From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 4E89D2D8A9 for ; Wed, 15 May 2019 04:37:08 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Pb_z1OO86SjL for ; Wed, 15 May 2019 04:37:08 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 0535D2C9BF for ; Wed, 15 May 2019 04:37:07 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 0/3] box: run checks on insertions in LUA spaces References: <20190514170050.GB5201@atlas> <60C9520B-20B2-4944-B44E-99B6A3E454EB@tarantool.org> <20190514184106.GC5201@atlas> <35C5E394-EC1A-421B-8868-F4451CC96DE1@tarantool.org> From: Kirill Shcherbatov Message-ID: Date: Wed, 15 May 2019 11:37:03 +0300 MIME-Version: 1.0 In-Reply-To: <35C5E394-EC1A-421B-8868-F4451CC96DE1@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, Nikita Pettik , Konstantin Osipov > 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; }; /*