From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
---|---|
To: | David Fetter <david(at)fetter(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
Cc: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Early WIP/PoC for inlining CTEs |
Date: | 2018-10-01 12:55:12 |
Message-ID: | dccb37dd-2f52-dbd7-a4c4-72a5853f4838@proxel.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 07/27/2018 10:10 AM, David Fetter wrote:
> On Fri, Jul 27, 2018 at 02:55:26PM +1200, Thomas Munro wrote:
>> On Thu, Jul 26, 2018 at 7:14 AM, David Fetter <david(at)fetter(dot)org> wrote:
>>> Please find attached the next version, which passes 'make check'.
>>
>> ... but not 'make check-world' (contrib/postgres_fdw's EXPLAIN is different).
>
> Please find attached a patch that does.
>
> It doesn't always pass make installcheck-world, but I need to sleep
> rather than investigate that at the moment.
I took a quick look at this patch and I have a couple of comments.
1) Is it really safe, for backwards compatibility reasons, to inline
when there are volatile functions? I imagine that it is possible that
there are people who rely on that volatile functions inside a CTE are
always run.
Imagine this case:
WITH cte AS (
SELECT x, volatile_f(x) FROM tab ORDER BY x
)
SELECT * FROM cte LIMIT 10;
It could change behavior if volatile_f() has side effects and we inline
the CTE. Is backwards compatibility for cases like this worth
preserving? People can get the old behavior by adding OFFSET 0 or
MATERIALIZED, but existing code would break.
2) The code in inline_cte_walker() is quite finicky. It looks correct to
me but it is hard to follow and some simple bug could easily be hiding
in there. I wonder if this code could be rewritten in some way to make
it easier to follow.
3) Can you recall what the failing test was because I have so far not
managed to reproduce it?
Andreas
From | Date | Subject | |
---|---|---|---|
Next Message | John Naylor | 2018-10-01 13:19:21 | Re: Sync ECPG scanner with core |
Previous Message | Etsuro Fujita | 2018-10-01 12:54:26 | Re: de-deduplicate code in DML execution hooks in postgres_fdw |