[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