Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org
Cc: "v.shpilevoy@tarantool.org" <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 1/1] Removed Expr pointer from SQL Column structure.
Date: Tue, 17 Apr 2018 14:02:24 +0300	[thread overview]
Message-ID: <07b96d39-dcd4-9900-defe-3f11b9b20537@tarantool.org> (raw)
In-Reply-To: <71db35b8-be0a-34be-07ab-09d63c4c39cd@tarantool.org>

New commit message:
    sql: remove Expr pointer from SQL Column struct
    
    Introduced space_def field in SQL Table structure which
    already contains Expr field as it is required by
    global Parser refactoring.
    
    Needed for #3051.


From 876a96547dfe69af760bfc39bdd0759b8dcadd1f Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Tue, 17 Apr 2018 13:57:10 +0300
Subject: [PATCH] Accounted Vald's commits.

---
 src/box/space_def.c     |  2 --
 src/box/sql.c           | 11 +--------
 src/box/sql.h           | 11 ---------
 src/box/sql/alter.c     |  4 ++--
 src/box/sql/build.c     | 60 +++++++++++++++++++++----------------------------
 src/box/sql/fkey.c      | 12 ++++++----
 src/box/sql/insert.c    | 27 ++++++++++++----------
 src/box/sql/pragma.c    | 18 ++++++++-------
 src/box/sql/sqliteInt.h |  3 +--
 src/box/sql/update.c    | 16 +++++++++++--
 10 files changed, 77 insertions(+), 87 deletions(-)

diff --git a/src/box/space_def.c b/src/box/space_def.c
index 5a4fd6d..22bd3ca 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -239,8 +239,6 @@ space_def_destroy_fields(struct field_def *fields, uint32_t field_count)
 void
 space_def_delete(struct space_def *def)
 {
-	if (def == NULL)
-		return;
 	space_opts_destroy(&def->opts);
 	tuple_dictionary_unref(def->dict);
 	space_def_destroy_fields(def->fields, def->field_count);
diff --git a/src/box/sql.c b/src/box/sql.c
index 6418cbd..6bc82c2 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1466,7 +1466,7 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf)
 	for (i = 0; i < n; i++) {
 		const char *t;
 		struct coll *coll = NULL;
-		struct field_def *field = sql_field_get(pTable, i);
+		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) {
@@ -1712,12 +1712,3 @@ space_column_default_expr(uint32_t space_id, uint32_t fieldno)
 
 	return space->def->fields[fieldno].default_value_expr;
 }
-
-struct field_def *
-sql_field_get(struct Table *pTable, int id)
-{
-	assert(pTable->def);
-	assert((uint32_t)id < pTable->def->exact_field_count);
-	assert((uint32_t)id < pTable->def->field_count);
-	return &pTable->def->fields[id];
-}
diff --git a/src/box/sql.h b/src/box/sql.h
index d177341..54981a3 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -145,17 +145,6 @@ sql_expr_dup(struct sqlite3 *db, struct Expr *p, int flags, char **buffer);
 void
 sql_expr_free(struct sqlite3 *db, struct Expr *expr, bool extern_alloc);
 
-/**
- * Get field by id.
- * @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.
- */
-struct field_def *
-sql_field_get(struct Table *pTable, int id);
-
 #if defined(__cplusplus)
 } /* extern "C" { */
 #endif
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index d2e0968..39ae070 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -161,8 +161,8 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
 
 	zTab = &pNew->zName[16];	/* Skip the "sqlite_altertab_" prefix on the name */
 	pCol = &pNew->aCol[pNew->nCol - 1];
-	struct field_def *field = sql_field_get(pNew, pNew->nCol - 1);
-	pDflt = field->default_value_expr;
+	assert(pNew->def);
+	pDflt = pNew->def->fields[pNew->nCol - 1].default_value_expr;
 	pTab = sqlite3HashFind(&db->pSchema->tblHash, zTab);;
 	assert(pTab);
 
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index d6033c9..394ee3a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -495,28 +495,26 @@ 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 changed.
+ * @retval not NULL on success.
+ */
 static Table *
