[tarantool-patches] [PATCH v1 2/4] sql: use 64b bitmasks instead of 32b where possible

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

In some cases(like foreign keys) the SQL code is still used
32-bit bit mask, while 64-bit bit masks will perform better
column optimizations. There was refactored code to work with 64b
bitmasks where required.
The 32b bitmasks are still used to specify constant OP_Function
arguments because this change would require changing the P1 type
of the VDBE p1 argument, which is not desirable. Moreover, the
64 function's arguments is an explicit overkill.

Part of #3571
---
 src/box/alter.cc        | 10 ++-------
 src/box/sql/delete.c    |  8 +++----
 src/box/sql/resolve.c   | 19 ++++++-----------
 src/box/sql/sqliteInt.h | 46 +++++++++++++++++++++++------------------
 src/box/sql/trigger.c   | 14 ++++++-------
 src/box/sql/update.c    | 21 ++++++++-----------
 6 files changed, 53 insertions(+), 65 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9656a4189..cdc7aa0d7 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 #include "alter.h"
+#include "column_mask.h"
 #include "schema.h"
 #include "user.h"
 #include "space.h"
@@ -3732,13 +3733,6 @@ fkey_grab_by_name(struct rlist *list, const char *fkey_name)
 	return NULL;
 }
 
-/**
- * FIXME: as SQLite legacy temporary we use such mask throught
- * SQL code. It should be replaced later with regular
- * mask from column_mask.h
- */
-#define FKEY_MASK(x) (((x)>31) ? 0xffffffff : ((uint64_t)1<<(x)))
-
 /**
  * Set bits of @mask which correspond to fields involved in
  * given foreign key constraint.
@@ -3752,7 +3746,7 @@ static inline void
 fkey_set_mask(const struct fkey *fk, uint64_t *mask, int type)
 {
 	for (uint32_t i = 0; i < fk->def->field_count; ++i)
-		*mask |= FKEY_MASK(fk->def->links[i].fields[type]);
+		column_mask_set_fieldno(mask, fk->def->links[i].fields[type]);
 }
 
 /**
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f9c42fdec..5e134ab0c 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -468,7 +468,7 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
 	    (fkey_is_required(table->def->id, NULL) || trigger_list != NULL)) {
 		/* Mask of OLD.* columns in use */
 		/* TODO: Could use temporary registers here. */
-		uint32_t mask =
+		uint64_t mask =
 			sql_trigger_colmask(parse, trigger_list, 0, 0,
 					    TRIGGER_BEFORE | TRIGGER_AFTER,
 					    table, onconf);
