On Mon, Jun 23, 2025 at 9:13 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> >
> > Note: Support for composite types in virtual generated columns is
> > currently partial.
> > for example:
> >
> > CREATE TYPE double_int as (a int, b int);
> > --ok
> > CREATE TABLE gtest4 (
> > a int,
> > b double_int GENERATED ALWAYS AS ((a * 2, a * 3)) VIRTUAL
> > );
> > --not ok.
> > CREATE TABLE gtest4 (
> > a int,
> > b double_int GENERATED ALWAYS AS ((a * 2, a * 3)::double_int) VIRTUAL
> > );
>
> Your CheckAttributeType() change is conditional on TYPTYPE_BASE, but if
> you remove that and check it for all types, then you get the right error
> in both cases.
>
At that time, I was thinking that
CREATE TABLE gtest4 (a int, b double_int GENERATED ALWAYS AS ((a * 2,
a * 3)) VIRTUAL );
should be ok.
because in CheckAttributeType, we already recursively checked each
TYPTYPE_BASE type.
If each type within the composite type is built-in types, overall
should be just fine.
but then
CREATE TABLE gtest4 (a int, b double_int GENERATED ALWAYS AS ((a * 2,
a * 3)::double_int) VIRTUAL );
error out would make it look like inconsistency.
So overall I guess we have to disallow TYPTYPE_COMPOSITE too.
> I have attached a patch that is similar to yours but with that change.
> I've also written the test cases a bit differently, but it also covers
> everything now.
>
> (The two patches should be squashed. I'm just keeping them separate to
> show what is changed.)
seems we didn't check the ALTER TABLE case.
CREATE TYPE double_int as (a int, b int);
CREATE TABLE y (a int);
alter table y add column b double_int GENERATED ALWAYS AS ((a * 2, a *
3)) VIRTUAL;
in ATExecAddColumn, we can change it to:
CheckAttributeType(NameStr(attribute->attname),
attribute->atttypid, attribute->attcollation,
list_make1_oid(rel->rd_rel->reltype),
(attribute->attgenerated ==
ATTRIBUTE_GENERATED_VIRTUAL ? CHKATYPE_IS_VIRTUAL : 0));
user-defined function ALTER TABLE ADD COLUMN works as expected.
maybe add two error case tests for ALTER TABLE ADD COLUMN, one for
type, one for function.
Other than that, it looks good to me.
