From: Konstantin Osipov <kostja@tarantool.org> To: tarantool-patches@freelists.org Cc: korablev@tarantool.org, Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v1 4/4] sql: make sql checks on server side Date: Fri, 18 Jan 2019 21:11:21 +0300 [thread overview] Message-ID: <20190118181121.GC24439@chai> (raw) In-Reply-To: <2d7c73e83bcee024ffcc095854a31bcd46f6ccab.1547128310.git.kshcherbatov@tarantool.org> * Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/01/10 23:12]: > diff --git a/src/box/space.c b/src/box/space.c > index 4d174f7ec..b139a0905 100644 > --- a/src/box/space.c > +++ b/src/box/space.c > @@ -29,6 +29,7 @@ > * SUCH DAMAGE. > */ > #include "space.h" > +#include "sql.h" Just STOP. Please read the SOP. You can't add an arbitrary header to another file just because you happen to need it. What the hell is going on here, why do you need to add entire sql layer to space.c? > +/** > + * SQL-specific actions for space. > + */ > +static void > +on_space_before_replace(struct trigger *trigger, void *event) > +{ > + uint32_t space_id = (uint32_t)(uintptr_t)trigger->data; > + struct space *space = space_cache_find(space_id); > + assert(space != NULL); > + struct txn *txn = (struct txn *) event; > + struct txn_stmt *stmt = txn_current_stmt(txn); > + if (stmt == NULL) > + return; > + struct tuple *new_tuple = stmt->new_tuple; > + struct tuple *old_tuple = stmt->old_tuple; > + if (new_tuple != NULL && space->sql_checks != NULL) { > + const char *new_tuple_raw = tuple_data(new_tuple); > + const char *old_tuple_raw = old_tuple != NULL ? > + tuple_data(old_tuple) : NULL; > + if (sql_checks_run(space->sql_checks, > + space->def->opts.checks_ast, space->def, > + old_tuple_raw, new_tuple_raw) != 0) { > + diag_raise(); > + } > + } > +} Why couldn't you define this function wherever you like? Please read the sop carefully about introducing new header file dependencies. > + if (def->opts.checks_ast != NULL) { > + struct sql_checks *checks = > + sql_checks_create(sql_get(), def->opts.checks_ast, def); > + if (unlikely(checks == NULL)) > + goto fail_free_indexes; > + space->sql_checks = checks; > + > + struct trigger *sql_actions_trigger = > + (struct trigger *)malloc(sizeof(struct trigger)); Please don't use plural. Why do you call it a different name? Why not ck_constraint_trigger? Even if it does an action, well, action is part of a check, so why invent a separate name for it? > + * UPDATE operations. > + */ > + struct sql_checks *sql_checks; struct ck_constraint > index 26b84c5db..8961c4fd9 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -35,9 +35,10 @@ > #include "sql/tarantoolInt.h" > #include "sql/vdbeInt.h" > #include "mpstream.h" > +#include "execute.h" You're not writing PHP! Please watch your dependencies carefully. > +/** SQL Checks object. */ > +struct sql_checks { > + /** > + * The first in an array variables representing the > + * new tuple to be inserted. > + */ > + int new_tuple_var; > + /** > + * The first in array of variables representing the state > + * (1 for ON and 0 for OFF) of corresponding Check. This > + * is the UPDATE optimization - disabled checks would be > + * skipped. > + */ > + int checks_state_var; I don't understand what these variables are for even though there are comments :/ > + /** > + * Precompiled reusable VDBE program for SQL Checks > + * raising error on Check constraint failure. VDBE > + * program assumes new_tuple and checks_state actual > + * parameters to be placed in VDBE memory before run. > + */ > + struct sqlite3_stmt *stmt; > +}; -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov
next prev parent reply other threads:[~2019-01-18 18:11 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-10 13:54 [tarantool-patches] [PATCH v1 0/4] sql: Checks " Kirill Shcherbatov 2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 1/4] box: rename space->opts checks to checks_ast Kirill Shcherbatov 2019-01-11 14:05 ` [tarantool-patches] " Konstantin Osipov 2019-01-18 18:00 ` Konstantin Osipov 2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 2/4] sql: disallow use of TYPEOF in Check Kirill Shcherbatov 2019-01-11 14:06 ` [tarantool-patches] " Konstantin Osipov 2019-01-11 14:07 ` Konstantin Osipov 2019-01-18 18:04 ` Konstantin Osipov 2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 3/4] box: exported sql_bind structure and API Kirill Shcherbatov 2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 4/4] sql: make sql checks on server side Kirill Shcherbatov 2019-01-11 14:12 ` [tarantool-patches] " Konstantin Osipov 2019-01-11 14:14 ` Konstantin Osipov 2019-01-18 18:11 ` Konstantin Osipov [this message] 2019-01-21 14:47 ` n.pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190118181121.GC24439@chai \ --to=kostja@tarantool.org \ --cc=korablev@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 4/4] sql: make sql checks on server side' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox