Re: pg18: Virtual generated columns are not (yet) safe when superuser selects from them

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Feike Steenbergen <feikesteenbergen(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg18: Virtual generated columns are not (yet) safe when superuser selects from them
Date: 2025-06-21 05:29:40
Message-ID: CACJufxEAppTVN1US8ypndz6C7Yhac6+e61LuXpz90-bmpHS20w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 19, 2025 at 5:11 AM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> Here is a new patch.
>
> My previous patch was a bit too simple. I had thought that
> check_functions_in_node() does the node walking itself, but that was
> wrong, so the patch only worked at the top-level of the expression. So
> I had to build some node-walking scaffolding around it to make it work.
> Also, check_functions_in_node() has some comments about what node type
> it doesn't check, so I had to add some code to handle those. This also
> requires that in addition to requiring built-in functions, we require
> built-in types. This shouldn't move the needle, since non-builtin types
> can't do much without non-builtin functions. Finally, it seems that
> most code actually uses FirstUnpinnedObjectId, not FirstNormalObjectId,
> to check for "built-in" status, so I changed to that, to be on the safe
> side.

+ /*
+ * check_functions_in_node() doesn't check some node types (see
+ * comment there). We handle CoerceToDomain and MinMaxExpr by
+ * checking for built-in types. The other listed node types cannot
+ * call user-definable SQL-visible functions.
+ *
+ * We furthermore need this type check to handle built-in, immutable
+ * polymorphic functions such as array_eq().
+ */
+ if (exprType(node) >= FirstUnpinnedObjectId)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("generation expression uses user-defined type"),
+ errdetail("Virtual generated columns that make use of user-defined
types are not yet supported."),
+ parser_errposition(pstate, exprLocation(node)));
this part doesn't have tests. we can have some tests based on in
src/test/regress/sql/create_type.sql
then I found an strange case:
( the following excerpted from create_type.sql)

BEGIN;
CREATE TYPE int42;
-- Make dummy I/O routines using the existing internal support for int4, text
CREATE FUNCTION int42_in(cstring)
RETURNS int42
AS 'int4in'
LANGUAGE internal STRICT IMMUTABLE;
CREATE FUNCTION int42_out(int42)
RETURNS cstring
AS 'int4out'
LANGUAGE internal STRICT IMMUTABLE;
CREATE TYPE int42 (
internallength = 4,
input = int42_in,
output = int42_out,
alignment = int4,
default = 42,
passedbyvalue
);
COMMIT;

CREATE TABLE gtest1 (a int42 GENERATED ALWAYS AS ('1') VIRTUAL);
CREATE TABLE gtest2 (a int42 GENERATED ALWAYS AS ('1'::int42) VIRTUAL);
ERROR: generation expression uses user-defined type
LINE 1: CREATE TABLE gtest2 (a int42 GENERATED ALWAYS AS ('1'::int42...
^
DETAIL: Virtual generated columns that make use of user-defined types
are not yet supported.

Do we need error out for the first case?

+ if (!IsA(node, List))
Is this "IF" branch necessary?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Devrim Gündüz 2025-06-21 11:13:54 Re: pgv18: simple table scan take more time than pgv14
Previous Message Michael Paquier 2025-06-21 03:54:15 Re: Custom reloptions in TableAM