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 34E2928A18 for ; Tue, 21 Aug 2018 12:31:39 -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 RUqu6oC-uXwu for ; Tue, 21 Aug 2018 12:31:39 -0400 (EDT) Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 D98F82877E for ; Tue, 21 Aug 2018 12:31:38 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH 09/10] sql: disable ON CONFLICT actions for indexes From: "n.pettik" In-Reply-To: <317b019c-61ad-83f4-5a47-3818e90fa4a8@tarantool.org> Date: Tue, 21 Aug 2018 19:31:37 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <75ebea5a7bc472e83a844cc98fc6fb88369e4b10.1534001739.git.korablev@tarantool.org> <317b019c-61ad-83f4-5a47-3818e90fa4a8@tarantool.org> 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 Cc: Vladislav Shpilevoy >> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c >> index cb120384a..271886ec6 100644 >> --- a/src/box/sql/insert.c >> +++ b/src/box/sql/insert.c >> +vdbe_emit_constraint_checks(struct Parse *parse_context, struct = Table *tab, >> + int new_tuple_reg, >> + enum on_conflict_action on_conflict, >> + int ignore_label, int *upd_cols) >> { >> - Vdbe *v; /* VDBE under constrution */ >> - Index *pIdx; /* Pointer to one of the indices */ >> - Index *pPk =3D 0; /* The PRIMARY KEY index */ >> - sqlite3 *db; /* Database connection */ >> - int addr1; /* Address of jump instruction */ >> - int seenReplace =3D 0; /* True if REPLACE is used to resolve = INT PK conflict */ >> - u8 isUpdate; /* True if this is an UPDATE operation = */ >> - u8 bAffinityDone =3D 0; /* True if the OP_Affinity operation has = been run */ >> struct session *user_session =3D current_session(); >> - >> - isUpdate =3D regOldData !=3D 0; >> - db =3D pParse->db; >> - v =3D sqlite3GetVdbe(pParse); >> - assert(v !=3D 0); >> - struct space_def *def =3D pTab->def; >> - /* This table is not a VIEW */ >> + struct sqlite3 *db =3D parse_context->db; >> + struct Vdbe *v =3D sqlite3GetVdbe(parse_context); >> + assert(v !=3D NULL); >> + struct space_def *def =3D tab->def; >> + /* Insertion into VIEW is prohibited. */ >> assert(!def->opts.is_view); >> - >> - pPk =3D sqlite3PrimaryKeyIndex(pTab); >> - >> - /* Record that this module has started */ >> - VdbeModuleComment((v, "BEGIN: GenCnstCks(%d,%d,%d,%d,%d)", >> - iDataCur, iIdxCur, regNewData, regOldData, = pkChng)); >> - >> - enum on_conflict_action override_error =3D = on_conflict->override_error; >> - enum on_conflict_action on_error; >> - /* Test all NOT NULL constraints. >> - */ >> + bool is_update =3D upd_cols !=3D NULL; >> + /* Test all NOT NULL constraints. */ >> for (uint32_t i =3D 0; i < def->field_count; i++) { >> - if (aiChng && aiChng[i] < 0) { >> - /* Don't bother checking for NOT NULL on columns = that do not change */ >> + /* >> + * Don't bother checking for NOT NULL on columns >> + * that do not change. >> + */ >> + if (is_update && upd_cols[i] < 0) >> continue; >> - } >> - if (def->fields[i].is_nullable || pTab->iAutoIncPKey =3D=3D= (int) i) >> - continue; /* This column is allowed to be = NULL */ >> - >> - on_error =3D table_column_nullable_action(pTab, i); >> - if (override_error !=3D ON_CONFLICT_ACTION_DEFAULT) >> - on_error =3D override_error; >> - /* ABORT is a default error action */ >> - if (on_error =3D=3D ON_CONFLICT_ACTION_DEFAULT) >> - on_error =3D ON_CONFLICT_ACTION_ABORT; >> - >> - struct Expr *dflt =3D NULL; >> - dflt =3D space_column_default_expr(pTab->def->id, i); >> - if (on_error =3D=3D ON_CONFLICT_ACTION_REPLACE && dflt = =3D=3D 0) >> - on_error =3D ON_CONFLICT_ACTION_ABORT; >> - >> - assert(on_error !=3D ON_CONFLICT_ACTION_NONE); >> - switch (on_error) { >> + /* This column is allowed to be NULL. */ >> + if (def->fields[i].is_nullable || tab->iAutoIncPKey =3D=3D= (int) i) >> + continue; >> + enum on_conflict_action on_conflict_nullable =3D >> + on_conflict !=3D ON_CONFLICT_ACTION_DEFAULT ? >> + on_conflict : table_column_nullable_action(tab, = i); >=20 > 1. Can you remove table_column_nullable_action? It is the single place = of its > usage. You can do space_by_id once at the begin of the function and = use > the space explicitly both here and below for space_checks_expr_list = (that > also would be removed in such a case). Ok, surely. Space is not even required here, we can fetch nullable = action from table->def: +++ b/src/box/sql/insert.c @@ -859,7 +859,7 @@ vdbe_emit_constraint_checks(struct Parse = *parse_context, struct Table *tab, continue; enum on_conflict_action on_conflict_nullable =3D on_conflict !=3D ON_CONFLICT_ACTION_DEFAULT ? - on_conflict : table_column_nullable_action(tab, = i); + on_conflict : = tab->def->fields[i].nullable_action; @@ -4796,9 +4858,6 @@ extern int sqlSubProgramsRemaining; =20 extern int sqlite3InitDatabase(sqlite3 * db); =20 -enum on_conflict_action -table_column_nullable_action(struct Table *tab, uint32_t column); - +++ b/src/box/sql/vdbeaux.c @@ -3869,30 +3869,3 @@ sqlite3VdbeRecordUnpackMsgpack(struct key_def = *key_def, /* Information about the pMem++; } } - -/** - * Return action on nullable constraint violation of given column in = given table. - * FIXME: This is implemented in expensive way. For each invocation = table lookup - * is performed. In future, first param will be replaced with pointer = to struct - * space. - * - * @param tab pointer to the table - * @param column column number for which action to be returned - * @retval return action found for given column - */ -enum on_conflict_action -table_column_nullable_action(struct Table *tab, uint32_t column) -{ - struct space *space =3D space_cache_find(tab->def->id); - - assert(space !=3D NULL); - - struct tuple_format *format =3D space->format; - - assert(format !=3D NULL); - assert(format->field_count > column); - - struct tuple_field field =3D format->fields[column]; - - return field.nullable_action; -} >> --- a/src/box/sql/update.c >> +++ b/src/box/sql/update.c >> @@ -152,34 +144,17 @@ sqlite3Update(Parse * pParse, /* The = parser context */ >> } >> struct space_def *def =3D pTab->def; >> - >> - /* Allocate a cursors for the main database table and for all = indices. >> - * The index cursors might not be used, but if they are used = they >> - * need to occur right after the database cursor. So go ahead = and >> - * allocate enough space, just in case. >> - */ >> - pTabList->a[0].iCursor =3D iBaseCur =3D iDataCur =3D = pParse->nTab++; >> - iIdxCur =3D iDataCur + 1; >> - pPk =3D is_view ? 0 : sqlite3PrimaryKeyIndex(pTab); >> - for (nIdx =3D 0, pIdx =3D pTab->pIndex; pIdx; pIdx =3D = pIdx->pNext, nIdx++) { >> - if (sql_index_is_primary(pIdx) && pPk !=3D 0) { >> - iDataCur =3D pParse->nTab; >> - pTabList->a[0].iCursor =3D iDataCur; >> - } >> - pParse->nTab++; >> - } >> - >> - /* Allocate space for aXRef[], aRegIdx[], and aToOpen[]. >> - * Initialize aXRef[] and aToOpen[] to their default values. >> - */ >> - aXRef =3D sqlite3DbMallocRawNN(db, sizeof(int) * >> - (def->field_count + nIdx) + nIdx + = 2); >> - if (aXRef =3D=3D 0) >> + int nIdx =3D 0; >> + for (struct Index *idx =3D pTab->pIndex; idx !=3D NULL; >> + idx =3D idx->pNext, nIdx++); >=20 > 2. In the same function below we do space_by_id from the > result of which we can get space->index_count. It is not? Well, it is useless code, actually. I forgot to remove it: +++ b/src/box/sql/update.c @@ -144,9 +144,6 @@ sqlite3Update(Parse * pParse, /* The = parser context */ } =20 struct space_def *def =3D pTab->def; - int nIdx =3D 0; - for (struct Index *idx =3D pTab->pIndex; idx !=3D NULL; - idx =3D idx->pNext, nIdx++);