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 7CBC72947A for ; Tue, 21 Aug 2018 12:31:20 -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 WBTsfKklhE5L for ; Tue, 21 Aug 2018 12:31:20 -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 B2DF429359 for ; Tue, 21 Aug 2018 12:31:19 -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 05/10] sql: remove affinity string of columns from Index From: "n.pettik" In-Reply-To: <9ad38333-305b-7c55-a682-6b473d76e4f4@tarantool.org> Date: Tue, 21 Aug 2018 19:31:17 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <026cd8e04034ad26529a2b8e76e1ae9aabf9076f.1534001739.git.korablev@tarantool.org> <9ad38333-305b-7c55-a682-6b473d76e4f4@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/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 =3D 0; >> struct Index *pk =3D = sqlite3PrimaryKeyIndex(table); >> const char *zAff =3D is_view ? NULL : >> - = sqlite3IndexAffinityStr(parse->db, pk); >> + = sql_index_affinity_str(parse->db, >> + = pk->def); >=20 > 1. sql_index_affinity_str makes extra lookup in space cache. Here > (in sql_table_delete_from) space ptr is known already. >=20 > 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. Ok, time to pull up struct space: +++ b/src/box/sql/delete.c @@ -352,8 +352,9 @@ sql_table_delete_from(struct Parse *parse, struct = SrcList *tab_list, key_len =3D 0; struct Index *pk =3D = sqlite3PrimaryKeyIndex(table); const char *zAff =3D is_view ? NULL : - = sql_index_affinity_str(parse->db, - = pk->def); + = sql_space_index_affinity_str(parse->db, + = space->def, + = pk->def); sqlite3VdbeAddOp4(v, OP_MakeRecord, reg_pk, = pk_len, +++ b/src/box/sql/fkey.c @@ -256,7 +256,8 @@ fkey_lookup_parent(struct Parse *parse_context, = struct space *parent, struct index *idx =3D space_index(parent, referenced_idx); assert(idx !=3D NULL); sqlite3VdbeAddOp4(v, OP_MakeRecord, temp_regs, field_count, = rec_reg, - sql_index_affinity_str(parse_context->db, = idx->def), + = sql_space_index_affinity_str(parse_context->db, + parent->def, = idx->def), +++ b/src/box/sql/insert.c @@ -59,25 +59,24 @@ sqlite3OpenTable(Parse * pParse, /* Generate code = into this VDBE */ } =20 char * -sql_index_affinity_str(struct sqlite3 *db, struct index_def *def) +sql_space_index_affinity_str(struct sqlite3 *db, struct space_def = *space_def, + struct index_def *idx_def) { - uint32_t column_count =3D def->key_def->part_count; + uint32_t column_count =3D idx_def->key_def->part_count; char *aff =3D (char *)sqlite3DbMallocRaw(db, column_count + 1); if (aff =3D=3D NULL) return NULL; - struct space *space =3D space_by_id(def->space_id); - assert(space !=3D NULL); /* * Table may occasionally come from Lua, so lets * gentle process this case by setting default * affinity for it. */ - if (space->def->fields =3D=3D NULL) { + if (space_def->fields =3D=3D NULL) { memset(aff, AFFINITY_BLOB, column_count); } else { for (uint32_t i =3D 0; i < column_count; i++) { - uint32_t x =3D def->key_def->parts[i].fieldno; - aff[i] =3D space->def->fields[x].affinity; + uint32_t x =3D = idx_def->key_def->parts[i].fieldno; + aff[i] =3D space_def->fields[x].affinity; +++ b/src/box/sql/sqliteInt.h @@ -4297,11 +4297,14 @@ int sqlite3VarintLen(u64 v); * is allocated on heap. * * @param db Database handle. - * @param def index_def where from affinity to be extracted. + * @param space_def Definition of space index belongs to. + * @param idx_def Definition of index from which affinity + * to be extracted. * @retval Allocated affinity string, or NULL on OOM. */ char * -sql_index_affinity_str(struct sqlite3 *db, struct index_def *def); +sql_space_index_affinity_str(struct sqlite3 *db, struct space_def = *space_def, + struct index_def *idx_def); +++ b/src/box/sql/update.c @@ -347,7 +347,8 @@ sqlite3Update(Parse * pParse, /* The = parser context */ regKey =3D iPk; } else { const char *zAff =3D is_view ? 0 : - sql_index_affinity_str(pParse->db, = pPk->def); + = sql_space_index_affinity_str(pParse->db, def, + = pPk->def); +++ b/src/box/sql/wherecode.c @@ -724,11 +724,17 @@ codeAllEqualityTerms(Parse * pParse, /* = Parsing context */ =20 char *zAff; if (pIdx !=3D NULL) { + struct space *space =3D = space_by_id(pIdx->def->space_id); + assert(space !=3D NULL); zAff =3D sqlite3DbStrDup(pParse->db, - = sql_index_affinity_str(pParse->db, - = pIdx->def)); + = sql_space_index_affinity_str(pParse->db, + = space->def, + = pIdx->def)); } else { - zAff =3D sql_index_affinity_str(pParse->db, idx_def); + struct space *space =3D space_by_id(idx_def->space_id); + assert(space !=3D NULL); + zAff =3D sql_space_index_affinity_str(pParse->db, = space->def, + idx_def); >> 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 =3D space_by_id(def->space_id); >> assert(space !=3D NULL); >> - >> + /* >> + * Table may occasionally come from Lua, so lets >> + * gentle process this case by setting default >> + * affinity for it. >> + */ >> + if (space->def->fields =3D=3D NULL) { >> + memset(aff, AFFINITY_BLOB, column_count); >> + goto exit; >> + } >> for (uint32_t i =3D 0; i < column_count; i++) { >> uint32_t x =3D def->key_def->parts[i].fieldno; >> aff[i] =3D space->def->fields[x].affinity; >> if (aff[i] =3D=3D AFFINITY_UNDEFINED) >> - aff[i] =3D 'A'; >> + aff[i] =3D AFFINITY_BLOB; >> } >> +exit: >=20 > 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. Ok, got it. Thx for fix. >=20 >> aff[column_count] =3D '\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 =3D -1; >> int nUpper =3D index->def->opts.stat->sample_count + 1; >> int rc =3D SQLITE_OK; >> - u8 aff =3D sqlite3IndexColumnAffinity(db, p, nEq); >> + u8 aff =3D sql_index_part_affinity(p->def, nEq); >=20 > 3. Same as 1 - extra lookup in the space cache. You have the space > few lines above. Fixed: @@ -4611,13 +4614,15 @@ sql_stat4_column(struct sqlite3 *db, const char = *record, uint32_t col_num, /** * Return the affinity for a single column of an index. * + * @param def Definition of space @idx belongs to. * @param idx Index to be investigated. * @param partno Affinity of this part to be returned. * * @retval Affinity of @partno index part. */ enum affinity_type -sql_index_part_affinity(struct index_def *idx, uint32_t partno); +sql_space_index_part_affinity(struct space_def *def, struct index_def = *idx, + uint32_t partno); +++ b/src/box/sql/where.c @@ -1159,13 +1159,12 @@ whereRangeAdjust(WhereTerm * pTerm, LogEst nNew) } +++ b/src/box/sql/vdbemem.c @@ -39,6 +39,7 @@ #include "sqliteInt.h" #include "vdbeInt.h" #include "tarantoolInt.h" +#include "box/schema.h" =20 #ifdef SQLITE_DEBUG /* @@ -1548,11 +1549,14 @@ sqlite3Stat4ProbeSetValue(Parse * pParse, = /* Parse context */ alloc.pIdx =3D pIdx; alloc.ppRec =3D ppRec; =20 + struct space *space =3D = space_by_id(pIdx->def->space_id); + assert(space !=3D NULL); for (i =3D 0; i < nElem; i++) { sqlite3_value *pVal =3D 0; Expr *pElem =3D (pExpr ? sqlite3VectorFieldSubexpr(pExpr, i) = : 0); - u8 aff =3D sql_index_part_affinity(pIdx->def, = iVal + i); + u8 aff =3D = sql_space_index_part_affinity(space->def, pIdx->def, + iVal + = i); =20 enum affinity_type -sql_index_part_affinity(struct index_def *idx, uint32_t partno) +sql_space_index_part_affinity(struct space_def *def, struct index_def = *idx, + uint32_t partno) { assert(partno < idx->key_def->part_count); - struct space *space =3D space_by_id(idx->space_id); - assert(space !=3D NULL); uint32_t fieldno =3D idx->key_def->parts[partno].fieldno; - return space->def->fields[fieldno].affinity; + return def->fields[fieldno].affinity; }=20 @@ -1220,7 +1219,7 @@ whereRangeSkipScanEst(Parse * pParse, = /* Parsing & code generating context */ int nLower =3D -1; int nUpper =3D index->def->opts.stat->sample_count + 1; int rc =3D SQLITE_OK; - u8 aff =3D sql_index_part_affinity(p->def, nEq); + u8 aff =3D sql_space_index_part_affinity(space->def, p->def, = nEq);