Tarantool development patches archive
 help / color / mirror / Atom feed
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?

  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