From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Serge Petrenko <sergepetrenko@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 1/1] box: make UUID part of SCALAR Date: Mon, 26 Apr 2021 18:31:51 +0300 [thread overview] Message-ID: <20210426153151.GA156387@tarantool.org> (raw) In-Reply-To: <4d65f57c-2477-c95a-f199-0af890ae148a@tarantool.org> Thank you for the review! My answers, diff and new patch below. On Sun, Apr 25, 2021 at 12:07:25PM +0300, Serge Petrenko wrote: > > > 23.04.2021 14:40, imeevma@tarantool.org пишет: > > Prior to this patch, UUID was not part of SCALAR. However, this should > > be changed to comply with the RFC "Consistent Lua/SQL types". > > > > Closes #6042 > > > > @TarantoolBot document > > Title: UUID is now part of SCALAR > > > > The UUID field type is now part of the SCALAR field type. This means > > that now values of type UUID can be inserted into the SCALAR field, and > > these values can participate in the sorting of the SCALAR fields. The > > order is as follows: boolean < number < string < varbinary < uuid. > > --- > > Thanks for the patch! > Generally LGTM with 2 comments. > > > https://github.com/tarantool/tarantool/issues/6042 > > https://github.com/tarantool/tarantool/tree/imeevma/gh-6042-make-uuid-part-of-scalar > > > > .../unreleased/make-uuid-part-of-scalar.md | 3 ++ > > src/box/field_def.c | 2 +- > > src/box/tuple_compare.cc | 2 + > > test/app/uuid.result | 46 +++++++++++++++++++ > > test/app/uuid.test.lua | 18 ++++++++ > > test/engine/ddl.result | 4 +- > > test/engine/ddl.test.lua | 2 +- > > 7 files changed, 72 insertions(+), 5 deletions(-) > > create mode 100644 changelogs/unreleased/make-uuid-part-of-scalar.md > > > > ... > > > > diff --git a/test/app/uuid.test.lua b/test/app/uuid.test.lua > > index 867bbd832..7c0942fd6 100644 > > --- a/test/app/uuid.test.lua > > +++ b/test/app/uuid.test.lua > > @@ -164,6 +164,24 @@ u1 = nil > > u1_str = nil > > u2_str = nil > > +-- > > +-- gh-6042: add UUID to SCALAR. > > +-- > > +s = box.schema.space.create('s', {format={{'s', 'scalar'}}}) > > +_ = s:create_index('i') > > +s:insert({1}) > > +s:insert({'1'}) > > +s:insert({uuid.fromstr('11111111-1111-1111-1111-111111111111')}) > > +s:insert({uuid.fromstr('11111111-1111-2222-1111-111111111111')}) > > + > > +-- > > +-- Make sure that comparison in right. Comparison in SCALAR field: > > +-- bool < number < string < varbinary < uuid. > > +-- > > I'd also insert a varbinary value to the index, just to check. > Is it possible from lua? > Fixed. It is possible, but it is easier to use SQL. Also, I inserted boolean. > > +s:select() > > +s:select({}, {iterator='LE'}) > > +s:drop() > > + > > uuid = nil > > test_run:cmd("clear filter") > > diff --git a/test/engine/ddl.result b/test/engine/ddl.result > > index 08ad1a57b..e044bc094 100644 > > --- a/test/engine/ddl.result > > +++ b/test/engine/ddl.result > > @@ -1429,10 +1429,8 @@ fail_format_change(12, 'number') > > number, got extension' > > ... > > -- uuid --X--> scalar > > The comment should be "-- uuid -----> scalar" now. > Fixed. > > -fail_format_change(12, 'scalar') > > +ok_format_change(12, 'scalar') > > --- > > -- 'Tuple field 12 (field12) type does not match one required by operation: expected > > - scalar, got extension' > > ... > > -- uuid --X--> string > > fail_format_change(12, 'string') > > diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua > > index 5ba80e075..0cf25aa46 100644 > > --- a/test/engine/ddl.test.lua > > +++ b/test/engine/ddl.test.lua > > @@ -523,7 +523,7 @@ ok_format_change(12, 'any') > > -- uuid --X--> number > > fail_format_change(12, 'number') > > -- uuid --X--> scalar > > -fail_format_change(12, 'scalar') > > +ok_format_change(12, 'scalar') > > -- uuid --X--> string > > fail_format_change(12, 'string') > > -- uuid --X--> integer > > -- > Serge Petrenko > Diff: diff --git a/test/app/uuid.result b/test/app/uuid.result index 6b22b1d2c..c085f9b6d 100644 --- a/test/app/uuid.result +++ b/test/app/uuid.result @@ -460,6 +460,14 @@ s:insert({'1'}) --- - ['1'] ... +s:insert({true}) +--- +- [true] +... +box.execute([[INSERT INTO "s" VALUES (x'303030')]]) +--- +- row_count: 1 +... s:insert({uuid.fromstr('11111111-1111-1111-1111-111111111111')}) --- - [11111111-1111-1111-1111-111111111111] @@ -474,8 +482,10 @@ s:insert({uuid.fromstr('11111111-1111-2222-1111-111111111111')}) -- s:select() --- -- - [1] +- - [true] + - [1] - ['1'] + - ['000'] - [11111111-1111-1111-1111-111111111111] - [11111111-1111-2222-1111-111111111111] ... @@ -483,8 +493,10 @@ s:select({}, {iterator='LE'}) --- - - [11111111-1111-2222-1111-111111111111] - [11111111-1111-1111-1111-111111111111] + - ['000'] - ['1'] - [1] + - [true] ... s:drop() --- diff --git a/test/app/uuid.test.lua b/test/app/uuid.test.lua index 7c0942fd6..d2c34fadd 100644 --- a/test/app/uuid.test.lua +++ b/test/app/uuid.test.lua @@ -171,6 +171,8 @@ s = box.schema.space.create('s', {format={{'s', 'scalar'}}}) _ = s:create_index('i') s:insert({1}) s:insert({'1'}) +s:insert({true}) +box.execute([[INSERT INTO "s" VALUES (x'303030')]]) s:insert({uuid.fromstr('11111111-1111-1111-1111-111111111111')}) s:insert({uuid.fromstr('11111111-1111-2222-1111-111111111111')}) diff --git a/test/engine/ddl.result b/test/engine/ddl.result index e044bc094..0fed1cbe4 100644 --- a/test/engine/ddl.result +++ b/test/engine/ddl.result @@ -1428,7 +1428,7 @@ fail_format_change(12, 'number') - 'Tuple field 12 (field12) type does not match one required by operation: expected number, got extension' ... --- uuid --X--> scalar +-- uuid ----> scalar ok_format_change(12, 'scalar') --- ... diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua index 0cf25aa46..08162f253 100644 --- a/test/engine/ddl.test.lua +++ b/test/engine/ddl.test.lua @@ -522,7 +522,7 @@ fail_format_change(11, 'uuid') ok_format_change(12, 'any') -- uuid --X--> number fail_format_change(12, 'number') --- uuid --X--> scalar +-- uuid ----> scalar ok_format_change(12, 'scalar') -- uuid --X--> string fail_format_change(12, 'string') New patch: commit e4a47eb977a40454b834b4df6b1aaa40be406024 Author: Mergen Imeev <imeevma@gmail.com> Date: Thu Apr 22 18:00:32 2021 +0300 box: make UUID part of SCALAR Prior to this patch, UUID was not part of SCALAR. However, this should be changed to comply with the RFC "Consistent Lua/SQL types". Closes #6042 @TarantoolBot document Title: UUID is now part of SCALAR The UUID field type is now part of the SCALAR field type. This means that now values of type UUID can be inserted into the SCALAR field, and these values can participate in the sorting of the SCALAR fields. The order is as follows: boolean < number < string < varbinary < uuid. diff --git a/changelogs/unreleased/make-uuid-part-of-scalar.md b/changelogs/unreleased/make-uuid-part-of-scalar.md new file mode 100644 index 000000000..eeead4ffb --- /dev/null +++ b/changelogs/unreleased/make-uuid-part-of-scalar.md @@ -0,0 +1,3 @@ +## feature/core + + * Field type UUID is now part of field type SCALAR (gh-6042). diff --git a/src/box/field_def.c b/src/box/field_def.c index 213e91699..51acb8025 100644 --- a/src/box/field_def.c +++ b/src/box/field_def.c @@ -83,7 +83,7 @@ const uint32_t field_ext_type[] = { /* [FIELD_TYPE_INTEGER] = */ 0, /* [FIELD_TYPE_BOOLEAN] = */ 0, /* [FIELD_TYPE_VARBINARY] = */ 0, - /* [FIELD_TYPE_SCALAR] = */ 1U << MP_DECIMAL, + /* [FIELD_TYPE_SCALAR] = */ (1U << MP_DECIMAL) | (1U << MP_UUID), /* [FIELD_TYPE_DECIMAL] = */ 1U << MP_DECIMAL, /* [FIELD_TYPE_UUID] = */ 1U << MP_UUID, /* [FIELD_TYPE_ARRAY] = */ 0, diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc index 0946d77f8..3ae1dcc8f 100644 --- a/src/box/tuple_compare.cc +++ b/src/box/tuple_compare.cc @@ -1790,6 +1790,8 @@ field_hint_scalar(const char *field, struct coll *coll) decimal_t dec; return hint_decimal(decimal_unpack(&field, len, &dec)); } + case MP_UUID: + return hint_uuid_raw(field); default: unreachable(); } diff --git a/test/app/uuid.result b/test/app/uuid.result index b4870302e..c085f9b6d 100644 --- a/test/app/uuid.result +++ b/test/app/uuid.result @@ -443,6 +443,64 @@ u1_str = nil u2_str = nil --- ... +-- +-- gh-6042: add UUID to SCALAR. +-- +s = box.schema.space.create('s', {format={{'s', 'scalar'}}}) +--- +... +_ = s:create_index('i') +--- +... +s:insert({1}) +--- +- [1] +... +s:insert({'1'}) +--- +- ['1'] +... +s:insert({true}) +--- +- [true] +... +box.execute([[INSERT INTO "s" VALUES (x'303030')]]) +--- +- row_count: 1 +... +s:insert({uuid.fromstr('11111111-1111-1111-1111-111111111111')}) +--- +- [11111111-1111-1111-1111-111111111111] +... +s:insert({uuid.fromstr('11111111-1111-2222-1111-111111111111')}) +--- +- [11111111-1111-2222-1111-111111111111] +... +-- +-- Make sure that comparison in right. Comparison in SCALAR field: +-- bool < number < string < varbinary < uuid. +-- +s:select() +--- +- - [true] + - [1] + - ['1'] + - ['000'] + - [11111111-1111-1111-1111-111111111111] + - [11111111-1111-2222-1111-111111111111] +... +s:select({}, {iterator='LE'}) +--- +- - [11111111-1111-2222-1111-111111111111] + - [11111111-1111-1111-1111-111111111111] + - ['000'] + - ['1'] + - [1] + - [true] +... +s:drop() +--- +... uuid = nil --- ... diff --git a/test/app/uuid.test.lua b/test/app/uuid.test.lua index 867bbd832..d2c34fadd 100644 --- a/test/app/uuid.test.lua +++ b/test/app/uuid.test.lua @@ -164,6 +164,26 @@ u1 = nil u1_str = nil u2_str = nil +-- +-- gh-6042: add UUID to SCALAR. +-- +s = box.schema.space.create('s', {format={{'s', 'scalar'}}}) +_ = s:create_index('i') +s:insert({1}) +s:insert({'1'}) +s:insert({true}) +box.execute([[INSERT INTO "s" VALUES (x'303030')]]) +s:insert({uuid.fromstr('11111111-1111-1111-1111-111111111111')}) +s:insert({uuid.fromstr('11111111-1111-2222-1111-111111111111')}) + +-- +-- Make sure that comparison in right. Comparison in SCALAR field: +-- bool < number < string < varbinary < uuid. +-- +s:select() +s:select({}, {iterator='LE'}) +s:drop() + uuid = nil test_run:cmd("clear filter") diff --git a/test/engine/ddl.result b/test/engine/ddl.result index 08ad1a57b..0fed1cbe4 100644 --- a/test/engine/ddl.result +++ b/test/engine/ddl.result @@ -1428,11 +1428,9 @@ fail_format_change(12, 'number') - 'Tuple field 12 (field12) type does not match one required by operation: expected number, got extension' ... --- uuid --X--> scalar -fail_format_change(12, 'scalar') +-- uuid ----> scalar +ok_format_change(12, 'scalar') --- -- 'Tuple field 12 (field12) type does not match one required by operation: expected - scalar, got extension' ... -- uuid --X--> string fail_format_change(12, 'string') diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua index 5ba80e075..08162f253 100644 --- a/test/engine/ddl.test.lua +++ b/test/engine/ddl.test.lua @@ -522,8 +522,8 @@ fail_format_change(11, 'uuid') ok_format_change(12, 'any') -- uuid --X--> number fail_format_change(12, 'number') --- uuid --X--> scalar -fail_format_change(12, 'scalar') +-- uuid ----> scalar +ok_format_change(12, 'scalar') -- uuid --X--> string fail_format_change(12, 'string') -- uuid --X--> integer
next prev parent reply other threads:[~2021-04-26 15:31 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-23 11:40 Mergen Imeev via Tarantool-patches 2021-04-25 9:07 ` Serge Petrenko via Tarantool-patches 2021-04-26 15:31 ` Mergen Imeev via Tarantool-patches [this message] 2021-04-28 14:43 ` Serge Petrenko via Tarantool-patches 2021-05-16 9:31 Mergen Imeev via Tarantool-patches 2021-05-19 10:48 ` Mergen Imeev via Tarantool-patches 2021-05-21 19:09 ` Vladislav Shpilevoy via Tarantool-patches 2021-05-24 9:49 ` Kirill Yukhin via Tarantool-patches
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=20210426153151.GA156387@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=sergepetrenko@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/1] box: make UUID part of SCALAR' \ /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