Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: support anonymous compound statement #4316

Open
Totktonada opened this issue Jun 28, 2019 · 5 comments
Open

sql: support anonymous compound statement #4316

Totktonada opened this issue Jun 28, 2019 · 5 comments
Labels
feature A new functionality sql
Milestone

Comments

@Totktonada
Copy link
Member

Totktonada commented Jun 28, 2019

It is part of SQL/PMS standard.

BEGIN
    <statement>;
    <statement>;
    ...
    <statement>;
END

It is necessary to execute multiple statements in one transaction at least until #2016 will be implemented for both vinyl and memtx. The discussion about that was initiated in #4123.

SQLite3 supports several statements with semicolons between them w/o BEGIN / END. Not sure why we're not going this way, but I suspect it may contradict with the standard.

SQLite3 grammar is the following (input is the root non-terminal, SEMI is a semicolon, the version is 3.29.0 2019-06-28 07:08:13 eab4297577e4d325fed4757867fc77860de7448998d86f098c8a50272e17alt1):

input ::= cmdlist.
cmdlist ::= cmdlist ecmd.
cmdlist ::= ecmd.
ecmd ::= SEMI.
ecmd ::= cmdx SEMI.
<...explain skipped...>
cmdx ::= cmd.           { sqlite3FinishCoding(pParse); }

I verified that is works:

sqlite> create table t(id int primary key, v varchar(100));
sqlite> insert into t values(1, 'one');
sqlite> insert into t values(2, 'two');
sqlite> insert into t values(3, 'three');
sqlite> select id from t;
1
2
3
sqlite> select id from t; select id from t;
1
2
3
1
2
3

Tarantool grammar is the following now (2.2.0-482-g8c84932ad):

// Input is a single SQL command
input ::= ecmd.
ecmd ::= explain cmdx SEMI. {   
    if (!pParse->parse_only)    
        sql_finish_coding(pParse);
}
ecmd ::= SEMI. {
  diag_set(ClientError, ER_SQL_STATEMENT_EMPTY);
  pParse->is_aborted = true;    
}
<...skipped explain...>
cmdx ::= cmd.

I tried to add the compound statement support to tarantool in this way:

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 010feffd4..117327962 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -109,7 +109,11 @@ struct TrigEvent { int a; IdList * b; };
 } // end %include
 
 // Input is a single SQL command
-input ::= ecmd.
+input ::= cmdlistx.
+cmdlist ::= ecmd.
+cmdlist ::= cmdlist ecmd.
+cmdlistx ::= BEGIN cmdlist END.
+cmdlistx ::= ecmd.
 ecmd ::= explain cmdx SEMI. {
        if (!pParse->parse_only)
                sql_finish_coding(pParse);
@@ -216,7 +220,7 @@ columnname(A) ::= nm(A) typedef(Y). {sqlAddColumn(pParse,&A,&Y);}
 //
 %fallback ID
   ABORT ACTION ADD AFTER AUTOINCREMENT BEFORE CASCADE
-  CONFLICT DEFERRED END FAIL
+  CONFLICT DEFERRED FAIL
   IGNORE INITIALLY INSTEAD NO MATCH PLAN
   QUERY KEY OFFSET RAISE RELEASE REPLACE RESTRICT
   RENAME CTIME_KW IF

(I don't know to be honest why it is necessary to remove END from %fallback, but my rules does not work w/o this change. It seems that the parser step into a wrong branch somewhere.)

This way it the following request gives the following assertion fail:

tarantool> \set language sql
tarantool> create table t(id int primary key, v varchar(100))
tarantool> insert into t values(1, 'one')
tarantool> insert into t values(2, 'two')
tarantool> insert into t values(3, 'three')
tarantool> select id from t
---
- metadata:
  - name: ID
    type: integer
  rows:
  - [1]
  - [2]
  - [3]
...
tarantool> begin select id from t; select id from t; end
tarantool: /home/alex/p/tarantool-meta/tarantool/src/box/sql/vdbeaux.c:448: sqlVdbeMakeLabel: Assertion `v->magic == VDBE_MAGIC_INIT' failed.
Aborted

It is possible to avoid sql_finish_coding() calls (create a list of cmdx instead of ecmd in the grammar) and it even 'works'. Resulting rows contain values from all requests in a compound statement and the metadata contains a metainfo for one of them. This is certainly invalid result. I hope that we should return result of the last statement in a compound statement.

So we should first agree on syntax and then implement proper vdbe state cleaining, or results discarding, or kinda.

@Totktonada Totktonada added the sql label Jun 28, 2019
@Totktonada
Copy link
Member Author

Points suggested by @kostja:

  • We should not allow a list of statements w/o begin <...> end. Just implement it with begin <...> end.
  • We should show results of all stataments in a compound statement:
    • Write them to console one by one.
    • Use iproto push for all responses except a last one.

@pgulutzan
Copy link
Contributor

pgulutzan commented Jun 30, 2019

SAVEPOINT x;
START TRANSACTION;
BEGIN NOT ATOMIC        /* [NOT ATOMIC] okay? */
  BEGIN                 /* BEGIN within BEGIN okay? */
    ROLLBACK TO x;      /* transaction environment is irrelevant? */
    DROP TABLE t;       /* Is this "last statement in a compound statement? */
  END;                  /* semicolon mandatory? */
END;
BEGIN
END;                    /* Will client wait till I type END;? */
select id from t; select id from t; /* why do you think this may be bad? */
EXPLAIN 'BEGIN ... END';/* this will be supported someday? */

@zilveer
Copy link

zilveer commented Jul 11, 2019

@kyukhin kyukhin added the feature A new functionality label Jul 18, 2019
@kyukhin kyukhin added this to the wishlist milestone Jul 18, 2019
@Totktonada
Copy link
Member Author

Related: #4809.

@Totktonada
Copy link
Member Author

An SQL transaction may not be used from a connector without defining an extra Lua function, which will do something like:

box.execute('START TRANSACTION')
for _, sql_request in ipairs(sql_requests) do
    box.execute(sql_request)
end
box.execute('COMMIT')

To overcome this, either this issue or #2016 should be resolved.

Note: Each of those issues resolves the problem with an SQL transaction, but each of them is not purely about this. Don't consider one as a replacement of another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality sql
Projects
None yet
Development

No branches or pull requests

4 participants