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 25F1A21BF7 for ; Mon, 28 Jan 2019 12:30:34 -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 EDXUH4mjoCqY for ; Mon, 28 Jan 2019 12:30:33 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 B0D7D217F2 for ; Mon, 28 Jan 2019 12:30:33 -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: [PATCH] sql: do not let VALUES statement treat the type definition keywords as values From: "n.pettik" In-Reply-To: <20190125105031.31745-1-szudin@tarantool.org> Date: Mon, 28 Jan 2019 20:30:31 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <2A6AF308-C6A5-4122-AC73-B429C1487CC5@tarantool.org> References: <20190125105031.31745-1-szudin@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 Decorations are not such vital as content of patch surely, but still should be OK. Firstly, try to limit the subject line to 50 characters or so. Then, if you send patch v2, please indicate it to format-patch utility by passing --subject-prefix=3D'PATCH v2=E2=80=99 > The "box.sql.execute('values(blob)')" causes an accert Nit: accert -> assert. > in the expression processing, because the parser doesn't distinguish = the keyword "BLOB" from the binary value (in the form X'hex=E2=80=99). >=20 > This fix adds an additional checks in the SQL grammar. Thus the = expressions such as "VALUES(BLOB)", "SELECT FLOAT" and so on are treated = as a syntax errors. Wrap the body of commit message to 72 characters or so. >=20 > Closes #3888 > =E2=80=94 Place links to your branch and issue here (not at the very bottom of patch). Patch itself is OK, just few nits. > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index 0bcf41594..73a324522 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -211,7 +211,7 @@ columnname(A) ::=3D nm(A) typedef(Y). = {sqlite3AddColumn(pParse,&A,&Y);} > IGNORE INITIALLY INSTEAD NO MATCH PLAN > QUERY KEY OFFSET RAISE RELEASE REPLACE RESTRICT > %ifdef SQLITE_OMIT_COMPOUND_SELECT > - INTERSECT=20 > + INTERSECT Extra diff. > %endif SQLITE_OMIT_COMPOUND_SELECT > RENAME CTIME_KW IF > . > @@ -823,12 +823,15 @@ idlist(A) ::=3D nm(Y). > p->affinity =3D AFFINITY_TEXT; > break; > case TK_BLOB: > + case TK_BLOB_KW: > p->affinity =3D AFFINITY_BLOB; Why do we need to assign affinity to keywords? > break; > case TK_INTEGER: > + case TK_INTEGER_KW: > p->affinity =3D AFFINITY_INTEGER; > break; > case TK_FLOAT: > + case TK_FLOAT_KW: > p->affinity =3D AFFINITY_REAL; > break; > } > diff --git a/test/sql/gh-3888-values-blob-assert.test.lua = b/test/sql/gh-3888-values-blob-assert.test.lua > new file mode 100644 > index 000000000..dafb73716 > --- /dev/null > +++ b/test/sql/gh-3888-values-blob-assert.test.lua > @@ -0,0 +1,37 @@ > +-- sql: assertion fault on VALUES #3888 > +-- Very simple query leads to assertion fault: > +-- > +-- box.cfg{} > +-- box.sql.execute("values(blob)") > +-- Assertion failed: (pExpr->u.zToken[0] =3D=3D 'x' || = pExpr->u.zToken[0] =3D=3D 'X'), > +-- function sqlite3ExprCodeTarget, > +-- file /Users/n.pettik/tarantool/src/box/sql/expr.c, line 3759. > +-- Abort trap: 6 To be honest, I=E2=80=99d rather put short description of bug, then copying content of ticket (which is available on github). Sort of: Make sure that tokens representing values of integer, float, and blob constants are different from tokens representing keywords of the same names. > --=20 > 2.17.1 >=20 > --- > Branch: = https://github.com/tarantool/tarantool/tree/sz/gh-3888-values-blob-assert > Issue: https://github.com/tarantool/tarantool/issues/3888 Next time move these links up.