Hi,

On Tue, Aug 18, 2026 at 5:52 AM Euler Taveira <[email protected]> wrote:
>
> I took another look at it.

Thanks for reviewing.

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

Yes, reworded it to just this: -- check that virtual generated columns
are not printed

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

Sounds good. I'm all in favor of having fewer in tests, so trimmed it
down to just inserting 1 row.

> +               /*
> +                * 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.

Agreed.

Please find the attached v6 patch.

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

Attachment: v6-0001-Omit-virtual-generated-columns-from-test_decoding.patch
Description: Binary data

Reply via email to