Re: Align tests for stored and virtual generated columns

On Thu, Aug 7, 2025 at 7:52 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:
> I noticed that the tests for virtual and stored generated columns
> contain the following comment;
>
>  -- keep these tests aligned with generated_stored.sql (or generated_virtual.sql)
>
> However, it seems that some additional tests for virtual generated columns
> disrupted that alignment, as they were not added to generated_stored.sql.
>
> I've attached a patch to restore the alignment.

Hello,

Thanks for this patch! Mutaama Maasha, Surya Poondla, and I reviewed
it together. Here are our thoughts:

We agree we should try to keep these tests in sync, so if they are
diverging we should bring them back into line.

The patch still applies. Tests pass.

Going through the tests made me curious about trying to break virtual
columns. I couldn't come up with anything, although one scenario that
doesn't seem be tested is changing the collation of a column used by a
generated column. For instance:

```
-- English vs Turkish upper/lower i:
create table t2 ( x text COLLATE "en-x-icu", y text COLLATE "tr-x-icu" );
insert into t2 values ('i', 'i'), ('I', 'I');
select upper(x), ascii(upper(x)), lower(x), ascii(lower(x)), upper(y),
ascii(upper(y)), lower(y), ascii(lower(y)) from t2;

create table t3 (
  x text collate "en-x-icu",
  lx text collate "en-x-icu" generated always as (lower(x)),
  ux text collate "en-x-icu" generated always as (upper(x)),
  y text collate "tr-x-icu",
  ly text collate "tr-x-icu" generated always as (lower(y)),
  uy text collate "tr-x-icu" generated always as (upper(y))
);
insert into t3 (x, y) values ('i', 'i'), ('I', 'I');
alter table t3 add constraint x check (ascii(lx) < 128 and ascii(ux) < 128);
alter table t3 alter column x type text collate "tr-x-icu";
ERROR:  cannot alter type of a column used by a generated column
DETAIL:  Column "x" is used by generated column "lx".
```

Perhaps we could add a test like that? (We do have a test for changing
the *type* of a column used by a generated column though.)

Is there a way we can make it easier to compare the two test scripts
for differences? Could we write a meta-test that compares them for
differences (in the spirit of `opr_sanity.sql`)? I experimented with
using psql variables to limit `STORED` vs `VIRTUAL` to only the top of
each SQL file. Then I could easily diff the two files and see how
diverged they were. Attached is a patch to do this and the results of
my diff (after applying the author's patch). It seems like there are
still a few trivial discrepancies that we could clean up.

To call out one less-trivial discrepancy:

```
--- sql/generated_stored.sql    2025-09-21 19:52:14.554930323 -0700
+++ sql/generated_virtual.sql   2025-09-21 19:52:21.447016340 -0700
...
-INSERT INTO gtest12 VALUES (3, 30), (4, 40);  -- currently not
allowed because of function permissions, should
 arguably be allowed
-SELECT a, c FROM gtest12;  -- allowed (does not actually invoke the function)
+--INSERT INTO gtest12 VALUES (3, 30), (4, 40);  -- allowed (does not
actually invoke the function)
+--SELECT a, c FROM gtest12;  -- currently not allowed because of
function permissions, should arguably be allowed
```

Why are the VIRTUAL tests commented out? The explanatory comments
suggest they should have opposite results from the STORED tests, which
makes sense, but shouldn't we be running them?

Similarly we noticed that the test for expansion of virtual generated
columns is not applied to stored columns. Is there a reason why not?

We found a couple places where this patch adds new test tables whose
numbering is out of sequence compared to the rest of the file. For
instance:

> @@ -806,6 +803,9 @@ SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT
>  -- these tests are specific to generated_virtual.sql
>  --
>
> +-- using user-defined type not yet supported
> +CREATE TABLE gtest24xxx (a gtestdomain1, b gtestdomain1, c int GENERATED ALWAYS AS (greatest(a, b)) VIRTUAL);  --
error
> +
>  create table gtest32 (
>    a int primary key,
>    b int generated always as (a * 2),
> --
> 2.43.0

Why add gtest24xxx in between gtest28b and gtest32? Maybe it should be
gtest30 or 31?

Also here:

> +INSERT INTO gtest21b (a) VALUES (2), (0);  -- violates constraint
> +INSERT INTO gtest21b (a) VALUES (NULL);  -- error
>  ALTER TABLE gtest21b ALTER COLUMN b DROP NOT NULL;
>  INSERT INTO gtest21b (a) VALUES (0);  -- ok now
>
> +-- not-null constraint with partitioned table
> +CREATE TABLE gtestnn_parent (
> +    f1 int,
> +    f2 bigint,
> +    f3 bigint GENERATED ALWAYS AS (nullif(f1, 1) + nullif(f2, 10)) STORED NOT NULL
> +) PARTITION BY RANGE (f1);
> +CREATE TABLE gtestnn_child PARTITION OF gtestnn_parent FOR VALUES FROM (1) TO (5);
> +CREATE TABLE gtestnn_childdef PARTITION OF gtestnn_parent default;

Should gtestnn_parent have a number? It is between gtest21b and
gtest22a. Perhaps gtest21nn_parent? This is a tougher choice since 21
and 22 are taken.

Yours,

--
Paul              ~{:-)
pj@illuminatedcomputing.com