Re: Ignore heap rewrites for materialized views in logical replication

On Sat, May 28, 2022 at 2:44 AM Euler Taveira <euler@eulerto.com> wrote:
>
> While investigating an internal report, I concluded that it is a bug. The
> reproducible test case is simple (check 0002) and it consists of a FOR ALL
> TABLES publication and a non-empty materialized view on publisher. After the
> setup, if you refresh the MV, you got the following message on the subscriber:
>
> ERROR:  logical replication target relation "public.pg_temp_NNNNN" does not exist
>
> That's because the commit 1a499c2520 (that fixes the heap rewrite for tables)
> forgot to consider that materialized views can also create transient heaps and
> they should also be skipped. The affected version is only 10 because 11
> contains a different solution (commit 325f2ec555) that provides a proper fix
> for the heap rewrite handling in logical decoding.
>
> 0001 is a patch to skip MV too.
>

I agree with your analysis and the fix looks correct to me.

> I attached 0002 to demonstrate the issue but it
> doesn't seem appropriate to be included. The test was written to detect the
> error and bail out. After this fix, it takes a considerable amount of time to
> finish the test because it waits for a message that never arrives.
>

Instead of waiting for an error, we can try to insert into a new table
created by the test case after the 'Refresh ..' command and wait for
the change to be replicated by using wait_for_caught_up.

> Since nobody
> reports this bug in 5 years and considering that version 10 will be EOL in 6
> months, I don't think an additional test is crucial here.
>

Let's try to see if we can simplify the test so that it can be
committed along with a fix. If we are not able to find any reasonable
way then we can think of skipping it.

-- 
With Regards,
Amit Kapila.