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 97CDA260A6 for ; Fri, 8 Jun 2018 04:25:38 -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 E7PniqaeN1iz for ; Fri, 8 Jun 2018 04:25:38 -0400 (EDT) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 51F2B26091 for ; Fri, 8 Jun 2018 04:25:37 -0400 (EDT) Date: Fri, 8 Jun 2018 11:25:30 +0300 From: Kirill Yukhin Subject: [tarantool-patches] Re: [PATCH 1/3] sql: fetch primary index for affinity only Message-ID: <20180608082530.iymkyugaizlpuluj@tarantool.org> References: <5bc1cc0b-529b-62b7-6ed3-36f657db1cd2@tarantool.org> <20180607120324.h2laztbxfnhnpe2s@tarantool.org> <5b1597db-8cba-1c71-2337-5d570fa3bc3a@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5b1597db-8cba-1c71-2337-5d570fa3bc3a@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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org Hello Vlad, Updated patch in the bottom. On 07 июн 18:01, Vladislav Shpilevoy wrote: > Hello. Thanks for the fixes! See one minor comment below. > > And I do not see the new commit on the branch. Branch still has the > old one + 2 my review fixes commits. Pushed. > > -- > > Regards, Kirill Yukhin > > > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > > index ddad54b..a4a8da6 100644 > > --- a/src/box/sql/delete.c > > +++ b/src/box/sql/delete.c > > @@ -325,8 +338,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > > (void *)space, P4_SPACEPTR); > > sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, > > table->tnum, space_ptr_reg); > > - sql_vdbe_set_p4_key_def(parse, pk); > > - VdbeComment((v, "%s", pk->zName)); > > + struct key_def *def = key_def_dup(pk_def); > > + if (def == NULL) { > > + sqlite3OomFault(parse->db); > > + goto delete_from_cleanup; > > + } > > + sqlite3VdbeAppendP4(v, def, P4_KEYDEF); > Why could not you at first dup key_def, and then just use AddOp4? Instead of > AddOp3 + dup + AppendP4. Done. -- Regards, Kirill Yukhin commit 34e707406484a144e538c05677e1e6ea5a819fd3 Author: Kirill Yukhin Date: Wed May 23 15:31:37 2018 +0300 sql: fetch primary index for affinity only This small patch removes usages of primary index throughout code sql_table_delete_from, limiting use to fetching of affinities only. We cannot use space_def here, since primary index might contain calculated columns. Part of #3235 diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index ddad54b..eff4da1 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -76,6 +76,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, struct Expr *where) { struct sqlite3 *db = parse->db; + struct Table loc_table; if (parse->nErr || db->mallocFailed) goto delete_from_cleanup; @@ -183,6 +184,12 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, struct NameContext nc; memset(&nc, 0, sizeof(nc)); nc.pParse = parse; + if (tab_list->a[0].pTab == NULL) { + assert(space != NULL); + memset(&loc_table, 0, sizeof(struct Table)); + loc_table.def = space->def; + tab_list->a[0].pTab = &loc_table; + } nc.pSrcList = tab_list; if (sqlite3ResolveExprNames(&nc, where)) goto delete_from_cleanup; @@ -196,7 +203,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, * is held in ephemeral table, there is no PK for * it, so columns should be loaded manually. */ - struct Index *pk = NULL; + struct key_def *pk_def = NULL; int reg_pk = parse->nMem + 1; int pk_len; int eph_cursor = parse->nTab++; @@ -207,13 +214,17 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, eph_cursor, pk_len); } else { - pk = sqlite3PrimaryKeyIndex(table); - assert(pk != NULL); - pk_len = index_column_count(pk); + assert(space->index_count > 0); + pk_def = key_def_dup(space->index[0]->def->key_def); + if(pk_def == NULL) { + sqlite3OomFault(parse->db); + goto delete_from_cleanup; + } + pk_len = pk_def->part_count; parse->nMem += pk_len; - sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, eph_cursor, - pk_len); - sql_vdbe_set_p4_key_def(parse, pk); + sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, eph_cursor, + pk_len, 0, + (char *)pk_def, P4_KEYDEF); } /* Construct a query to find the primary key for @@ -252,11 +263,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, /* Extract the primary key for the current row */ if (!is_view) { for (int i = 0; i < pk_len; i++) { - assert(pk->aiColumn[i] >= 0); sqlite3ExprCodeGetColumnOfTable(v, table->def, tab_cursor, - pk-> - aiColumn[i], + pk_def->parts[i].fieldno, reg_pk + i); } } else { @@ -287,10 +296,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, * key. */ key_len = 0; - const char *zAff = is_view ? NULL : - sqlite3IndexAffinityStr(parse->db, pk); + const char *aff = NULL; + struct Index *pk = sqlite3PrimaryKeyIndex(table); + if (pk != NULL) { + aff = is_view ? NULL : + sqlite3IndexAffinityStr(parse->db, pk); + } sqlite3VdbeAddOp4(v, OP_MakeRecord, reg_pk, pk_len, - reg_key, zAff, pk_len); + reg_key, aff, pk_len); /* Set flag to save memory allocating one * by malloc. */ @@ -323,10 +336,16 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, int space_ptr_reg = ++parse->nMem; sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void *)space, P4_SPACEPTR); - sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, - table->tnum, space_ptr_reg); - sql_vdbe_set_p4_key_def(parse, pk); - VdbeComment((v, "%s", pk->zName)); + struct key_def *def = key_def_dup(pk_def); + if (def == NULL) { + sqlite3OomFault(parse->db); + goto delete_from_cleanup; + } + sqlite3VdbeAddOp4(v, OP_OpenWrite, tab_cursor, + table->tnum, space_ptr_reg, + (void *)def, P4_KEYDEF); + + VdbeComment((v, "%s", space->index[0]->def->name)); if (one_pass == ONEPASS_MULTI) sqlite3VdbeJumpHere(v, iAddrOnce); @@ -339,7 +358,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, if (one_pass != ONEPASS_OFF) { /* OP_Found will use an unpacked key. */ assert(key_len == pk_len); - assert(pk != NULL || table->pSelect != NULL); + assert(pk_def != NULL || table->pSelect != NULL); sqlite3VdbeAddOp4Int(v, OP_NotFound, tab_cursor, addr_bypass, reg_key, key_len);