[tarantool-patches] Re: [PATCH v3 0/3] box: run checks on insertions in LUA spaces
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed May 15 11:37:03 MSK 2019
> 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;
};
/*
More information about the Tarantool-patches
mailing list