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 B5216278EF for ; Fri, 20 Jul 2018 08:11:49 -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 HvEBA62KlRxb for ; Fri, 20 Jul 2018 08:11:49 -0400 (EDT) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 6C953278DE for ; Fri, 20 Jul 2018 08:11:49 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH] sql: refactor primary index creation From: "n.pettik" In-Reply-To: Date: Fri, 20 Jul 2018 15:11:39 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: 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: Kirill Yukhin Now you also can remove keyConf field from struct Table. > @@ -931,6 +925,22 @@ sqlite3AddPrimaryKey(Parse * pParse, /* = Parsing context */ > } > if (pList) > pParse->iPkSortOrder =3D pList->a[0].sort_order; > + > + struct sqlite3 *db =3D pParse->db; > + struct ExprList *list; > + struct Token ipkToken; > + sqlite3TokenInit(&ipkToken, = pTab->def->fields[iCol].name); > + list =3D sql_expr_list_append(db, NULL, > + sqlite3ExprAlloc(db, TK_ID, > + &ipkToken, = 0)); > + if (list =3D=3D NULL) > + return; You don=E2=80=99t need to create list from scratch: sql_create_index = accepts NULL as list of columns and processes it exactly in the same way (i.e. uses only last column). > + list->a[0].sort_order =3D pParse->iPkSortOrder; > + sql_create_index(pParse, 0, 0, list, pTab->keyConf, 0, = 0, > + SORT_ORDER_ASC, false, > + SQL_INDEX_TYPE_CONSTRAINT_PK); > + if (db->mallocFailed) > + return; goto primary_key_exit? > } else if (autoInc) { > sqlite3ErrorMsg(pParse, "AUTOINCREMENT is only allowed = on an " > "INTEGER PRIMARY KEY or INT PRIMARY = KEY"); > @@ -941,7 +951,16 @@ sqlite3AddPrimaryKey(Parse * pParse, /* = Parsing context */ > pList =3D 0; > } >=20 > - primary_key_exit: > + /* Mark every PRIMARY KEY column as NOT NULL (except for = imposter tables) > + */ Fix comment: we don=E2=80=99t have imposter tables; fit it into 66 = chars. > + for (uint32_t i =3D 0; i < pTab->def->field_count; i++) { > + if (pTab->aCol[i].is_primkey) { > + pTab->def->fields[i].nullable_action > + =3D ON_CONFLICT_ACTION_ABORT; > + pTab->def->fields[i].is_nullable =3D false; > + } > + } > +primary_key_exit: > sql_expr_list_delete(pParse->db, pList); > return; > } > @@ -1215,63 +1234,6 @@ createTableStmt(sqlite3 * db, Table * p) > return zStmt; > } >=20 > -/* > - * This routine runs at the end of parsing a CREATE TABLE statement. > - * The job of this routine is to convert both > - * internal schema data structures and the generated VDBE code. > - * Changes include: > - * > - * (1) Set all columns of the PRIMARY KEY schema object to be = NOT NULL. > - * (2) Set the Index.tnum of the PRIMARY KEY Index object in the > - * schema to the rootpage from the main table. > - * (3) Add all table columns to the PRIMARY KEY Index object > - * so that the PRIMARY KEY is a covering index. > - */ > -static void > -convertToWithoutRowidTable(Parse * pParse, Table * pTab) I see reference to this function in comment to struct Index. Remove it as well. >=20 > @@ -2559,7 +2519,9 @@ tnt_error: > static bool > constraint_is_named(const char *name) > { > - return strncmp(name, "sql_autoindex_", = strlen("sql_autoindex_")); > + return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")) = && > + strncmp(name, "pk_unnamed_", strlen("pk_unnamed_")) && > + strncmp(name, "unique_unnamed_", = strlen("unique_unnamed_")); > } >=20 > void > @@ -2648,29 +2610,36 @@ sql_create_index(struct Parse *parse, struct = Token *token, > sqlite3NameFromToken(db, > = &parse->constraintName); >=20 > + /* > + * This naming is temporary. Now it's not > + * possible (since we implement UNIQUE > + * and PK constraints with indexes and > + * indexes can not have same names), but > + * in future we would use names exactly > + * as they are set by user. > + */ > + const char *prefix =3D "sql_autoindex_%s_%d=E2=80=9D; If we trapped so far, index is definitely UNIQUE or PK constraint, so this initialisation with default prefix is redundant. And actually I don=E2=80=99t understand why do you need this renaming... > + if (idx_type =3D=3D SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) { > + prefix =3D constraint_name =3D=3D NULL ? > + "unique_unnamed_%s_%d" : "unique_%s_%d"; > + } > + else { > + if (idx_type =3D=3D = SQL_INDEX_TYPE_CONSTRAINT_PK) > + prefix =3D constraint_name =3D=3D NULL ? > + "pk_unnamed_%s_%d" : "pk_%s_%d"; > + } > + > + uint32_t n =3D 1; > + for (struct Index *idx =3D table->pIndex; idx !=3D NULL; > + idx =3D idx->pNext, n++); > + > if (constraint_name =3D=3D NULL || > strcmp(constraint_name, "") =3D=3D 0) { > - uint32_t n =3D 1; > - for (struct Index *idx =3D table->pIndex; idx !=3D= NULL; > - idx =3D idx->pNext, n++); > - name =3D sqlite3MPrintf(db, = "sql_autoindex_%s_%d", > + name =3D sqlite3MPrintf(db, prefix, > table->def->name, n); > } else { > - /* > - * This naming is temporary. Now it's not > - * possible (since we implement UNIQUE > - * and PK constraints with indexes and > - * indexes can not have same names), but > - * in future we would use names exactly > - * as they are set by user. > - */ > - if (idx_type =3D=3D = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) > - name =3D sqlite3MPrintf(db, > - = "unique_constraint_%s", > - constraint_name); > - if (idx_type =3D=3D = SQL_INDEX_TYPE_CONSTRAINT_PK) > - name =3D sqlite3MPrintf(db, = "pk_constraint_%s", > - constraint_name); > + name =3D sqlite3MPrintf(db, prefix, > + constraint_name, n); > } > sqlite3DbFree(db, constraint_name); > } >=20 > diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c > index 7b1f6ec..cabe22b 100644 > --- a/src/box/sql/pragma.c > +++ b/src/box/sql/pragma.c > @@ -726,22 +726,20 @@ sqlite3Pragma(Parse * pParse, Token * pId, = /* First part of [schema.]id field */ > int iKey =3D = pFK->aCol[0].iFrom; > assert(iKey >=3D 0 && = iKey < > = (int)pTab->def->field_count); > - if (iKey !=3D = pTab->iPKey) { > - = sqlite3VdbeAddOp3(v, > - = OP_Column, > - = 0, > - = iKey, > - = regRow); > - = sqlite3ColumnDefault(v, > - = pTab->def, > - = iKey, > - = regRow); > - = sqlite3VdbeAddOp2(v, > - = OP_IsNull, > - = regRow, > - = addrOk); > - VdbeCoverage(v); > - } > + sqlite3VdbeAddOp3(v, > + = OP_Column, > + 0, > + iKey, > + = regRow); > + sqlite3ColumnDefault(v, > + = pTab->def, > + = iKey, > + = regRow); > + sqlite3VdbeAddOp2(v, > + = OP_IsNull, > + = regRow, > + = addrOk); > + VdbeCoverage(v); > VdbeCoverage(v); Double call of VdbeCoverage();