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 425F02947A for ; Tue, 21 Aug 2018 12:31:32 -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 hicqrr1ofF5G for ; Tue, 21 Aug 2018 12:31:32 -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 CB9A229359 for ; Tue, 21 Aug 2018 12:31:31 -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 10/10] sql: move autoincrement field number to server From: "n.pettik" In-Reply-To: Date: Tue, 21 Aug 2018 19:31:30 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <685EC40E-4A7E-4A5A-8BAD-A597ADC67E67@tarantool.org> References: <893c37395b2536af932aac033f6c54718720b63f.1534001739.git.korablev@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 > On 13 Aug 2018, at 23:24, Vladislav Shpilevoy = wrote: >=20 > Are you sure it is the only way to pass autoinc fieldno? And that it = can > not be dropped and calculated from other fields without significant = problems? >=20 > Now this flag looks very ugly inside _space tuples. >=20 > I think, autoinc is rather primary index option than the space, and = that it > can be detected via checking >=20 > pk->part_count =3D=3D 1 and space->sequence !=3D NULL >=20 > then pk->parts[0].fieldno is autoinc field. It is not? Okay, it seems to be possible. Check this out: sql: remove autoincrement field number from Table =20 During INSERT SQL statement execution we may need to know field = which stores INTEGER AUTOINCREMENT PRIMARY KEY. Since we are going to get = rid of struct Table, lets remove this member from struct Table. Instead, = it can be calculated using struct space: if PK consists from one part = and sequence not NULL - table features autoincrement. =20 Part of #3561 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index b38a934a1..dc00b5d8c 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -401,7 +401,6 @@ sql_table_new(Parse *parser, char *name) strcpy(table->def->engine_name, = sql_storage_engine_strs[current_session()->sql_default_engine]); =20 - table->iAutoIncPKey =3D -1; table->pSchema =3D db->pSchema; table->nTabRef =3D 1; return table; @@ -868,8 +867,7 @@ sqlite3AddPrimaryKey(Parse * pParse, /* = Parsing context */ pTab->def->fields[iCol].type =3D=3D FIELD_TYPE_INTEGER && sortOrder !=3D SORT_ORDER_DESC) { assert(autoInc =3D=3D 0 || autoInc =3D=3D 1); - if (autoInc) - pTab->iAutoIncPKey =3D iCol; + pParse->is_new_table_autoinc =3D autoInc; struct sqlite3 *db =3D pParse->db; struct ExprList *list; struct Token token; @@ -1698,7 +1696,7 @@ sqlite3EndTable(Parse * pParse, /* Parse context = */ * Check to see if we need to create an _sequence table * for keeping track of autoincrement keys. */ - if (p->iAutoIncPKey >=3D 0) { + if (pParse->is_new_table_autoinc) { assert(reg_space_id !=3D 0); /* Do an insertion into _sequence. */ int reg_seq_id =3D ++pParse->nMem; diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index d9e230aee..7780bf749 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -100,6 +100,22 @@ sql_emit_table_affinity(struct Vdbe *v, struct = space_def *def, int reg) P4_DYNAMIC); } =20 +/** + * In SQL table can be created with AUTOINCREMENT. + * In Tarantool it can be detected as primary key which consists + * from one field with not NULL space's sequence. + */ +static uint32_t +sql_space_autoinc_fieldno(struct space *space) +{ + assert(space !=3D NULL); + struct index *pk =3D space_index(space, 0); + if (pk =3D=3D NULL || pk->def->key_def->part_count !=3D 1 || + space->sequence =3D=3D NULL) + return UINT32_MAX; + return pk->def->key_def->parts[0].fieldno; +} + /** * This routine is used to see if a statement of the form * "INSERT INTO SELECT ..." can run for the results of the @@ -556,7 +572,9 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ sqlite3VdbeAddOp1(v, OP_Yield, dest.iSDParm); VdbeCoverage(v); } - + struct space *space =3D space_by_id(pTab->def->id); + assert(space !=3D NULL); + uint32_t autoinc_fieldno =3D sql_space_autoinc_fieldno(space); /* Run the BEFORE and INSTEAD OF triggers, if there are any */ endOfLoop =3D sqlite3VdbeMakeLabel(v); @@ -574,7 +592,7 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ } if ((!useTempTable && !pList) || (pColumn && j >=3D pColumn->nId)) { - if (i =3D=3D pTab->iAutoIncPKey) { + if (i =3D=3D (int) autoinc_fieldno) { sqlite3VdbeAddOp2(v, OP_Integer, = -1, regCols + i + = 1); } else { @@ -637,7 +655,7 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ } if (j < 0 || nColumn =3D=3D 0 || (pColumn && j >=3D pColumn->nId)) { - if (i =3D=3D pTab->iAutoIncPKey) { + if (i =3D=3D (int) autoinc_fieldno) { sqlite3VdbeAddOp2(v, = OP_NextAutoincValue, pTab->def->id, @@ -652,7 +670,7 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ dflt, iRegStore); } else if (useTempTable) { - if (i =3D=3D pTab->iAutoIncPKey) { + if (i =3D=3D (int) autoinc_fieldno) { int regTmp =3D ++pParse->nMem; /* Emit code which doesn't = override * autoinc-ed value with select = result @@ -671,7 +689,7 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ } } else if (pSelect) { if (regFromSelect !=3D regData) { - if (i =3D=3D pTab->iAutoIncPKey) = { + if (i =3D=3D (int) = autoinc_fieldno) { /* Emit code which = doesn't override * autoinc-ed value with = select result * in case that result = is NULL @@ -693,7 +711,7 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ } } else { =20 - if (i =3D=3D pTab->iAutoIncPKey) { + if (i =3D=3D (int) autoinc_fieldno) { if (pList->a[j].pExpr->op =3D=3D = TK_NULL) { sqlite3VdbeAddOp2(v, = OP_Null, 0, iRegStore); continue; @@ -730,8 +748,6 @@ sqlite3Insert(Parse * pParse, /* Parser = context */ on_error, endOfLoop, 0); fkey_emit_check(pParse, pTab, 0, regIns, 0); int pk_cursor =3D pParse->nTab++; - struct space *space =3D space_by_id(pTab->def->id); - assert(space !=3D NULL); vdbe_emit_open_cursor(pParse, pk_cursor, 0, space); vdbe_emit_insertion_completion(v, pk_cursor, regIns + 1, pTab->def->field_count, @@ -846,6 +862,9 @@ vdbe_emit_constraint_checks(struct Parse = *parse_context, struct Table *tab, /* Insertion into VIEW is prohibited. */ assert(!def->opts.is_view); bool is_update =3D upd_cols !=3D NULL; + struct space *space =3D space_by_id(tab->def->id); + assert(space !=3D NULL); + uint32_t autoinc_fieldno =3D sql_space_autoinc_fieldno(space); /* Test all NOT NULL constraints. */ for (uint32_t i =3D 0; i < def->field_count; i++) { /* @@ -855,7 +874,7 @@ vdbe_emit_constraint_checks(struct Parse = *parse_context, struct Table *tab, if (is_update && upd_cols[i] < 0) continue; /* This column is allowed to be NULL. */ - if (def->fields[i].is_nullable || tab->iAutoIncPKey =3D=3D= (int) i) + if (def->fields[i].is_nullable || autoinc_fieldno =3D=3D = i) continue; enum on_conflict_action on_conflict_nullable =3D on_conflict !=3D ON_CONFLICT_ACTION_DEFAULT ? @@ -948,7 +967,7 @@ vdbe_emit_constraint_checks(struct Parse = *parse_context, struct Table *tab, int reg_pk =3D new_tuple_reg + fieldno; if (tab->def->fields[fieldno].affinity =3D=3D = AFFINITY_INTEGER) { int skip_if_null =3D sqlite3VdbeMakeLabel(v); - if (tab->iAutoIncPKey >=3D 0) { + if (autoinc_fieldno !=3D UINT32_MAX) { sqlite3VdbeAddOp2(v, OP_IsNull, reg_pk, skip_if_null); } @@ -1000,7 +1019,7 @@ vdbe_emit_constraint_checks(struct Parse = *parse_context, struct Table *tab, process_index: ; int cursor =3D parse_context->nTab++; vdbe_emit_open_cursor(parse_context, cursor, = idx->def->iid, - space_by_id(tab->def->id)); + space); /* * If there is no conflict in current index, just * jump to the start of next iteration. Label is diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 9a45acedd..d2ef85846 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1850,8 +1850,6 @@ struct Savepoint { struct Table { Index *pIndex; /* List of SQL indexes on this table. */ u32 nTabRef; /* Number of pointers to this Table */ - i16 iAutoIncPKey; /* If PK is marked INTEGER PRIMARY KEY = AUTOINCREMENT, store - column number here, -1 otherwise = Tarantool specifics */ /** * Estimated number of entries in table. * Used only when table represents temporary objects, @@ -2838,6 +2836,8 @@ struct Parse { */ struct rlist new_fkey; bool initiateTTrans; /* Initiate Tarantool transaction */ + /** True, if table to be created has AUTOINCREMENT PK. */ + bool is_new_table_autoinc; /** If set - do not emit byte code at all, just parse. */ bool parse_only; /** Type of parsed_ast member. */