[tarantool-patches] Re: [PATCH v2 1/1] Removed Expr pointer from SQL Column structure.

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hello. Thanks for fixes. I force pushed a pair of new ones into the same commit.
Nikita, please, review the patch.

diff --git a/src/box/sql.c b/src/box/sql.c
index a6713f1f0..6bc82c280 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1466,7 +1466,8 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf)
  	for (i = 0; i < n; i++) {
  		const char *t;
  		struct coll *coll = NULL;
-		struct Expr *def = aCol[i].pDflt;
+		struct field_def *field = &pTable->def->fields[i];
+		struct Expr *def = field->default_value_expr;
  		if (aCol[i].zColl != NULL &&
  		    strcasecmp(aCol[i].zColl, "binary") != 0) {
  			coll = sqlite3FindCollSeq(aCol[i].zColl);
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 129ef823c..cf8be5d41 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -35,6 +35,7 @@
   */
  #include "sqliteInt.h"
  #include "src/box/session.h"
+#include "tarantoolInt.h"
  
  /*
   * The code in this file only exists if we are not omitting the
@@ -161,7 +162,9 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
  
  	zTab = &pNew->zName[16];	/* Skip the "sqlite_altertab_" prefix on the name */
  	pCol = &pNew->aCol[pNew->nCol - 1];
-	pDflt = pCol->pDflt;
+	assert(pNew->def != NULL);
+	pDflt = space_column_default_expr(SQLITE_PAGENO_TO_SPACEID(pNew->tnum),
+					  pNew->nCol - 1);
  	pTab = sqlite3HashFind(&db->pSchema->tblHash, zTab);;
  	assert(pTab);
  
@@ -297,7 +300,6 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
  		Column *pCol = &pNew->aCol[i];
  		pCol->zName = sqlite3DbStrDup(db, pCol->zName);
  		pCol->zColl = 0;
-		pCol->pDflt = 0;
  	}
  	pNew->pSchema = db->pSchema;
  	pNew->addColOffset = pTab->addColOffset;
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 92f3cb607..ab6df467d 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -299,7 +299,6 @@ sqlite3DeleteColumnNames(sqlite3 * db, Table * pTable)
  	if ((pCol = pTable->aCol) != 0) {
  		for (i = 0; i < pTable->nCol; i++, pCol++) {
  			sqlite3DbFree(db, pCol->zName);
-			sql_expr_free(db, pCol->pDflt, false);
  			sqlite3DbFree(db, pCol->zColl);
  		}
  		sqlite3DbFree(db, pTable->aCol);
@@ -397,6 +396,12 @@ deleteTable(sqlite3 * db, Table * pTable)
  	sqlite3DbFree(db, pTable->zColAff);
  	sqlite3SelectDelete(db, pTable->pSelect);
  	sqlite3ExprListDelete(db, pTable->pCheck);
+	if (pTable->def != NULL) {
+		/* Fields has been allocated independently. */
+		struct field_def *fields = pTable->def->fields;
+		space_def_delete(pTable->def);
+		sqlite3DbFree(db, fields);
+	}
  	sqlite3DbFree(db, pTable);
  
  	/* Verify that no lookaside memory was used by schema tables */
@@ -490,6 +495,43 @@ sqlite3PrimaryKeyIndex(Table * pTab)
  	return p;
  }
  
+/**
+ * Create and initialize a new SQL Table object.
+ * @param pParse SQL Parser object.
+ * @param zName Table to create name.
+ * @retval NULL on memory allocation error, Parser state is
+ *         changed.
+ * @retval not NULL on success.
+ */
+static Table *
+sql_table_new(Parse *pParse, char *zName)
+{
+	sqlite3 *db = pParse->db;
+
+	Table *pTable = sqlite3DbMallocZero(db, sizeof(Table));
+	struct space_def *def = space_def_new(0, 0, 0, NULL, 0, NULL, 0,
+					      &space_opts_default, NULL, 0);
+	if (pTable == NULL || def == NULL) {
+		if (def != NULL)
+			space_def_delete(def);
+		sqlite3DbFree(db, pTable);
+		pParse->rc = SQLITE_NOMEM_BKPT;
+		pParse->nErr++;
+		return NULL;
+	}
+
+	pTable->def = def;
+	pTable->zName = zName;
+	pTable->iPKey = -1;
+	pTable->iAutoIncPKey = -1;
+	pTable->pSchema = db->pSchema;
+	sqlite3HashInit(&pTable->idxHash);
+	pTable->nTabRef = 1;
+	pTable->nRowLogEst = 200;
+	assert(200 == sqlite3LogEst(1048576));
+	return pTable;
+}
+
  /*
   * Begin constructing a new table representation in memory.  This is
   * the first of several action routines that get called in response
@@ -547,21 +589,10 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
  		goto begin_table_error;
  	}
  
-	pTable = sqlite3DbMallocZero(db, sizeof(Table));
-	if (pTable == 0) {
-		assert(db->mallocFailed);
-		pParse->rc = SQLITE_NOMEM_BKPT;
-		pParse->nErr++;
+	pTable = sql_table_new(pParse, zName);
+	if (pTable == NULL)
  		goto begin_table_error;
-	}
-	pTable->zName = zName;
-	pTable->iPKey = -1;
-	pTable->iAutoIncPKey = -1;
-	pTable->pSchema = db->pSchema;
-	sqlite3HashInit(&pTable->idxHash);
-	pTable->nTabRef = 1;
-	pTable->nRowLogEst = 200;
-	assert(200 == sqlite3LogEst(1048576));
+
  	assert(pParse->pNewTable == 0);
  	pParse->pNewTable = pTable;
  
@@ -585,6 +616,47 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
  	return;
  }
  
+/**
+ * Get field by id. Allocate memory if needed.
+ * @param pParse SQL Parser object.
+ * @param pTable SQL Table object.
+ * @param id column identifier.
+ * @retval not NULL on success.
+ * @retval NULL on out of memory.
+ */
+static struct field_def *
+sql_field_retrieve(Parse *pParse, Table *pTable, uint32_t id)
+{
+	sqlite3 *db = pParse->db;
+	struct field_def *field;
+	assert(pTable->def);
+	assert(pTable->def->exact_field_count >= (uint32_t)pTable->nCol);
+	assert(id < (uint32_t)db->aLimit[SQLITE_LIMIT_COLUMN]);
+
+	if (id >= pTable->def->exact_field_count) {
+		uint32_t nCol = pTable->def->exact_field_count;
+		nCol = (nCol > 0) ? 2 * nCol : 1;
+		field = sqlite3DbRealloc(db, pTable->def->fields,
+					 nCol * sizeof(pTable->def->fields[0]));
+		if (field == NULL) {
+			pParse->rc = SQLITE_NOMEM_BKPT;
+			pParse->nErr++;
+			return NULL;
+		}
+
+		for (uint32_t i = nCol / 2; i < nCol; i++) {
+			memcpy(&field[i], &field_def_default,
+			       sizeof(struct field_def));
+		}
+
+		pTable->def->fields = field;
+		pTable->def->exact_field_count = nCol;
+	}
+
+	field = &pTable->def->fields[id];
+	return field;
+}
+
  /*
   * Add a new column to the table currently being constructed.
   *
@@ -610,6 +682,8 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
  		return;
  	}
  #endif
+	if (sql_field_retrieve(pParse, p, (uint32_t) p->nCol) == NULL)
+		return;
  	z = sqlite3DbMallocRaw(db, pName->n + 1);
  	if (z == 0)
  		return;
@@ -668,6 +742,7 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
  		}
  	}
  	p->nCol++;
+	p->def->field_count++;
  	pParse->constraintName.n = 0;
  }
  
@@ -813,7 +888,11 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan)
  			 * is required by pragma table_info.
  			 */
  			Expr x;
-			sql_expr_free(db, pCol->pDflt, false);
+			assert(p->def != NULL);
+			struct field_def *field =
+				&p->def->fields[p->nCol - 1];
+			sql_expr_free(db, field->default_value_expr, false);
+
  			memset(&x, 0, sizeof(x));
  			x.op = TK_SPAN;
  			x.u.zToken = sqlite3DbStrNDup(db, (char *)pSpan->zStart,
@@ -821,7 +900,9 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan)
  							    pSpan->zStart));
  			x.pLeft = pSpan->pExpr;
  			x.flags = EP_Skip;
