Date: Tue, 16 Nov 2021 13:25:37 -0300
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Daniel Westermann <dwe@dbi-services.com>,
	pgsql-hackers@lists.postgresql.org,
	Simon Riggs <simon.riggs@enterprisedb.com>,
	Pavan Deolasee <pavan.deolasee@gmail.com>
Subject: Re: support for MERGE
Message-ID: <202111161625.ypdd2kxce64b@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqF0CEYoWCDj71smZ9WiaZkbk_coj9mokXb+_q1=A6M8Kw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202111161625.ypdd2kxce64b%40alvherre.pgsql>
Precedence: bulk

Hi Amit

On 2021-Nov-16, Amit Langote wrote:

> AFAICS, MERGE operating on an inheritance parent that is not
> partitioned should work mostly the same as the case where it is
> partitioned (good thing that it works at all without needing any
> special code!), though only the INSERT actions would have to be
> handled appropriately by the user using triggers and such.  And also I
> guess any UPDATE actions that need to move rows between child tables
> because that too involves tuple routing logic.  As long as we're clear
> on that in the documentation, I don't see why this case should not be
> covered in the initial version.

Yeah, I think the reason it works so cleanly is that the code you and/or
Tom added to be able to get rid of inheritance_planner is superb,
including the new row identity stuff.  For the same reason, I suspect
that adding support for foreign tables should be reasonably simple --
just add explicit support for handling "wholerow" in a few places.  I
have not tried.

> I thought for a second about the cases where child tables have columns
> not present in the root parent mentioned in the command, but I guess
> that possibility doesn't present problems given that the command
> wouldn't be able to mention such columns to begin with; it can only
> refer to the root parent's column which must be present in all of the
> affected child tables.

Right.  On the other hand, if we did have a problem with extra columns,
ISTM that would be on the user's head, not our responsibility.  In the
example I added, there is one child table with variant column layout; it
did require that the insertion trigger explicitly lists the columns in
the INSERT statement for that table, but otherwise it work correctly.

> In any case, I have a feeling that the planner would catch any
> problematic cases if there're any while converting MergeAction
> expressions into the individual child table layouts.

Yeah, AFAICS it worked fine for the case I tried.  Maybe there are more
elaborate ones that I didn't think of, of course.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"