[tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY

[n.pettik]

>> Plase, make up a list of tests to be enabled after multi-directional
>> iterators are introduced. Don’t fix tests which fail now, simply comment
>> them. Finally, add a few simple tests verifying that different sorting
>> orders are not supported now.
> 
> I did as you say. Added list of tests as a comment to #3309.
> But I would like to know, why commenting tests & making a list
> of them is better than change tests & comment old expected results?
> The main advantage is that one can not ignore/miss tests, which
> would fail after #3309 is done.

Firstly, they won’t automatically start to fail, since check is SQL
code won’t disappear itself. Secondly, such approach allows to
significantly reduce efforts to turn tests on/off after/before fixes.

> +    -- Tests are commented because of the reason described in
> +    -- NOTE at the beginning of the file.
> +    -- <begin>
> +    --local data = {
> +    --    {limit=0, offset=4, orderby="+b,+a"},
> +    --    {limit=0, offset=5, orderby="+b,+a"},
> +    --    {limit=0, offset=6, orderby="+b,+a"},
> +    --    {limit=0, offset=9, orderby="+b,+a"},
> +    --    {limit=0, offset=0, orderby="+b,+a"},
> +    --    {limit=0, offset=1, orderby="+b,+a"},
> +    --    {limit=7, offset=4, orderby="+b,+a"},
> +    --    {limit=7, offset=9, orderby="+b,+a”},

These first tests come with the same sorting orders,
can we avoid commenting them?

> +    --    {limit=0, offset=4, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=5, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=6, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=9, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=0, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=1, orderby="+b DESC,+a"},
> +    --    {limit=7, offset=4, orderby="+b DESC,+a"},
> +    --    {limit=7, offset=9, orderby="+b DESC,+a"},
> 
> +void
> +sql_expr_check_sort_orders(struct Parse *parse,
> +			   const struct ExprList *expr_list)
> +{
> +	if(expr_list == NULL)
> +		return;
> +	enum sort_order reference_order = expr_list->a[0].sort_order;
> +	for (int i = 1; i < expr_list->nExpr; i++) {
> +		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
> +		if (expr_list->a[i].sort_order != reference_order) {
> +			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
> +				 "different sorting orders”);

Different sorting orders and LIMIT clause at the same time OR
“ORDER BY with LIMIT”, “different sorting orders”.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190315/d602338d/attachment.html>