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 316D92E52A for ; Sun, 19 May 2019 12:02:13 -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 SSgyqB1kZZwq for ; Sun, 19 May 2019 12:02:13 -0400 (EDT) Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (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 E5A482E581 for ; Sun, 19 May 2019 12:02:12 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v4 3/4] box: introduce column_mask for ck constraint References: <16fabe763ee57c8cd5c807b24d0fc500ad5a8bae.1558014727.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <148cfacb-757b-0c4a-7b7d-efaa18c6ffac@tarantool.org> Date: Sun, 19 May 2019 19:02:10 +0300 MIME-Version: 1.0 In-Reply-To: <16fabe763ee57c8cd5c807b24d0fc500ad5a8bae.1558014727.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Kirill Shcherbatov , tarantool-patches@freelists.org Thanks for the patch! See 3 comments below. On 16/05/2019 16:56, Kirill Shcherbatov wrote: > This patch introduces a smart bitmask of fields are mentioned in 1. Remove 'are'. 'mentioned' here works as adjective. The same in other places. Please, find them by yourself. > ck constraint. This allows to do not bind useless fields and > do not parse more fields that is required. > > Needed for #3691 > --- > src/box/ck_constraint.c | 66 ++++++++++++++++++++++++++++++++++------ > src/box/ck_constraint.h | 6 ++++ > src/box/sql.h | 5 ++- > src/box/sql/build.c | 2 +- > src/box/sql/resolve.c | 19 +++++++++--- > src/box/sql/sqlInt.h | 6 ++++ > test/sql/checks.result | 59 +++++++++++++++++++++++++++++++++++ > test/sql/checks.test.lua | 20 ++++++++++++ > 8 files changed, 167 insertions(+), 16 deletions(-) > > diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c > index 43798be76..4756f7970 100644 > --- a/src/box/ck_constraint.c > +++ b/src/box/ck_constraint.c > @@ -46,17 +46,21 @@ const char *ck_constraint_language_strs[] = {"SQL"}; > * tree traversal. > * @param ck_constraint Check constraint object to update. > * @param space_def Space definition to use. > + * @param column_mask[out] The "smart" column mask of fields are > + * referenced by AST. > * @retval 0 on success. > * @retval -1 on error. > */ > static int > ck_constraint_resolve_field_names(struct Expr *expr, > - struct space_def *space_def) > + struct space_def *space_def, > + uint64_t *column_mask) > { > struct Parse parser; > sql_parser_create(&parser, sql_get(), default_flags); > 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, > + column_mask); 2. Next to last argument 'expr_list' is always NULL. Please, drop it. > int rc = parser.is_aborted ? -1 : 0; > sql_parser_destroy(&parser); > return rc; > @@ -98,10 +105,24 @@ ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def, > * bind tuple fields there before execution. > */ > uint32_t field_count = space_def->field_count; > + /* > + * Use column mask to prepare binding only for > + * referenced fields. > + */ > int bind_tuple_reg = sqlGetTempRange(&parser, field_count); > - for (uint32_t i = 0; i < field_count; i++) { > + struct bit_iterator it; > + bit_iterator_init(&it, &column_mask, sizeof(uint64_t), true); > + size_t used_fieldno = bit_iterator_next(&it); > + for (; used_fieldno < SIZE_MAX; used_fieldno = bit_iterator_next(&it)) { > sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar, > - bind_tuple_reg + i); > + bind_tuple_reg + used_fieldno); > + } > + if (column_mask_fieldno_is_set(column_mask, 63)) { 3. Please, do not hardcode that value. Add a one-line function to column_mask.h for that. On other places too. > + used_fieldno = 64; > + for (; used_fieldno < (size_t)field_count; used_fieldno++) { > + sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar, > + bind_tuple_reg + used_fieldno); > + } > } > /* Generate ck constraint test code. */ > vdbe_emit_ck_constraint(&parser, expr, ck_constraint_def->name,