[tarantool-patches] Re: [PATCH v3 0/3] box: run checks on insertions in LUA spaces

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> Sorry,  meant: "Why can’t we simply do this…"
> 
> So this is suggestion how to fix redundant fields decoding.

Not exactly like this, but quite similar(see attachment below).

At first, the bytecode is compiled with vdbe_emit_ck_constraint
based on sqlExprIfTrue encoder. So, this code refers BY FIELDNO
to some fields in memory having the following layout:
[0 X 0 0 X X 0 0 0 0]
So we may bind only min_field_count fields, there the min_field_count
it the fieldno + 1 of the rightmost referenced field.

Actually we may use a bitmap of used fields and bind only them, but
managing such bitmap (that may not fit into uint64_t e.g. in general case)
is costly and there is no difference between binding NULL and field
while we must decode via mp_next all min_field_count tuple fields anyway. 

=================================================

diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
index 52f615227..a6352f005 100644
--- a/src/box/ck_constraint.c
+++ b/src/box/ck_constraint.c
@@ -48,12 +48,14 @@
  */
 static int
 ck_constraint_resolve_field_names(struct Expr *expr,
-				  struct space_def *space_def)
+				  struct space_def *space_def,
+				  uint32_t *min_field_count)
 {
 	struct Parse parser;
 	sql_parser_create(&parser, sql_get());
 	parser.parse_only = true;
-	sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
+	sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL,
+				   min_field_count);
 	int rc = parser.is_aborted ? -1 : 0;
 	sql_parser_destroy(&parser);
 	return rc;
@@ -150,7 +152,7 @@ ck_constraint_program_run(struct ck_constraint *ck_constraint,
 	 */
 	uint32_t tuple_field_count = mp_decode_array(&new_tuple);
 	uint32_t field_count =
-		MIN(tuple_field_count, space->def->field_count);
+		MIN(tuple_field_count, ck_constraint->min_field_count);
 	for (uint32_t i = 0; i < field_count; i++) {
 		struct sql_bind bind;
 		if (sql_bind_decode(&bind, i + 1, &new_tuple) != 0 ||
@@ -214,6 +216,7 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
 	ck_constraint->def = NULL;
 	ck_constraint->stmt = NULL;
 	ck_constraint->space_id = space_def->id;
+	ck_constraint->min_field_count = 0;
 	rlist_create(&ck_constraint->link);
 	trigger_create(&ck_constraint->trigger,
 		       ck_constraint_on_replace_trigger, ck_constraint,
@@ -222,7 +225,9 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
 		sql_expr_compile(sql_get(), ck_constraint_def->expr_str,
 				 strlen(ck_constraint_def->expr_str));
 	if (expr == NULL ||
-	    ck_constraint_resolve_field_names(expr, space_def) != 0) {
+	    ck_constraint_resolve_field_names(expr, space_def,
+					      &ck_constraint->
+						  min_field_count) != 0) {
 		diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
 			 ck_constraint_def->name,
 			 box_error_message(box_error_last()));
diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
index abbfd3a8f..89fdaf806 100644
--- a/src/box/ck_constraint.h
+++ b/src/box/ck_constraint.h
@@ -87,6 +87,8 @@ struct ck_constraint {
 	 * built for.
 	 */
 	uint32_t space_id;
+	/** The index of the rightmost referenced field. */
+	uint32_t min_field_count;
 	/**
 	 * Organize check constraint structs into linked list
 	 * with space::ck_constraint.
diff --git a/src/box/sql.h b/src/box/sql.h
index 4bb27da8e..7be1210cd 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -278,12 +278,15 @@ sql_expr_list_append(struct sql *db, struct ExprList *expr_list,
  * @param def The definition of space being referenced.
  * @param type NC_IsCheck or NC_IdxExpr.
  * @param expr Expression to resolve.  May be NULL.
- * @param expr_list Expression list to resolve.  May be NUL.
+ * @param expr_list Expression list to resolve.  May be NULL.
+ * @param min_field_count[out] Return the index of the rightmost
+ *                             referenced field.
  */
 void
 sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
 			   int type, struct Expr *expr,
-			   struct ExprList *expr_list);
+			   struct ExprList *expr_list,
+			   uint32_t *min_field_count);
 
 /**
  * Initialize a new parser object.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 3709f6b1d..b515f52a5 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2195,7 +2195,7 @@ index_fill_def(struct Parse *parse, struct index *index,
 	for (int i = 0; i < expr_list->nExpr; i++) {
 		struct Expr *expr = expr_list->a[i].pExpr;
 		sql_resolve_self_reference(parse, space_def, NC_IdxExpr,
-					   expr, 0);
+					   expr, NULL, NULL);
 		if (parse->is_aborted)
 			goto cleanup;
 
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e6d..76baaa142 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -556,8 +556,13 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 	case TK_ID:{
 			if ((pNC->ncFlags & NC_AllowAgg) != 0)
 				pNC->ncFlags |= NC_HasUnaggregatedId;
-			return lookupName(pParse, 0, pExpr->u.zToken, pNC,
-					  pExpr);
+			int rc = lookupName(pParse, 0, pExpr->u.zToken, pNC,
+					    pExpr);
+			assert(pExpr->iColumn >= 0);
+			pNC->min_field_count =
+				MAX(pNC->min_field_count,
+				    (uint32_t)pExpr->iColumn + 1);
+			return rc;
 		}
 
 		/* A table name and column name:     ID.ID
@@ -1611,7 +1616,8 @@ sqlResolveSelectNames(Parse * pParse,	/* The parser context */
 void
 sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
 			   int type, struct Expr *expr,
-			   struct ExprList *expr_list)
+			   struct ExprList *expr_list,
+			   uint32_t *min_field_count)
 {
 	/* Fake SrcList for parser->create_table_def */
 	SrcList sSrc;
@@ -1631,8 +1637,11 @@ sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
 	sNC.pParse = parser;
 	sNC.pSrcList = &sSrc;
 	sNC.ncFlags = type;
+	sNC.min_field_count = 0;
 	if (sqlResolveExprNames(&sNC, expr) != 0)
 		return;
 	if (expr_list != NULL)
 		sqlResolveExprListNames(&sNC, expr_list);
+	if (min_field_count != NULL)
+		*min_field_count = sNC.min_field_count;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 64f5f3d59..b2a354043 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2352,6 +2352,8 @@ struct NameContext {
 	int nRef;		/* Number of names resolved by this context */
 	int nErr;		/* Number of errors encountered while resolving names */
 	u16 ncFlags;		/* Zero or more NC_* flags defined below */
+	/** The rightmost tuple field referenced by AST. */
+	uint32_t min_field_count;
 };
 
 /*