Thanks for the patch! See 2 comments below.
commit 5d1010dfcd7d0666774afaaf5934de018f99e181
Author: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
Date: Mon Jul 16 14:21:54 2018 +0300
sql: get rid of Column structure
Get rid of is_primkey in Column structure as it become
redundant. Moved the last member coll with collation pointer
to field_def structure. Finally, dropped Column.
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index fe54e5531..c2a211c95 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -146,7 +146,6 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
Table *pNew; /* Copy of pParse->pNewTable */
Table *pTab; /* Table being altered */
const char *zTab; /* Table name */
- Column *pCol; /* The new column */
Expr *pDflt; /* Default value for the new column */
sqlite3 *db; /* The database connection; */
Vdbe *v = pParse->pVdbe; /* The prepared statement under
construction */
@@ -161,7 +160,6 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
/* Skip the "sqlite_altertab_" prefix on the name. */
zTab = &pNew->def->name[16];
- pCol = &pNew->aCol[pNew->def->field_count - 1];
assert(pNew->def != NULL);
pDflt = space_column_default_expr(SQLITE_PAGENO_TO_SPACEID(pNew->tnum),
pNew->def->field_count - 1);
@@ -181,7 +179,8 @@ 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->is_primkey) {
+ struct Index *pk = sqlite3PrimaryKeyIndex(pTab);
+ if (index_has_column(pk, pNew->def->field_count - 1)) {
sqlite3ErrorMsg(pParse, "Cannot add a PRIMARY KEY column");
return;
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index ee97ef9ee..ce4012081 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -114,40 +114,70 @@ sql_finish_coding(struct Parse *parse_context)
}
}
+/**
+ * Test if @column_idx is a part of @index.
+ *
+ * @param index to lookup.
+ * @column_idx index of column to test.
+ * @retval true if index contains column_idx.
+ * @retval false else.
+ */
+bool
+index_has_column(struct Index *index, uint32_t column_idx)
+{
+ uint32_t parts_count = index->def->key_def->part_count;
+ struct key_part *parts = index->def->key_def->parts;
+ for (uint32_t i = 0; i < parts_count; i++) {
+ if (column_idx == parts[i].fieldno)
+ return true;
+ }
+ return false;
+}
+
/**
* This is a function which should be called during execution
- * of sqlite3EndTable. It ensures that only PRIMARY KEY
- * constraint may have ON CONFLICT REPLACE clause.
+ * of sqlite3EndTable. It set defaults for columns having no
+ * separate NULL/NOT NULL specifiers and ensures that only
+ * PRIMARY KEY constraint may have ON CONFLICT REPLACE clause.
*
+ * @param parser SQL Parser object.
* @param table Space which should be checked.
- * @retval False, if only primary key constraint has
- * ON CONFLICT REPLACE clause or if there are no indexes
- * with REPLACE as error action. True otherwise.
+ * @retval -1 on error. Parser SQL_TARANTOOL_ERROR is set.
+ * @retval 0 on success.
*/
-static bool
-check_on_conflict_replace_entries(struct Table *table)
-{
- /* Check all NOT NULL constraints. */
- for (int i = 0; i < (int)table->def->field_count; i++) {
- enum on_conflict_action on_error =
- table->def->fields[i].nullable_action;
- if (on_error == ON_CONFLICT_ACTION_REPLACE &&
- table->aCol[i].is_primkey == false) {
- return true;
+static int
+actualize_on_conflict_actions(struct Parse *parser, struct Table *table)
+{
+ const char *err_msg = NULL;
+ struct field_def *field = table->def->fields;
+ struct Index *pk = sqlite3PrimaryKeyIndex(table);
+ for (uint32_t i = 0; i < table->def->field_count; ++i, ++field) {
+ if (field->nullable_action == on_conflict_action_MAX) {
+ /* Set default. */
+ field->nullable_action = ON_CONFLICT_ACTION_NONE;
+ field->is_nullable = true;
}
+ if (field->nullable_action == ON_CONFLICT_ACTION_REPLACE &&
+ !index_has_column(pk, i))
+ goto non_pk_on_conflict_error;
}
- /* Check all UNIQUE constraints. */
+
for (struct Index *idx = table->pIndex; idx; idx = idx->pNext) {
if (idx->onError == ON_CONFLICT_ACTION_REPLACE &&
- !IsPrimaryKeyIndex(idx)) {
- return true;
- }
+ !IsPrimaryKeyIndex(idx))
+ goto non_pk_on_conflict_error;
}
- /*
- * CHECK constraints are not allowed to have REPLACE as
- * error action and therefore can be skipped.
- */
- return false;
+
+ return 0;
+
+non_pk_on_conflict_error:
+ err_msg = tt_sprintf("only PRIMARY KEY constraint can have "
+ "ON CONFLICT REPLACE clause - %s",
+ table->def->name);
+ diag_set(ClientError, ER_SQL, err_msg);
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+ return -1;
}