FOR PORTION OF should reject GENERATED columns

On Wed, May 13, 2026 at 8:39 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

On Tue, May 12, 2026 at 1:34 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:

FOR PORTION OF doesn't seem to work well with virtual generated columns,
either. The following example seg-faults on my machine:

create table t (a int, b int4range generated always as (int4range(a, a + 1)) virtual);
insert into t values (1);
delete from t for portion of b from 1 to 2;

I posted a fix for this yesterday, but I wanted to make a separate
thread so the commitfest app could track it correctly. Here is the
start of the conversation:

/messages/by-id/CA+renyWqeWxSUoohRQ4htfSLCcDVsZ=XwVR7F8-e9GXeH_O13w@mail.gmail.com

My first fix was doing this check in the analysis phase because I
thought it would be hard to separate from moving other validations
into the planner/executor[1]/messages/by-id/626986.1776785090@sss.pgh.pa.us, but doing it as a stand-alone patch was
easier than I thought. Here is v2.

[1]: /messages/by-id/626986.1776785090@sss.pgh.pa.us

--
Paul ~{:-)
pj@illuminatedcomputing.com

On Fri, May 15, 2026 at 2:04 PM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

On Tue, May 12, 2026 at 1:34 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:

FOR PORTION OF doesn't seem to work well with virtual generated columns,
either. The following example seg-faults on my machine:

create table t (a int, b int4range generated always as (int4range(a, a + 1)) virtual);
insert into t values (1);
delete from t for portion of b from 1 to 2;

My first fix was doing this check in the analysis phase because I
thought it would be hard to separate from moving other validations
into the planner/executor[1], but doing it as a stand-alone patch was
easier than I thought. Here is v2.

[1] /messages/by-id/626986.1776785090@sss.pgh.pa.us

This needed a rebase; attached.

Also I noticed it had some extra tests from the work to move FDW
checks out of parse analysis. I've removed that from here and I'll
make sure we have it in the right patch.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On 11.06.26 22:57, Paul A Jungwirth wrote:

On Fri, May 15, 2026 at 2:04 PM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

On Tue, May 12, 2026 at 1:34 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:

FOR PORTION OF doesn't seem to work well with virtual generated columns,
either. The following example seg-faults on my machine:

create table t (a int, b int4range generated always as (int4range(a, a + 1)) virtual);
insert into t values (1);
delete from t for portion of b from 1 to 2;

My first fix was doing this check in the analysis phase because I
thought it would be hard to separate from moving other validations
into the planner/executor[1], but doing it as a stand-alone patch was
easier than I thought. Here is v2.

[1] /messages/by-id/626986.1776785090@sss.pgh.pa.us

This needed a rebase; attached.

Also I noticed it had some extra tests from the work to move FDW
checks out of parse analysis. I've removed that from here and I'll
make sure we have it in the right patch.

I don't understand why this proposed check is being done in the
executor. It seems to me that it should be done in the parser, in
transformForPortionOfClause(), where you check other properties of the
for-portion-of target column. It is not possible to turn a normal
column into a generated column, so once we have checked that the column
exists and has the right type and is not generated, I don't think there
is then any risk that that check becomes invalidated between parsing and
execution.

On Wed, Jun 24, 2026 at 1:48 AM Peter Eisentraut <peter@eisentraut.org> wrote:

I don't understand why this proposed check is being done in the
executor. It seems to me that it should be done in the parser, in
transformForPortionOfClause(), where you check other properties of the
for-portion-of target column. It is not possible to turn a normal
column into a generated column, so once we have checked that the column
exists and has the right type and is not generated, I don't think there
is then any risk that that check becomes invalidated between parsing and
execution.

We were worried about BEGIN ATOMIC functions in particular, but you're
right that there is no way to change an ordinary column to a GENERATED
column later, without dropping it. And the BEGIN ATOMIC function
records the dependency, so Postgres won't let you do that. (Actually
you *can* change an integer column to GENERATED AS IDENTITY, but I
don't think you will ever be able to use an integer column in FOR
PORTION OF.)

Here is v4 moving the check into analysis. This lets us give a nicer
error message in a couple cases (captured in the tests).

The test about BEGIN ATOMIC functions now shows that the analysis-time
check prevents you from defining the function.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On 24.06.26 19:21, Paul A Jungwirth wrote:

On Wed, Jun 24, 2026 at 1:48 AM Peter Eisentraut <peter@eisentraut.org> wrote:

I don't understand why this proposed check is being done in the
executor. It seems to me that it should be done in the parser, in
transformForPortionOfClause(), where you check other properties of the
for-portion-of target column. It is not possible to turn a normal
column into a generated column, so once we have checked that the column
exists and has the right type and is not generated, I don't think there
is then any risk that that check becomes invalidated between parsing and
execution.

