Horton

Thread

Commits

  1. Disallow generated columns in COPY WHERE clause

  2. Tighten check for generated column in partition key expression

  1. COPY WHERE clause generated/system column reference

    jian he <jian.universality@gmail.com> — 2025-10-27T08:20:37Z

    hi.

CREATE TABLE gtest0 (a int, b int GENERATED ALWAYS AS (a + 1) VIRTUAL);
copy gtest0 from stdin where (b <> 1);
0
\.

ERROR:  unexpected virtual generated column reference
CONTEXT:  COPY gtest0, line 1: "0"

We need to apply expand_generated_columns_in_expr to the whereClause in DoCopy.
However, handling STORED generated columns appears to be less straightforward.

currently:
ExecQual(cstate->qualexpr, econtext))
happen before
ExecComputeStoredGenerated.

when calling ExecQual, the stored generated column values have not been
populated yet. so we may need ExecComputeStoredGenerated beforehand.
Since ExecComputeStoredGenerated is likely expensive, I added logic to detect
whether the WHERE clause actually have stored generated columns reference before
calling it.

generated column allow tableoid system column reference, COPY WHERE clause also
allow tableoid column reference, should be fine.

please check the attached file:
v1-0001 fix COPY WHERE with system column reference
v1-0002 fix COPY WHERE with generated column reference

  2. Re: COPY WHERE clause generated/system column reference

    Kirill Reshke <reshkekirill@gmail.com> — 2025-10-27T18:02:13Z

    On Mon, 27 Oct 2025 at 13:21, jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> CREATE TABLE gtest0 (a int, b int GENERATED ALWAYS AS (a + 1) VIRTUAL);
> copy gtest0 from stdin where (b <> 1);
> 0
> \.
>
> ERROR:  unexpected virtual generated column reference
> CONTEXT:  COPY gtest0, line 1: "0"
>
> We need to apply expand_generated_columns_in_expr to the whereClause in DoCopy.
> However, handling STORED generated columns appears to be less straightforward.
>
> currently:
> ExecQual(cstate->qualexpr, econtext))
> happen before
> ExecComputeStoredGenerated.
>
> when calling ExecQual, the stored generated column values have not been
> populated yet. so we may need ExecComputeStoredGenerated beforehand.
> Since ExecComputeStoredGenerated is likely expensive, I added logic to detect
> whether the WHERE clause actually have stored generated columns reference before
> calling it.
>
> generated column allow tableoid system column reference, COPY WHERE clause also
> allow tableoid column reference, should be fine.
>
> please check the attached file:
> v1-0001 fix COPY WHERE with system column reference
> v1-0002 fix COPY WHERE with generated column reference

Hi! Indeed, copying from with generated column in where clause is
broken on HEAD.

I applied your patches, they indeed fix the issue.

Small comment: in 0002:

> + if (has_stored_generated)
> + ExecComputeStoredGenerated(resultRelInfo, estate, myslot,
> +   CMD_INSERT);

Should we use CMD_UTILITY here? Comment in nodes.h suggests so. Also,
ExecComputeStoredGenerated only check for equality with CMD_UPDATE, so
this is just a cosmetic change.



-- 
Best regards,
Kirill Reshke

  3. Re: COPY WHERE clause generated/system column reference

    jian he <jian.universality@gmail.com> — 2025-10-28T05:28:55Z

    On Tue, Oct 28, 2025 at 2:02 AM Kirill Reshke <reshkekirill@gmail.com> wrote:
>
> Small comment: in 0002:
>
> > + if (has_stored_generated)
> > + ExecComputeStoredGenerated(resultRelInfo, estate, myslot,
> > +   CMD_INSERT);
>
> Should we use CMD_UTILITY here? Comment in nodes.h suggests so. Also,
> ExecComputeStoredGenerated only check for equality with CMD_UPDATE, so
> this is just a cosmetic change.
>
hi.

use CMD_UTILITY will also work as expected.
ExecComputeStoredGenerated expects the command type (cmdtype) to be either
UPDATE or INSERT.

in ExecComputeStoredGenerated, we have:
    if (cmdtype == CMD_UPDATE)
    else
    {
        if (resultRelInfo->ri_GeneratedExprsI == NULL)
            ExecInitGenerated(resultRelInfo, estate, cmdtype);
        /* Early exit is impossible given the prior Assert */
        Assert(resultRelInfo->ri_NumGeneratedNeededI > 0);
        ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsI;
    }

in struct ResultRelInfo also has comments like:
    /*
     * Arrays of stored generated columns ExprStates for INSERT/UPDATE/MERGE.
     */
    ExprState **ri_GeneratedExprsI;
    ExprState **ri_GeneratedExprsU;

I think using CMD_INSERT should be fine. Also, note that below
ExecComputeStoredGenerated uses CMD_INSERT too.

  4. Re: COPY WHERE clause generated/system column reference

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-11-04T00:27:03Z

    On Mon, Oct 27, 2025 at 1:21 AM jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> CREATE TABLE gtest0 (a int, b int GENERATED ALWAYS AS (a + 1) VIRTUAL);
> copy gtest0 from stdin where (b <> 1);
> 0
> \.
>
> ERROR:  unexpected virtual generated column reference
> CONTEXT:  COPY gtest0, line 1: "0"
>
> We need to apply expand_generated_columns_in_expr to the whereClause in DoCopy.
> However, handling STORED generated columns appears to be less straightforward.
>
> currently:
> ExecQual(cstate->qualexpr, econtext))
> happen before
> ExecComputeStoredGenerated.
>
> when calling ExecQual, the stored generated column values have not been
> populated yet. so we may need ExecComputeStoredGenerated beforehand.
> Since ExecComputeStoredGenerated is likely expensive, I added logic to detect
> whether the WHERE clause actually have stored generated columns reference before
> calling it.

