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

  • From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx, Nikita Pettik <korablev@xxxxxxxxxxxxx>
  • Date: Mon, 23 Jul 2018 11:31:45 +0300

Hi again!
I've rebased my patch on Kirill's Y. branch kyukhin/gh-3482-remove-ipkey;
I've also have succeed in using  ON_CONFLICT_ACTION_DEFAULT instead of 
on_conflict_action_MAX.
This changes represented as "[draft] don't use on_conflict_default_MAX" commit 
on my branch.
I've have had to introduce small rule index_onconf that looks very similar with 
onconf. If I not mistaken,
you are going to get rid of Index on_conflict action; and this rudiment crap 
would disappear. 

On 20.07.2018 10:29, Kirill Shcherbatov wrote:

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.

Well, it seems to be total mess. DEFAULT (for nullable on conflict action)
is always converted into ABORT. If you move this conversation right after
parser’s pass, you can get rid off using additional enum value:

create table t1(a NULL PRIMARY KEY NOT NULL , b);

a == DEFAULT —> NONE —> ABORT
b == DEFAULT —> NONE
Please, read my previous message again. 

I need to distinguish no-initialized and initialized columns.
+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'''

I need a marker. I've tried to use ON_CONFLICT_ACTION_DEFAULT in this role, 
but it looks not
workable until Index->onError present: onconf parse.y rule should be 
universal.

Let's discuss this verbally if you need.



Other related posts: