public inbox for [email protected]  

help / color / mirror / Atom feed

From: SATYANARAYANA NARLAPURAM <[email protected]>
To: Euler Taveira <[email protected]>
Cc: [email protected]
Subject: Re: [Patch] Omit virtual generated columns from test_decoding output
Date: Mon, 4 May 2026 22:16:26 -0700
Message-ID: <CAHg+QDdPuDskxeSvL1Y+pSkEHmycBv+tn5Rpp4ra8+NZj_se+w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAHg+QDfTh3UbB-Ed--o2Bd=SBDJoEiG-qp3C0+ETDibF63y=dw@mail.gmail.com>
	<[email protected]>

Hi,

On Mon, May 4, 2026 at 8:09 PM Euler Taveira <[email protected]> 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

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [Patch] Omit virtual generated columns from test_decoding output
  In-Reply-To: <CAHg+QDdPuDskxeSvL1Y+pSkEHmycBv+tn5Rpp4ra8+NZj_se+w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox