[tarantool-patches] Re: [PATCH 05/10] sql: remove affinity string of columns from Index
n.pettik
korablev at tarantool.org
Tue Aug 21 19:31:17 MSK 2018
>> 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.
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 = 0;
struct Index *pk = sqlite3PrimaryKeyIndex(table);
const char *zAff = 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 = space_index(parent, referenced_idx);
assert(idx != 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 */
}
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 = def->key_def->part_count;
+ uint32_t column_count = idx_def->key_def->part_count;
char *aff = (char *)sqlite3DbMallocRaw(db, column_count + 1);
if (aff == NULL)
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) {
+ if (space_def->fields == NULL) {
memset(aff, AFFINITY_BLOB, column_count);
} else {
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;
+ uint32_t x = idx_def->key_def->parts[i].fieldno;
+ aff[i] = 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 = iPk;
} else {
const char *zAff = 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 */
char *zAff;
if (pIdx != NULL) {
+ struct space *space = space_by_id(pIdx->def->space_id);
+ assert(space != NULL);
zAff = sqlite3DbStrDup(pParse->db,
- sql_index_affinity_str(pParse->db,
- pIdx->def));
+ sql_space_index_affinity_str(pParse->db,
+ space->def,
+ pIdx->def));
} else {
- zAff = sql_index_affinity_str(pParse->db, idx_def);
+ struct space *space = space_by_id(idx_def->space_id);
+ assert(space != NULL);
+ zAff = 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 = 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.
Ok, got it. Thx for fix.
>
>> 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.
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"
#ifdef SQLITE_DEBUG
/*
@@ -1548,11 +1549,14 @@ sqlite3Stat4ProbeSetValue(Parse * pParse, /* Parse context */
alloc.pIdx = pIdx;
alloc.ppRec = ppRec;
+ struct space *space = space_by_id(pIdx->def->space_id);
+ assert(space != NULL);
for (i = 0; i < nElem; i++) {
sqlite3_value *pVal = 0;
Expr *pElem =
(pExpr ? sqlite3VectorFieldSubexpr(pExpr, i) : 0);
- u8 aff = sql_index_part_affinity(pIdx->def, iVal + i);
+ u8 aff = sql_space_index_part_affinity(space->def, pIdx->def,
+ iVal + i);
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 = space_by_id(idx->space_id);
- assert(space != NULL);
uint32_t fieldno = idx->key_def->parts[partno].fieldno;
- return space->def->fields[fieldno].affinity;
+ return def->fields[fieldno].affinity;
}
@@ -1220,7 +1219,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 = sql_index_part_affinity(p->def, nEq);
+ u8 aff = sql_space_index_part_affinity(space->def, p->def, nEq);
More information about the Tarantool-patches
mailing list