Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Ilya Kosarev <i.kosarev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] key_def: support composite types extraction
Date: Thu, 1 Oct 2020 14:38:30 +0300	[thread overview]
Message-ID: <20201001113830.p7af3m647h6vwem6@tkn_work_nb> (raw)
In-Reply-To: <1601157194.729560301@f185.i.mail.ru>

> >How about lbox_key_def_merge() and underlying functions? I'm not sure
> >they will work correct. At least I tried this on the branch:
> >
> > | tarantool> key_def = require('key_def')
> > | tarantool> kd1 = key_def.new({{fieldno = 1, type = 'array'}})
> > | tarantool> kd2 = key_def.new({{fieldno = 1, type = 'map'}})
> > | tarantool> kd1:merge(kd2)
> > | ---
> > | - - type: array
> > | is_nullable: false
> > | fieldno: 1
> > | ...
> >
> >It does not look correct.
> This turns out to be the correct behavior:
>  
> tarantool> kd1 = key_def.new({{fieldno = 1, type = 'unsigned'}})
> ---
> ...
> tarantool> kd2 = key_def.new({{fieldno = 1, type = 'string'}})
> ---
> ...
> tarantool> kd1:merge(kd2)
> ---
> - - type: unsigned
>     is_nullable: false
>     fieldno: 1
> …

I'm not sure it is correct. But at least you proved that it is not in
the scope of your current task :)

An attempt to create conflicting indices gives an error, so I guess this
case is not handled, because it was not possible until we exposed
key_def directly to Lua.

But another case makes me doubt even more:

 | tarantool> key_def = require('key_def')
 | tarantool> kd1 = key_def.new({{fieldno = 1, type = 'scalar'}})
 | tarantool> kd2 = key_def.new({{fieldno = 1, type = 'integer'}})
 | tarantool> kd1:merge(kd2)
 | ---
 | - - type: scalar
 |     is_nullable: false
 |     fieldno: 1
 | ...
 |
 | tarantool> kd1 = key_def.new({{fieldno = 1, type = 'integer'}})
 | tarantool> kd2 = key_def.new({{fieldno = 1, type = 'scalar'}})
 | tarantool> kd1:merge(kd2)
 | ---
 | - - type: integer
 |     is_nullable: false
 |     fieldno: 1
 | ...

It would be natural to get most restricted type, when one type is subset
of another. I assume the following invariant: for any given kd1 and kd2
kd1:merge(kd2) should accept only those tuples that both kd1 and kd2
accept (kd accepts a tuple when it is valid against kd).

OTOH, the main use case for :merge() function is to compare tuples in
the same way as a non-unique secondary index do (when a key parts of a
primary index are implicitly merged into the secondary index ones). In
this sense we can choose either subtype or supertype: it will not affect
an order of tuples. So choosing of the first (secondary index's) key
part looks okay.

I don't know whether we should change anything or not, so just shared my
findings and thoughts.

If I'll somehow end with an idea that we should do something here, I'll
file an issue.

WBR, Alexander Turenko.

  reply	other threads:[~2020-10-01 11:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-21 12:11 Ilya Kosarev
2020-09-25 18:34 ` Alexander Turenko
2020-09-26 21:53   ` Ilya Kosarev
2020-10-01 11:38     ` Alexander Turenko [this message]
2020-10-01 15:26       ` Ilya Kosarev

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=20201001113830.p7af3m647h6vwem6@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=i.kosarev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] key_def: support composite types extraction' \
    /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