-sql_table_new(Parse *pParse, char *zName, uint32_t nFields)
+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 /* space id */, 0 /* user id */,
-					      0, "ephemeral",
-					      strlen("ephemeral"), "memtx",
-					      strlen("memtx"),
+	struct space_def *def = space_def_new(0, 0, 0, NULL, 0, NULL, 0,
 					      &space_opts_default,
-					      &field_def_default,
-					      0/* length of field_def */);
-	struct field_def *fields =
-		sqlite3DbMallocZero(db,
-				    nFields*sizeof(struct field_def));
-	if (pTable == NULL || def == NULL || fields == NULL) {
+					      &field_def_default, 0);
+	if (pTable == NULL || def == NULL) {
 		assert(db->mallocFailed);
-		space_def_delete(def);
-		sqlite3DbFree(db, fields);
+		if (def != NULL)
+			space_def_delete(def);
 		sqlite3DbFree(db, pTable);
 		pParse->rc = SQLITE_NOMEM_BKPT;
 		pParse->nErr++;
@@ -524,12 +522,10 @@ sql_table_new(Parse *pParse, char *zName, uint32_t nFields)
 	}
 
 	pTable->def = def;
-	pTable->def->fields = fields;
-	for (uint32_t i = 0; i < nFields; i++)
-		memcpy(&def->fields[i], &field_def_default,
-		       sizeof(struct field_def));
 	/* store allocated fields count */
-	def->exact_field_count = nFields;
+	def->exact_field_count = 0;
+	def->field_count = 0;
+	pTable->def->fields = NULL;
 
 	pTable->zName = zName;
 	pTable->iPKey = -1;
@@ -599,7 +595,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
 		goto begin_table_error;
 	}
 
-	pTable = sql_table_new(pParse, zName, 1);
+	pTable = sql_table_new(pParse, zName);
 	if (pTable == NULL)
 		goto begin_table_error;
 
@@ -639,16 +635,15 @@ sql_field_retrieve(Parse *pParse, Table *pTable, uint32_t id)
 {
 	sqlite3 *db = pParse->db;
 	struct field_def *field;
-	assert(pTable->def && pTable->def->fields);
+	assert(pTable->def);
 	assert(pTable->def->exact_field_count >= (uint32_t)pTable->nCol);
-	assert(id < pTable->def->exact_field_count * 2);
 	assert(id < (uint32_t)db->aLimit[SQLITE_LIMIT_COLUMN]);
 
 	if (id >= pTable->def->exact_field_count) {
-		field =
-			sqlite3DbRealloc(db, pTable->def->fields,
-					 pTable->def->exact_field_count * 2
-					 * sizeof(pTable->def->fields[0]));
+		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) {
 			assert(db->mallocFailed);
 			pParse->rc = SQLITE_NOMEM_BKPT;
@@ -656,13 +651,12 @@ sql_field_retrieve(Parse *pParse, Table *pTable, uint32_t id)
 			return NULL;
 		}
 
-		for (uint32_t i = pTable->def->exact_field_count;
-			i < 2*pTable->def->exact_field_count; i++)
+		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 *= 2;
+		pTable->def->exact_field_count = nCol;
 	}
 
 	field = &pTable->def->fields[id];
@@ -900,9 +894,9 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan)
 			 * is required by pragma table_info.
 			 */
 			Expr x;
-
+			assert(p->def);
 			struct field_def *field =
-				sql_field_get(p, (uint32_t) p->nCol - 1);
+				&p->def->fields[p->nCol - 1];
 			sql_expr_free(db, field->default_value_expr, false);
 
 			memset(&x, 0, sizeof(x));
@@ -915,8 +909,6 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan)
 
 			field->default_value_expr =
 				sqlite3ExprDup(db, &x, EXPRDUP_REDUCE);
-
-			pCol->pDflt = 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 625cf3c..d196fd4 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -36,6 +36,8 @@
 #include <box/coll.h>
 #include "sqliteInt.h"
 #include "box/session.h"
+#include "box/schema.h"
+#include "tarantoolInt.h"
 
 #ifndef SQLITE_OMIT_FOREIGN_KEY
 #ifndef SQLITE_OMIT_TRIGGER