@@ -484,10 +484,8 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
 		 */
 		sqlite3VdbeAddOp2(v, OP_Copy, reg_pk, first_old_reg);
 		for (int i = 0; i < (int)table->def->field_count; i++) {
-			testcase(mask != 0xffffffff && iCol == 31);
-			testcase(mask != 0xffffffff && iCol == 32);
-			if (mask == 0xffffffff
-			    || (i <= 31 && (mask & MASKBIT32(i)) != 0)) {
+			if (i >= COLUMN_MASK_SIZE ||
+			    column_mask_fieldno_is_set(mask, i)) {
 				sqlite3ExprCodeGetColumnOfTable(v, table->def,
 								cursor, i,
 								first_old_reg +
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 6462467bc..1f6ba0418 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -341,20 +341,13 @@ lookupName(Parse * pParse,	/* The parsing context */
 					if (iCol < 0) {
 						pExpr->affinity =
 							AFFINITY_INTEGER;
-					} else if (pExpr->iTable == 0) {
-						testcase(iCol == 31);
-						testcase(iCol == 32);
-						pParse->oldmask |=
-						    (iCol >=
-						     32 ? 0xffffffff
-						     : (((u32) 1) << iCol));
 					} else {
-						testcase(iCol == 31);
-						testcase(iCol == 32);
-						pParse->newmask |=
-						    (iCol >=
-						     32 ? 0xffffffff
-						     : (((u32) 1) << iCol));
+						uint64_t *mask =
+							pExpr->iTable == 0 ?
+							&pParse->oldmask :
+							&pParse->newmask;
+						column_mask_set_fieldno(mask,
+									iCol);
 					}
 					pExpr->iColumn = (i16) iCol;
 					pExpr->space_def = pTab->def;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ee24e0337..4280c1938 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -67,6 +67,7 @@
 
 #include <stdbool.h>
 
+#include "box/column_mask.h"
 #include "box/field_def.h"
 #include "box/sql.h"
 #include "box/txn.h"
@@ -2630,22 +2631,24 @@ struct SelectDest {
 #endif
 
 /*
- * At least one instance of the following structure is created for each
- * trigger that may be fired while parsing an INSERT, UPDATE or DELETE
- * statement. All such objects are stored in the linked list headed at
- * Parse.pTriggerPrg and deleted once statement compilation has been
- * completed.
- *
- * A Vdbe sub-program that implements the body and WHEN clause of trigger
- * TriggerPrg.pTrigger, assuming a default ON CONFLICT clause of
- * TriggerPrg.orconf, is stored in the TriggerPrg.pProgram variable.
- * The Parse.pTriggerPrg list never contains two entries with the same
- * values for both pTrigger and orconf.
- *
- * The TriggerPrg.aColmask[0] variable is set to a mask of old.* columns
- * accessed (or set to 0 for triggers fired as a result of INSERT
- * statements). Similarly, the TriggerPrg.aColmask[1] variable is set to
- * a mask of new.* columns used by the program.
+ * At least one instance of the following structure is created for
+ * each trigger that may be fired while parsing an INSERT, UPDATE
+ * or DELETE statement. All such objects are stored in the linked
+ * list headed at Parse.pTriggerPrg and deleted once statement
+ * compilation has been completed.
+ *
+ * A Vdbe sub-program that implements the body and WHEN clause of
+ * trigger TriggerPrg.pTrigger, assuming a default ON CONFLICT
+ * clause of TriggerPrg.orconf, is stored in the
+ * TriggerPrg.pProgram variable. The Parse.pTriggerPrg list never
+ * contains two entries with the same values for both pTrigger
+ * and orconf.
+ *
+ * The TriggerPrg.column_mask[0] variable is set to a mask of
+ * old.* columns accessed (or set to 0 for triggers fired as a
+ * result of INSERT statements). Similarly, the
+ * TriggerPrg.column_mask[1] variable is set to a mask of new.*
+ * columns used by the program.
  */
 struct TriggerPrg {
 	/** Trigger this program was coded from. */
@@ -2653,7 +2656,8 @@ struct TriggerPrg {
 	TriggerPrg *pNext;	/* Next entry in Parse.pTriggerPrg list */
 	SubProgram *pProgram;	/* Program implementing pTrigger/orconf */
 	int orconf;		/* Default ON CONFLICT policy */
-	u32 aColmask[2];	/* Masks of old.*, new.* columns accessed */
+	/* Masks of old.*, new.* columns accessed. */
+	uint64_t column_mask[2];
 };
 
 enum ast_type {
@@ -2735,8 +2739,10 @@ struct Parse {
 	Parse *pToplevel;	/* Parse structure for main program (or NULL) */
 	Table *pTriggerTab;	/* Table triggers are being coded for */
 	u32 nQueryLoop;		/* Est number of iterations of a query (10*log2(N)) */
-	u32 oldmask;		/* Mask of old.* columns referenced */
-	u32 newmask;		/* Mask of new.* columns referenced */
+	/* Mask of old.* columns referenced. */
+	uint64_t oldmask;
+	/* Mask of new.* columns referenced. */
+	uint64_t newmask;
 	u8 eTriggerOp;		/* TK_UPDATE, TK_INSERT or TK_DELETE */
 	u8 eOrconf;		/* Default ON CONFLICT policy for trigger steps */
 	/** Region to make SQL temp allocations. */
@@ -4124,7 +4130,7 @@ TriggerStep *sqlite3TriggerDeleteStep(sqlite3 *, Token *, Expr *);
  *
  * @retval mask value.
  */
-u32
+uint64_t
 sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
 		    ExprList *changes_list, int new, int tr_tm,
 		    Table *table, int orconf);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 7d5dc9e23..039350550 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -786,8 +786,8 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 	sqlite3VdbeLinkSubProgram(pTop->pVdbe, pProgram);
 	pPrg->trigger = trigger;
 	pPrg->orconf = orconf;
-	pPrg->aColmask[0] = 0xffffffff;
-	pPrg->aColmask[1] = 0xffffffff;
+	pPrg->column_mask[0] = COLUMN_MASK_FULL;
+	pPrg->column_mask[1] = COLUMN_MASK_FULL;
 
 	/*
 	 * Allocate and populate a new Parse context to use for
@@ -859,8 +859,8 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 		pProgram->nMem = pSubParse->nMem;
 		pProgram->nCsr = pSubParse->nTab;
 		pProgram->token = (void *)trigger;
-		pPrg->aColmask[0] = pSubParse->oldmask;
-		pPrg->aColmask[1] = pSubParse->newmask;
+		pPrg->column_mask[0] = pSubParse->oldmask;
+		pPrg->column_mask[1] = pSubParse->newmask;
 		sqlite3VdbeDelete(v);
 	}
 
@@ -978,13 +978,13 @@ vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 	}
 }
 
-u32
+uint64_t
 sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
 		    ExprList *changes_list, int new, int tr_tm,
 		    Table *table, int orconf)
 {
 	const int op = changes_list != NULL ? TK_UPDATE : TK_DELETE;
-	u32 mask = 0;
+	uint64_t mask = 0;
 
 	assert(new == 1 || new == 0);
 	for (struct sql_trigger *p = trigger; p != NULL; p = p->next) {
@@ -993,7 +993,7 @@ sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
 			TriggerPrg *prg =
 				sql_row_trigger(parser, p, table, orconf);
 			if (prg != NULL)
-				mask |= prg->aColmask[new];
+				mask |= prg->column_mask[new];
 		}
 	}
 
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index dbef22fd8..50b81c188 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -101,7 +101,6 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 	/* List of triggers on pTab, if required. */
 	struct sql_trigger *trigger;
 	int tmask;		/* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
-	int newmask;		/* Mask of NEW.* columns accessed by BEFORE triggers */
 	int iEph = 0;		/* Ephemeral table holding all primary key values */
 	int nKey = 0;		/* Number of elements in regKey */
 	int aiCurOnePass[2];	/* The write cursors opened by WHERE_ONEPASS */
@@ -335,15 +334,14 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 	if (is_pk_modified || hasFK != 0 || trigger != NULL) {
 		struct space *space = space_by_id(pTab->def->id);
 		assert(space != NULL);
-		u32 oldmask = hasFK ? space->fkey_mask : 0;
+		uint64_t oldmask = hasFK ? space->fkey_mask : 0;
 		oldmask |= sql_trigger_colmask(pParse, trigger, pChanges, 0,
 					       TRIGGER_BEFORE | TRIGGER_AFTER,
 					       pTab, on_error);
 		for (i = 0; i < (int)def->field_count; i++) {
-			if (oldmask == 0xffffffff
-			    || (i < 32 && (oldmask & MASKBIT32(i)) != 0) ||
-				sql_space_column_is_in_pk(pTab->space, i)) {
-				testcase(oldmask != 0xffffffff && i == 31);
+			if (i >= COLUMN_MASK_SIZE ||
+			    column_mask_fieldno_is_set(oldmask, i) ||
+			    sql_space_column_is_in_pk(pTab->space, i)) {
 				sqlite3ExprCodeGetColumnOfTable(v, def,
 								pk_cursor, i,
 								regOld + i);
@@ -367,22 +365,21 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 	 * may have modified them). So not loading those that are not going to
 	 * be used eliminates some redundant opcodes.
 	 */
-	newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1,
-				      TRIGGER_BEFORE, pTab, on_error);
+	uint64_t newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1,
+					       TRIGGER_BEFORE, pTab, on_error);
 	for (i = 0; i < (int)def->field_count; i++) {
 		j = aXRef[i];
 		if (j >= 0) {
 			sqlite3ExprCode(pParse, pChanges->a[j].pExpr,
 					regNew + i);
-		} else if (0 == (tmask & TRIGGER_BEFORE) || i > 31
-			   || (newmask & MASKBIT32(i))) {
+		} else if ((tmask & TRIGGER_BEFORE) == 0 ||
+			   i >= COLUMN_MASK_SIZE ||
+			   column_mask_fieldno_is_set(newmask, i) != 0) {
 			/* This branch loads the value of a column that will not be changed
 			 * into a register. This is done if there are no BEFORE triggers, or
 			 * if there are one or more BEFORE triggers that use this value via
 			 * a new.* reference in a trigger program.
 			 */
-			testcase(i == 31);
-			testcase(i == 32);
 			sqlite3ExprCodeGetColumnToReg(pParse, def, i,
 						      pk_cursor, regNew + i);
 		} else {
-- 
2.20.1