[tarantool-patches] Re: [PATCH v1 3/3] sql: dissallow bindings for DDL

Kirill Shcherbatov kshcherbatov at tarantool.org
Thu Sep 6 16:04:58 MSK 2018


> Previous two patches looks OK to me.
>> Bindings could not be used in stored ACTs because they allocate
> Nit: “AST”.
>> memory registers and makes
> Nit: “make” (or better “process").
>> assignments on parse sequentially.
> Nit: “during parsing”.
> Also, I guess problem is quite more sophisticated as it seems:
> a) For check constraint bindings really make no sense
> (I can’t come up with example how they can be used there at all).
> b) For triggers currently we don’t have proper mechanism, which
> would allow to use bindings. Original SQLite also lacks it.
> To reuse the same trigger’s body with different parameters we should
> not only be able to store it as prepared statement and substitute literals,
> but also give trigger new name. 
>> Original sqlite3 did validations that persistent AST doesn't have
>> auto-assigment Varibles on triggers and checks creation.
>> On DDL integration complete we've get rid this mechanism.
> Nits: “completion”, “got rid of”.
>> Now it should be returned.
> 
> Well, actually your approach is slightly different: explain that
> DDL (to be more precise - triggers and checks creation) relies on
> parse_only flag in parser. Hence, you can check it and throw an
> error during parsing.

    sql: dissallow bindings for DDL
    
    Bindings could not be used in stored ASTs because they allocate
    memory registers and process assignments during parsing(temporal register
    should be allocated for each variable in parse context, but it is temporal).
    Original sqlite3 did validations that AST to be persisted doesn't have
    auto-assigment Varibles on triggers and checks creation.
    On DDL integration completion we've got rid this mechanism.
    Now it should be returned.
    We use flag 'parse_only' which is set by parser on compiling AST to
    determine attempt to use bindings in DDL(triggers, defaults and checks
    creation) that is incorrect and raise an error.
    
    Closes #3653.




More information about the Tarantool-patches mailing list