Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)
Date: 2022-12-23 12:10:31
Message-ID: CAApHDvpJgZ35NDac3fYi=VXjJma8AyyBFXQ2jM8RVVLRR00QmQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 23 Dec 2022 at 17:04, Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> Thanks for the test! I looked at this and found that with WCO
> constraints we can also hit the buggy code. Based on David's test case,
> I came up with the following in the morning.

I ended up going with a WCO option test in the end. I wanted to steer
clear of having a test that has expected broken results from the
generated column code. Also, I just couldn't help thinking the
generated column test felt like it was just glued on the end and not
really in the correct place in the file.

I've put the new WCO test in along with the existing one. I also
considered modifying one of the existing tests to add another
partitioning level, but I ended up staying clear of that as I felt
like it caused a bit more churn than I wanted with an existing test.
The test I put together tests for the bug and also checks the WCO
works by not updating the row that's outside of the scope of the WCO
view and updating the row that is in the scope of the view.

I've now pushed your fix plus that test.

It feels a bit like famine to feast when it comes to tests for this bug today.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2022-12-23 12:54:15 RE: Exit walsender before confirming remote flush in logical replication
Previous Message John Naylor 2022-12-23 11:47:08 Re: [PoC] Improve dead tuple storage for lazy vacuum