Message-ID: <d217eb59-835f-3844-8279-829fac9fd22d@enterprisedb.com>
Date: Thu, 21 Apr 2022 22:45:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: Assert failure in CTE inlining with view and correlated subquery
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Richard Guo <guofenglinux@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <29196a1e-ed47-c7ca-9be2-b1c636816183@enterprisedb.com>
 <CAMbWs498tSxj0eiR13JvgYq=hPjwqG086_Fo8X34ESp4OQHDug@mail.gmail.com>
 <CAMbWs48n9MAfokXPoJZ3La8KMru8rJ7bVB31Ywv=x2adtDjCMg@mail.gmail.com>
 <48ee60e4-7ec3-ab21-ff67-e7f9dc2e37b1@enterprisedb.com>
 <3399045.1650567813@sss.pgh.pa.us>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <3399045.1650567813@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/d217eb59-835f-3844-8279-829fac9fd22d%40enterprisedb.com>
Precedence: bulk

On 4/21/22 21:03, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
>> On 4/21/22 10:29, Richard Guo wrote:
>>> Further debugging shows that in this repro the reference to the CTE is
>>> removed when generating paths for the subquery 'sub', where we would try
>>> to remove subquery targetlist items that are not needed. So for the
>>> items we are to remove, maybe we need to check if they contain CTEs and
>>> if so decrease cterefcount of the CTEs correspondingly.
> 
>> Right, at some point we remove the unnecessary targetlist entries, but
>> that ignores the entry may reference a CTE. That's pretty much what I
>> meant by the counter being "out of sync".
>> Updating the counter while removing the entry is one option, but maybe
>> we could simply delay counting the CTE references until after that?
> 
> I think we should just drop this cross-check altogether; it is not nearly
> useful enough to justify the work that'd be involved in maintaining
> cterefcount accurately for all such transformations.  All it's really
> there for is to be sure that we don't need to make a subplan for the
> inlined CTE.
> 
> There are two downstream consumers of cte_plan_ids, which currently just
> have Asserts that we made a subplan.  I think it'd be worth converting
> those to real run-time tests, which leads me to something more or less as
> attached.
> 

WFM. I'm not particularly attached to the assert, so if you say it's not
worth it let's get rid of it.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company