Hi David, The sort expressions stay in the targetlist, only sortClause is cleared, so your example still raises division by zero. There is a regression test for it.
Stock:
InitPlan expr_1
-> Sort
Output: t.a, ((1 / (t.a - 2)))
Sort Key: ((1 / (t.a - 2)))
-> Seq Scan on public.t
Output: t.a, (1 / (t.a - 2))
Filter: (t.a = 2)
ERROR: division by zero
Patched:
InitPlan expr_1
-> Seq Scan on public.t
Output: t.a, (1 / (t.a - 2))
Filter: (t.a = 2)
ERROR: division by zero
In the attached v2 (against b536d5a0bd7 and REL_13 to REL_19)
I drop ORDER BY for volatile sort keys too, but keep it
for set operations like here:
SELECT (SELECT a FROM t1 UNION ALL SELECT a FROM t2 ORDER BY 1);
Best regards,
Iliia
On Mon, Oct 19, 2026 at 8:30 PM David Geier <[email protected]> wrote:
>
> Hi Iliia,
>
> > The query is valid, the plan is bad. ORDER BY can't change the outcome
> > so it is wasteful. The Sort reads the whole input first.
>
> Agreed.
>
> Worth pointing out that this works because the executor will return an
> error if the sub-query returns > 1 row. So it can only ever return 0
> rows (result is NULL, no sort needed) or 1 row (result is well defined
> without sort).
>
> > The Planner already drops the ORDER BY for EXISTS in
> > simplify_EXISTS_query.
> > This does the same for scalar subqueries.
>
> Good point that there's precedence for this type of change. This is
> especially important for the error case, e.g.
>
> SELECT (SELECT a FROM t WHERE a = 2 ORDER BY 1 / (a - 2));
>
> Without the optimization, this results in a division-by-zero error. With
> the optimization this is no longer the case. There's a comment in the
> optimization for EXISTS which states the accepted semantic change:
>
> * Note: by suppressing the targetlist we could cause an observable
> behavioral
> * change, namely that any errors that might occur in evaluating the tlist
> * won't occur, nor will other side-effects of volatile functions. This
> seems
> * unlikely to bother anyone in practice.
>
> So for EXISTS we apply the optimization even in the presence of volatile
> expressions and accept that those side effects may vanish.
>
> Your patch for sublinks is stricter. Why not do it the same way it's
> done for EXISTS?
>
> --
> David Geier
REL_13-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
master-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_15-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_16-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_14-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_18-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_19-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
REL_17-v2-0001-Remove-useless-ORDER-BY-from-scalar-subqueries.patch
Description: Binary data
