From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type Date: Mon, 29 Jul 2019 22:55:53 +0200 [thread overview] Message-ID: <24a34647-0d0c-7950-b92e-6c8bf3f16d92@tarantool.org> (raw) In-Reply-To: <1293710C-85CC-4F5A-A7CF-4677D901DCE9@tarantool.org> Thanks for the fixes! On 29/07/2019 02:03, n.pettik wrote: > >> >> 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 > > Ok, applied. > >> 2. Please, add tests on varbinary values binding. > > Could you suggest the way to force MP_BIN format from Lua? > I see the only test on VARBINARY type (test/box/varbinary_type.test.lua), > but this approach is unlikely to be applicable for bindings. I can, but you won't like it. box.cfg{listen = 3313} netbox = require('net.box') buffer = require('buffer') ffi = require('ffi') ffi.cdef[[ typedef struct box_tuple_format_t box_tuple_format_t; typedef struct box_tuple_t box_tuple_t; int box_insert(uint32_t space_id, const char *tuple, const char *tuple_end, box_tuple_t **result); ]] s = box.schema.create_space('test') pk = s:create_index('pk', {parts = {{1, 'scalar'}}}) box.schema.user.grant('guest','read, write, execute', 'universe') box.schema.user.grant('guest', 'create', 'space') cn = netbox.connect(box.cfg.listen) data = buffer.static_alloc('char', 1 + 6) data[0] = 0x91 data[1] = 0xc4 data[2] = 3 data[3] = string.byte('b') data[4] = string.byte('i') data[5] = string.byte('n') ffi.C.box_insert(s.id, data, data + 6, nil) cn:execute('SELECT typeof(?);', s:select()[1]) This returns: --- - metadata: - name: typeof(?) type: string rows: - ['varbinary'] ... I used the fact that netbox encodes 'execute' bind array as a tuple. So I inserted MP_BIN into a tuple, and used it as a bind array. >> 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? > > It’s rule for declaring blob (aka binary string) literals. > I can rename it, but TBO it looks OK to me. Could you please provide an example, how to use BLOB keyword to declare a literal? I can't find any test, using BLOB in a query string for anything. Also, I've found, that 'blob' type is still mentioned in sql-tap/e_expr.test.lua quite a lot. And in sql-tap/types2.test.lua, where it is still used as a column type. These tests are disabled. Why?
next prev parent reply other threads:[~2019-07-29 20:53 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 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <49a188eb-dafe-44e7-a0fd-e9244b68e721@tarantool.org> 2019-07-29 0:03 ` n.pettik 2019-07-29 20:55 ` Vladislav Shpilevoy [this message] 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=24a34647-0d0c-7950-b92e-6c8bf3f16d92@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@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