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

n.pettik korablev at tarantool.org
Wed Jul 18 23:36:38 MSK 2018


> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index 54f78a9..c035691 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -3893,14 +3893,14 @@ flattenSubquery(Parse * pParse,		/* Parsing context */
> 	 * queries.
> 	 */
> 	if (pSub->pPrior) {
> -		if (pSub->pOrderBy) {
> -			return 0;	/* Restriction 20 */
> -		}
> 		if (isAgg || (p->selFlags & SF_Distinct) != 0
> 		    || pSrc->nSrc != 1) {
> 			return 0;
> 		}
> 		for (pSub1 = pSub; pSub1; pSub1 = pSub1->pPrior) {
> +			if (pSub1->pOrderBy) {
> +				return 0;	/* Restriction 20 */
> +			}

According to our code style:
- use explicit != NULL comparison;
- don’t put bracers around one-line if statement;
- put comment above the code to be commented;

>diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
>--- a/test/sql-tap/subquery.test.lua
>+++ b/test/sql-tap/subquery.test.lua
>+test:do_execsql_test(
>+       "subquery-9.2",
>+       [[
>+               SELECT 'abc' FROM (SELECT * FROM table1 UNION ALL
>+                                   SELECT * FROM table1 UNION ALL
>+                                  SELECT * FROM table1 ORDER BY 1);
>+       ]], {
>+               -- <subquery-9.2>
>+               'abc','abc','abc','abc','abc','abc'
>+               -- <subquery-9.2>
>+       })

Why did you use here SELECT ‘abc’ and in other tests - SELECT *?
It is nitpicking, but just tests look a little weird..



More information about the Tarantool-patches mailing list