Re: Virtual generated columns

On 16.12.24 15:34, jian he wrote:
> hi. some minor issues...
>
> <varlistentry id="sql-altertable-desc-set-expression">
> <term><literal>SET EXPRESSION AS</literal></term>
> <listitem>
> <para>
> This form replaces the expression of a generated column. Existing data
> in the column is rewritten and all the future changes will apply the new
> generation expression.
> </para>
> </listitem>
> </varlistentry>
> the second sentence seems not to apply to a virtual generated column?

Tweaked the wording in v11.

> doc/src/sgml/ref/alter_table.sgml
> seems does not explicitly mention the difference of
> ALTER TABLE tp ALTER COLUMN b SET EXPRESSION AS (a * 3);
> ALTER TABLE ONLY tp ALTER COLUMN b SET EXPRESSION AS (a * 3);
> ?
> the first one will recurse to the child tables and replace any
> generated expression in the child table
> for the to be altered column, the latter won't.

This is implied by the general meaning of the ONLY clause in ALTER
TABLE. This applies to all ALTER TABLE actions. Is there anything we
need to explain specifically for this action?

> CheckAttributeType, we can change it to
> <<<
> else if (att_typtype == TYPTYPE_DOMAIN)
> {
> if ((flags & CHKATYPE_IS_VIRTUAL) && DomainHasConstraints(atttypid))
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("virtual generated column \"%s\" cannot
> have a domain type", attname)));
> }
> <<<
> so we can support the domain without any constraints for now.
> (I don't have a huge opinion though).

I don't think this would be correct, since constraints could be added to
the domain later.

> ALTER COLUMN SET NOT NULL, if already not-null, then it will become a no-op.
> Similarly if old and new generated expressions are the same,
> ATExecSetExpression can return InvalidObjectAddress, making it a no-op.
>
> For example, in ATExecSetExpression, can we make the following ALTER
> TABLE a no-op?
> CREATE TABLE gtest20 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a
> * 3) VIRTUAL );
> ALTER TABLE gtest20 ALTER COLUMN b SET EXPRESSION AS (a * 3);
>
> since ATExecSetExpression is not recursive,
> Each input argument (AlteredTableInfo *tab) is separated for
> partitioned tables and partitions.
> so does AlteredTableInfo->newvals, AlteredTableInfo->rewrite information.
> so for no-op ATExecSetExpression return InvalidObjectAddress
> will also work for partitioned tables, inheritance.

I don't know why we would need to make this a no-op. I mean, we also
don't make UPDATE ... SET x = x a no-op.

> attached file trying to do that. While testing it,
> I found out there is no test case for ALTER COLUMN SET EXPRESSION
> for inheritance cases. even though it works.
> in src/test/regress/sql/generated_virtual.sql
> after line 161, we can add following tests:
>
> <<<
> ALTER TABLE ONLY gtest1 ALTER COLUMN b SET EXPRESSION AS (a * 10);
> select tableoid::regclass, * from gtest1;
> ALTER TABLE gtest1 ALTER COLUMN b SET EXPRESSION AS (a * 100);
> select tableoid::regclass, * from gtest1;
> <<<

There was already a test for this:

+-- alter only parent's and one child's generation expression
+ALTER TABLE ONLY gtest_parent ALTER COLUMN f3 SET EXPRESSION AS (f2 * 4);
+ALTER TABLE gtest_child ALTER COLUMN f3 SET EXPRESSION AS (f2 * 10);

Is there anything you think this doesn't cover?