Re: [Tarantool-patches] [PATCH v2 1/1] sql: enable autoindex optimization

[Nikita Pettik <korablev@tarantool.org>]

On 06 Oct 14:00, Mergen Imeev wrote:
> Hi! My answer below.
> 
> On Tue, Oct 06, 2020 at 10:44:40AM +0000, Nikita Pettik wrote:
> > On 06 Oct 10:08, imeevma@tarantool.org wrote:
> > > This patch enables "autoindex" optimization. This optimization uses an
> > > ephemeral space that contains all the fields used in the query. This
> > > ephemeral space is called "autoindex". Autoindex may have fewer fields
> > > than the original space, and the order of these fields in autoindex may
> > > be different from their order in the original space. In addition to the
> > > mentioned fields, autoindex contains a rowid.
> > > 
> > > The primary key of autoindex is a covering index, i.e. it contains all
> > > of its fields.
> > > 
> > > Currently this optimization mainly used in cases when values SELECTED
> > > from more than one space and WHERE specified.
> > > 
> > > At the moment, this optimization does not take into account the number
> > > of tuples in spaces. This should be fixed.
> > > 
> > > Closes #4933
> > > 
> > > Co-authored-by: Mergen Imeev <imeevma@gmail.com>
> > > Co-authored-by: Timur Safin <tsafin@tarantool.org>
> > > ---
> > > https://github.com/tarantool/tarantool/issues/4933
> > > https://github.com/tarantool/tarantool/tree/imeevma/gh-4933-autoindex
> > > 
> > > @ChangeLog
> > >  - "Auto-index" optimization is now enabled (gh-4933).
> > > diff --git a/test/sql/misc.result b/test/sql/misc.result
> > > index 6af11bfba..d86d55081 100644
> > > --- a/test/sql/misc.result
> > > +++ b/test/sql/misc.result
> > > @@ -204,3 +204,32 @@ box.execute('SELECT field66, field68, field70 FROM test')
> > >  box.space.TEST:drop()
> > >  ---
> > >  ...
> > > +-- gh-4933: Make sure that autoindex optimization is used.
> > > +box.execute('CREATE TABLE t1(i int primary key, a int);')
> > > +---
> > > +- row_count: 1
> > > +...
> > > +box.execute('CREATE TABLE t2(i int primary key, b int);')
> > > +---
> > > +- row_count: 1
> > > +...
> > > +--
> > > +-- There is no need to insert values in the tables since planner assumes a
> > > +-- default number of tuples for each space, regardless of how many tuples there
> > > +-- actually are in those spaces. The default value is 1048576 (== 2^20).
> > > +--
> > > +box.execute('EXPLAIN QUERY PLAN SELECT a, b FROM t1, t2 WHERE a = b;')
> > > +---
> > > +- metadata:
> > > +  - name: selectid
> > > +    type: integer
> > > +  - name: order
> > > +    type: integer
> > > +  - name: from
> > > +    type: integer
> > > +  - name: detail
> > > +    type: text
> > > +  rows:
> > > +  - [0, 0, 0, 'SCAN TABLE T1 (~1048576 rows)']
> > > +  - [0, 1, 1, 'SEARCH TABLE T2 USING AUTOMATIC COVERING INDEX (B=?) (~20 rows)']
> > > +...
> > 
> > I haven't reviewed the whole patch yet, but do you really think autoindex
> > qp optimization deserves this single insignificant test..?
> > 
> The main test is TPC-H Q13, Q17 and Q20. Here we show that the optimization is
> enabled.
>

Sorry, I won't accept this patch with no tests. Please look and adopt
corresponding tests from sqlite:

https://github.com/sqlite/sqlite/blob/master/test/autoindex1.test
https://github.com/sqlite/sqlite/blob/master/test/autoindex3.test
https://github.com/sqlite/sqlite/blob/master/test/autoindex2.test
https://github.com/sqlite/sqlite/blob/master/test/autoindex4.test
https://github.com/sqlite/sqlite/blob/master/test/autoindex5.test

You also can simplify queries from tpc-h and use them as well.