[Patch] Omit virtual generated columns from test_decoding output

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. 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?

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?

--
Euler Taveira
EDB https://www.enterprisedb.com/

Hi,

On Mon, May 4, 2026 at 8:09 PM Euler Taveira <euler@eulerto.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

On Tue, May 5, 2026 at 2:16 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

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

Seems you forgot to attached the patch.

Regards,

--
Fujii Masao

Hi,

On Thu, May 7, 2026 at 9:31 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, May 5, 2026 at 2:16 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

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

patch.

Seems you forgot to attached the patch.

Attached now, thanks for letting ne know!

Hi

On Sat, May 9, 2026 at 1:58 PM SATYANARAYANA NARLAPURAM <
satyanarlapuram@gmail.com> wrote:

Hi,

On Thu, May 7, 2026 at 9:31 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, May 5, 2026 at 2:16 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

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

v2 patch.

Seems you forgot to attached the patch.

Attached now, thanks for letting ne know!

Please find the v3 patch.

Thanks,
Satya

Hi,

On Thu, May 7, 2026 at 9:31 PM Fujii Masao <masao.fujii@gmail.com> wrote:

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

Seems you forgot to attached the patch.

I agree we must fix this for test_decoding. Here's my take.

When a column holding a genuine NULL value gets decoded, it shows up
as null in the output. But a virtual generated column also shows up as
null, so the two are hard to tell apart in the test_decoding output
today.

table public.t: INSERT: a[integer]:1 b[integer]:null c[integer]:null
d[integer]:100

Here b is a genuine NULL and c is a virtual generated column whose
value is 10 (for example), but both show up as null.

That gives me two reasons to +1 this patch:

1/ It's hard to distinguish in the test_decoding output whether a
column is a genuine NULL or a virtual generated column. For example:
2/ The pgoutput already skips virtual generated columns in
logicalrep_should_publish_column().

I quickly reviewed the v3 patch and it looks good to me. However,
pgindent was not happy, so I ran it, tweaked the comments and commit
message a bit, ran the tests, and attached a v4 patch. Please have a
look.

I prefer to back-patch this through PG18, where virtual generated
columns were introduced (commit 83ea6c54025).

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 30, 2026 at 9:32 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Thu, May 7, 2026 at 9:31 PM Fujii Masao <masao.fujii@gmail.com> wrote:

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

Seems you forgot to attached the patch.

I agree we must fix this for test_decoding. Here's my take.

When a column holding a genuine NULL value gets decoded, it shows up
as null in the output. But a virtual generated column also shows up as
null, so the two are hard to tell apart in the test_decoding output
today.

table public.t: INSERT: a[integer]:1 b[integer]:null c[integer]:null
d[integer]:100

Here b is a genuine NULL and c is a virtual generated column whose
value is 10 (for example), but both show up as null.

That gives me two reasons to +1 this patch:

1/ It's hard to distinguish in the test_decoding output whether a
column is a genuine NULL or a virtual generated column. For example:
2/ The pgoutput already skips virtual generated columns in
logicalrep_should_publish_column().

I quickly reviewed the v3 patch and it looks good to me. However,
pgindent was not happy, so I ran it, tweaked the comments and commit
message a bit, ran the tests, and attached a v4 patch. Please have a
look.

I prefer to back-patch this through PG18, where virtual generated
columns were introduced (commit 83ea6c54025).

I agree with the proposed change. It's quite confusing to distinguish
between a genuine NULL value and a NULL value in a virtual generated
column, as Bharath mentioned.

Here are some review comments:

+-- Check that virtual generated columns are omitted from the output (their
+-- values are not stored on disk so heap_getattr() would otherwise emit a
+-- wrong NULL), while stored generated columns are emitted normally.

How about rewriting it to:

+-- Virtual generated columns are always stored as null in the tuple, so they
+-- are not printed at all; a printed null would not be distinguishable from a
+-- column that really contains a null.  Stored generated columns are printed
+-- as usual.

A similar change would be required to the comments in test_decoding.c.