We were worried about BEGIN ATOMIC functions in particular, but you're
right that there is no way to change an ordinary column to a GENERATED
column later, without dropping it. And the BEGIN ATOMIC function
records the dependency, so Postgres won't let you do that. (Actually
you *can* change an integer column to GENERATED AS IDENTITY, but I
don't think you will ever be able to use an integer column in FOR
PORTION OF.)

Here is v4 moving the check into analysis. This lets us give a nicer
error message in a couple cases (captured in the tests).

The test about BEGIN ATOMIC functions now shows that the analysis-time
check prevents you from defining the function.

Hmm, I think doing it in the parser won't actually work if you are
writing through a view. For example, with the v4 patch, the following
does not error:

CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

So we need to push it later after all.

But maybe it would fit in the planner, near where the volatility check
is being moved to?

On Sat, Jun 27, 2026 at 12:05 AM Peter Eisentraut <peter@eisentraut.org> wrote:

The test about BEGIN ATOMIC functions now shows that the analysis-time
check prevents you from defining the function.

Hmm, I think doing it in the parser won't actually work if you are
writing through a view. For example, with the v4 patch, the following
does not error:

CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

So we need to push it later after all.

But maybe it would fit in the planner, near where the volatility check
is being moved to?

That's a good catch. Here is a version that checks in the planner,
with your example added to the test cases. This is a little nicer than
the executor, since rangeVar hasn't been converted to an expression
yet (for VIRTUAL columns).

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On 29.06.26 08:29, Paul A Jungwirth wrote:

On Sat, Jun 27, 2026 at 12:05 AM Peter Eisentraut <peter@eisentraut.org> wrote:

The test about BEGIN ATOMIC functions now shows that the analysis-time
check prevents you from defining the function.

Hmm, I think doing it in the parser won't actually work if you are
writing through a view. For example, with the v4 patch, the following
does not error:

CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

So we need to push it later after all.

But maybe it would fit in the planner, near where the volatility check
is being moved to?

That's a good catch. Here is a version that checks in the planner,
with your example added to the test cases. This is a little nicer than
the executor, since rangeVar hasn't been converted to an expression
yet (for VIRTUAL columns).

It seems like this still doesn't fully work in some situations:

-- as before
CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;

-- now with an INSTEAD OF trigger
CREATE FUNCTION tfunc() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN RETURN OLD; END;
$$;

CREATE TRIGGER tg1
INSTEAD OF DELETE ON v
FOR EACH ROW EXECUTE FUNCTION tfunc();

-- This should fail but doesn't.
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

On Thu, Jul 2, 2026 at 12:02 AM Peter Eisentraut <peter@eisentraut.org> wrote:

It seems like this still doesn't fully work in some situations:

-- as before
CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;

-- now with an INSTEAD OF trigger
CREATE FUNCTION tfunc() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN RETURN OLD; END;
$$;

CREATE TRIGGER tg1
INSTEAD OF DELETE ON v
FOR EACH ROW EXECUTE FUNCTION tfunc();

-- This should fail but doesn't.
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

In this case the statement with FOR PORTION OF is never executed
(because we do the trigger instead), so I think not failing is good.
It lets people provide an alternative way of doing something that
Postgres can't normally provide (like updating views). But if we
disable FOR PORTION OF with INSTEAD OF triggers (per the other
thread), this goes away, right? So whichever choice we make there
resolves this case.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On 02.07.26 16:27, Paul A Jungwirth wrote:

On Thu, Jul 2, 2026 at 12:02 AM Peter Eisentraut <peter@eisentraut.org> wrote:

It seems like this still doesn't fully work in some situations:

-- as before
CREATE TABLE t (a int, b int4range GENERATED ALWAYS AS (int4range(a, a +
1)) STORED);
CREATE VIEW v AS SELECT * FROM t;

-- now with an INSTEAD OF trigger
CREATE FUNCTION tfunc() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN RETURN OLD; END;
$$;

CREATE TRIGGER tg1
INSTEAD OF DELETE ON v
FOR EACH ROW EXECUTE FUNCTION tfunc();

-- This should fail but doesn't.
DELETE FROM v FOR PORTION OF b FROM 1 TO 2;

In this case the statement with FOR PORTION OF is never executed
(because we do the trigger instead), so I think not failing is good.
It lets people provide an alternative way of doing something that
Postgres can't normally provide (like updating views). But if we
disable FOR PORTION OF with INSTEAD OF triggers (per the other
thread), this goes away, right? So whichever choice we make there
resolves this case.

Ok, committed.