Re: Re: Patch to add functionality to specify ORDER BY in CREATE FUNCTION for SRFs

On Mon, Jan 5, 2015 at 11:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David G Johnston <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> > Atri Sharma wrote
> >> If order of result rows is not the same as required, an error is raised:
> >>
> >> SELECT * FROM incorrect_order_nulls() ORDER BY e NULLS LAST;
> >> ERROR: Order not same as specified
>
> > First reaction for the error was unfavorable but (see below) it likely is
> > the best option and does adequately cover the reason for failure -
> > programmer error.
>
> TBH, my first reaction to this entire patch is unfavorable: it's a
> solution in search of a problem. It adds substantial complication not
> only for users but for PG developers in order to solve a rather narrow
> performance issue.
>

I could agree about the scope of the performance issue, but am not sure
about the added complication. It essentially is similar to, say, a
combination of how Unique is implemented with a flavour or ORDINALITY
implementation. A new path that is added in a certain number of cases plus
a low overhead node does not seem too bad to me IMO. This is inline with a
lot of real world cases I have seen, where the data is *bubbled* up to
SRFs, which does give a possibility of an existing order. Couple it with
the power to specify ORDER BY in your SRF function and you could save a lot.

I am not sure how it complicates for hackers. Could you please elaborate a
bit?

> What would make sense to me is to teach the planner about inlining
> SQL functions that include ORDER BY clauses, so that the performance
> issue of a double sort could be avoided entirely transparently to
> the user.
>

It sounds good, but inlining in current way shall restrict the scope of
optimization (which is not applicable for current design). For eg, you
cannot inline RECORD returning SRFs...

--
Regards,

Atri
*l'apprenant*