Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Cc: tarantool-patches@freelists.org,
	Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] box: introduce VARBINARY field type
Date: Thu, 27 Jun 2019 11:01:32 +0300	[thread overview]
Message-ID: <20190627080132.GC4229@atlas> (raw)
In-Reply-To: <f43ff4bc-c085-fef0-2922-3b970ec1b016@tarantool.org>

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/06/27 10:40]:
> > please add docbot request.
> 
> In fact, I did not include TarantoolBot query consciously: this patch is preparatory for #4206, and only
> introduces a new field type, while preparing data corresponding to this field in Tarantool now
> is not easy (see the test case below).
> 
> But in general, the documentation may be updated. So I've updated a commit message.

since it's usable in format/index definitions, it's an external
behaviour change and has to be documented. Actually it's going to
be a sweeping changes in many places in the docs, since the type
system has multiple mentions in the docs.

> ==========================================
> 
> A new VARBINARY field type would be useful for SQL type system.
> 
> Closes #4201
> Needed for #4206
> 
> @TarantoolBot document
> Title: new varbinary field type
> 
> Introduced a new field type varbinary to represent mp_bin values.
> The new type varbinary may be used in format or index definition.
> 
> Example:
> s = box.schema.space.create('withdata')
> s:format({{"b", "varbinary"}})
> pk = s:create_index('pk', {parts = {1, "varbinary"}})
> ---
>  src/box/field_def.c              |  23 +++--
>  src/box/field_def.h              |   1 +
>  src/box/lua/key_def.c            |   1 +
>  src/box/tuple_compare.cc         |  17 ++++
>  test/box/varbinary_type.result   | 152 +++++++++++++++++++++++++++++++
>  test/box/varbinary_type.test.lua |  51 +++++++++++
>  6 files changed, 235 insertions(+), 10 deletions(-)
>  create mode 100644 test/box/varbinary_type.result
>  create mode 100644 test/box/varbinary_type.test.lua
> 
> diff --git a/src/box/field_def.c b/src/box/field_def.c
> index 0ba3d3294..346042b98 100644
> --- a/src/box/field_def.c
> +++ b/src/box/field_def.c
> @@ -55,6 +55,7 @@ const uint32_t field_mp_type[] = {
>  		(1U << MP_FLOAT) | (1U << MP_DOUBLE),
>  	/* [FIELD_TYPE_INTEGER]  =  */ (1U << MP_UINT) | (1U << MP_INT),
>  	/* [FIELD_TYPE_BOOLEAN]  =  */ 1U << MP_BOOL,
> +	/* [FIELD_TYPE_VARBINARY] =  */ 1U << MP_BIN,
>  	/* [FIELD_TYPE_SCALAR]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
>  		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_STR) |
>  		(1U << MP_BIN) | (1U << MP_BOOL),
> @@ -69,6 +70,7 @@ const char *field_type_strs[] = {
>  	/* [FIELD_TYPE_NUMBER]   = */ "number",
>  	/* [FIELD_TYPE_INTEGER]  = */ "integer",
>  	/* [FIELD_TYPE_BOOLEAN]  = */ "boolean",
> +	/* [FIELD_TYPE_VARBINARY] = */"varbinary",
>  	/* [FIELD_TYPE_SCALAR]   = */ "scalar",
>  	/* [FIELD_TYPE_ARRAY]    = */ "array",
>  	/* [FIELD_TYPE_MAP]      = */ "map",
> @@ -96,16 +98,17 @@ field_type_by_name_wrapper(const char *str, uint32_t len)
>   * values can be stored in the j type.
>   */
>  static const bool field_type_compatibility[] = {
> -	   /*   ANY   UNSIGNED  STRING   NUMBER  INTEGER  BOOLEAN   SCALAR   ARRAY     MAP */
> -/*   ANY    */ true,   false,   false,   false,   false,   false,   false,   false,   false,
> -/* UNSIGNED */ true,   true,    false,   true,    true,    false,   true,    false,   false,
> -/*  STRING  */ true,   false,   true,    false,   false,   false,   true,    false,   false,
> -/*  NUMBER  */ true,   false,   false,   true,    false,   false,   true,    false,   false,
> -/*  INTEGER */ true,   false,   false,   true,    true,    false,   true,    false,   false,
> -/*  BOOLEAN */ true,   false,   false,   false,   false,   true,    true,    false,   false,
> -/*  SCALAR  */ true,   false,   false,   false,   false,   false,   true,    false,   false,
> -/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,   true,    false,
> -/*    MAP   */ true,   false,   false,   false,   false,   false,   false,   false,   true,
> +	   /*   ANY   UNSIGNED  STRING   NUMBER  INTEGER  BOOLEAN VARBINARY SCALAR   ARRAY     MAP */
> +/*   ANY    */ true,   false,   false,   false,   false,   false,   false,  false,   false,   false,
> +/* UNSIGNED */ true,   true,    false,   true,    true,    false,   false,  true,    false,   false,
> +/*  STRING  */ true,   false,   true,    false,   false,   false,   false,  true,    false,   false,
> +/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,  true,    false,   false,
> +/*  INTEGER */ true,   false,   false,   true,    true,    false,   false,  true,    false,   false,
> +/*  BOOLEAN */ true,   false,   false,   false,   false,   true,    false,  true,    false,   false,
> +/* VARBINARY*/ true,   false,   false,   false,   false,   false,   true,   true,    false,   false,
> +/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,  true,    false,   false,
> +/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,  false,   true,    false,
> +/*    MAP   */ true,   false,   false,   false,   false,   false,   false,  false,   false,   true,
>  };
>  
>  bool
> diff --git a/src/box/field_def.h b/src/box/field_def.h
> index f944de9d6..c1a7ec0a9 100644
> --- a/src/box/field_def.h
> +++ b/src/box/field_def.h
> @@ -56,6 +56,7 @@ enum field_type {
>  	FIELD_TYPE_NUMBER,
>  	FIELD_TYPE_INTEGER,
>  	FIELD_TYPE_BOOLEAN,
> +	FIELD_TYPE_VARBINARY,
>  	FIELD_TYPE_SCALAR,
>  	FIELD_TYPE_ARRAY,
>  	FIELD_TYPE_MAP,
> diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
> index dfcc89442..052a1c85d 100644
> --- a/src/box/lua/key_def.c
> +++ b/src/box/lua/key_def.c
> @@ -111,6 +111,7 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part,
>  	part->type = field_type_by_name(type_name, type_len);
>  	switch (part->type) {
>  	case FIELD_TYPE_ANY:
> +	case FIELD_TYPE_VARBINARY:
>  	case FIELD_TYPE_ARRAY:
>  	case FIELD_TYPE_MAP:
>  		/* Tuple comparators don't support these types. */
> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index c1a70a087..95a0f58c9 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -404,6 +404,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
>  		return mp_compare_number(field_a, field_b);
>  	case FIELD_TYPE_BOOLEAN:
>  		return mp_compare_bool(field_a, field_b);
> +	case FIELD_TYPE_VARBINARY:
> +		return mp_compare_bin(field_a, field_b);
>  	case FIELD_TYPE_SCALAR:
>  		return coll != NULL ?
>  		       mp_compare_scalar_coll(field_a, field_b, coll) :
> @@ -434,6 +436,8 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
>  						   field_b, b_type);
>  	case FIELD_TYPE_BOOLEAN:
>  		return mp_compare_bool(field_a, field_b);
> +	case FIELD_TYPE_VARBINARY:
> +		return mp_compare_bin(field_a, field_b);
>  	case FIELD_TYPE_SCALAR:
>  		return coll != NULL ?
>  		       mp_compare_scalar_coll(field_a, field_b, coll) :
> @@ -1502,6 +1506,14 @@ field_hint_string(const char *field, struct coll *coll)
>  			      hint_str_coll(field, len, coll);
>  }
>  
> +static inline hint_t
> +field_hint_varbinary(const char *field)
> +{
> +	assert(mp_typeof(*field) == MP_BIN);
> +	uint32_t len = mp_decode_binl(&field);
> +	return hint_bin(field, len);
> +}
> +
>  static inline hint_t
>  field_hint_scalar(const char *field, struct coll *coll)
>  {
> @@ -1547,6 +1559,8 @@ field_hint(const char *field, struct coll *coll)
>  		return field_hint_number(field);
>  	case FIELD_TYPE_STRING:
>  		return field_hint_string(field, coll);
> +	case FIELD_TYPE_VARBINARY:
> +		return field_hint_varbinary(field);
>  	case FIELD_TYPE_SCALAR:
>  		return field_hint_scalar(field, coll);
>  	default:
> @@ -1648,6 +1662,9 @@ key_def_set_hint_func(struct key_def *def)
>  	case FIELD_TYPE_STRING:
>  		key_def_set_hint_func<FIELD_TYPE_STRING>(def);
>  		break;
> +	case FIELD_TYPE_VARBINARY:
> +		key_def_set_hint_func<FIELD_TYPE_VARBINARY>(def);
> +		break;
>  	case FIELD_TYPE_SCALAR:
>  		key_def_set_hint_func<FIELD_TYPE_SCALAR>(def);
>  		break;
> diff --git a/test/box/varbinary_type.result b/test/box/varbinary_type.result
> new file mode 100644
> index 000000000..c7cd5700c
> --- /dev/null
> +++ b/test/box/varbinary_type.result
> @@ -0,0 +1,152 @@
> +env = require('test_run')
> +---
> +...
> +test_run = env.new()
> +---
> +...
> +--
> +-- gh-4201: Introduce varbinary field type.
> +--
> +s = box.schema.space.create('withdata')
> +---
> +...
> +s:format({{"b", "integer"}})
> +---
> +...
> +_ = s:create_index('pk', {parts = {1, "varbinary"}})
> +---
> +- error: Field 1 has type 'integer' in space format, but type 'varbinary' in index
> +    definition
> +...
> +s:format({{"b", "varbinary"}})
> +---
> +...
> +_ = s:create_index('pk', {parts = {1, "integer"}})
> +---
> +- error: Field 1 has type 'varbinary' in space format, but type 'integer' in index
> +    definition
> +...
> +pk = s:create_index('pk', {parts = {1, "varbinary"}})
> +---
> +...
> +buffer = require('buffer')
> +---
> +...
> +ffi = require('ffi')
> +---
> +...
> +test_run:cmd("setopt delimiter ';'")
> +---
> +- true
> +...
> +function bintuple_insert(space, bytes)
> +	local tmpbuf = buffer.IBUF_SHARED
> +	tmpbuf:reset()
> +	local p = tmpbuf:alloc(3 + #bytes)
> +	p[0] = 0x91
> +	p[1] = 0xC4
> +	p[2] = #bytes
> +	for i, c in pairs(bytes) do p[i + 3 - 1] = c end
> +	ffi.cdef[[int box_insert(uint32_t space_id, const char *tuple, const char *tuple_end, box_tuple_t **result);]]
> +	ffi.C.box_insert(space.id, tmpbuf.rpos, tmpbuf.wpos, nil)
> +end
> +test_run:cmd("setopt delimiter ''");
> +---
> +...
> +bintuple_insert(s, {0xDE, 0xAD, 0xBE, 0xAF})
> +---
> +...
> +bintuple_insert(s, {0xFE, 0xED, 0xFA, 0xCE})
> +---
> +...
> +s:select()
> +---
> +- - [!!binary 3q2+rw==]
> +  - [!!binary /u36zg==]
> +...
> +box.execute("SELECT * FROM \"withdata\" WHERE \"b\" < x'FEEDFACE';")
> +---
> +- metadata:
> +  - name: b
> +    type: varbinary
> +  rows:
> +  - [!!binary 3q2+rw==]
> +...
> +pk:alter({parts = {1, "scalar"}})
> +---
> +...
> +s:format({{"b", "scalar"}})
> +---
> +...
> +s:insert({11})
> +---
> +- [11]
> +...
> +s:insert({22})
> +---
> +- [22]
> +...
> +s:insert({"11"})
> +---
> +- ['11']
> +...
> +s:insert({"22"})
> +---
> +- ['22']
> +...
> +s:select()
> +---
> +- - [11]
> +  - [22]
> +  - ['11']
> +  - ['22']
> +  - [!!binary 3q2+rw==]
> +  - [!!binary /u36zg==]
> +...
> +box.execute("SELECT * FROM \"withdata\" WHERE \"b\" <= x'DEADBEAF';")
> +---
> +- metadata:
> +  - name: b
> +    type: scalar
> +  rows:
> +  - [11]
> +  - [22]
> +  - ['11']
> +  - ['22']
> +  - [!!binary 3q2+rw==]
> +...
> +pk:alter({parts = {1, "varbinary"}})
> +---
> +- error: 'Tuple field 1 type does not match one required by operation: expected varbinary'
> +...
> +s:delete({11})
> +---
> +- [11]
> +...
> +s:delete({22})
> +---
> +- [22]
> +...
> +s:delete({"11"})
> +---
> +- ['11']
> +...
> +s:delete({"22"})
> +---
> +- ['22']
> +...
> +bintuple_insert(s, {0xFA, 0xDE, 0xDE, 0xAD})
> +---
> +...
> +pk:alter({parts = {1, "varbinary"}})
> +---
> +...
> +s:select()
> +---
> +- - [!!binary 3q2+rw==]
> +  - [!!binary +t7erQ==]
> +  - [!!binary /u36zg==]
> +...
> +s:drop()
> +---
> +...
> diff --git a/test/box/varbinary_type.test.lua b/test/box/varbinary_type.test.lua
> new file mode 100644
> index 000000000..7895a1d22
> --- /dev/null
> +++ b/test/box/varbinary_type.test.lua
> @@ -0,0 +1,51 @@
> +env = require('test_run')
> +test_run = env.new()
> +
> +--
> +-- gh-4201: Introduce varbinary field type.
> +--
> +s = box.schema.space.create('withdata')
> +s:format({{"b", "integer"}})
> +_ = s:create_index('pk', {parts = {1, "varbinary"}})
> +s:format({{"b", "varbinary"}})
> +_ = s:create_index('pk', {parts = {1, "integer"}})
> +pk = s:create_index('pk', {parts = {1, "varbinary"}})
> +
> +buffer = require('buffer')
> +ffi = require('ffi')
> +
> +test_run:cmd("setopt delimiter ';'")
> +function bintuple_insert(space, bytes)
> +	local tmpbuf = buffer.IBUF_SHARED
> +	tmpbuf:reset()
> +	local p = tmpbuf:alloc(3 + #bytes)
> +	p[0] = 0x91
> +	p[1] = 0xC4
> +	p[2] = #bytes
> +	for i, c in pairs(bytes) do p[i + 3 - 1] = c end
> +	ffi.cdef[[int box_insert(uint32_t space_id, const char *tuple, const char *tuple_end, box_tuple_t **result);]]
> +	ffi.C.box_insert(space.id, tmpbuf.rpos, tmpbuf.wpos, nil)
> +end
> +test_run:cmd("setopt delimiter ''");
> +
> +bintuple_insert(s, {0xDE, 0xAD, 0xBE, 0xAF})
> +bintuple_insert(s, {0xFE, 0xED, 0xFA, 0xCE})
> +s:select()
> +box.execute("SELECT * FROM \"withdata\" WHERE \"b\" < x'FEEDFACE';")
> +pk:alter({parts = {1, "scalar"}})
> +s:format({{"b", "scalar"}})
> +s:insert({11})
> +s:insert({22})
> +s:insert({"11"})
> +s:insert({"22"})
> +s:select()
> +box.execute("SELECT * FROM \"withdata\" WHERE \"b\" <= x'DEADBEAF';")
> +pk:alter({parts = {1, "varbinary"}})
> +s:delete({11})
> +s:delete({22})
> +s:delete({"11"})
> +s:delete({"22"})
> +bintuple_insert(s, {0xFA, 0xDE, 0xDE, 0xAD})
> +pk:alter({parts = {1, "varbinary"}})
> +s:select()
> +s:drop()
> -- 
> 2.21.0
> 

-- 
Konstantin Osipov, Moscow, Russia

  reply	other threads:[~2019-06-27  8:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-26 16:40 Kirill Shcherbatov
2019-06-26 19:11 ` [tarantool-patches] " Konstantin Osipov
2019-06-27  7:38   ` [tarantool-patches] " Kirill Shcherbatov
2019-06-27  8:01     ` Konstantin Osipov [this message]
2019-07-03 10:10 ` Vladimir Davydov

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=20190627080132.GC4229@atlas \
    --to=kostja@tarantool.org \
    --cc=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [tarantool-patches] Re: [PATCH v1 1/1] box: introduce VARBINARY field 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