@@ -1346,10 +1348,12 @@ fkActionTrigger(Parse * pParse,	/* Parse context */
 									     &tToCol,
 									     0));
 				} else if (action == OE_SetDflt) {
-					struct field_def *field =
-						sql_field_get(pFKey->pFrom,
-							      iFromCol);
-					Expr *pDflt = field->default_value_expr;
+					uint32_t space_id =
+						SQLITE_PAGENO_TO_SPACEID(
+							pFKey->pFrom->tnum);
+					Expr *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 f6db89a..067f50a 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1801,18 +1801,21 @@ xferOptimization(Parse * pParse,	/* Parser context */
 		}
 		/* Default values for second and subsequent columns need to match. */
 		if (i > 0) {
-			struct field_def *pSrcField = sql_field_get(pSrc, i);
-			struct field_def *pDestField = sql_field_get(pSrc, i);
-			Expr *pSrcDflt = pSrcField->default_value_expr;
-			Expr *pDestDflt = pDestField->default_value_expr;
-			assert(pDestDflt == 0
-			       || pDestDflt->op == TK_SPAN);
-			assert(pSrcDflt == 0
-			       || pSrcDflt->op == TK_SPAN);
-			if ((pDestDflt == 0) != (pSrcDflt == 0)
-			    || (pDestDflt
-				&& strcmp(pDestDflt->u.zToken,
-					  pSrcDflt->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 c0bf7fd..3ca94a1 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,
@@ -359,9 +361,6 @@ 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++) {
-					struct field_def *field =
-						sql_field_get(pTab, i);
-					Expr *pDflt = field->default_value_expr;
 					if (!table_column_is_in_pk(pTab, i)) {
 						k = 0;
 					} else if (pPk == 0) {
@@ -373,18 +372,21 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 						     i; k++) {
 						}
 					}
-					assert(pDflt == 0
-					       || 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,
-							     pDflt ?
-							     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 9a7d99c..90b7e08 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,7 +1969,7 @@ 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 */
-	struct space_def *def;
+	struct space_def *def;	/* Space definition with Tarantool metadata */
 };
 
 /*
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index f6aa24b..2505a5d 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,8 +77,18 @@ 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);
-		Expr *expr = pTab->def ?
-			     sql_field_get(pTab, i)->default_value_expr : NULL;
+
+		Expr *expr = NULL;
+		struct space *space =
+			space_cache_find(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
+		/* FIXME: ephemeral spaces are not present in the cache now */
+		if (space != NULL) {
+			expr = space->def->fields[i].default_value_expr;
+		} else {
+			expr = pTab->def ?
+			       pTab->def->fields[i].default_value_expr : NULL;
+		}
+
 		sqlite3ValueFromExpr(sqlite3VdbeDb(v),
 				     expr,
 				     pCol->affinity, &pValue);
-- 
2.7.4



On 16.04.2018 22:23, Vladislav Shpilevoy wrote:
> Hello. Thanks for the patch, it looks almost ok, your patches become 
> better and better! See my 11 comments below.
> 
> 
> On 16/04/2018 19:35, Kirill Shcherbatov wrote:
>> Introduced space_def field in SQL Table structure which
>> already contains Expr field.
>>
>> Needed for #3051.
> 
> 1. Please, use this guide to write commit messages: 
> https://tarantool.io/en/doc/1.9/dev_guide/developer_guidelines.html#how-to-write-a-commit-message
> 
> In particular this:
> - limit the subject line to 50 characters,
> - prefix the subject with a subsystem name. Here it is 'sql: ',
> - do not end the subject line with a period,
> - use the imperative mood in the subject line.
> 
>> diff --git a/src/box/space_def.c b/src/box/space_def.c
>> index 22bd3ca..5a4fd6d 100644
>> --- a/src/box/space_def.c
>> +++ b/src/box/space_def.c
>> @@ -239,6 +239,8 @@ space_def_destroy_fields(struct field_def *fields, uint32_t field_count)
>>   void
>>   space_def_delete(struct space_def *def)
>>   {
>> +	if (def == NULL)
>> +		return;
> 
> 2. The space_def_delete must not be called with NULL. Please, fix the
> caller function instead of space_def_delete.
> 
>>   	space_opts_destroy(&def->opts);
>>   	tuple_dictionary_unref(def->dict);
>>   	space_def_destroy_fields(def->fields, def->field_count);
>> diff --git a/src/box/sql.c b/src/box/sql.c
>> index a6713f1..6418cbd 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 = sql_field_get(pTable, i);
> 
> 3. The wrapper for assertions only is useless. Please, inline it with
> no these obvious assertions.
> 
>> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
>> index 129ef82..d2e0968 100644
>> --- a/src/box/sql/alter.c
>> +++ b/src/box/sql/alter.c
>> @@ -297,7 +298,6 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
>>   		Column *pCol = &pNew->aCol[i];
>>   		pCol->zName = sqlite3DbStrDup(db, pCol->zName);
>>   		pCol->zColl = 0;
>> -		pCol->pDflt = 0;
> 
> 4. I jumped around the patch and it looks, that you forgot the main part
> of it - removal of struct Column.pDflt. Thanks to you, now it is very
> easy.
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 92f3cb6..d6033c9 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -490,6 +495,53 @@ sqlite3PrimaryKeyIndex(Table * pTab)
>>   	return p;
>>   }
>>   
>> +static Table *
>> +sql_table_new(Parse *pParse, char *zName, uint32_t nFields)
> 
> 5. Why do you need nFields argument? It is always 1. And I
> can not understand why 1? When a parser sees 'CREATE TABLE name (', it
> does not know even about a first column. Please, write a comments for
> the function.
> 
>> +{
>> +
>> +	sqlite3 *db = pParse->db;
>> +
>> +
>> +	Table *pTable = sqlite3DbMallocZero(db, sizeof(Table));
>> +	struct space_def *def = space_def_new(0 /* space id */, 0 /* user id */,
>> +					      0, "ephemeral",
>> +					      strlen("ephemeral"), "memtx",
>> +					      strlen("memtx"),
>> +					      &space_opts_default,
>> +					      &field_def_default,
>> +					      0/* length of field_def */);
> 
> 6. Looks like you copy-pasted this code. The space_def is created for
> defaults only, and it must be mentioned here in comments. Other
> space_def fields are dummy and invalid. It means that its name must be
> NULL, engine must be NULL, fields array must be NULL. And do not put
> comments on each argument - it must be done in function declaration, but
> not on each function call. Ok? Please, fix it. I do not know, why
> ephemeral tables violate these rules(
>> @@ -585,6 +626,49 @@ 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)
> 
> Off topic: good name.
> 
>> diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
>> index f56b6d9..625cf3c 100644
>> --- a/src/box/sql/fkey.c
>> +++ b/src/box/sql/fkey.c
>> @@ -1346,8 +1346,10 @@ fkActionTrigger(Parse * pParse,	/* Parse context */
>>   									     &tToCol,
>>   									     0));
>>   				} else if (action == OE_SetDflt) {
>> -					Expr *pDflt =
>> -					    pFKey->pFrom->aCol[iFromCol].pDflt;
>> +					struct field_def *field =
>> +						sql_field_get(pFKey->pFrom,
>> +							      iFromCol);
> 
> 7. It is not DDL, so here please use space_def from the space, not from
> the table. Use here space_column_default_expr.
>> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
>> index b24d55b..f6db89a 100644
>> --- a/src/box/sql/insert.c
>> +++ b/src/box/sql/insert.c
>> @@ -1801,14 +1801,18 @@ 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)
>> +			struct field_def *pSrcField = sql_field_get(pSrc, i);
>> +			struct field_def *pDestField = sql_field_get(pSrc, i);
> 
> 8. Same as 7.
>> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
>> index b724c98..c0bf7fd 100644
>> --- a/src/box/sql/pragma.c
>> +++ b/src/box/sql/pragma.c
>> @@ -359,6 +359,9 @@ 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++) {
>> +					struct field_def *field =
>> +						sql_field_get(pTab, i);
>> +					Expr *pDflt = field->default_value_expr;
> 
> 9. Same as 7.
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 59662cf..9a7d99c 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -1970,6 +1970,7 @@ 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 */
>> +	struct space_def *def;
> 
> 10. Please, write a comment, for what this member is used, and what
> stores.
> 
> 
>>   };
>>   
>>   /*
>> diff --git a/src/box/sql/update.c b/src/box/sql/update.c
>> index 83c05ab..f6aa24b 100644
>> --- a/src/box/sql/update.c
>> +++ b/src/box/sql/update.c
>> @@ -75,7 +75,10 @@ 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 = pTab->def ?
>> +			     sql_field_get(pTab, i)->default_value_expr : NULL;
>> +		sqlite3ValueFromExpr(sqlite3VdbeDb(v),
>> +				     expr,
>>   				     pCol->affinity, &pValue);
> 
> 11. Same as 7.
> 
>>   		if (pValue) {
>>   			sqlite3VdbeAppendP4(v, pValue, P4_MEM);
>>
> 

  reply	other threads:[~2018-04-17 11:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16 16:35 [tarantool-patches] " Kirill Shcherbatov
2018-04-16 19:23 ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-17 11:02   ` Kirill Shcherbatov [this message]
2018-04-17 18:29     ` Vladislav Shpilevoy
2018-04-17 18:30       ` Vladislav Shpilevoy
2018-04-18  7:31         ` Kirill Shcherbatov
2018-04-18  9:47           ` Vladislav Shpilevoy
2018-04-19 12:58             ` n.pettik
2018-04-19 14:07               ` Kirill Shcherbatov
2018-04-19 15:05                 ` n.pettik
2018-04-21  8:51                   ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07b96d39-dcd4-9900-defe-3f11b9b20537@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH v2 1/1] Removed Expr pointer from SQL Column structure.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox