[tarantool-patches] Re: [PATCH v2 1/3] box: an ability to disable CK constraints

[Kirill Shcherbatov]

>> +	struct memtx_engine *memtx =
>> +		(struct memtx_engine *)engine_by_name("memtx");
>> +	assert(memtx != NULL);
> 
> Wait a second, how it is supposed to work with vinyl?
All system spaces use memtx engine. Thus looking for memtx engine state is an
acceptable way to determine, whether Tarantool loaded.

> Nit: method is calles "enable", but it can also disable trigger.
> I'd rename it to "_set_status"/"set_state" etc
I don't like both your variants. "enable" has been proposed by Kostya.

>> +	if (space_foreach(space_run_ck_constraints, (void *)true) != 0)
>> +		unreachable();
> 
> What does this change do? Why do you need param if it is always true?
In the first patch version I used to call this function space_run_ck_constraints with
false and true in two places to enable/disable all check constraint. In my opinion
hardcoding true in smth like space_enable_ck_constraints is not really good idea.

> Is space_is_system() check vital?
It may be omitted. Initially I assumed that system spaces cold not have ck constraints
but actually it is not so now, they could.