While I agree we can improve the error message in that case as the
message "unexpected virtual generated column reference" isn't helpful
much, I'm not sure that generated column values should be considered
when filtering rows by the WHERE clause. The documentation[1] says:

A stored generated column is computed when it is written (inserted or
updated) and occupies storage as if it were a normal column. A virtual
generated column occupies no storage and is computed when it is read.

The proposed patch (the 0002 patch) allows COPY FROM ... WHERE to
filter rows by checking tuples including generated column values but
it's somewhat odd as it seems not to be the time of reading tuples
from a table.

Also, the patch calls ExecComputeStoredGenerated() before ExecQual(),
which is also before we trigger the BEFORE INSERT trigger. It clearly
violates what the documentation describes[1]:

Generated columns are, conceptually, updated after BEFORE triggers
have run. Therefore, changes made to base columns in a BEFORE trigger
will be reflected in generated columns. But conversely, it is not
allowed to access generated columns in BEFORE triggers.

For example, the tuples passed to a BEFORE INSERT trigger varies
depending on the WHERE clause as follows:

-- preparation
create table t (a int, s int generated always as (a + 10) stored);
create table tt (a int, s int);
create function trig_fn() returns trigger as
$$
begin
    insert into tt select NEW.*;
    return NEW;
end;
$$ language plpgsql;
create trigger trig before insert on t for each row execute function trig_fn();

-- copy a row without the WHERE clause.
copy t from program 'echo 1';
table tt;
 a | s
---+---
 1 |
(1 row)

-- copy a row with the where clause
copy t from program 'echo 1' where s > 0;
table tt;
 a | s
---+----
 1 |
 1 | 11

> generated column allow tableoid system column reference, COPY WHERE clause also
> allow tableoid column reference, should be fine.
>
> 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.

Regards,

[1] https://www.postgresql.org/docs/devel/ddl-generated-columns.html#DDL-GENERATED-COLUMNS

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

  5. Re: COPY WHERE clause generated/system column reference

    jian he <jian.universality@gmail.com> — 2025-11-04T11:43:49Z

    On Tue, Nov 4, 2025 at 8:27 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> The proposed patch (the 0002 patch) allows COPY FROM ... WHERE to
> filter rows by checking tuples including generated column values but
> it's somewhat odd as it seems not to be the time of reading tuples
> from a table.
>
> Also, the patch calls ExecComputeStoredGenerated() before ExecQual(),
> which is also before we trigger the BEFORE INSERT trigger. It clearly
> violates what the documentation describes[1]:
>
> For example, the tuples passed to a BEFORE INSERT trigger varies
> depending on the WHERE clause as follows:
>
> -- preparation
> create table t (a int, s int generated always as (a + 10) stored);
> create table tt (a int, s int);
> create function trig_fn() returns trigger as
> $$
> begin
>     insert into tt select NEW.*;
>     return NEW;
> end;
> $$ language plpgsql;
> create trigger trig before insert on t for each row execute function trig_fn();
>
> -- copy a row without the WHERE clause.
> copy t from program 'echo 1';
> table tt;
>  a | s
> ---+---
>  1 |
> (1 row)
>
> -- copy a row with the where clause
> copy t from program 'echo 1' where s > 0;
> table tt;
>  a | s
> ---+----
>  1 |
>  1 | 11
>
> > 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.

> > 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);

Another possible reason:
tableoid can be referenced in virtual generated column expression.
COPY WHERE clause can be supported for virtual general columns.

CREATE TABLE gtest4 (a int, b oid GENERATED ALWAYS AS ((tableoid)));
COPY gtest4 from stdin where b <> 26420;
COPY gtest4 from stdin where tableoid <> 26420;

we should expect the above two COPY statements behave the same.

please check the attached file:
v2-0001: fix COPY WHERE with system column reference
v2-0002: fix COPY WHERE with virtual generated column reference
v2-0003: fix COPY WHERE with stored generated column reference (experimental)

  6. Re: COPY WHERE clause generated/system column reference

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-05T11:43:19Z

    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.

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.

>>> 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.

  7. Re: COPY WHERE clause generated/system column reference

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-11-05T18:19:09Z

    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

  8. Re: COPY WHERE clause generated/system column reference

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-06T13:17:46Z

    On 05.11.25 19:19, Masahiko Sawada wrote:
>> 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.

Thanks.  I have committed it with these corrections.

  9. Re: COPY WHERE clause generated/system column reference

    Peter Eisentraut <peter@eisentraut.org> — 2026-01-05T08:16:50Z

    On 06.11.25 14:17, Peter Eisentraut wrote:
> On 05.11.25 19:19, Masahiko Sawada wrote:
>>> 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.
> 
> Thanks.  I have committed it with these corrections.

The commitfest entry associated with this thread is still open.

The result of the last commit is that generated columns are prohibited 
in COPY WHERE expressions.

Earlier in the thread there were proposed patches to allow them to work. 
  At least for virtual generated columns this was relatively simple, for 
stored generated columns it was harder because of the timing relative to 
triggers.

After thinking about this some more, I think we should just not do that. 
  This seems fundamentally wrong, because generated columns are a 
property of the target table, but the COPY WHERE clause is meant to 
filter the input data.  This means the practical use is already quite 
questionable.  And then as discussed the execution timing relative to 
triggers and perhaps other things make this potentially pretty 
complicated and semantically dubious.