Re: COPY WHERE clause generated/system column reference - Mailing list pgsql-hackers

On Wed, Nov 5, 2025 at 3:43 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 04.11.25 12:43, jian he wrote:
> >>> generated column allow tableoid system column reference, COPY WHERE clause also
> >>> allow tableoid column reference, should be fine.
> >>>
> >
> > for virtual generated column, adding
> > ``whereClause = expand_generated_columns_in_expr(whereClause, rel, 1);``
> >
> > should be able to solve the problem.
> >
> > For stored generated columns, we can either
> > A. document that the stored generated column is not yet computed, it
> > will be NULL
> > B. error out if the WHERE clause has a stored generated column.
> > C. add a temp slot and the computed stored generated column value
> > stored in the temp slot.
> >
> > attached v2-0003 using option C to address this problem.
>
> For backpatching, I suggest that we prohibit both stored and virtual
> generated column in the COPY WHERE clause.  They don't work anyway, so
> this doesn't change anything except get a better error message.

+1

>
> We can then consider adding support in future releases, similar to how
> we are expanding their use in other contexts in other patches.
>
> Attached is my proposed patch.  I kept it similar to the recently
> committed fix in commit ba99c9491c4.  Note that we also need to consider
> whole-row references, as that patch did.

Here are some minor comments for the proposed patch:

+                   ereport(ERROR,
+                           errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                           errmsg("generated columns are not
supported in COPY FROM WHERE conditions"),
+                           errdetail("Column \"%s\" is a generated column.",
+
get_attname(RelationGetRelid(rel), attno, false)));

How about using ERRCODE_INVALID_COLUMN_REFERENCE instead? It's more
consistent with other places where we check the column references.

---
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -161,7 +161,6 @@ COPY x from stdin WHERE a IN (generate_series(1,5));

 COPY x from stdin WHERE a = row_number() over(b);

-
 -- check results of copy in
 SELECT * FROM x;

Unnecessary line removal.

The rest looks good to me.

>
> >>> please check the attached file:
> >>> v1-0001 fix COPY WHERE with system column reference
> >>
> >> It seems to make sense to disallow users to specify system columns in
> >> the WHERE clause of COPY FROM. But why do we need to have an exception
> >> for tableoid? In the context of COPY FROM, specifying tableoid doesn't
> >> not make sense to me as tuples don't come from any relations. If we
> >> accept tableoid, I think it's better to explain why here.
> >>
> > In function CopyFrom, we have below comment, which indicates
> > At that time, tableoid was considered in the WHERE clause.
> >
> >          /*
> >           * Constraints and where clause might reference the tableoid column,
> >           * so (re-)initialize tts_tableOid before evaluating them.
> >           */
> >          myslot->tts_tableOid =
> > RelationGetRelid(target_resultRelInfo->ri_RelationDesc);
>
> I think this doesn't actually work correctly.  I started a separate
> thread about this:
>
> https://www.postgresql.org/message-id/flat/30c39ee8-bb11-4b8f-9697-45f7e018a8d3%40eisentraut.org
>
> Until that is solved, I think we don't need to do anything about system
> columns.  System columns other than tableoid are already rejected.  Once
> we know what, if anything, to do about tableoid, we can implement a more
> complete check.

Agreed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com