Hi, On Thu, May 7, 2026 at 9:31 PM Fujii Masao <[email protected]> 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
From a101142d945186af245ac6c67ddd3d7eebb8758d Mon Sep 17 00:00:00 2001 From: Satyanarayana Narlapuram <[email protected]> Date: Tue, 5 May 2026 00:51:03 +0000 Subject: [PATCH v4] Omit virtual generated columns from test_decoding output. Virtual generated columns are not stored on disk, so heap_getattr() in tuple_to_stringinfo() always returns NULL for them. test_decoding therefore emitted a NULL value for such a column even though the user can observe a computed, possibly non-null value via SELECT. This makes the output ambiguous: a virtual generated column is shown as NULL and cannot be told apart from a column that genuinely holds NULL, even though the two mean very different things. The logical replication pgoutput plugin already skips virtual generated columns in logicalrep_should_publish_column(). Fix this by skipping virtual generated columns in tuple_to_stringinfo(), the single place that renders every tuple, so INSERT, UPDATE and DELETE output are all covered. Stored generated columns continue to be emitted as before because their values do live in the heap tuple. Back-patch to 18, where virtual generated columns were introduced in commit 83ea6c54025. Author: Satyanarayana Narlapuram <[email protected]> Reviewed-by: Bharath Rupireddy <[email protected]> Reviewed-by: Euler Taveira <[email protected]> Reviewed-by: Fujii Masao <[email protected]> Discussion: https://postgr.es/m/CAHg%2BQDfTh3UbB-Ed--o2Bd%3DSBDJoEiG-qp3C0%2BETDibF63y%3Ddw%40mail.gmail.com --- contrib/test_decoding/expected/ddl.out | 45 ++++++++++++++++++++++++++ contrib/test_decoding/sql/ddl.sql | 25 ++++++++++++++ contrib/test_decoding/test_decoding.c | 11 +++++++ 3 files changed, 81 insertions(+) diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out index 6819812e806..ad0db58e18c 100644 --- a/contrib/test_decoding/expected/ddl.out +++ b/contrib/test_decoding/expected/ddl.out @@ -895,6 +895,51 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc data (0 rows) \pset format aligned +-- 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. +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 +); +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'); + data +---------------------------------------------------------------------- + BEGIN + table public.gtest1: INSERT: a[integer]:1 b[integer]:10 d[integer]:2 + table public.gtest1: INSERT: a[integer]:2 b[integer]:20 d[integer]:4 + COMMIT + BEGIN + table public.gtest1: UPDATE: a[integer]:1 b[integer]:99 d[integer]:2 + COMMIT + BEGIN + table public.gtest1: DELETE: a[integer]:2 + COMMIT +(10 rows) + +-- 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'); + data +-------------------------------------------- + BEGIN + table public.gtest2: INSERT: a[integer]:10 + table public.gtest2: INSERT: a[integer]:20 + COMMIT +(4 rows) + +DROP TABLE gtest1; +DROP TABLE gtest2; SELECT pg_drop_replication_slot('regression_slot'); pg_drop_replication_slot -------------------------- diff --git a/contrib/test_decoding/sql/ddl.sql b/contrib/test_decoding/sql/ddl.sql index 6d0b7d77778..2ee84ffe693 100644 --- a/contrib/test_decoding/sql/ddl.sql +++ b/contrib/test_decoding/sql/ddl.sql @@ -467,6 +467,31 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); \pset format aligned +-- 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. +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 +); +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'); + +-- 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'); +DROP TABLE gtest1; +DROP TABLE gtest2; + SELECT pg_drop_replication_slot('regression_slot'); /* check that the slot is gone */ diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index d5cf0fa02b0..93b15bd1270 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -554,6 +554,17 @@ tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple, bool skip_ if (attr->attnum < 0) continue; + /* + * Don't print virtual generated columns. Their values are not stored + * in the heap tuple, so heap_getattr() would always return a wrong + * NULL. This matches pgoutput's policy of never publishing virtual + * generated columns (see logicalrep_should_publish_column()). Stored + * generated columns are emitted as usual since their values are + * actually on disk. + */ + if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL) + continue; + typid = attr->atttypid; /* get Datum from tuple */ -- 2.47.3
