Feedback-ID: ie0a040ee:Fastmail
Message-ID: <91aa5405-d9b3-4832-baea-0b5169609f75@eisentraut.org>
Date: Fri, 27 Mar 2026 16:08:20 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Align tests for stored and virtual generated columns
To: Paul A Jungwirth <pj@illuminatedcomputing.com>,
 Yugo Nagata <nagata@sraoss.co.jp>, Surya Poondla <s_poondla@apple.com>,
 Mutaamba Maasha <maasha@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <20250808115142.e9ccb81f35466a9a131a4c55@sraoss.co.jp>
 <CA+renyWSegxAW4f+OBdhEeVLL6kLNiN_zxy2ddE=YGhXkyM+=g@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CA+renyWSegxAW4f+OBdhEeVLL6kLNiN_zxy2ddE=YGhXkyM+=g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/91aa5405-d9b3-4832-baea-0b5169609f75%40eisentraut.org>
Precedence: bulk

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