Re: [PATCH] Remove useless ORDER BY from scalar subqueries

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