From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type Date: Fri, 26 Jul 2019 00:12:21 +0200 [thread overview] Message-ID: <d14cdbe4-d3a3-1a00-4ba9-bdd590908b46@tarantool.org> (raw) In-Reply-To: <ff84ee4c2b351d2a8b8fea3e43187bc8de9e92c2.1563967510.git.korablev@tarantool.org> Thanks for the patch! See 4 comments below. On 24/07/2019 13:42, Nikita Pettik wrote: > Current patch introduces new type available in SQL: > - VARBINARY now is reserved keyword; > - Allow to specify VARBINARY column and CAST type; > - All literals which start from 'x' are assumed to be of this type; > - There's no available implicit or explicit conversions between > VARBINARY and other types; > - Under the hood all values of VARBINARY type are stored as MP_BIN > msgpack format type. > > Closes #4206 > --- > extra/mkkeywordhash.c | 1 + > src/box/sql/expr.c | 2 +- > src/box/sql/func.c | 4 +- > src/box/sql/parse.y | 3 +- > src/box/sql/vdbe.c | 26 ++- > src/box/sql/vdbemem.c | 5 + > test/sql-tap/keyword1.test.lua | 3 +- > test/sql/gh-3888-values-blob-assert.result | 4 +- > test/sql/iproto.result | 4 +- > test/sql/misc.result | 2 +- > test/sql/types.result | 256 ++++++++++++++++++++++++++++- > test/sql/types.test.lua | 60 +++++++ > 12 files changed, 345 insertions(+), 25 deletions(-) > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index 826232f99..d0f0cb4f5 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -2094,20 +2098,24 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ > } > break; > } > - } else if ((flags1 | flags3) & MEM_Bool) { > + } else if (((flags1 | flags3) & MEM_Bool) != 0 || > + ((flags1 | flags3) & MEM_Blob) != 0) { 1. Nit: diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index d0f0cb4f5..ea7c9f25f 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2098,8 +2098,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ } break; } - } else if (((flags1 | flags3) & MEM_Bool) != 0 || - ((flags1 | flags3) & MEM_Blob) != 0) { + } else if (((flags1 | flags3) & (MEM_Bool | MEM_Blob)) != 0) { /* * If one of values is of type BOOLEAN or VARBINARY, * then the second one must be of the same type as 2. Please, add tests on varbinary values binding. 3. BLOB keyword is reserved, but also it is used in parse.y:980. Should not it be deleted from all the rules, and be just reserved? 4. Additionally (I bet, you knew I would ask), what are we going to do with all mentionings of blob in the code? We have word 'blob' mentioned 368 times in all the SQL sources, including comments, parts of names. In wherecode.c:644 we have a CREATE TABLE example, using BLOB - this one definitely should be fixed. What about other places? We could replace 'blob' -> 'bin','binary', 'vbin', ... . It should not take much time.
next prev parent reply other threads:[~2019-07-25 22:10 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-24 11:42 [tarantool-patches] [PATCH 0/5] Introduce VARBINARY in SQL Nikita Pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 1/5] sql: always erase numeric flag after stringifying Nikita Pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt Nikita Pettik 2019-07-25 22:12 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <a061e845-eeb1-00d1-9141-3b9bb87768f5@tarantool.org> 2019-07-28 23:56 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob' Nikita Pettik 2019-07-25 22:11 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <2e655514-0fec-8baf-20a8-d49e5586b047@tarantool.org> 2019-07-28 23:56 ` n.pettik 2019-07-29 21:03 ` Vladislav Shpilevoy 2019-07-30 13:43 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 4/5] sql: make built-ins raise errors for varbin args Nikita Pettik 2019-07-25 22:11 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <05d15035-2552-1f05-b7ce-facfbbc3a520@tarantool.org> 2019-07-28 23:59 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 5/5] sql: introduce VARBINARY column type Nikita Pettik 2019-07-25 22:12 ` Vladislav Shpilevoy [this message] [not found] ` <49a188eb-dafe-44e7-a0fd-e9244b68e721@tarantool.org> 2019-07-29 0:03 ` [tarantool-patches] " n.pettik 2019-07-29 20:55 ` Vladislav Shpilevoy 2019-07-30 13:44 ` n.pettik 2019-07-30 19:41 ` Vladislav Shpilevoy 2019-07-30 19:52 ` Vladislav Shpilevoy 2019-07-31 14:51 ` n.pettik 2019-08-01 8:42 ` [tarantool-patches] Re: [PATCH 0/5] Introduce VARBINARY in SQL Kirill Yukhin
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=d14cdbe4-d3a3-1a00-4ba9-bdd590908b46@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type' \ /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