[tarantool-patches] Re: [PATCH v2 9/9] sql: run check constraint tests on space alter
Kirill Shcherbatov
kshcherbatov at tarantool.org
Tue Apr 16 16:51:13 MSK 2019
> Mention that format is required condition to use check
> constraints: otherwise, name of field can’t be resolved.
> Now it leads to assertion fault:
>
> s = box.schema.create_space('test’)
> s:create_index('pk’)
> box.space._ck_constraint:insert({'physics', s.id, 'X<Y’})
> Assertion failed: (space_def->field_count > 0), function lookupName, file tarantool/src/box/sql/resolve.c, line 242.
Thank you. Fixed it in previous patch.
>
>> box.space._ck_constraint:insert({'physics', s.id, 'X<Y'})
>> box.space.test:insert({6, 5})
>> - error: 'Check constraint failed: physics’
>
> To finish this patch-set I suggest to add Lua-wrapper to create
> check constraints on any space using NoSQL interface and introduce
> ALTER TABLE ADD CONSTRAINT CHECK().
> Last issue you can implement in a separate patch or delegate its
> implementation to smb else.
Consider my new commit.
>> + vdbe_emit_ck_constraint(&parser, expr, ck_constraint->def->name,
>> + new_tuple_reg);
>> + sql_finish_coding(&parser);
>
> Do we need to call this function at all?
Yes. I've tried to drop it.
>
>> + if (parser.is_aborted) {
>> + diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
>> + ck_constraint->def->name,
>> + "can not compile expression”);
>
> This error will re-set original parsing error. I suggest to
> concatenate them.
Done.
>> + sql_reset(ck_constraint->stmt);
>
> This function returns error code, don’t ignore it.
Now I use it's result to determine execution state at the end of trigger.
>> + parser->ckBase = new_tuple_reg;
>> + struct Vdbe *v = sqlGetVdbe(parser);
>> + const char *ck_constraint_name = sqlDbStrDup(parser->db, name);
>
> Where's this pointer released?
>
>> + VdbeNoopComment((v, "BEGIN: ck constraint %s test", name));
>> + /* Skip check when it is turned off. */
Now this string is a part of VdbeNoopComment so VDBE must release it.
> When it can be turned off?
Outdated comment.
> Please, add descent set of tests verifying that check constraints
> work in any possible scenario.
> Make sure that check occurs before replace action.
Many new good tests are the part of previous patch now.
More information about the Tarantool-patches
mailing list