Re: [Patch] Omit virtual generated columns from test_decoding output

From: SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>
To: Euler Taveira <euler(at)eulerto(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [Patch] Omit virtual generated columns from test_decoding output
Date: 2026-05-05 05:16:26
Message-ID: CAHg+QDdPuDskxeSvL1Y+pSkEHmycBv+tn5Rpp4ra8+NZj_se+w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Mon, May 4, 2026 at 8:09 PM Euler Taveira <euler(at)eulerto(dot)com> wrote:

> On Mon, May 4, 2026, at 10:11 PM, SATYANARAYANA NARLAPURAM wrote:
> >
> > Virtual generated columns are not stored on disk, so heap_getattr() in
> > tuple_to_stringinfo() always returned NULL for them, producing
> > misleading output such as
> >
> > table public.t: INSERT: a[integer]:1 b[integer]:10 c[integer]:null
> >
> > even though the user could observe a non-null value via SELECT. Stored
> > generated columns continue to be emitted as before because their values
> > do live in the heap tuple.
> >
>
> I wouldn't say misleading but expected. Logical decoding relies on WAL and
> virtual generated columns are not stored in the WAL.
>
> > This matches the pgoutput's logicalrep_should_publish_column()
> > which never publishes virtual generated columns. Added a regression test.
> > Please find the patch attached.
> >
>
> There is no guarantee that test_decoding should match the pgoutput.

Agreed, not trying to keep them in sync but giving as a reference.

> I agree that
> test_decoding shouldn't output virtual generated columns. The problem is
> that it
> already does it. I'm afraid that removing it should break existing
> applications.
> (I heard that some solutions rely on test_decoding for CDC.) Should we
> change it
> as you proposed or add an option to put it back to keep the old behavior?
>

It is emitting null, I am not sure if it is meaningful for the consumers to
consume this or
have taken dependency on this. Adding an extra option isn't an overkill for
this? I am open
to this idea if others feel the same.

> I didn't review your patch but I noticed that there is a new test file for
> this
> change. There are some concerns about the total test execution time. Do you
> really need to include this test? If so, should you combine it with an
> existing
> test file?

Fair concern, I moved the tests to ddl.sql. Please find the attached v2
patch.

Thanks,
Satya

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2026-05-05 05:25:44 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Amit Kapila 2026-05-05 04:57:47 Re: Include schema-qualified names in publication error messages.