On 24.06.26 19:21, Paul A Jungwirth wrote:
On Wed, Jun 24, 2026 at 1:48 AM Peter Eisentraut <[email protected]> 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?