---
+-- table with only virtual generated columns alongside the key
+CREATE TABLE gtest2 (
+    a int PRIMARY KEY,
+    b int GENERATED ALWAYS AS (a + 1) VIRTUAL,
+    c text GENERATED ALWAYS AS ('row-' || a::text) VIRTUAL
+);
+INSERT INTO gtest2 (a) VALUES (10), (20);
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
NULL, 'include-xids', '0', 'skip-empty-xacts', '1');

I don't think this test is needed as it doesn't improve test coverages.

---
As for backpatching, IIUC it's not a correctness bug nor causes a
server crash or data corruption. Given it might affect the existing
consumers using test_decoding, I think it should be only for HEAD.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Hi,

On Mon, Aug 17, 2026 at 2:54 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I agree with the proposed change. It's quite confusing to distinguish
between a genuine NULL value and a NULL value in a virtual generated
column, as Bharath mentioned.

Here are some review comments:

Thanks for reviewing.

+-- Check that virtual generated columns are omitted from the output (their
+-- values are not stored on disk so heap_getattr() would otherwise emit a
+-- wrong NULL), while stored generated columns are emitted normally.

How about rewriting it to:

+-- Virtual generated columns are always stored as null in the tuple, so they
+-- are not printed at all; a printed null would not be distinguishable from a
+-- column that really contains a null.  Stored generated columns are printed
+-- as usual.

Looks better. Used that.

A similar change would be required to the comments in test_decoding.c.

Changed.

---
+-- table with only virtual generated columns alongside the key
+CREATE TABLE gtest2 (
+    a int PRIMARY KEY,
+    b int GENERATED ALWAYS AS (a + 1) VIRTUAL,
+    c text GENERATED ALWAYS AS ('row-' || a::text) VIRTUAL
+);
+INSERT INTO gtest2 (a) VALUES (10), (20);
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
NULL, 'include-xids', '0', 'skip-empty-xacts', '1');

I don't think this test is needed as it doesn't improve test coverages.

Makes sense. I added a null column to the one test that we are left with.

---
As for backpatching, IIUC it's not a correctness bug nor causes a
server crash or data corruption. Given it might affect the existing
consumers using test_decoding, I think it should be only for HEAD.

Agreed.

Please have a look at the v5 patch.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Mon, Aug 17, 2026, at 9:00 PM, Bharath Rupireddy wrote:

Please have a look at the v5 patch.

I took another look at it.

+-- Virtual generated columns are always stored as null in the tuple, so they
+-- are not printed at all; a printed null would not be distinguishable from a
+-- column that really contains a null. Stored generated columns are printed
+-- as usual.
+CREATE TABLE gtest1 (
+    a int PRIMARY KEY,
+    b int,
+    c int GENERATED ALWAYS AS (a + b) VIRTUAL,
+    d int GENERATED ALWAYS AS (a * 2) STORED,
+    e int
+);
+INSERT INTO gtest1 (a, b) VALUES (1, 10), (2, 20);
+UPDATE gtest1 SET b = 99 WHERE a = 1;
+DELETE FROM gtest1 WHERE a = 2;
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+DROP TABLE gtest1;

It seems repetitive to say the same explanation in the commit message
and at the top of this test. It is sufficient to follow the same pattern
from the tests in this file.

-- check generated columns

Do you really need to test the 3 commands (I, U, D) here? I'm asking
because all of them use the same function (tuple_to_stringinfo) behind
the scenes. If so, I suggest that you use a single transaction instead
of 3 separate transactions.

+		/*
+		 * Virtual generated columns are always stored as null in the tuple,
+		 * so don't print them at all; a printed null would not be
+		 * distinguishable from a column that really contains a null. pgoutput
+		 * likewise never publishes virtual generated columns (see
+		 * logicalrep_should_publish_column()). Stored generated columns are
+		 * printed as usual since their values are actually on disk.
+		 */

No need to mention the pgoutput here. It is sufficient (for historical
reason) that the commit message says it.

--
Euler Taveira
EDB https://www.enterprisedb.com/