Re: [PATCH TEST] Fix logical replication setup in subscription test `t/009_matviews.pl`

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Грем Снорт <grem(dot)snoort(at)gmail(dot)com>
Subject: Re: [PATCH TEST] Fix logical replication setup in subscription test `t/009_matviews.pl`
Date: 2025-10-14 05:04:24
Message-ID: aO3Z2Krz6KQX1VyI@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 13, 2025 at 06:12:07PM +0530, Ashutosh Bapat wrote:
> For now this makes sense.

The arguments and the patches I am seeing do not really make sense
here.

> We could avoid running a full test, and save time and resources, if we
> piggy back MV vs logical replication testing on 100_bugs.pl. If we do
> that, it should be master-only and can be a separate discussion.

Moving the test would be one thing, I'd be OK to do that only on HEAD
and not bother about the back-branches. Still, that's a separate
discussion.

As far as I can see, this test is not able to check what it wants to
check since v17. Hence, doing something only on HEAD, if that's
really what you are implying here (I understand that you are not
implying that but I am not entirely sure, either), is not sufficient.

I also fail to understand how Kuroda-san's patch helps in solving
everything at hand: more is required. If apply the patch posted at
[1] on HEAD, the test passes. If I revert the change introduced by
bc1adc651b8e in pgoutput.c with the patch posted at [1], the test
passes as well, but I would expect that the test *fails* in some way.

I did not take the time to dive into the details here, but should
there be at least some pattern detected in the TAP test? Or do we
actually need to patch pgoutput.c at all now and remove the check
based on is_publishable_relation(). Or is there any meaning in
spending cycles for such a test now if we cannot detect a difference
in behavior?

In any case, none of the proposals posted on this thread seem
sufficient to me: 009_matviews.pl is not useful if it does not choke
in some way if we reintroduce a problem equivalent to what
bc1adc651b8e has done. If it is not useful anymore and if pgoutput.c
can be simplified, there may be a point in considering these options.

Good find about b4da732fd64e, by the way, Kuroda-san. That's the
commit that has changed the way 009_matviews.pl behaves. If I revert
b4da732fd64e, 009_matviews.pl would fail on timeout, as expected based
on the way the test is currently written.

[1]: https://www.postgresql.org/message-id/OSCPR01MB149660172D9F21AA75365DC9BF5EDA@OSCPR01MB14966.jpnprd01.prod.outlook.com
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2025-10-14 05:21:11 Re: Logical Replication of sequences
Previous Message Hayato Kuroda (Fujitsu) 2025-10-14 04:36:01 RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE