On 28.06.24 02:00, jian he wrote:
> the test structure you made ( generated_stored.sql,
> generated_virtual.sq) looks ok to me.
> but do we need to reset the search_path at the end of
> generated_stored.sql, generated_virtual.sql?
No, the session ends at the end of the test file, so we don't need to
reset session state.
> + /*
> + * TODO: This could be done, but it would need a different implementation:
> + * no rewriting, but still need to recheck any constraints.
> + */
> + if (attTup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("ALTER TABLE / SET EXPRESSION is not supported for virtual
> generated columns"),
> + errdetail("Column \"%s\" of relation \"%s\" is a virtual generated column.",
> + colName, RelationGetRelationName(rel))));
>
> minor typo, should be
> + errmsg("ALTER TABLE SET EXPRESSION is not supported for virtual
> generated columns"),
This style "ALTER TABLE / something else" is also used for other error
messages related to ALTER TABLE subcommands, so I am using the same here.
> insert/update/delete/merge returning have problems:
> CREATE TABLE t2 (
> a int ,
> b int GENERATED ALWAYS AS (a * 2),
> d int default 22);
> insert into t2(a) select g from generate_series(1,10) g;
>
> insert into t2 select 100 returning *, (select t2.b), t2.b = t2.a * 2;
> update t2 set a = 12 returning *, (select t2.b), t2.b = t2.a * 2;
> update t2 set a = 12 returning *, (select (select t2.b)), t2.b = t2.a * 2;
> delete from t2 where t2.b = t2.a * 2 returning *, 1,((select t2.b));
>
> currently all these query, error message is "unexpected virtual
> generated column reference"
> we expect above these query work?
Yes, this is a bug. I'm looking into it.
> issue with merge:
> CREATE TABLE t0 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
> insert into t0(a) select g from generate_series(1,10) g;
> MERGE INTO t0 t USING t0 AS s ON 2 * t.a = s.b WHEN MATCHED THEN
> DELETE returning *;
>
> the above query returns zero rows, but for stored generated columns it
> will return 10 rows.
>
> in transformMergeStmt(ParseState *pstate, MergeStmt *stmt)
> add
> `qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;`
> before
> `assign_query_collations(pstate, qry);`
> solve the problem.
Good catch. Will fix.
Thanks for this review. I will work on fixing the issues above and come
back with a new patch set.