[tarantool-patches] Re: [PATCH v1 1/3] sql: restrict nullable action definitions

[Kirill Shcherbatov]

> This name sounds really weird.. Why not sql_column_add_nullable_action ?
If you wish.
-sql_column_nullable_action_add(struct Parse *parser,
+sql_column_add_nullable_action(struct Parse *parser,

> 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.
This is the central idea of the patch.  on_conflict_action_MAX is a marker that this field wasn't
initialized yet manually. This allows to detect second attempt to specify NULL/NOT NULL etc.
There is a comment about this concept in sqlite3AddColumn where on_conflict_action_MAX is set.
The default behavior is  ON_CONFLICT_ACTION_NONE and we have to distinguish non-initialized
columns and initialized with ON_CONFLICT_ACTION_DEFAULT.

> Could we avoid using iPKey in this function? We are going to remove it soon.
I don't increase a complexity here and believe that is patch is not about to suggest a way to exclude iPkey.
No idea.

-		sqlite3TokenInit(&ipkToken, pTab->def->fields[pTab->iPKey].name);
+		struct field_def *field = &fields[pTab->iPKey];


> Lets write ‘can’t be declared with NULL’. Otherwise, it seems to be confusing.
--- gh-3473: Primary key can be declared with NULL.
+-- gh-3473: Primary key can't be declared with NULL.