[patches] [PATCH] sql: remove unnecessary SCopy opcodes
Bulat Niatshin
niatshin at tarantool.org
Sat Mar 10 17:06:04 MSK 2018
Since OP_NoConflict opcode appears in INSERT/UPDATE VDBE listings
only when UNIQUE constraint check can't be handled by Tarantool
(after pushing 2255 to 2.0), related OP_SCopy should appear in
VDBE listing only when OP_NoConflict is present. This patch
contains a small fix for that.
Also there was a small refactoring in code which adds
OP_MakeRecord for INSERT/UPDATE in order to make that moment
clearer.
Fix for #2255.
---
Branch:
https://github.com/tarantool/tarantool/tree/bn/remove-useless-scopy
Issue:
There is no issue related to this commit, because this patch
contains only a small fix, which was forgotten in patch for 2255
(optimize UNIQUE constraints checks).
Brief overview:
The purpose of 2255 was to remove unnecessary OP_NoConflict
and MakeRecord opcodes from UPDATE/INSERT VDBE listing,
because only in special cases uniqueness is preserved by VDBE,
not by Tarantool. And OP_SCopy used to be a opcode, which copied
value in a separate register, which lately would be used by
OP_NoConflict.
Unnecessary OP_NoConflict opcodes were removed, but not all
OP_SCopy's. In this patch I prepared a small fix for that.
src/box/sql/insert.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 54fcca5c9..99fc27afb 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1240,14 +1240,24 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
VdbeComment((v, "%s column %d", pIdx->zName,
i));
} else {
- if (iField == pTab->iPKey) {
- x = regNewData;
- } else {
- x = iField + regNewData + 1;
+ /* OP_SCopy copies value in separate register,
+ * which lately will be used by OP_NoConflict.
+ * But OP_NoConflict is necessary only in cases
+ * when bytecode is needed for proper UNIQUE
+ * constraint handling.
+ */
+ if (uniqueByteCodeNeeded) {
+ if (iField == pTab->iPKey) {
+ x = regNewData;
+ } else {
+ x = iField + regNewData + 1;
+ }
+ assert(iField >= 0);
+ sqlite3VdbeAddOp2(v, OP_SCopy,
+ x, regIdx + i);
+ VdbeComment((v, "%s",
+ pTab->aCol[iField].zName));
}
- assert(iField >= 0);
- sqlite3VdbeAddOp2(v, OP_SCopy, x, regIdx + i);
- VdbeComment((v, "%s", pTab->aCol[iField].zName));
}
}
@@ -1272,14 +1282,15 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
sqlite3VdbeResolveLabel(v, skip_if_null);
}
}
- if (IsPrimaryKeyIndex(pIdx) || uniqueByteCodeNeeded) {
- sqlite3VdbeAddOp3(v, OP_MakeRecord, regNewData + 1,
+ if (uniqueByteCodeNeeded) {
+ sqlite3VdbeAddOp3(v, OP_MakeRecord,
+ regNewData + 1,
pTab->nCol, aRegIdx[ix]);
VdbeComment((v, "for %s", pIdx->zName));
}
} else {
/* kyukhin: for Tarantool, this should be evaluated to NOP. */
- if (IsPrimaryKeyIndex(pIdx) || uniqueByteCodeNeeded) {
+ if (uniqueByteCodeNeeded) {
sqlite3VdbeAddOp3(v, OP_MakeRecord, regIdx,
pIdx->nColumn, aRegIdx[ix]);
VdbeComment((v, "for %s", pIdx->zName));
--
2.14.1
More information about the Tarantool-patches
mailing list