* [tarantool-patches] [PATCH v2 1/3] sql: remove unnecessary AUTOINCREMENT ID generation
2019-07-04 11:42 [tarantool-patches] [PATCH v2 0/3] sql: do not show IDs generated by trigger imeevma
@ 2019-07-04 11:42 ` imeevma
2019-07-08 18:54 ` [tarantool-patches] " n.pettik
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 2/3] sql: do not show IDs generated by trigger imeevma
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 3/3] sql: remove VDBE from TXN imeevma
2 siblings, 1 reply; 6+ messages in thread
From: imeevma @ 2019-07-04 11:42 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
Currently, if we perform something like
CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT);
INSERT INTO t(a) VALUES (NULL);
we generate a new identifier in a special way. This was necessary
for the cases described in issue #2981. Here it is:
CREATE TABLE t1 (
s1 INTEGER PRIMARY KEY AUTOINCREMENT,
s2 INTEGER,
CHECK (s1 <> 19)
);
INSERT INTO t1 VALUES (18, NULL);
INSERT INTO t1 (s2) VALUES (NULL);
In this case, the CHECK did not work properly. This is not
necessary now, because CHECK was moved to BOX due to issue #3691.
This patch removes the mentioned special way.
Part of #4188
---
src/box/sql/insert.c | 5 +----
test/sql/gh-2981-check-autoinc.result | 32 ++++++++++++++++++++++++++++++++
test/sql/gh-2981-check-autoinc.test.lua | 11 ++++++++++-
3 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b353148..d2b4e17 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -628,10 +628,7 @@ sqlInsert(Parse * pParse, /* Parser context */
if (j < 0 || nColumn == 0
|| (pColumn && j >= pColumn->nId)) {
if (i == (int) autoinc_fieldno) {
- sqlVdbeAddOp2(v,
- OP_NextAutoincValue,
- space->def->id,
- iRegStore);
+ sqlVdbeAddOp2(v, OP_Null, 0, iRegStore);
continue;
}
struct Expr *dflt = NULL;
diff --git a/test/sql/gh-2981-check-autoinc.result b/test/sql/gh-2981-check-autoinc.result
index f03858a..8583115 100644
--- a/test/sql/gh-2981-check-autoinc.result
+++ b/test/sql/gh-2981-check-autoinc.result
@@ -23,6 +23,14 @@ box.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER,
---
- row_count: 1
...
+box.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
+---
+- row_count: 1
+...
+box.execute("CREATE TABLE t5 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
+---
+- row_count: 1
+...
box.execute("insert into t1 values (18, null);")
---
- row_count: 1
@@ -55,6 +63,22 @@ box.execute("insert into t3(s2) values (null)")
---
- error: 'Check constraint failed ''CK_CONSTRAINT_1_T3'': s1 < 10'
...
+box.execute("insert into t4 values (9, null)")
+---
+- row_count: 1
+...
+box.execute("insert into t4 values (null, null)")
+---
+- error: 'Check constraint failed ''CK_CONSTRAINT_1_T4'': s1 < 10'
+...
+box.execute("INSERT INTO t5 VALUES (18, NULL);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t5 SELECT NULL, NULL FROM t5;")
+---
+- error: 'Check constraint failed ''CK_CONSTRAINT_1_T5'': s1 <> 19'
+...
box.execute("DROP TABLE t1")
---
- row_count: 1
@@ -67,3 +91,11 @@ box.execute("DROP TABLE t3")
---
- row_count: 1
...
+box.execute("DROP TABLE t4")
+---
+- row_count: 1
+...
+box.execute("DROP TABLE t5")
+---
+- row_count: 1
+...
diff --git a/test/sql/gh-2981-check-autoinc.test.lua b/test/sql/gh-2981-check-autoinc.test.lua
index 0eb8f73..ac5624e 100644
--- a/test/sql/gh-2981-check-autoinc.test.lua
+++ b/test/sql/gh-2981-check-autoinc.test.lua
@@ -7,6 +7,8 @@ box.cfg{}
box.execute("CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
box.execute("CREATE TABLE t2 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19 AND s1 <> 25));");
box.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
+box.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
+box.execute("CREATE TABLE t5 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
box.execute("insert into t1 values (18, null);")
box.execute("insert into t1(s2) values (null);")
@@ -19,7 +21,14 @@ box.execute("insert into t2(s2) values (null);")
box.execute("insert into t3 values (9, null)")
box.execute("insert into t3(s2) values (null)")
+box.execute("insert into t4 values (9, null)")
+box.execute("insert into t4 values (null, null)")
+
+box.execute("INSERT INTO t5 VALUES (18, NULL);")
+box.execute("INSERT INTO t5 SELECT NULL, NULL FROM t5;")
+
box.execute("DROP TABLE t1")
box.execute("DROP TABLE t2")
box.execute("DROP TABLE t3")
-
+box.execute("DROP TABLE t4")
+box.execute("DROP TABLE t5")
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [tarantool-patches] [PATCH v2 2/3] sql: do not show IDs generated by trigger
2019-07-04 11:42 [tarantool-patches] [PATCH v2 0/3] sql: do not show IDs generated by trigger imeevma
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 1/3] sql: remove unnecessary AUTOINCREMENT ID generation imeevma
@ 2019-07-04 11:42 ` imeevma
2019-07-08 18:54 ` [tarantool-patches] " n.pettik
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 3/3] sql: remove VDBE from TXN imeevma
2 siblings, 1 reply; 6+ messages in thread
From: imeevma @ 2019-07-04 11:42 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
Hi! Thank you for review. My answers and new patch below.
On 7/3/19 3:25 AM, n.pettik wrote:
>
>
>> On 25 Jun 2019, at 17:09, Imeev Mergen <imeevma@tarantool.org> wrote:
>> Hi! Thank you for review! My answers below.
>> On 6/7/19 3:38 AM, n.pettik wrote:
>>>> On 29 May 2019, at 20:09, imeevma@tarantool.org wrote:
>>>>
>>>> Currently, if INSERT is executed by a trigger, the new generated
>>>> identifiers will be stored in the VDBE. This is wrong, and this
>>>> patch fixes it.
>>> Why is it so?
>> Currently, we are not able to divide the received IDS into those
>> that belong to the mentioned table and those that do not belong.
>
> Ok, now it is clear. Add explanation to the commit message.
>
Added.
>>>> Only identifiers generated during INSERT run by
>>>> the user will be saved.
>>> Firstly, I truly do not understand why this behaviour
>>> is considered to be buggy. I don’t see discussion or
>>> bug description.
>> Now this discussion can be viewed on the dev mailing list.
>>> Secondly, I do not understand what happens in this patch.
>>> Could you please explain these changes and argue why
>>> they are needed?
>> Changes in this patch:
>> 1) At first I removed changes made in patch #2981 since it isn't
>> necessary now. Due to this, all the generation of new identifiers
>> is now performed in BOX.
>> 2) I removed VDBE from TXN. So now sequence_next() does not save
>> the generated IDs in VDBE.
>> 3) I changed the way OP_NextAutoincValue works. Now it is executed
>> when there is an insert in the field with AUTOINCREMENT. If NULL
>> was inserted it receives the last generated ID using the new
>> function sequence_get_value(), and stores it in VDBE. Otherwise it
>> is a no-op.
>
> Please, split this patch into patches according to your points,
> and re-send new patch-set.
>
Done.
>> 4) I added an extra argument to the sqlInsert() function, which
>> makes it clear that the insertion was done in a trigger.
>>
>> My point about these changes:
>> 1 - I think it should be done anyway.
>> 2 - It doesn't look like VDBE should be stored in TXN.
>
> Please, support such statements with arguments.
>
1 - it worked only for cases like:
CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));
INSERT INTO t1 VALUES (18, NULL);
INSERT INTO t1 (s2) VALUES (NULL);
But it doesn't worked for these cases:
CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));
INSERT INTO t1 VALUES (18, NULL);
INSERT INTO t1 VALUES (NULL, NULL);
CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));
INSERT INTO t1 VALUES (18, NULL);
INSERT INTO t1 SELECT NULL, NULL FROM t1;
In addition, after the CHECK has been moved to the BOX, it works
correctly for all cases. So now it is not necessary.
However, I can actually leave it at that. Then I need to create a
new opcode if I want to solve #4188 using my solution.
2 - sorry i can't say exactly why. But Gosha told me about it when
I asked him verbally, and Kostja O. and Gosha mentioned it in the
server chat. I think this can be found in messages for April or
March.
New patch:
From d275de15fb14cf8ca9c8361602ed62d6d68e8aaf Mon Sep 17 00:00:00 2001
Date: Tue, 28 May 2019 17:41:15 +0300
Subject: [PATCH] sql: do not show IDs generated by trigger
Currently, if an INSERT was executed by a trigger during the
execution of a statement, if there were any generated identifiers,
they can be displayed as a result of the statement. This is
incorrect, since we are not able to divide the IDs obtained into
those that belong to the table mentioned in the statement and
those that do not belong to this table.
For example:
box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
box.execute('CREATE TRIGGER r AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (null); END')
box.execute('INSERT INTO t2 VALUES (100);')
box.execute('INSERT INTO t1 VALUES (NULL), (NULL), (NULL);')
Result should be:
---
- autoincrement_ids:
- 1
- 2
- 3
row_count: 3
...
Closes #4188
diff --git a/src/box/errcode.h b/src/box/errcode.h
index be8dab2..6d22b09 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -250,6 +250,7 @@ struct errcode_record {
/*195 */_(ER_CREATE_CK_CONSTRAINT, "Failed to create check constraint '%s': %s") \
/*196 */_(ER_CK_CONSTRAINT_FAILED, "Check constraint failed '%s': %s") \
/*197 */_(ER_SQL_COLUMN_COUNT, "Unequal number of entries in row expression: left side has %u, but right side - %u") \
+ /*198 */_(ER_SEQUENCE_NO_ELEMENTS, "Sequence '%s' has no elements") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sequence.c b/src/box/sequence.c
index c9828c0..ee8a8d1 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -195,9 +195,6 @@ sequence_next(struct sequence *seq, int64_t *result)
new_data) == light_sequence_end)
return -1;
*result = def->start;
- if (txn_vdbe() != NULL &&
- vdbe_add_new_autoinc_id(txn_vdbe(), *result) != 0)
- return -1;
return 0;
}
old_data = light_sequence_get(&sequence_data_index, pos);
@@ -232,9 +229,6 @@ done:
new_data, &old_data) == light_sequence_end)
unreachable();
*result = value;
- if (txn_vdbe() != NULL &&
- vdbe_add_new_autoinc_id(txn_vdbe(), value) != 0)
- return -1;
return 0;
overflow:
if (!def->cycle) {
@@ -347,3 +341,19 @@ sequence_data_iterator_create(void)
light_sequence_iterator_freeze(&sequence_data_index, &iter->iter);
return &iter->base;
}
+
+int
+sequence_get_value(struct sequence *seq, int64_t *out_value)
+{
+ uint32_t key = seq->def->id;
+ uint32_t hash = sequence_hash(key);
+ uint32_t pos = light_sequence_find_key(&sequence_data_index, hash, key);
+ if (pos == light_sequence_end) {
+ diag_set(ClientError, ER_SEQUENCE_NO_ELEMENTS, seq->def->name);
+ return -1;
+ }
+ struct sequence_data data = light_sequence_get(&sequence_data_index,
+ pos);
+ *out_value = data.value;
+ return 0;
+}
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 4419427..2acb6da 100644
--- a/src/box/sequence.h
+++ b/src/box/sequence.h
@@ -140,9 +140,6 @@ sequence_next(struct sequence *seq, int64_t *result);
int
access_check_sequence(struct sequence *seq);
-int
-vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);
-
/**
* Create an iterator over sequence data.
*
@@ -153,6 +150,16 @@ vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);
struct snapshot_iterator *
sequence_data_iterator_create(void);
+/**
+ * Get last element of given sequence.
+ *
+ * @param seq sequence to get value from.
+ * @param[out] out_value last element of sequence.
+ * @retval 0 on success, -1 on error.
+ */
+int
+sequence_get_value(struct sequence *seq, int64_t *out_value);
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index d2b4e17..c331cc5 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -233,7 +233,8 @@ sqlInsert(Parse * pParse, /* Parser context */
SrcList * pTabList, /* Name of table into which we are inserting */
Select * pSelect, /* A SELECT statement to use as the data source */
IdList * pColumn, /* Column names corresponding to IDLIST. */
- enum on_conflict_action on_error)
+ enum on_conflict_action on_error,
+ bool is_triggered)
{
sql *db; /* The main database structure */
char *zTab; /* Name of the table into which we are inserting */
@@ -738,6 +739,13 @@ sqlInsert(Parse * pParse, /* Parser context */
vdbe_emit_insertion_completion(v, space, regIns + 1,
space->def->field_count,
on_error);
+ /*
+ * Save the newly autogenerated value to VDBE.
+ */
+ if (!is_triggered && autoinc_fieldno < UINT32_MAX) {
+ sqlVdbeAddOp2(v, OP_NextAutoincValue, space->def->id,
+ regData + autoinc_fieldno);
+ }
}
/* Update the count of rows that are inserted
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 010feff..50ddd4b 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -826,7 +826,7 @@ cmd ::= with(W) insert_cmd(R) INTO fullname(X) idlist_opt(F) select(S). {
sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS;
/* Instruct SQL to initate Tarantool's transaction. */
pParse->initiateTTrans = true;
- sqlInsert(pParse, X, S, F, R);
+ sqlInsert(pParse, X, S, F, R, false);
}
cmd ::= with(W) insert_cmd(R) INTO fullname(X) idlist_opt(F) DEFAULT VALUES.
{
@@ -834,7 +834,7 @@ cmd ::= with(W) insert_cmd(R) INTO fullname(X) idlist_opt(F) DEFAULT VALUES.
sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS;
/* Instruct SQL to initate Tarantool's transaction. */
pParse->initiateTTrans = true;
- sqlInsert(pParse, X, 0, F, R);
+ sqlInsert(pParse, X, 0, F, R, false);
}
%type insert_cmd {int}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 73dc6e4..5dcfd34 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3003,7 +3003,7 @@ sql_store_select(struct Parse *parse_context, struct Select *select);
void
sql_drop_table(struct Parse *);
void sqlInsert(Parse *, SrcList *, Select *, IdList *,
- enum on_conflict_action);
+ enum on_conflict_action, bool);
void *sqlArrayAllocate(sql *, void *, int, int *, int *);
/**
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index d746ef8..32a7052 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -631,14 +631,10 @@ codeTriggerProgram(Parse * pParse, /* The parser context */
break;
}
case TK_INSERT:{
- sqlInsert(pParse,
- targetSrcList(pParse, pStep),
- sqlSelectDup(db,
- pStep->pSelect,
- 0),
- sqlIdListDup(db,
- pStep->pIdList),
- pParse->eOrconf);
+ sqlInsert(pParse, targetSrcList(pParse, pStep),
+ sqlSelectDup(db, pStep->pSelect, 0),
+ sqlIdListDup(db, pStep->pIdList),
+ pParse->eOrconf, true);
break;
}
case TK_DELETE:{
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c8887f9..5ecfcf4 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -559,7 +559,7 @@ vdbe_autoinc_id_list(struct Vdbe *vdbe)
return &vdbe->autoinc_id_list;
}
-int
+static int
vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id)
{
assert(vdbe != NULL);
@@ -1150,29 +1150,30 @@ case OP_String: { /* out2 */
}
/* Opcode: NextAutoincValue P1 P2 * * *
- * Synopsis: r[P2] = next value from space sequence, which pageno is r[P1]
*
- * Get next value from space sequence, which pageno is written into register
- * P1, write this value into register P2. If space doesn't exists (invalid
- * space_id or something else), raise an error. If space with
- * specified space_id doesn't have attached sequence, also raise an error.
+ * If the value in the register P2 is NULL, get the last value
+ * from the sequence that belongs to the space with id P1, and
+ * save this value in the VDBE. If the value in register in not
+ * NULL than this opcode is a no-op.
*/
case OP_NextAutoincValue: {
assert(pOp->p1 > 0);
assert(pOp->p2 > 0);
+ pIn2 = &p->aMem[pOp->p2];
+ if ((pIn2->flags & MEM_Null) == 0)
+ break;
+
struct space *space = space_by_id(pOp->p1);
if (space == NULL)
goto abort_due_to_error;
int64_t value;
struct sequence *sequence = space->sequence;
- if (sequence == NULL || sequence_next(sequence, &value) != 0)
+ if (sequence == NULL || sequence_get_value(sequence, &value) != 0)
goto abort_due_to_error;
- pOut = out2Prerelease(p, pOp);
- pOut->flags = MEM_Int;
- pOut->u.i = value;
+ vdbe_add_new_autoinc_id(p, value);
break;
}
diff --git a/test/box/misc.result b/test/box/misc.result
index dab8549..0f8bcb1 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -525,6 +525,7 @@ t;
195: box.error.CREATE_CK_CONSTRAINT
196: box.error.CK_CONSTRAINT_FAILED
197: box.error.SQL_COLUMN_COUNT
+ 198: box.error.SEQUENCE_NO_ELEMENTS
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 9639ba7..3580d6b 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -552,8 +552,6 @@ cn:execute('insert into test values (null, 1)')
---
- autoincrement_ids:
- 127
- - 1
- - 2
row_count: 1
...
box.execute('create table test3 (id int primary key autoincrement)')
diff --git a/test/sql/triggers.result b/test/sql/triggers.result
index 307b390..cc52727 100644
--- a/test/sql/triggers.result
+++ b/test/sql/triggers.result
@@ -551,3 +551,60 @@ box.execute("DROP TABLE t1;")
---
- row_count: 1
...
+--
+-- gh-4188: Additional generated identifiers when INSERT is
+-- executed by triggers.
+--
+box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
+---
+- row_count: 1
+...
+box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
+---
+- row_count: 1
+...
+box.execute('CREATE TRIGGER r AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (null); END')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t2 VALUES (100);')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t1 VALUES (NULL), (NULL), (NULL);')
+---
+- autoincrement_ids:
+ - 1
+ - 2
+ - 3
+ row_count: 3
+...
+box.execute('SELECT * FROM t1;')
+---
+- metadata:
+ - name: I
+ type: integer
+ rows:
+ - [1]
+ - [2]
+ - [3]
+...
+box.execute('SELECT * FROM t2;')
+---
+- metadata:
+ - name: I
+ type: integer
+ rows:
+ - [100]
+ - [101]
+ - [102]
+ - [103]
+...
+box.execute('DROP TABLE t1;')
+---
+- row_count: 1
+...
+box.execute('DROP TABLE t2;')
+---
+- row_count: 1
+...
diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua
index a056e79..3d3b393 100644
--- a/test/sql/triggers.test.lua
+++ b/test/sql/triggers.test.lua
@@ -191,3 +191,17 @@ box.execute([[CREATE TRIGGER r1 AFTER INSERT ON t1 FOR EACH ROW BEGIN SELECT 1;
box.session.su('admin')
box.schema.user.drop('tester')
box.execute("DROP TABLE t1;")
+
+--
+-- gh-4188: Additional generated identifiers when INSERT is
+-- executed by triggers.
+--
+box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
+box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
+box.execute('CREATE TRIGGER r AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (null); END')
+box.execute('INSERT INTO t2 VALUES (100);')
+box.execute('INSERT INTO t1 VALUES (NULL), (NULL), (NULL);')
+box.execute('SELECT * FROM t1;')
+box.execute('SELECT * FROM t2;')
+box.execute('DROP TABLE t1;')
+box.execute('DROP TABLE t2;')
^ permalink raw reply [flat|nested] 6+ messages in thread* [tarantool-patches] [PATCH v2 3/3] sql: remove VDBE from TXN
2019-07-04 11:42 [tarantool-patches] [PATCH v2 0/3] sql: do not show IDs generated by trigger imeevma
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 1/3] sql: remove unnecessary AUTOINCREMENT ID generation imeevma
2019-07-04 11:42 ` [tarantool-patches] [PATCH v2 2/3] sql: do not show IDs generated by trigger imeevma
@ 2019-07-04 11:42 ` imeevma
2 siblings, 0 replies; 6+ messages in thread
From: imeevma @ 2019-07-04 11:42 UTC (permalink / raw)
To: korablev; +Cc: tarantool-patches
VDBE was added to TXN because the generated identifiers were added
to VDBE in the sequence_next() function. Since they are now stored
in the VDBE itself, it is not necessary to have it in TXN.
Follow-up #4188
---
src/box/sql/vdbe.c | 4 ++--
src/box/sql/vdbe.h | 3 +--
src/box/sql/vdbeInt.h | 2 +-
src/box/sql/vdbeaux.c | 10 ++++------
src/box/txn.h | 17 -----------------
5 files changed, 8 insertions(+), 28 deletions(-)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 5ecfcf4..3534f48 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2927,7 +2927,7 @@ case OP_CheckViewReferences: {
* Otherwise, raise an error with appropriate error message.
*/
case OP_TransactionBegin: {
- if (sql_txn_begin(p) != 0)
+ if (sql_txn_begin() != 0)
goto abort_due_to_error;
p->auto_commit = false ;
break;
@@ -2983,7 +2983,7 @@ case OP_TransactionRollback: {
*/
case OP_TTransaction: {
if (!box_txn()) {
- if (sql_txn_begin(p) != 0)
+ if (sql_txn_begin() != 0)
goto abort_due_to_error;
} else {
p->anonymous_savepoint = sql_savepoint(p, NULL);
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 4f94643..fde898d 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -175,11 +175,10 @@ Vdbe *sqlVdbeCreate(Parse *);
* Allocate and initialize SQL-specific struct which completes
* original Tarantool's txn struct using region allocator.
*
- * @param v Vdbe with which associate this transaction.
* @retval NULL on OOM, new psql_txn struct on success.
**/
struct sql_txn *
-sql_alloc_txn(struct Vdbe *v);
+sql_alloc_txn();
/**
* Prepare given VDBE to execution: initialize structs connected
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 6bfeecc..30cac07 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -440,7 +440,7 @@ u32 sqlVdbeSerialGet(const unsigned char *, u32, Mem *);
int sqlVdbeExec(Vdbe *);
int sqlVdbeList(Vdbe *);
int
-sql_txn_begin(Vdbe *p);
+sql_txn_begin();
Savepoint *
sql_savepoint(Vdbe *p,
const char *zName);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index baeeb46..0912eaf 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -77,7 +77,7 @@ sqlVdbeCreate(Parse * pParse)
}
struct sql_txn *
-sql_alloc_txn(struct Vdbe *v)
+sql_alloc_txn()
{
struct sql_txn *txn = region_alloc_object(&fiber()->gc,
struct sql_txn);
@@ -86,7 +86,6 @@ sql_alloc_txn(struct Vdbe *v)
"struct sql_txn");
return NULL;
}
- txn->vdbe = v;
txn->pSavepoint = NULL;
txn->fk_deferred_count = 0;
return txn;
@@ -106,11 +105,10 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
* check FK violations, at least now.
*/
if (txn->psql_txn == NULL) {
- txn->psql_txn = sql_alloc_txn(vdbe);
+ txn->psql_txn = sql_alloc_txn();
if (txn->psql_txn == NULL)
return -1;
}
- txn->psql_txn->vdbe = vdbe;
}
return 0;
}
@@ -1997,7 +1995,7 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
}
int
-sql_txn_begin(Vdbe *p)
+sql_txn_begin()
{
if (in_txn()) {
diag_set(ClientError, ER_ACTIVE_TRANSACTION);
@@ -2006,7 +2004,7 @@ sql_txn_begin(Vdbe *p)
struct txn *ptxn = txn_begin(false);
if (ptxn == NULL)
return -1;
- ptxn->psql_txn = sql_alloc_txn(p);
+ ptxn->psql_txn = sql_alloc_txn();
if (ptxn->psql_txn == NULL) {
box_txn_rollback();
return -1;
diff --git a/src/box/txn.h b/src/box/txn.h
index a19becc..e51d5a2 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -118,8 +118,6 @@ struct sql_txn {
* VDBE to the next in the same transaction.
*/
uint32_t fk_deferred_count;
- /** Current VDBE. */
- struct Vdbe *vdbe;
};
/**
@@ -397,21 +395,6 @@ void
txn_on_stop(struct trigger *trigger, void *event);
/**
- * Return VDBE that is being currently executed.
- *
- * @retval VDBE that is being currently executed.
- * @retval NULL Either txn or ptxn_sql or vdbe is NULL;
- */
-static inline struct Vdbe *
-txn_vdbe()
-{
- struct txn *txn = in_txn();
- if (txn == NULL || txn->psql_txn == NULL)
- return NULL;
- return txn->psql_txn->vdbe;
-}
-
-/**
* FFI bindings: do not throw exceptions, do not accept extra
* arguments
*/
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread