From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B85C2254C9 for ; Tue, 22 Jan 2019 09:15:53 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YUIURwqX6kE4 for ; Tue, 22 Jan 2019 09:15:53 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 741B2254B4 for ; Tue, 22 Jan 2019 09:15:53 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: sql: assertion fault on VALUES (#3888) From: "n.pettik" In-Reply-To: <35ac4879-5140-15ea-6db8-1e61dbe687dd@tarantool.org> Date: Tue, 22 Jan 2019 17:15:51 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <16c923db-f9fc-1317-0468-fdb24740b1ff@tarantool.org> <35ac4879-5140-15ea-6db8-1e61dbe687dd@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: szudin@tarantool.org > On 22 Jan 2019, at 16:42, Stanislav Zudin = wrote: >=20 > Branch: https://github.com/tarantool/tarantool/tree/2.1 > Issue: https://github.com/tarantool/tarantool/issues/3888 You should add link to branch where your patch is located, so that reviewer can pull and test it. For instance, https://github.com/tarantool/tarantool/tree/np/gh-3934-IN-operator-fix Links to branch and issue should be placed after --- delimiter: Closes #3888 --- > =46rom ff49a389a68488b13a0e3093fce9f51ac4c186bf Mon Sep 17 00:00:00 = 2001 > From: Stanislav Zudin > Date: Tue, 22 Jan 2019 15:41:26 +0300 > Subject: [PATCH] sql: if a keyword 'blob' is used as an ID treat it as = error These headers shouldn=E2=80=99t appear if you send patch via git = send-email. >=20 > Closes #3888 > --- > src/box/sql/expr.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) >=20 > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index b67b22c23..3eea7f4a3 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -3751,10 +3751,24 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * = pExpr, int target) > } > #ifndef SQLITE_OMIT_BLOB_LITERAL > case TK_BLOB:{ > - int n; > - const char *z; > + int n =3D sqlite3Strlen30(pExpr->u.zToken); > + const char *z =3D pExpr->u.zToken; > char *zBlob; > + > assert(!ExprHasProperty(pExpr, EP_IntValue)); > + > + if ((n < 3) || /* 'X' + opening ' + closing ' =3D=3D 3 = */ > + (z[0] !=3D 'x' && z[0] !=3D 'X') || > + (z[1] !=3D '\'') || (z[n - 1] !=3D '\'')) { > + /* It's definitely not a blob, report an error. */ > + sqlite3ErrorMsg(pParse, > + "Unexpected keyword '%s' within request", > + z); If we get so far, then we have already passed through Lemon parser and about to generate byte code. Hence it is too late to raise an error about unexpected keywords. Look at parse.y: 30 // This code runs whenever there is a syntax error 31 // 32 %syntax_error { 33 UNUSED_PARAMETER(yymajor); /* Silence some compiler warnings = */ 34 assert( TOKEN.z[0] ); /* The tokenizer always gives us a = token */ 35 if (yypParser->is_fallback_failed && TOKEN.isReserved) { 36 sqlite3ErrorMsg(pParse, "keyword \"%T\" is reserved", = &TOKEN); 37 } else { 38 sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", = &TOKEN); 39 } 40 } I guess we should terminate here. > + sqlite3MayAbort(pParse); > + return 0; > + } > + > + /* ---------------------------------------------- */ We don=E2=80=99t use such comments as delimiters or whatever. What about other cases which I mentioned? For instance, SELECT FLOAT returns 0, but must raise an error. Yep, it may seem as a slightly different problem. You could extend you patch and fix it right now. Otherwise, file an issue(s), please. > assert(pExpr->u.zToken[0] =3D=3D 'x' > || pExpr->u.zToken[0] =3D=3D 'X'); > assert(pExpr->u.zToken[1] =3D=3D '\'=E2=80=99); Also, note that every patch should come with test verifying that the problem has been resolved (whenever it is possible).