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 102B627B63 for ; Thu, 19 Jul 2018 07:49:02 -0400 (EDT) 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 lqsFaxtRI05G for ; Thu, 19 Jul 2018 07:49:01 -0400 (EDT) Received: from mail-lj1-f170.google.com (mail-lj1-f170.google.com [209.85.208.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9C2DC27B59 for ; Thu, 19 Jul 2018 07:49:01 -0400 (EDT) Received: by mail-lj1-f170.google.com with SMTP id v9-v6so7003563ljk.4 for ; Thu, 19 Jul 2018 04:49:01 -0700 (PDT) MIME-Version: 1.0 References: <1530787337-18302-1-git-send-email-hollow653@gmail.com> In-Reply-To: From: Nikita Tatunov Date: Thu, 19 Jul 2018 14:48:48 +0300 Message-ID: Subject: [tarantool-patches] Re: [PATCH] sql: Remove 'BEGIN TRANSACTION' Content-Type: multipart/alternative; boundary="000000000000d552c9057158c348" 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: korablev@tarantool.org Cc: tarantool-patches@freelists.org --000000000000d552c9057158c348 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =D1=87=D1=82, 19 =D0=B8=D1=8E=D0=BB. 2018 =D0=B3. =D0=B2 3:24, n.pettik : > > diff --git a/extra/lempar.c b/extra/lempar.c > index 00fd79c..d043e39 100644 > --- a/extra/lempar.c > +++ b/extra/lempar.c > @@ -336,8 +336,8 @@ void *ParseAlloc(void *(*mallocProc)(YYMALLOCARGTYPE)= ){ > if( pParser ){ > #ifdef YYTRACKMAXSTACKDEPTH > pParser->yyhwm =3D 0; > - pParser->is_fallback_failed =3D false; > #endif > + pParser->is_fallback_failed =3D false; > > > Could you please explain this code movement? > "Also found initialization in ifdef that caused some problems with error messages in occasions when the wrong syntax was used." To be more accurate it's just variable initialization that was in wrong place. Moreover in most cases (almost always) after allocating memory for this variable there was non-zero value ('true', basically), that was causing "keyword 'BEGIN' is reserved" error message if we use "BEGIN" instead of "START TRANSACTION", after the patch changes are applied. > > +++ b/test/sql-tap/start-transaction.test.lua > @@ -0,0 +1,266 @@ > +#!/usr/bin/env tarantool > +test =3D require("sqltester") > +test:plan(21) > + > +test:do_catchsql_test( > + "start-transaction-1.0", > + [[ > + CREATE TABLE IF NOT EXISTS t(id int PRIMARY KEY); > + DELETE FROM t; > + BEGIN; > + INSERT INTO t VALUES (1); > + INSERT INTO t VALUES (2); > + COMMIT; > + ]], { > + -- > + 1, "near \"BEGIN\": syntax error" > + -- > + }) > + > +test:do_execsql_test( > + "start-transaction-1.1", > + [[ > + SELECT * FROM t; > + ]], { > + -- > + > + -- > + }) > + > +test:do_catchsql_test( > + "start-transaction-1.2", > + [[ > + CREATE TABLE IF NOT EXISTS t(id int primary key); > + delete from t; > > > We write SQL queries in upper-case. Fix it in other places too. > > Fixed it. Diff with the previous pushed patch: diff --git a/test/sql-tap/start-transaction.test.lua b/test/sql-tap/start-transaction.test.lua index eece4cd..98ca531 100755 --- a/test/sql-tap/start-transaction.test.lua +++ b/test/sql-tap/start-transaction.test.lua @@ -30,8 +30,8 @@ test:do_execsql_test( test:do_catchsql_test( "start-transaction-1.2", [[ - CREATE TABLE IF NOT EXISTS t(id int primary key); - delete from t; + CREATE TABLE IF NOT EXISTS t(id INT PRIMARY KEY); + DELETE FROM t; BEGIN TRANSACTION; INSERT INTO t VALUES (1); INSERT INTO t VALUES (2); @@ -256,7 +256,7 @@ test:do_execsql_test( test:do_execsql_test( "start-transaction-1.20", [[ - drop table t; + DROP TABLE t; ]], { -- diff --git a/test/sql/tokenizer.result b/test/sql/tokenizer.result index 7c0ca8f..95063b5 100644 --- a/test/sql/tokenizer.result +++ b/test/sql/tokenizer.result @@ -102,9 +102,9 @@ sql_tokenizer.split_sql('select 1; -- test comment\n; select 2;') - - select 1; - ' select 2;' ... -sql_tokenizer.split_sql("start transaction; insert into table values ('example kek--'); commit;") +sql_tokenizer.split_sql("START TRANSACTION; insert into table values ('example kek--'); commit;") --- -- - start transaction; +- - START TRANSACTION; - ' insert into table values (''example kek--'');' - ' commit;' ... diff --git a/test/sql/tokenizer.test.lua b/test/sql/tokenizer.test.lua index 5c1cadc..4a4e289 100644 --- a/test/sql/tokenizer.test.lua +++ b/test/sql/tokenizer.test.lua @@ -32,7 +32,7 @@ test_run:cmd("setopt delimiter ''"); sql_tokenizer.split_sql('select 1; -- test comment\n; select 2;') -sql_tokenizer.split_sql("start transaction; insert into table values ('example kek--'); commit;") +sql_tokenizer.split_sql("START TRANSACTION; insert into table values ('example kek--'); commit;") sql_tokenizer.split_sql("insert into quoted(a) value 'lalala'") --000000000000d552c9057158c348 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=D1=87= =D1=82, 19 =D0=B8=D1=8E=D0=BB. 2018 =D0=B3. =D0=B2 3:24, n.pettik <korablev@tarantool.org>:

diff --git a/extra/lempar.c b/extra/lempar.c
index 00fd79c..d043e39 100644
--- a/extra/lempar.c
<= div>+++ b/extra/lempar.c
@@ -336,8 +336,8 @@ void *ParseAlloc(voi= d *(*mallocProc)(YYMALLOCARGTYPE)){
=C2=A0 =C2=A0if( pParser ){
=C2=A0#ifdef YYTRACKMAXSTACKDEPTH
=C2=A0 =C2=A0 =C2=A0pP= arser->yyhwm =3D 0;
-=C2=A0 =C2=A0 pParser->is_fallback_fai= led =3D false;
=C2=A0#endif
+=C2=A0 =C2=A0 pParser->= is_fallback_failed =3D false;

Could you please explain this code movement?

"Also found
initialization in ifdef that caused some problems with error
messages in occasions when the wrong syntax was used."
To be more= accurate it's just variable initialization that was in wrong place.
Moreover in most cases (almost always) after allocating memory for = this
variable there was non-zero value ('true', basically= ), that was causing
"keyword 'BEGIN' is reserved&quo= t; error message if we use "BEGIN" instead
of "STA= RT TRANSACTION", after the patch changes are applied.
=C2=A0=

+++ b/test/sql-tap/start-transaction.test.lua
@@ -0,0 +1,266 @@
+#!/usr/bin/env tarantool
+te= st =3D require("sqltester")
+test:plan(21)
+<= /div>
+test:do_catchsql_test(
+ "start-transaction-1.0",
+ [[
+ CREATE TABLE IF NOT EXISTS t(id int PRIMARY KEY);
+ DELETE FROM t;
+<= span style=3D"white-space:pre-wrap"> BEGIN;
+ INSERT INTO t VALUES (1);
+ INSERT INTO t VALUES (2);
+ COMMIT;
+ ]], {
+ -- <start-transaction-1.0>
+<= span style=3D"white-space:pre-wrap"> 1, "near \"BEGIN\&qu= ot;: syntax error"
+ <= /span>-- <start-transaction-1.0>
+ })
+
+test:do_execsql_test(
+ "start-transaction-1.= 1",
+ [[
+ SELECT * FROM t;
+ ]], {
+ -- <start-transaction-1.1>
<= div>+
+ -- <start= -transaction-1.1>
+ })
+
+test:do_catchsql_test(
+ "start-transaction-1.2",
<= div>+ [[
+ CREATE TABLE IF NOT EXISTS t(id int prim= ary key);
+ delete f= rom t;

We write SQL= queries in upper-case. Fix it in other places too.

Fixed it.

Diff with the previous p= ushed patch:

diff --git a/test/sql-tap/start-transaction.test.l= ua b/test/sql-tap/start-transaction.test.lua
index eece4cd..98ca5= 31 100755
--- a/test/sql-tap/start-transaction.test.lua
+++ b/test/sql-tap/start-transaction.test.lua
@@ -30,8 +30,8 @@ = test:do_execsql_test(
=C2=A0test:do_catchsql_test(
=C2= =A0 "start-transaction-1.2"= ;,
=C2=A0 [[
- CREATE TABLE IF NOT EXISTS t(id int = primary key);
- delete fr= om t;
+ CREATE TABLE IF N= OT EXISTS t(id INT PRIMARY KEY);
+ DELETE FROM t;
=C2=A0 = BEGIN TRANSACTION;
=C2=A0 = INSERT INTO t VALUES (1);
=C2=A0 INSERT INTO t VALUES (2);
@@ -256,7 +256,7 @@ t= est:do_execsql_test(
=C2=A0test:do_execsql_test(
=C2=A0= "start-transaction-1.20",=
=C2=A0 [[
- drop table t;
+ DROP TABLE t;
=C2=A0 ]], {
=C2=A0 -- <start0transaction-1.20>
=C2=A0
d= iff --git a/test/sql/tokenizer.result b/test/sql/tokenizer.result
index 7c0ca8f..95063b5 100644
--- a/test/sql/tokenizer.result
+++ b/test/sql/tokenizer.result
@@ -102,9 +102,9 @@ sql_t= okenizer.split_sql('select 1; -- test comment\n; select 2;')
<= div>=C2=A0- - select 1;
=C2=A0 =C2=A0- ' select 2;'
=
=C2=A0...
-sql_tokenizer.split_sql("start transaction; = insert into table values ('example kek--'); commit;")
+sql_tokenizer.split_sql("START TRANSACTION; insert into table value= s ('example kek--'); commit;")
=C2=A0---
-= - - start transaction;
+- - START TRANSACTION;
=C2=A0 = =C2=A0- ' insert into table values (''example kek--'');= '
=C2=A0 =C2=A0- ' commit;'
=C2=A0...
=
diff --git a/test/sql/tokenizer.test.lua b/test/sql/tokenizer.test.lua=
index 5c1cadc..4a4e289 100644
--- a/test/sql/tokenizer= .test.lua
+++ b/test/sql/tokenizer.test.lua
@@ -32,7 +3= 2,7 @@ test_run:cmd("setopt delimiter ''");
=C2= =A0
=C2=A0sql_tokenizer.split_sql('select 1; -- test comment\= n; select 2;')
=C2=A0
-sql_tokenizer.split_sql(&quo= t;start transaction; insert into table values ('example kek--'); co= mmit;")
+sql_tokenizer.split_sql("START TRANSACTION; in= sert into table values ('example kek--'); commit;")
= =C2=A0
=C2=A0sql_tokenizer.split_sql("insert into quoted(a) = value 'lalala'")

--000000000000d552c9057158c348--