-			pCol->pDflt = sqlite3ExprDup(db, &x, EXPRDUP_REDUCE);
+
+			field->default_value_expr =
+				sqlite3ExprDup(db, &x, EXPRDUP_REDUCE);
  			sqlite3DbFree(db, x.u.zToken);
  		}
  	}
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index f56b6d9cd..9b88b5f3a 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -36,6 +36,7 @@
  #include <box/coll.h>
  #include "sqliteInt.h"
  #include "box/session.h"
+#include "tarantoolInt.h"
  
  #ifndef SQLITE_OMIT_FOREIGN_KEY
  #ifndef SQLITE_OMIT_TRIGGER
@@ -1346,8 +1347,12 @@ fkActionTrigger(Parse * pParse,	/* Parse context */
  									     &tToCol,
  									     0));
  				} else if (action == OE_SetDflt) {
+					uint32_t space_id =
+						SQLITE_PAGENO_TO_SPACEID(
+							pFKey->pFrom->tnum);
  					Expr *pDflt =
-					    pFKey->pFrom->aCol[iFromCol].pDflt;
+						space_column_default_expr(
+							space_id, (uint32_t)iFromCol);
  					if (pDflt) {
  						pNew =
  						    sqlite3ExprDup(db, pDflt,
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b24d55b07..067f50a0a 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1801,14 +1801,21 @@ xferOptimization(Parse * pParse,	/* Parser context */
  		}
  		/* Default values for second and subsequent columns need to match. */
  		if (i > 0) {
-			assert(pDestCol->pDflt == 0
-			       || pDestCol->pDflt->op == TK_SPAN);
-			assert(pSrcCol->pDflt == 0
-			       || pSrcCol->pDflt->op == TK_SPAN);
-			if ((pDestCol->pDflt == 0) != (pSrcCol->pDflt == 0)
-			    || (pDestCol->pDflt
-				&& strcmp(pDestCol->pDflt->u.zToken,
-					  pSrcCol->pDflt->u.zToken) != 0)
+			uint32_t src_space_id =
+				SQLITE_PAGENO_TO_SPACEID(pSrc->tnum);
+			struct space *src_space =
+				space_cache_find(src_space_id);
+			uint32_t dest_space_id =
+				SQLITE_PAGENO_TO_SPACEID(pDest->tnum);
+			struct space *dest_space =
+				space_cache_find(dest_space_id);
+			char *src_expr_str =
+				src_space->def->fields[i].default_value;
+			char *dest_expr_str =
+				dest_space->def->fields[i].default_value;
+			if ((dest_expr_str == NULL) != (src_expr_str == NULL)
+			    || (dest_expr_str
+				&& strcmp(src_expr_str, dest_expr_str) != 0)
  			    ) {
  				return 0;	/* Default values must be the same for all columns */
  			}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index b724c9845..3ca94a1c3 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -36,6 +36,7 @@
  #include <box/index.h>
  #include <box/box.h>
  #include <box/tuple.h>
+#include "box/schema.h"
  #include "sqliteInt.h"
  #include "vdbeInt.h"
  #include "box/session.h"
@@ -64,6 +65,7 @@
   * ../tool/mkpragmatab.tcl.
   */
  #include "pragma.h"
+#include "tarantoolInt.h"
  
  /*
   * Interpret the given string as a safety level.  Return 0 for OFF,
@@ -370,19 +372,21 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
  						     i; k++) {
  						}
  					}
-					assert(pCol->pDflt == 0
-					       || pCol->pDflt->op == TK_SPAN);
  					bool nullable = table_column_is_nullable(pTab, i);
+					uint32_t space_id =
+						SQLITE_PAGENO_TO_SPACEID(
+							pTab->tnum);
+					struct space *space =
+						space_cache_find(space_id);
+					char *expr_str = space->
+						def->fields[i].default_value;
  					sqlite3VdbeMultiLoad(v, 1, "issisi",
  							     i, pCol->zName,
  							     field_type_strs[
  							     sqlite3ColumnType
  							     (pCol)],
  							     nullable == 0,
-							     pCol->
-							     pDflt ? pCol->
-							     pDflt->u.zToken
-							     : 0, k);
+							     expr_str, k);
  					sqlite3VdbeAddOp2(v, OP_ResultRow, 1,
  							  6);
  				}
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 59662cf14..7ce5bac5d 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1877,7 +1877,6 @@ struct Savepoint {
  struct Column {
  	char *zName;		/* Name of this column */
  	enum field_type type;	/* Column type. */
-	Expr *pDflt;		/* Default value of this column */
  	char *zColl;		/* Collating sequence.  If NULL, use the default */
  	enum on_conflict_action notNull;  /* An ON_CONFLICT_ACTION code for
  					   * handling a NOT NULL constraint
@@ -1970,6 +1969,8 @@ struct Table {
  	Trigger *pTrigger;	/* List of triggers stored in pSchema */
  	Schema *pSchema;	/* Schema that contains this table */
  	Table *pNextZombie;	/* Next on the Parse.pZombieTab list */
+	/** Space definition with Tarantool metadata. */
+	struct space_def *def;
  };
  
  /*
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 83c05ab48..274305352 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -35,6 +35,8 @@
   */
  #include "sqliteInt.h"
  #include "box/session.h"
+#include "tarantoolInt.h"
+#include "box/schema.h"
  
  /*
   * The most recently coded instruction was an OP_Column to retrieve the
@@ -75,7 +77,14 @@ sqlite3ColumnDefault(Vdbe * v, Table * pTab, int i, int iReg)
  		Column *pCol = &pTab->aCol[i];
  		VdbeComment((v, "%s.%s", pTab->zName, pCol->zName));
  		assert(i < pTab->nCol);
-		sqlite3ValueFromExpr(sqlite3VdbeDb(v), pCol->pDflt,
+
+		Expr *expr = NULL;
+		struct space *space =
+			space_cache_find(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
+		if (space != NULL && space->def->fields != NULL)
+			expr = space->def->fields[i].default_value_expr;
+		sqlite3ValueFromExpr(sqlite3VdbeDb(v),
+				     expr,
  				     pCol->affinity, &pValue);
  		if (pValue) {
  			sqlite3VdbeAppendP4(v, pValue, P4_MEM);