[tarantool-patches] Re: [PATCH] sql: assertion fail on nested select

[Nikita Tatunov <hollow653@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

ср, 18 июл. 2018 г. в 15:34, Nikita Tatunov <hollow653@gmail.com>:

> Hello, Alexander! Please consider changes to the patch:
>
> вт, 17 июл. 2018 г. в 16:59, Alexander Turenko <
> alexander.turenko@tarantool.org>:
>
>> Hi Nikita!
>>
>> Please, consider comments below.
>>
>> WBR, Alexander Turenko.
>>
>> On Mon, Jul 02, 2018 at 03:11:40PM +0300, N.Tatunov wrote:
>> > To optimize the select-subquery tarantool uses
>> > subquery flattening function which is only used
>> > with respect to some restrictions. One of them
>> > was implemented improperly.
>> >
>>
>> Be certain: 'some restriction' and just 'improperly' are not good
>> problem descriptions.
>>
>> I would use wording like the following.
>>
>> > When a subquery is a compound select the subquery flattening
>> > optimization verify whether the ORDER BY clause presents, but only for
>> > the last subquery component. The commit fixes that to verify all
>> > subquery components.
>>
>> > In case of ORDER BY in non-last component of a compound select the
>> > error should be raised. Accepting of such subqueries for the
>> > flattening optimization prevents the error to be raised afterwards.
>> > Instead it stuck into assertion violation inside flattenSubquery (see
>> > the issue).
>>
>>
> Fixed:
>
> To optimize the select-subquery tarantool uses
> subquery flattening optimization which is only used
> with respect to some restrictions. When any of
> subselects but the last one has an ORDER BY
> clause we should raise an error. So subquery
> components containing it are not accepted
> to be optimized.
>
> Checking part was only checking if the last
> subselect contains the clause which led to an
> assertion fail. With the patch applied
> flattening is not used in case any of
> subselects has an ORDER BY.
>
>
>> I checked sqlite3 check-in 8000d230 (it is immediate predecessor of
>> c9104b59 where the flattening optimization seems to be changed, but
>> tarantool didn't updated with that check-in). It has the assert, but has
>> no a change like yours, and correctly shows the following error:
>>
>> Error: ORDER BY clause should come after UNION ALL not before
>>
>> Why it is so?
>>
>>
I tried to test that problem on this sqlite3 version with -DSQLITE_DEBUG
and assertion fails there.
Since flattening only makes another compound select-stmt with ORDER BY
clause I guess error was
occuring on flattened version of it. Moreover in latter versions of sqlite3
this assert was deleted and the
restriction check wasn't fixed. So I assume sqlite3 developers just don't
know about it or don't aim on
fixing it as the error anyways occurs but on flattened subquery.

> +test:do_catchsql_test(
>> > +     "subquery-9.3",
>> > +     [[
>> > +             SELECT * FROM (SELECT * FROM table1 ORDER BY 1 UNION All
>> > +                               SELECT * FROM table1 UNION All
>> > +                            SELECT * FROM table1);
>>
>> All -> ALL
>>
>
> Also fixed.
>

[-- Attachment #2: Type: text/html, Size: 4408 bytes --]