Tarantool development patches archive
 help / color / mirror / Atom feed
* Re: [Tarantool-patches] [PATCH v2 06/11] box, datetime: datetime comparison for indices
       [not found] ` <2939f293071df83ba612c431895adea49c9cb8de.1627430045.git.tsafin@tarantool.org>
@ 2021-07-28  8:19   ` Timur Safin via Tarantool-patches
  0 siblings, 0 replies; only message in thread
From: Timur Safin via Tarantool-patches @ 2021-07-28  8:19 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: TML

As testing in release_lto revealed there is subtle problem
with index sorting if we insert date which are very close 
to each over in their nanoseconds parts

https://github.com/tsafin/tarantool/runs/3176791534#step:5:5954

		[022] Test failed! Result content mismatch: 
		[022] --- engine/datetime.result Tue Jul 27 23:04:00 2021 
		[022] +++ /tmp/tnt/rejects/engine/datetime.reject Tue Jul 27 23:18:29 2021 
		[022] @@ -70,7 +70,8 @@ 
		[022] 
		[022] err 
		[022] | --- 
		[022] - | - null 
		[022] + | - - 2021-07-27T23:18:29.451Z 
		[022] + | - 2021-07-27T23:18:29.451020Z 
		[022] | ... 
		[022] box.space.T:drop() 
		[022] | --- 
		[022] 
		[022] Last 15 lines of Tarantool Log file [Instance "box"][/tmp/tnt/022_engine/box.log]:
		
		With expected engine/datetime.result:
		
		for i = 1, #a - 1 do\
		    if tostring(a[i][1]) >= tostring(a[i+1][1]) then\
		        err = {a[i][1], a[i+1][1]}\
		        break\
		    end\
		end
		 | ---
		 | ...
		
		err
		 | ---
		 | - null
		 | ...

We have 2 dates of values 2021-07-27T23:18:29.451Z and 
2021-07-27T23:18:29.451020Z which are in wrong order in
the index.

Will need to take into account nanoseconds part.

Timur

