public inbox for [email protected]  

help / color / mirror / Atom feed

From: Paul A Jungwirth <[email protected]>
To: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables
Date: Tue, 26 May 2026 09:14:04 -0700
Message-ID: <CA+renyVNmXityjqZNdKhG2SGkfKFM1dT72PTbkyQ5K6L9m9NuQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CA+renyV6BS+cUxAbxB1X-bBn8+9NDEwRah8pSD_EVdhtb1TcUA@mail.gmail.com>
	<[email protected]>

On Mon, May 25, 2026 at 11:55 PM Chao Li <[email protected]> wrote:
>
> Now v2 mixes two separate changes:
>
> 1) Adding ExecInitForPortionOf() to address the UPDATE OF trigger issue
> 2) Fixing the inheritance/leftover bug originally reported in this thread
>
> I’d prefer not to combine these in a single patch. Could you please split out the refactoring that adds ExecInitForPortionOf() into a separate patch, with tests showing that the UPDATE OF trigger issue is fixed? Then I can rework my inheritance/leftover fix on top of that.

Thanks for taking a look! I agree about keeping the fixes separate.
UPDATE OF triggers are fixed by the patch on the other thread (with
tests). Since we now include the application-time column in
updatedCols, there is nothing else to do there.

I thought the ExecInitForPortionOf refactoring was still nice to keep,
and it seemed to fit better here. (I should have updated the commit
message not to mention UPDATE OF though.)

But thinking about it this morning, I realized: the planner already
prunes partitions before we get here. Lazily initializing the structs
is trying too hard. Is there some scenario where that actually saves
work? If not, we could just set up all the structs in
ExecInitModifyTable. That's what I was doing in an older version of
the original patch. And then the refactor with ExecInitForPortionOf
seems unnecessary. I'll send a patch with those changes, unless you
have objections. (Probably two separate patches: one to remove lazy
initialization, another to actually fix traditional inheritance.) If
you disagree, please let me know.

Yours,

-- 
Paul              ~{:-)
[email protected]

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables
  In-Reply-To: <CA+renyVNmXityjqZNdKhG2SGkfKFM1dT72PTbkyQ5K6L9m9NuQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox