From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
cc: Alexander Lakhin <exclusion@gmail.com>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17803: Rule "ALSO INSERT ... SELECT ..." fails to substitute
 default values
In-reply-to: 
 <CAEZATCVMdsLhBj8BbGT3B-Y-4HpmBW8EdOZGhYk+KG6hXngrdQ@mail.gmail.com>
References: <17803-53c63ed4ecb4eac6@postgresql.org>
 <CAEZATCUQtGx6gQtryMbC3ZsEd4GUcc5THVnkXP1F0Fan_uLG-Q@mail.gmail.com>
 <3953951.1676991950@sss.pgh.pa.us>
 <CAEZATCWoXX9-+ZaaLiOMRsZgwmStayBGmCdA0BdwVzo5GCU51Q@mail.gmail.com>
 <3977948.1677002558@sss.pgh.pa.us>
 <7f8f1bbb-9fb3-80fb-a400-5c7e755b5aa2@gmail.com>
 <CAEZATCUrXMUK8F6daS_f0kH7o_=nDtEqA1Ny23SwHWbFvH61PA@mail.gmail.com>
 <60780f8c-3132-133e-8286-538b732bff19@gmail.com>
 <CAEZATCW8WkFajCvOzhcXeSmX-TrHY3mJgd3pS09jV=BY1mOS3g@mail.gmail.com>
 <5e09da43-aaba-7ea7-0a51-a2eb981b058b@gmail.com>
 <CAEZATCW2BPV8H_FoAj6V-_4aK3zn2+DX0pUSL76aXzqYUQsoWQ@mail.gmail.com>
 <716957.1677189004@sss.pgh.pa.us>
 <CAEZATCUo-GCtup4_ve89pp+3XTBirqppi8fYv16J+uxnAcA4AQ@mail.gmail.com>
 <CAEZATCVMdsLhBj8BbGT3B-Y-4HpmBW8EdOZGhYk+KG6hXngrdQ@mail.gmail.com>
Comments: In-reply-to Dean Rasheed <dean.a.rasheed@gmail.com>
	message dated "Fri, 24 Feb 2023 08:50:45 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <847246.1677257625.1@sss.pgh.pa.us>
Date: Fri, 24 Feb 2023 11:53:45 -0500
Message-ID: <847247.1677257625@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/847247.1677257625%40sss.pgh.pa.us>
Precedence: bulk

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Here's an updated patch, now with test cases involving both OLD and
> NEW references.

It might be worth doing

+        if (rte->rtekind == RTE_SUBQUERY && !rte->lateral &&
+            contain_vars_of_level((Node *) rte->subquery, 1))
+            rte->lateral = true;

so as to save the expense of contain_vars_of_level() when the flag
is already set.  However, it's arguable that no users would bother
with writing LATERAL, so this might be pointless.

More importantly, I think the comment could do with a bit more
information.  Maybe like

     /*
+     * Mark any subquery RTEs in the rule action as LATERAL if they contain
+     * Vars referring to the current query level (references to NEW/OLD).
+     * Those really are lateral references, but we've historically not
+     * required users to mark such subqueries with LATERAL explicitly.
+     * But the planner will complain if such Vars exist in a non-LATERAL
+     * subquery, so we have to fix things up here.
+     */

That might be overly verbose, but I think it's good to memorialize this
sort of info.

LGTM otherwise.

			regards, tom lane