: From: Timur Safin <tsafin@tarantool.org>
: Subject: [PATCH v2 06/11] box, datetime: datetime comparison for indices
: 
: * storage hints implemented for datetime_t values;
: * proper comparison for indices of datetime type.
: 
: Part of #5941
: Part of #5946
: ---
:  src/box/field_def.c           | 18 ++++++++
:  src/box/field_def.h           |  3 ++
:  src/box/memtx_space.c         |  3 +-
:  src/box/tuple_compare.cc      | 30 ++++++++++++++
:  src/box/vinyl.c               |  3 +-
:  test/engine/datetime.result   | 77 +++++++++++++++++++++++++++++++++++
:  test/engine/datetime.test.lua | 35 ++++++++++++++++
:  7 files changed, 165 insertions(+), 4 deletions(-)
:  create mode 100644 test/engine/datetime.result
:  create mode 100644 test/engine/datetime.test.lua
: 
: diff --git a/src/box/field_def.c b/src/box/field_def.c
: index 6964e3e9f..aaf5f9cff 100644
: --- a/src/box/field_def.c
: +++ b/src/box/field_def.c
: @@ -193,3 +193,21 @@ field_type_by_name(const char *name, size_t len)
:  		return FIELD_TYPE_ANY;
:  	return field_type_MAX;
:  }
: +
: +const bool field_type_index_allowed[] =
: +    {
: +	/* [FIELD_TYPE_ANY]      = */ false,
: +	/* [FIELD_TYPE_UNSIGNED] = */ true,
: +	/* [FIELD_TYPE_STRING]   = */ true,
: +	/* [FIELD_TYPE_NUMBER]   = */ true,
: +	/* [FIELD_TYPE_DOUBLE]   = */ true,
: +	/* [FIELD_TYPE_INTEGER]  = */ true,
: +	/* [FIELD_TYPE_BOOLEAN]  = */ true,
: +	/* [FIELD_TYPE_VARBINARY]= */ true,
: +	/* [FIELD_TYPE_SCALAR]   = */ true,
: +	/* [FIELD_TYPE_DECIMAL]  = */ true,
: +	/* [FIELD_TYPE_UUID]     = */ true,
: +	/* [FIELD_TYPE_ARRAY]    = */ false,
: +	/* [FIELD_TYPE_MAP]      = */ false,
: +	/* [FIELD_TYPE_DATETIME] = */ true,
: +};
: diff --git a/src/box/field_def.h b/src/box/field_def.h
: index 120b2a93d..bd02418df 100644
: --- a/src/box/field_def.h
: +++ b/src/box/field_def.h
: @@ -120,6 +120,9 @@ extern const uint32_t field_ext_type[];
:  extern const struct opt_def field_def_reg[];
:  extern const struct field_def field_def_default;
: 
: +/** helper table for checking allowed indices for types */
: +extern const bool field_type_index_allowed[];
: +
:  /**
:   * @brief Field definition
:   * Contains information about of one tuple field.
: diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
: index b71318d24..1ab16122e 100644
: --- a/src/box/memtx_space.c
: +++ b/src/box/memtx_space.c
: @@ -748,8 +748,7 @@ memtx_space_check_index_def(struct space *space, struct
: index_def *index_def)
:  	/* Check that there are no ANY, ARRAY, MAP parts */
:  	for (uint32_t i = 0; i < key_def->part_count; i++) {
:  		struct key_part *part = &key_def->parts[i];
: -		if (part->type <= FIELD_TYPE_ANY ||
: -		    part->type >= FIELD_TYPE_ARRAY) {
: +		if (!field_type_index_allowed[part->type]) {
:  			diag_set(ClientError, ER_MODIFY_INDEX,
:  				 index_def->name, space_name(space),
:  				 tt_sprintf("field type '%s' is not supported",
: diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
: index 48c833643..4adceebd3 100644
: --- a/src/box/tuple_compare.cc
: +++ b/src/box/tuple_compare.cc
: @@ -538,6 +538,8 @@ tuple_compare_field_with_type(const char *field_a, enum
: mp_type a_type,
:  						   field_b, b_type);
:  	case FIELD_TYPE_UUID:
:  		return mp_compare_uuid(field_a, field_b);
: +	case FIELD_TYPE_DATETIME:
: +		return mp_compare_datetime(field_a, field_b);
:  	default:
:  		unreachable();
:  		return 0;
: @@ -1630,6 +1632,18 @@ hint_uuid_raw(const char *data)
:  	return hint_create(MP_CLASS_UUID, val);
:  }
: 
: +static inline hint_t
: +hint_datetime(struct datetime_t *date)
: +{
: +	/*
: +	 * Use at most HINT_VALUE_BITS from datetime
: +	 * seconds field as a hint value
: +	 */
: +	uint64_t val = (uint64_t)date->secs & HINT_VALUE_MAX;
: +
: +	return hint_create(MP_CLASS_DATETIME, val);
: +}
: +
:  static inline uint64_t
:  hint_str_raw(const char *s, uint32_t len)
:  {
: @@ -1761,6 +1775,17 @@ field_hint_uuid(const char *field)
:  	return hint_uuid_raw(data);
:  }
: 
: +static inline hint_t
: +field_hint_datetime(const char *field)
: +{
: +	assert(mp_typeof(*field) == MP_EXT);
: +	int8_t ext_type;
: +	uint32_t len = mp_decode_extl(&field, &ext_type);
: +	assert(ext_type == MP_DATETIME);
: +	struct datetime_t date;
: +	return hint_datetime(datetime_unpack(&field, len, &date));
: +}
: +
:  static inline hint_t
:  field_hint_string(const char *field, struct coll *coll)
:  {
: @@ -1849,6 +1874,8 @@ field_hint(const char *field, struct coll *coll)
:  		return field_hint_decimal(field);
:  	case FIELD_TYPE_UUID:
:  		return field_hint_uuid(field);
: +	case FIELD_TYPE_DATETIME:
: +		return field_hint_datetime(field);
:  	default:
:  		unreachable();
:  	}
: @@ -1963,6 +1990,9 @@ key_def_set_hint_func(struct key_def *def)
:  	case FIELD_TYPE_UUID:
:  		key_def_set_hint_func<FIELD_TYPE_UUID>(def);
:  		break;
: +	case FIELD_TYPE_DATETIME:
: +		key_def_set_hint_func<FIELD_TYPE_DATETIME>(def);
: +		break;
:  	default:
:  		/* Invalid key definition. */
:  		def->key_hint = NULL;
: diff --git a/src/box/vinyl.c b/src/box/vinyl.c
: index c80b2d99b..360d1fa70 100644
: --- a/src/box/vinyl.c
: +++ b/src/box/vinyl.c
: @@ -662,8 +662,7 @@ vinyl_space_check_index_def(struct space *space, struct
: index_def *index_def)
:  	/* Check that there are no ANY, ARRAY, MAP parts */
:  	for (uint32_t i = 0; i < key_def->part_count; i++) {
:  		struct key_part *part = &key_def->parts[i];
: -		if (part->type <= FIELD_TYPE_ANY ||
: -		    part->type >= FIELD_TYPE_ARRAY) {
: +		if (!field_type_index_allowed[part->type]) {
:  			diag_set(ClientError, ER_MODIFY_INDEX,
:  				 index_def->name, space_name(space),
:  				 tt_sprintf("field type '%s' is not supported",
: diff --git a/test/engine/datetime.result b/test/engine/datetime.result
: new file mode 100644
: index 000000000..3ff474dee
: --- /dev/null
: +++ b/test/engine/datetime.result
: @@ -0,0 +1,77 @@
: +-- test-run result file version 2
: +env = require('test_run')
: + | ---
: + | ...
: +test_run = env.new()
: + | ---
: + | ...
: +engine = test_run:get_cfg('engine')
: + | ---
: + | ...
: +
: +date = require('datetime')
: + | ---
: + | ...
: +
: +_ = box.schema.space.create('T', {engine = engine})
: + | ---
: + | ...
: +_ = box.space.T:create_index('pk', {parts={1,'datetime'}})
: + | ---
: + | ...
: +
: +box.space.T:insert{date('1970-01-01')}\
: +box.space.T:insert{date('1970-01-02')}\
: +box.space.T:insert{date('1970-01-03')}\
: +box.space.T:insert{date('2000-01-01')}
: + | ---
: + | ...
: +
: +o = box.space.T:select{}
: + | ---
: + | ...
: +assert(tostring(o[1][1]) == '1970-01-01T00:00Z')
: + | ---
: + | - true
: + | ...
: +assert(tostring(o[2][1]) == '1970-01-02T00:00Z')
: + | ---
: + | - true
: + | ...
: +assert(tostring(o[3][1]) == '1970-01-03T00:00Z')
: + | ---
: + | - true
: + | ...
: +assert(tostring(o[4][1]) == '2000-01-01T00:00Z')
: + | ---
: + | - true
: + | ...
: +
: +for i = 1,16 do\
: +    box.space.T:insert{date.now()}\
: +end
: + | ---
: + | ...
: +
: +a = box.space.T:select{}
: + | ---
: + | ...
: +err = nil
: + | ---
: + | ...
: +for i = 1, #a - 1 do\
: +    if tostring(a[i][1]) >= tostring(a[i+1][1]) then\
: +        err = {a[i][1], a[i+1][1]}\
: +        break\
: +    end\
: +end
: + | ---
: + | ...
: +
: +err
: + | ---
: + | - null
: + | ...
: +box.space.T:drop()
: + | ---
: + | ...
: diff --git a/test/engine/datetime.test.lua b/test/engine/datetime.test.lua
: new file mode 100644
: index 000000000..180d9bb72
: --- /dev/null
: +++ b/test/engine/datetime.test.lua
: @@ -0,0 +1,35 @@
: +env = require('test_run')
: +test_run = env.new()
: +engine = test_run:get_cfg('engine')
: +
: +date = require('datetime')
: +
: +_ = box.schema.space.create('T', {engine = engine})
: +_ = box.space.T:create_index('pk', {parts={1,'datetime'}})
: +
: +box.space.T:insert{date('1970-01-01')}\
: +box.space.T:insert{date('1970-01-02')}\
: +box.space.T:insert{date('1970-01-03')}\
: +box.space.T:insert{date('2000-01-01')}
: +
: +o = box.space.T:select{}
: +assert(tostring(o[1][1]) == '1970-01-01T00:00Z')
: +assert(tostring(o[2][1]) == '1970-01-02T00:00Z')
: +assert(tostring(o[3][1]) == '1970-01-03T00:00Z')
: +assert(tostring(o[4][1]) == '2000-01-01T00:00Z')
: +
: +for i = 1,16 do\
: +    box.space.T:insert{date.now()}\
: +end
: +
: +a = box.space.T:select{}
: +err = nil
: +for i = 1, #a - 1 do\
: +    if tostring(a[i][1]) >= tostring(a[i+1][1]) then\
: +        err = {a[i][1], a[i+1][1]}\
: +        break\
: +    end\
: +end
: +
: +err
: +box.space.T:drop()
: --
: 2.29.2



^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-07-28  8:20 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1627430045.git.tsafin@tarantool.org>
     [not found] ` <2939f293071df83ba612c431895adea49c9cb8de.1627430045.git.tsafin@tarantool.org>
2021-07-28  8:19   ` [Tarantool-patches] [PATCH v2 06/11] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git