[tarantool-patches] Re: [PATCH v1 0/4] sql: remove Triggers to server
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu May 31 20:36:48 MSK 2018
Hello. Thanks for the patchset!
On 31/05/2018 14:22, Kirill Shcherbatov wrote:
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3273-no-sql-triggers
> Issue: https://github.com/tarantool/tarantool/issues/3273
>
> As we are going to call parser on box.cfg() to recreate triggers
> from SQL, we should initialize Schema as it used in sqlite3BeginTrigger.
> Inroduced box_space_id_by_name to get object id by name from custom space.
"Inroduced" - typo.
> Fixed bug in tarantoolSqlite3RenameTrigger: sql request have had invalid
> length in MsgPack after ALTER TABLE NAME.
> Introduced sql_triggers field in space structure.
> Changed parser logic to do not insert builded triggers, just only
"builded" - no such word.
> to do parsing. All triggers insertions and deletions are operated
> via on_replace_dd_trigger now.
Please, do not just copy-paste commit messages.
Out of the context I can not understand why "we are going to
call parser on box.cfg() to recreate triggers" is blocked by Schema and
a mystic function sqlite3BeginTrigger.
And why "Inroduced box_space_id_by_name to get object id by name
from custom space."? Box_space_id_by_name exists before you patch.
"Fixed bug in tarantoolSqlite3RenameTrigger" - this and the previous
hunks are minor patches and can be omitted here or just mentioned as
'helper patches'.
"Introduced sql_triggers field in space structure." - too minor
implementation detail.
>
> Kirill Shcherbatov (4):
> box: move db->pShchema init to sql_init
> sql: fix sql len in tarantoolSqlite3RenameTrigger
> box: introduce box_space_id_by_name
> sql: move Triggers to server
>
> src/box/alter.cc | 57 +++++++++++
> src/box/box.cc | 10 +-
> src/box/box.h | 14 +++
> src/box/space.h | 2 +
> src/box/sql.c | 60 ++++-------
> src/box/sql.h | 62 ++++++++++++
> src/box/sql/build.c | 8 +-
> src/box/sql/insert.c | 6 +-
> src/box/sql/sqliteInt.h | 2 -
> src/box/sql/tokenize.c | 43 +++++++-
> src/box/sql/trigger.c | 258 ++++++++++++++++++++++++++++--------------------
> src/box/sql/vdbe.c | 58 +++--------
> src/box/sql/vdbe.h | 1 -
> src/box/sql/vdbeaux.c | 9 --
> 14 files changed, 374 insertions(+), 216 deletions(-)
>
More information about the Tarantool-patches
mailing list