On Fri, Oct 10, 2025 at 1:20 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Michael,
>
> > On Thu, Oct 09, 2025 at 03:37:25PM +0300, Грем Снорт wrote:
> > > I've found a simple problem in one of subscription tests
> > > (`src/test/subscription/t/009_matviews.pl`).
> >
> > (Added a couple of folks in CC.)
>
> Thanks for adding.
>
> > The backend change coupled with this test comes from bc1adc651b8e,
> > first introduced in v11. At the top of REL_11_STABLE, which is the
> > first branch where the test has been introduced, if I update
> > pgoutput.c and remove the is_publishable_relation() call in
> > pgoutput_change() to undo the fix, then the test is able to hang as it
> > is designed.
> >
> > Now, if I do the same thing on HEAD, removing the check, then the test
> > passes! Something else is going on here: the test is not checking
> > what it has been written for. Applying your patch does not change
> > this state.
>
> I found that b4da732 [1] changes the behavior. It modifies how we create the
> materialized view.
>
> Since b4da732, CREATE MATERIALIZED VIEW ... AS ... behaves similarly with REFRESH
> MATERIALIZED VIEW command.
> Refresh command initially creates a transient table, insert tuples then swapping
> the relfilenumber between the mateview and transient one.
> Thus, from the WAL's perspective, INSERT happens to the temporary one, so it won't
> be replicated to the subscriber. See comments atop RefreshMatViewByOid().
>
> So...I think it is not caused by changes for logical replication.
Thanks for the investigation. Makes sense.
I agree with Michael that the test could be written better, rather
than testing for timeout. AFAIU, the test is testing that the changes
to materialized view are not sent downstream; are properly filtered
out. I think the test should create MV and make sure that it doesn't
get populated through the logical replication but when refreshed
explicitly on the subscriber. While checking for that we should
perform usual wait for LSN. However, after your explanation above, it
seems that MVs should be considered similar to unlogged tables or
temporary tables after that change. So we could just merge this test
into subscription/100_bugs.pl.
--
Best Wishes,
Ashutosh Bapat
