Re: Changed SRF in targetlist handling

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Changed SRF in targetlist handling
Date: 2016-05-23 02:09:25
Message-ID: CAKFQuwY5FvPmHi8x8_sXfAa2CwdaK_m7ds69zCipHViLzx=p9Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

tl;dr

Semantic changes to SRF-in-target-list processing are undesirable when they
are all but deprecated.

I'd accept a refactoring that trades a performance gain for unaffected
queries for a reasonable performance hit of those afflicted.

Preamble...

Most recent thread that I can recall seeing on the topic - and where I
believe the rewrite idea was first presented.

http://www.postgresql.org/message-id/flat/25750(dot)1458767514(at)sss(dot)pgh(dot)pa(dot)us#25750(dot)1458767514@sss.pgh.pa.us

On Sun, May 22, 2016 at 8:53 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> discussing executor performance with a number of people at pgcon,
> several hackers - me included - complained about the additional
> complexity, both code and runtime, required to handle SRFs in the target
> list.
>
> One idea I circulated was to fix that by interjecting a special executor
> node to process SRF containing targetlists (reusing Result possibly?).
> That'd allow to remove the isDone argument from ExecEval*/ExecProject*
> and get rid of ps_TupFromTlist which is fairly ugly.
>

​Conceptually I'm all for minimizing the impact on queries of this form.
It seems to be the most likely to get written and committed and the least
likely to cause unforeseen issues.

> Robert suggested - IIRC mentioning previous on-list discussion - to
> instead rewrite targetlist SRFs into lateral joins. My gut feeling is
> that that'd be a larger undertaking, with significant semantics changes.
>
​[...]​

> If we accept bigger semantical changes, I'm inclined to instead just get
> rid of targetlist SRFs in total; they're really weird and not needed
> anymore.
>

​I cannot see these, in isolation, being a good option. Nonetheless, I
don't think any semantic change should happen before 9.2 becomes no longer
supported. I'd be inclined to take a similar approach as with
standard_conforming_strings (minus the execution guc, just the warning one)
with whatever after-the-fact learning taken into account.

Its worth considering query rewrite and making it forbidden as a joint goal.

For something like a canonical version of this, especially for
composite-returning SRF:

WITH func_call (
SELECT func(tbl.col)
FROM tbl
)
​SELECT (func_call.func).*
FROM func_call;​

If we can rewrite the CTE portion into a lateral - with the exact same
semantics (specifically, returning the single-column composite) then check
the rewritten query the select list SRF would not longer be present and no
error would be thrown.

For situations where a rewrite cannot be made to behave properly we leave
the construct alone and let the query raise an error.

In considering what I just wrote I'm not particularly enamored with
it...hence my overall conclusion. Can't say I hate it and after re-reading
the aforementioned thread I'm inclined to like it for cases where, for
instance, we are susceptible to a LCM evaluation.

David J.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2016-05-23 02:13:03 Re: Adding an alternate syntax for Phrase Search
Previous Message Alvaro Herrera 2016-05-23 01:52:06 Re: [sqlsmith] PANIC: failed to add BRIN tuple