On Mon, Jan 5, 2015 at 11:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David G Johnston <david.g.johnston@gmail.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...
