Thread
Commits
-
Align tests for stored and virtual generated columns
- 9a9998163bda 19 (unreleased) landed
-
Align tests for stored and virtual generated columns
Yugo Nagata <nagata@sraoss.co.jp> — 2025-08-08T02:51:42Z
Hi, 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. In addition, I moved the test confirming that user-defined types are not allowed in virtual generated columns to the generated_virtual.sql-specific section, since this test is not necessary for stored generated columns. Regards, Yugo Nagata -- Yugo Nagata <nagata@sraoss.co.jp>
-
Re: Align tests for stored and virtual generated columns
Paul A Jungwirth <pj@illuminatedcomputing.com> — 2025-09-30T18:01:55Z
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 -
Re: Align tests for stored and virtual generated columns
Peter Eisentraut <peter@eisentraut.org> — 2026-03-27T15:00:42Z
On 08.08.25 04:51, Yugo Nagata 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. > > In addition, I moved the test confirming that user-defined types are not allowed > in virtual generated columns to the generated_virtual.sql-specific section, > since this test is not necessary for stored generated columns. Committed, thanks.
-
Re: Align tests for stored and virtual generated columns
Peter Eisentraut <peter@eisentraut.org> — 2026-03-27T15:08:20Z
On 30.09.25 20:01, Paul A Jungwirth wrote: > 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.) The example you show produces the error ERROR: cannot alter type of a column used by a generated column which suggests that it is internally the same thing, so it seems another test wouldn't add any new coverage. > 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. I thought about something like that initially, too, but then decided against it because it would make each test individually harder to understand and manage. > 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? My recollection is that some of these tests are commented out because running them would produce some behavior that would affect subsequent tests (for example, adding or removing rows that they shouldn't), and so you would have to do some extra work to undo some of that to make this all work smoothly. (Also, in some cases it would be a waste of time to run large blocks of tests for unsupported features, so all but the first few test statements are commented out.) > Similarly we noticed that the test for expansion of virtual generated > columns is not applied to stored columns. Is there a reason why not? I guess this tests something that doesn't really apply to stored columns. But maybe this could be double checked. > 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. Yeah, this whole test numbering turned into a disaster pretty early on. At this point, I consider these test files to be a monument against doing that again. In later feature work, I started to name test tables by random numbers. ;-)