From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id E499E21091 for ; Wed, 18 Jul 2018 16:12:29 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nHX49x2GD5p6 for ; Wed, 18 Jul 2018 16:12:29 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 2EF0E20FBA for ; Wed, 18 Jul 2018 16:12:29 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v1 1/3] sql: restrict nullable action definitions From: "n.pettik" In-Reply-To: <33314a00ce2e0602e0a0fa7e30257ff03e3eed16.1531932662.git.kshcherbatov@tarantool.org> Date: Wed, 18 Jul 2018 23:12:26 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <51802451-C9AA-4EB8-8926-D08446738FF1@tarantool.org> References: <33314a00ce2e0602e0a0fa7e30257ff03e3eed16.1531932662.git.kshcherbatov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov > void > -sqlite3AddNotNull(Parse * pParse, int onError) > +sql_column_nullable_action_add(struct Parse *parser, > + enum on_conflict_action nullable_action) This name sounds really weird.. Why not sql_column_add_nullable_action ? > { > - Table *p; > - p =3D pParse->pNewTable; > - if (p =3D=3D 0 || NEVER(p->def->field_count < 1)) > + struct Table *p =3D parser->pNewTable; > + if (p =3D=3D NULL || NEVER(p->def->field_count < 1)) > return; > - p->def->fields[p->def->field_count - 1].nullable_action =3D = (u8)onError; > - p->def->fields[p->def->field_count - 1].is_nullable =3D > - action_is_nullable(onError); > + struct field_def *field =3D &p->def->fields[p->def->field_count = - 1]; > + if (field->nullable_action !=3D on_conflict_action_MAX) { Why do you need on_conflict_action_MAX when you already have = ON_CONFLICT_ACTION_DEFAULT? Anyway, there is no action DEFAULT, it is sooner or later converted to = ABORT. > + /* Prevent defining nullable_action many times. */ > + const char *err_msg =3D > + 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); > + parser->rc =3D SQL_TARANTOOL_ERROR; > + parser->nErr++; > + return; > + } > + field->nullable_action =3D nullable_action; > + field->is_nullable =3D action_is_nullable(nullable_action); > } >=20 > /* > @@ -1251,7 +1271,10 @@ convertToWithoutRowidTable(Parse * pParse, = Table * pTab) > if (pTab->iPKey >=3D 0) { > ExprList *pList; > Token ipkToken; > - sqlite3TokenInit(&ipkToken, = pTab->def->fields[pTab->iPKey].name); > + struct field_def *field =3D &fields[pTab->iPKey]; Could we avoid using iPKey in this function? We are going to remove it = soon. > + if (field_def_create_for_pk(pParse, field, space_name) = !=3D 0) > + return; > + sqlite3TokenInit(&ipkToken, field->name); > pList =3D sql_expr_list_append(pParse->db, NULL, > sqlite3ExprAlloc(db, TK_ID, > &ipkToken, = 0)); > index b6d92f7..5ae6b0c 100644 > --- a/test/sql/on-conflict.test.lua > +++ b/test/sql/on-conflict.test.lua > @@ -38,3 +38,11 @@ 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. Lets write =E2=80=98can=E2=80=99t be declared with NULL=E2=80=99. = Otherwise, it seems to be confusing. > +-- > +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY NOT = NULL);') > +box.sql.execute('CREATE TABLE te17 (s1 INT NULL PRIMARY KEY);') > +box.sql.execute("CREATE TABLE test (a int PRIMARY KEY, b int NULL ON = CONFLICT IGNORE);") > +box.sql.execute("CREATE TABLE test (a int, b int NULL, c int, PRIMARY = KEY(a, b, c))") > --=20