Hi, On Mon, Aug 17, 2026 at 2:54 PM Masahiko Sawada <[email protected]> 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
v5-0001-Omit-virtual-generated-columns-from-test_decoding.patch
Description: Binary data
