Re: COPY WHERE clause generated/system column reference

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: COPY WHERE clause generated/system column reference
Date: 2025-11-05 18:19:09
Message-ID: CAD21AoA2OLz0OkGpDNDqhbOM2VnZCYris=tGw-vg8LYDYmdW_g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 5, 2025 at 3:43 AM Peter Eisentraut <peter(at)eisentraut(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matheus Alcantara 2025-11-05 19:02:52 Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
Previous Message Arseniy Mukhin 2025-11-05 18:17:19 Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue