[patches] [PATCH 3/3] sql: delegate SQL primary key check to Tarantool

Alex Khatskevich avkhatskevich at tarantool.org
Thu Feb 22 17:55:26 MSK 2018



On 14.02.2018 15:38, Nikita Pettik wrote:
> As last point of removing colflag attribute from struct Column,
> table_column_is_primkey() function is added. It checks via Tarantool
> facilities whether column belongs to PK or not (after space creation).
> However, during initializing of SQL iternals and before actual creation
> of struct space, colflag attribute (as indicator of PK) still can be used.
> Since this is the only purpose of attribute, one has been renamed.
>
> Closes #3118
>
> Signed-off-by: Nikita Pettik <korablev at tarantool.org>
> ---
>   src/box/sql/alter.c     |  2 +-
>   src/box/sql/build.c     | 34 ++++++++++++++++++++++++++++++----
>   src/box/sql/fkey.c      |  2 +-
>   src/box/sql/pragma.c    |  3 +--
>   src/box/sql/sqliteInt.h |  7 ++-----
>   src/box/sql/update.c    | 11 ++++-------
>   6 files changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
> index 2794c52cc..0bab035e0 100644
> --- a/src/box/sql/alter.c
> +++ b/src/box/sql/alter.c
> @@ -231,7 +231,7 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
>   	 * If there is a NOT NULL constraint, then the default value for the
>   	 * column must not be NULL.
>   	 */
> -	if (pCol->colFlags & COLFLAG_PRIMKEY) {
> +	if (pCol->is_primkey) {
>   		sqlite3ErrorMsg(pParse, "Cannot add a PRIMARY KEY column");
>   		return;
>   	}
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 851a8f760..8f69901bf 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -47,6 +47,7 @@
>   #include "vdbeInt.h"
>   #include "tarantoolInt.h"
>   #include "box/session.h"
> +#include "box/schema.h"
>   
>   /*
>    * This routine is called after a single SQL statement has been
> @@ -433,6 +434,32 @@ sqlite3DeleteColumnNames(sqlite3 * db, Table * pTable)
>   	}
>   }
>   
> +/* Return true if given column is part of primary key. */
> +bool
> +table_column_is_primkey(Table *table, uint32_t column)
I do not like primkey word &&
The name is a little confusing, because one can suppose that the
function checks if column is a primary key, while it actually checks
if pk contains the column.

I would like the name to be `is_column_in_pk` or something like this
> +{
> +	uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
> +	struct space *space = space_by_id(space_id);
> +	assert(space != NULL);
> +
> +	struct index *primary_idx = index_find(space, 0 /* PK */);
> +	/* Views don't have any indexes. */
> +	if (primary_idx == NULL)
> +		return false;
> +	struct index_def *idx_def = primary_idx->def;
> +	uint64_t pk_mask = idx_def->key_def->column_mask;
> +	if (column < 63) {
Are there any constants in Tarantool for this purpose? (63)
> +		return pk_mask & (1UL << column);
> +	} else if ((pk_mask & (1UL << 1)) != 0) {
magic
> +		for (uint32_t i = 0; i < idx_def->key_def->part_count; ++i) {
> +			struct key_part *part = &idx_def->key_def->parts[i];
> +			if (part->fieldno == column)
> +				return true;
> +		}
> +	}
> +	return false;
> +}
> +
>   /*
>    * Remove the memory data structures associated with the given
>    * Table.  No changes are made to disk by this routine.
> @@ -1040,7 +1067,7 @@ sqlite3AddPrimaryKey(Parse * pParse,	/* Parsing context */
>   	if (pList == 0) {
>   		iCol = pTab->nCol - 1;
>   		pCol = &pTab->aCol[iCol];
> -		pCol->colFlags |= COLFLAG_PRIMKEY;
> +		pCol->is_primkey = 1;
>   		nTerm = 1;
>   	} else {
>   		nTerm = pList->nExpr;
> @@ -1058,8 +1085,7 @@ sqlite3AddPrimaryKey(Parse * pParse,	/* Parsing context */
>   				    (zCName,
>   				     pTab->aCol[iCol].zName) == 0) {
>   					pCol = &pTab->aCol[iCol];
> -					pCol->colFlags |=
> -					    COLFLAG_PRIMKEY;
> +					pCol->is_primkey = 1;
>   					break;
>   				}
>   			}
> @@ -1423,7 +1449,7 @@ convertToWithoutRowidTable(Parse * pParse, Table * pTab)
>   	 */
>   	if (!db->init.imposterTable) {
>   		for (i = 0; i < pTab->nCol; i++) {
> -			if ((pTab->aCol[i].colFlags & COLFLAG_PRIMKEY) != 0) {
> +			if (pTab->aCol[i].is_primkey) {
>   				pTab->aCol[i].notNull = OE_Abort;
>   			}
>   		}
> diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
> index d803a0df9..75930ebda 100644
> --- a/src/box/sql/fkey.c
> +++ b/src/box/sql/fkey.c
> @@ -865,7 +865,7 @@ fkParentIsModified(Table * pTab, FKey * p, int *aChange)
>   					if (0 ==
>   					    strcmp(pCol->zName, zKey))
>   						return 1;
> -				} else if (pCol->colFlags & COLFLAG_PRIMKEY) {
> +				} else if (table_column_is_primkey(pTab, iKey)) {
>   					return 1;
>   				}
>   			}
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 4aa3b2961..2b5cca435 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -352,8 +352,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
>   				sqlite3ViewGetColumnNames(pParse, pTab);
>   				for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
>   				     i++, pCol++) {
> -					if ((pCol->
> -					     colFlags & COLFLAG_PRIMKEY) == 0) {
> +					if (!table_column_is_primkey(pTab, i)) {
>   						k = 0;
>   					} else if (pPk == 0) {
>   						k = 1;
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index 5fc612dea..fd732fa9e 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -1354,13 +1354,9 @@ struct Column {
>   	u8 notNull;		/* An OE_ code for handling a NOT NULL constraint */
>   	char affinity;		/* One of the SQLITE_AFF_... values */
>   	u8 szEst;		/* Estimated size of value in this column. sizeof(INT)==1 */
> -	u8 colFlags;		/* Boolean properties.  See COLFLAG_ defines below */
> +	u8 is_primkey;		/* Boolean propertie for being PK */
>   };
>   
> -/* Allowed values for Column.colFlags:
> - */
> -#define COLFLAG_PRIMKEY  0x0001	/* Column is part of the primary key */
> -
>   /*
>    * A sort order can be either ASC or DESC.
>    */
> @@ -3100,6 +3096,7 @@ void sqlite3ResetAllSchemasOfConnection(sqlite3 *);
>   void sqlite3ResetOneSchema(sqlite3 *);
>   void sqlite3CommitInternalChanges();
>   void sqlite3DeleteColumnNames(sqlite3 *, Table *);
> +bool table_column_is_primkey(Table *, uint32_t);
>   int sqlite3ColumnsFromExprList(Parse *, ExprList *, i16 *, Column **);
>   void sqlite3SelectAddColumnTypeAndCollation(Parse *, Table *, Select *);
>   Table *sqlite3ResultSetOfSelect(Parse *, Select *);
> diff --git a/src/box/sql/update.c b/src/box/sql/update.c
> index 8d0fe73ff..114f3c350 100644
> --- a/src/box/sql/update.c
> +++ b/src/box/sql/update.c
> @@ -232,12 +232,9 @@ sqlite3Update(Parse * pParse,		/* The parser context */
>   			goto update_cleanup;
>   		}
>   		for (j = 0; j < pTab->nCol; j++) {
> -			if (strcmp
> -			    (pTab->aCol[j].zName, pChanges->a[i].zName) == 0) {
> -				if (pPk
> -					   && (pTab->aCol[j].
> -					       colFlags & COLFLAG_PRIMKEY) !=
> -					   0) {
> +			if (strcmp(pTab->aCol[j].zName,
> +				   pChanges->a[i].zName) == 0) {
> +				if (pPk && table_column_is_primkey(pTab, j)) {
>   					chngPk = 1;
>   				}
>   				aXRef[j] = i;
> @@ -490,7 +487,7 @@ sqlite3Update(Parse * pParse,		/* The parser context */
>   		for (i = 0; i < pTab->nCol; i++) {
>   			if (oldmask == 0xffffffff
>   			    || (i < 32 && (oldmask & MASKBIT32(i)) != 0)
> -			    || (pTab->aCol[i].colFlags & COLFLAG_PRIMKEY) != 0) {
> +			    || table_column_is_primkey(pTab, i)) {
>   				testcase(oldmask != 0xffffffff && i == 31);
>   				sqlite3ExprCodeGetColumnOfTable(v, pTab,
>   								iDataCur, i,




More information about the Tarantool-patches mailing list