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
