[tarantool-patches] Re: [PATCH] sql: get rid off tnum field of struct Table
n.pettik
korablev at tarantool.org
Fri Jul 20 17:07:03 MSK 2018
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 3af9f9a..504701d 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -384,7 +384,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> if (pTab == NULL)
> goto insert_cleanup;
>
> - space_id = SQLITE_PAGENO_TO_SPACEID(pTab->tnum);
> + space_id = pTab->def->id;
>
> /* Figure out if we have any triggers and if the table being
> * inserted into is a view
> @@ -742,7 +742,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> if (i == pTab->iAutoIncPKey) {
> sqlite3VdbeAddOp2(v,
> OP_NextAutoincValue,
> - pTab->tnum,
> + SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(pTab->def->id, 0),
It looks disgusting.. Lets also refactor OP_NextAutoincValue so that
it accepts raw space_id. After that, remove these SQLITE_PAGENO
macroses at all.
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index cabe22b..0c838fa 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -432,9 +432,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> for (i = sqliteHashFirst(&db->pSchema->tblHash); i;
> i = sqliteHashNext(i)) {
> Table *pTab = sqliteHashData(i);
> - uint32_t space_id =
> - SQLITE_PAGENO_TO_SPACEID(pTab->tnum);
> - struct space *space = space_by_id(space_id);
> + struct space *space;
> + space = space_by_id(pTab->def->id);
Squash two lines into one:
struct space *space = space_by_id(pTab->def->id);
> @@ -161,18 +142,18 @@ extern int
> sqlite3InitDatabase(sqlite3 * db)
> {
> int rc;
> - InitData initData;
> + struct init_data init;
>
> assert(db->pSchema != NULL);
>
> - memset(&initData, 0, sizeof(InitData));
> - initData.db = db;
> + memset(&init, 0, sizeof(init));
> + init.db = db;
>
> /* Load schema from Tarantool - into the primary db only. */
> - tarantoolSqlite3LoadSchema(&initData);
> + tarantoolSqlite3LoadSchema(&init);
>
> - if (initData.rc) {
> - rc = initData.rc;
> + if (init.rc) {
> + rc = init.rc;
> goto error_out;
> }
>
> -/*
> +/**
> * A pointer to this structure is used to communicate information
> - * from sqlite3Init and OP_ParseSchema into the sqlite3InitCallback.
> - */
> -typedef struct {
> - sqlite3 *db; /* The database being initialized */
> - char **pzErrMsg; /* Error message stored here */
> - int rc; /* Result code stored here */
> -} InitData;
> + * from sqlite3Init and OP_ParseSchema into the sql_init_callback.
> + */
> +struct init_data {
> + /* The database being initialized */
> + sqlite3 *db;
> + /* Error message stored here */
> + char **pzErrMsg;
> + /* Result code stored here */
Comments inside struct definition should look like:
/** … */
And put dot at the end of sentences.
>
> diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
> index f043a60..b34e671 100644
> --- a/src/box/sql/tarantoolInt.h
> +++ b/src/box/sql/tarantoolInt.h
> @@ -44,7 +44,7 @@
> ((pgno) & 1023)
>
> /* Load database schema from Tarantool. */
> -void tarantoolSqlite3LoadSchema(InitData * init);
> +void tarantoolSqlite3LoadSchema(struct init_data * init);
struct init_data *init — remove extra space.
> @@ -2178,8 +2171,8 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
> }
> rc = whereLoopXfer(db, p, pTemplate);
> Index *pIndex = p->pIndex;
> - if (pIndex && pIndex->tnum == 0)
> - p->pIndex = 0;
> + if (pIndex != NULL && pIndex->pTable->def->opts.is_view)
> + p->pIndex = NULL;
> return rc;
> }
These conditions don’t seem to be equivalent: view features ordinary space_id AFAIK.
>
> @@ -2347,8 +2340,8 @@ whereRangeVectorLen(Parse * pParse, /* Parsing context */
> * terms only. If it is modified, this value is restored before this
> * function returns.
> *
> - * If pProbe->tnum==0, that means pIndex is a fake index used for the
> - * INTEGER PRIMARY KEY.
> + * If pProbe->def->space_id==0, that means pIndex is a fake index
> + * used for the INTEGER PRIMARY KEY.
But in fact you use another sign of fake index:
iid == UINT32_MAX
> */
> static int
> whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder, /* The WhereLoop factory */
> @@ -2391,12 +2384,11 @@ whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder, /* The WhereLoop factory */
> opMask =
> WO_EQ | WO_IN | WO_GT | WO_GE | WO_LT | WO_LE | WO_ISNULL;
> }
> - struct space *space =
> - space_by_id(SQLITE_PAGENO_TO_SPACEID(pProbe->tnum));
> + struct space *space = space_by_id(pProbe->def->space_id);
> struct index *idx = NULL;
> struct index_stat *stat = NULL;
> - if (space != NULL) {
> - idx = space_index(space, SQLITE_PAGENO_TO_INDEXID(pProbe->tnum));
> + if (space != NULL && pProbe->def->iid != UINT32_MAX) {
> + idx = space_index(space, pProbe->def->iid);
> assert(idx != NULL);
> stat = idx->def->opts.stat;
> }
>
> @@ -2577,7 +2569,7 @@ whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder, /* The WhereLoop factory */
> assert(eOp & (WO_ISNULL | WO_EQ | WO_IN));
>
> assert(pNew->nOut == saved_nOut);
> - if (pTerm->truthProb <= 0 && pProbe->tnum != 0 ) {
> + if (pTerm->truthProb <= 0 && !pProbe->pTable->def->opts.is_view) {
The same as previous remark: these conditions don’t seem to be equivalent
More information about the Tarantool-patches
mailing list