From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org, Nikita Pettik <korablev@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 05/10] sql: remove affinity string of columns from Index Date: Mon, 13 Aug 2018 23:24:30 +0300 [thread overview] Message-ID: <9ad38333-305b-7c55-a682-6b473d76e4f4@tarantool.org> (raw) In-Reply-To: <026cd8e04034ad26529a2b8e76e1ae9aabf9076f.1534001739.git.korablev@tarantool.org> Thanks for the patch! I have pushed my review fixes in a separate commit. Also see 3 comments below. On 12/08/2018 17:13, Nikita Pettik wrote: > Part of #3561 > --- > src/box/sql/build.c | 1 - > src/box/sql/delete.c | 3 ++- > src/box/sql/insert.c | 42 +++++++++++------------------------------- > src/box/sql/sqliteInt.h | 16 +++++++++++----- > src/box/sql/update.c | 2 +- > src/box/sql/vdbemem.c | 3 +-- > src/box/sql/where.c | 21 ++++++++------------- > src/box/sql/wherecode.c | 3 ++- > 8 files changed, 36 insertions(+), 55 deletions(-) > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > index 0be68830a..cb16a7c0d 100644 > --- a/src/box/sql/delete.c > +++ b/src/box/sql/delete.c > @@ -352,7 +352,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > key_len = 0; > struct Index *pk = sqlite3PrimaryKeyIndex(table); > const char *zAff = is_view ? NULL : > - sqlite3IndexAffinityStr(parse->db, pk); > + sql_index_affinity_str(parse->db, > + pk->def); 1. sql_index_affinity_str makes extra lookup in space cache. Here (in sql_table_delete_from) space ptr is known already. Same in fkey_lookup_parent. Maybe it would be better to pass space ptr alongside with index_def? And name it sql_space_index_affinity_str. > sqlite3VdbeAddOp4(v, OP_MakeRecord, reg_pk, pk_len, > reg_key, zAff, pk_len); > /* Set flag to save memory allocating one > diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c > index ddd7f932b..2b6100ad5 100644 > --- a/src/box/sql/insert.c > +++ b/src/box/sql/insert.c > @@ -112,15 +84,23 @@ sql_index_affinity_str(struct sqlite3 *db, struct index_def *def) > return NULL; > struct space *space = space_by_id(def->space_id); > assert(space != NULL); > - > + /* > + * Table may occasionally come from Lua, so lets > + * gentle process this case by setting default > + * affinity for it. > + */ > + if (space->def->fields == NULL) { > + memset(aff, AFFINITY_BLOB, column_count); > + goto exit; > + } > for (uint32_t i = 0; i < column_count; i++) { > uint32_t x = def->key_def->parts[i].fieldno; > aff[i] = space->def->fields[x].affinity; > if (aff[i] == AFFINITY_UNDEFINED) > - aff[i] = 'A'; > + aff[i] = AFFINITY_BLOB; > } > +exit: 2. Please, try to avoid extra labels when possible. They might help only when there are many error processing code snippets or when it allows to strongly reduce indentation. Otherwise it slightly confuses. I have fixed it on the branch. > aff[column_count] = '\0'; > - > return aff; > } > > diff --git a/src/box/sql/where.c b/src/box/sql/where.c > index db8d0bf80..794db0302 100644 > --- a/src/box/sql/where.c > +++ b/src/box/sql/where.c > @@ -1224,7 +1220,7 @@ whereRangeSkipScanEst(Parse * pParse, /* Parsing & code generating context */ > int nLower = -1; > int nUpper = index->def->opts.stat->sample_count + 1; > int rc = SQLITE_OK; > - u8 aff = sqlite3IndexColumnAffinity(db, p, nEq); > + u8 aff = sql_index_part_affinity(p->def, nEq); 3. Same as 1 - extra lookup in the space cache. You have the space few lines above. > > sqlite3_value *p1 = 0; /* Value extracted from pLower */ > sqlite3_value *p2 = 0; /* Value extracted from pUpper */
next prev parent reply other threads:[~2018-08-13 20:24 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-12 14:12 [tarantool-patches] [PATCH 00/10] sql: cleanup in struct Index and struct Table Nikita Pettik 2018-08-12 14:12 ` [tarantool-patches] [PATCH 01/10] sql: remove suport of ALTER TABLE ADD COLUMN Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-12 14:12 ` [tarantool-patches] [PATCH 02/10] sql: remove string of fields collation from Table Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-12 14:12 ` [tarantool-patches] [PATCH 03/10] sql: remove index hash from struct Table Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-12 14:13 ` [tarantool-patches] [PATCH 04/10] sql: remove flags " Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-12 14:13 ` [tarantool-patches] [PATCH 05/10] sql: remove affinity string of columns from Index Nikita Pettik 2018-08-13 20:24 ` Vladislav Shpilevoy [this message] 2018-08-21 16:31 ` [tarantool-patches] " n.pettik 2018-08-24 21:04 ` Vladislav Shpilevoy 2018-08-26 19:45 ` n.pettik 2018-08-12 14:13 ` [tarantool-patches] [PATCH 06/10] sql: completely remove support of partial indexes Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-21 16:31 ` n.pettik 2018-08-24 21:04 ` Vladislav Shpilevoy 2018-08-26 19:44 ` n.pettik 2018-08-12 14:13 ` [tarantool-patches] [PATCH 07/10] sql: remove index type from struct Index Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-21 16:31 ` n.pettik 2018-08-12 14:13 ` [tarantool-patches] [PATCH 08/10] sql: use secondary indexes to process OP_Delete Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-12 14:13 ` [tarantool-patches] [PATCH 09/10] sql: disable ON CONFLICT actions for indexes Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-21 16:31 ` n.pettik 2018-08-24 21:04 ` Vladislav Shpilevoy 2018-08-26 19:44 ` n.pettik 2018-08-27 17:24 ` Vladislav Shpilevoy 2018-08-12 14:13 ` [tarantool-patches] [PATCH 10/10] sql: move autoincrement field number to server Nikita Pettik 2018-08-13 20:24 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-21 16:31 ` n.pettik 2018-08-24 21:03 ` Vladislav Shpilevoy 2018-08-26 19:44 ` n.pettik 2018-08-27 17:24 ` Vladislav Shpilevoy 2018-08-27 17:24 ` [tarantool-patches] Re: [PATCH 00/10] sql: cleanup in struct Index and struct Table Vladislav Shpilevoy 2018-08-29 14:11 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9ad38333-305b-7c55-a682-6b473d76e4f4@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 05/10] sql: remove affinity string of columns from Index' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox