From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [tarantool-patches] [PATCH v1 1/2] sql: restrict nullable action definitions Date: Thu, 12 Jul 2018 19:34:27 +0300 [thread overview] Message-ID: <3155108e339f80906e3d4a3bd3a0885ed08e8ffc.1531413185.git.kshcherbatov@tarantool.org> (raw) In-Reply-To: <cover.1531413184.git.kshcherbatov@tarantool.org> In-Reply-To: <cover.1531413184.git.kshcherbatov@tarantool.org> This patch dissallows define multiple "NULL", "NOT NULL" options per column and fixes silent implicit behavior for invalid "NULL PRIMARY KEY" construction. Closes #3473. --- src/box/alter.cc | 1 + src/box/field_def.c | 1 + src/box/field_def.h | 3 ++- src/box/sql/build.c | 46 +++++++++++++++++++++++++++++++++++++++---- src/box/sql/parse.y | 2 +- test/sql/on-conflict.result | 13 ++++++++++++ test/sql/on-conflict.test.lua | 6 ++++++ 7 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 7b6bd1a..3791ac7 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -390,6 +390,7 @@ field_def_decode(struct field_def *field, const char **data, "nullable action", fieldno + TUPLE_INDEX_BASE)); } + assert(field->nullable_action != ON_CONFLICT_ACTION_UNDEFINED); if (!((field->is_nullable && field->nullable_action == ON_CONFLICT_ACTION_NONE) || (!field->is_nullable diff --git a/src/box/field_def.c b/src/box/field_def.c index 8dbead6..70a9e46 100644 --- a/src/box/field_def.c +++ b/src/box/field_def.c @@ -46,6 +46,7 @@ const char *field_type_strs[] = { }; const char *on_conflict_action_strs[] = { + /* [ON_CONFLICT_ACTION_UNDEFINED]= */ "undefined", /* [ON_CONFLICT_ACTION_NONE] = */ "none", /* [ON_CONFLICT_ACTION_ROLLBACK] = */ "rollback", /* [ON_CONFLICT_ACTION_ABORT] = */ "abort", diff --git a/src/box/field_def.h b/src/box/field_def.h index 05f80d4..c35e0b8 100644 --- a/src/box/field_def.h +++ b/src/box/field_def.h @@ -60,7 +60,8 @@ enum field_type { }; enum on_conflict_action { - ON_CONFLICT_ACTION_NONE = 0, + ON_CONFLICT_ACTION_UNDEFINED = 0, + ON_CONFLICT_ACTION_NONE, ON_CONFLICT_ACTION_ROLLBACK, ON_CONFLICT_ACTION_ABORT, ON_CONFLICT_ACTION_FAIL, diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 53c20a6..946b10c 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -653,7 +653,7 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType) struct field_def *column_def = &p->def->fields[p->def->field_count]; memcpy(column_def, &field_def_default, sizeof(field_def_default)); column_def->name = z; - column_def->nullable_action = ON_CONFLICT_ACTION_NONE; + column_def->nullable_action = ON_CONFLICT_ACTION_UNDEFINED; column_def->is_nullable = true; if (pType->n == 0) { @@ -701,9 +701,22 @@ sqlite3AddNotNull(Parse * pParse, int onError) p = pParse->pNewTable; if (p == 0 || NEVER(p->def->field_count < 1)) return; - p->def->fields[p->def->field_count - 1].nullable_action = (u8)onError; - p->def->fields[p->def->field_count - 1].is_nullable = - action_is_nullable(onError); + struct field_def *field = &p->def->fields[p->def->field_count - 1]; + if (field->nullable_action != ON_CONFLICT_ACTION_UNDEFINED) { + /* Prevent defining nullable_action many times. */ + const char *err_msg = + tt_sprintf("NULL declaration for column '%s' of table " + "'%s' has been already set to '%s'", + field->name, p->def->name, + on_conflict_action_strs[field-> + nullable_action]); + diag_set(ClientError, ER_SQL, err_msg); + pParse->rc = SQL_TARANTOOL_ERROR; + pParse->nErr++; + return; + } + field->nullable_action = (u8)onError; + field->is_nullable = action_is_nullable(onError); } /* @@ -1728,6 +1741,31 @@ sqlite3EndTable(Parse * pParse, /* Parse context */ p->def->id = SQLITE_PAGENO_TO_SPACEID(p->tnum); } + /* Set default on_nullable action if required. */ + for (uint32_t i = 0; i < p->def->field_count; i++) { + if (p->def->fields[i].nullable_action == + ON_CONFLICT_ACTION_UNDEFINED) { + p->def->fields[i].nullable_action = + ON_CONFLICT_ACTION_NONE; + p->def->fields[i].is_nullable = true; + } else if (p->iPKey == (int)i && + p->def->fields[i].nullable_action == + ON_CONFLICT_ACTION_NONE) { + /* + * PRIMARY KEY can not be defined with + * ON_CONFLICT_ACTION_NONE. + */ + const char *err_str = + tt_sprintf("Cannot define PRIMARY KEY " + "constraint on nullable column in " + "table '%s'", p->def->name); + diag_set(ClientError, ER_SQL, err_str); + pParse->rc = SQL_TARANTOOL_ERROR; + pParse->nErr++; + return; + } + } + if (!p->def->opts.is_view) { if ((p->tabFlags & TF_HasPrimaryKey) == 0) { sqlite3ErrorMsg(pParse, diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index ac935fd..6463019 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -276,7 +276,7 @@ ccons ::= DEFAULT id(X). { // In addition to the type name, we also care about the primary key and // UNIQUE constraints. // -ccons ::= NULL onconf. +ccons ::= NULL onconf. {sqlite3AddNotNull(pParse, ON_CONFLICT_ACTION_NONE);} ccons ::= NOT NULL onconf(R). {sqlite3AddNotNull(pParse, R);} ccons ::= PRIMARY KEY sortorder(Z) onconf(R) autoinc(I). {sqlite3AddPrimaryKey(pParse,0,R,I,Z);} diff --git a/test/sql/on-conflict.result b/test/sql/on-conflict.result index c0d0de0..bdc2c76 100644 --- a/test/sql/on-conflict.result +++ b/test/sql/on-conflict.result @@ -99,3 +99,16 @@ box.sql.execute('DROP TABLE t1') box.sql.execute('DROP TABLE t2') --- ... +-- +-- gh-3473: Primary key can be declared with NULL. +-- +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY NOT NULL);') +--- +- error: 'SQL error: NULL declaration for column ''S1'' of table ''TE17'' has been + already set to ''none''' +... +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY);') +--- +- error: 'SQL error: Cannot define PRIMARY KEY constraint on nullable column in table + ''TE17''' +... diff --git a/test/sql/on-conflict.test.lua b/test/sql/on-conflict.test.lua index b6d92f7..94eb84f 100644 --- a/test/sql/on-conflict.test.lua +++ b/test/sql/on-conflict.test.lua @@ -38,3 +38,9 @@ box.sql.execute('DROP TABLE p') box.sql.execute('DROP TABLE e') box.sql.execute('DROP TABLE t1') box.sql.execute('DROP TABLE t2') + +-- +-- gh-3473: Primary key can be declared with NULL. +-- +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY NOT NULL);') +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY);') -- 2.7.4
next prev parent reply other threads:[~2018-07-12 16:34 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-12 16:34 [tarantool-patches] [PATCH v1 0/2] " Kirill Shcherbatov 2018-07-12 16:34 ` Kirill Shcherbatov [this message] 2018-07-13 10:26 ` [tarantool-patches] Re: [PATCH v1 1/2] " Vladislav Shpilevoy 2018-07-13 16:05 ` Kirill Shcherbatov 2018-07-13 20:03 ` Vladislav Shpilevoy 2018-07-16 9:43 ` Kirill Shcherbatov 2018-07-16 10:20 ` Vladislav Shpilevoy 2018-07-16 12:27 ` Kirill Shcherbatov 2018-07-12 16:34 ` [tarantool-patches] [PATCH v1 2/2] sql: fixed possible leak in sqlite3EndTable Kirill Shcherbatov 2018-07-13 10:26 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-13 16:05 ` Kirill Shcherbatov 2018-07-13 10:26 ` [tarantool-patches] Re: [PATCH v1 0/2] sql: restrict nullable action definitions Vladislav Shpilevoy 2018-07-16 12:28 ` [tarantool-patches] [PATCH v1 2/4] sql: refactor sqlite3AddNotNull function Kirill Shcherbatov 2018-07-16 13:41 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-16 12:28 ` [tarantool-patches] [PATCH v1 4/4] sql: get rid of Column structure Kirill Shcherbatov 2018-07-16 13:40 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-16 16:33 ` Kirill Shcherbatov 2018-07-17 9:32 ` Vladislav Shpilevoy 2018-07-17 14:08 ` Kirill Shcherbatov 2018-07-17 22:01 ` Vladislav Shpilevoy 2018-07-18 7:25 ` Kirill Shcherbatov 2018-07-18 8:04 ` Vladislav Shpilevoy 2018-07-18 16:41 ` n.pettik
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=3155108e339f80906e3d4a3bd3a0885ed08e8ffc.1531413185.git.kshcherbatov@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [tarantool-patches] [PATCH v1 1/2] sql: restrict nullable action definitions' \ /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