Re: SQL:2011 Application Time Update & Delete

On 20.02.26 18:16, Paul A Jungwirth wrote:
> On Fri, Feb 13, 2026 at 12:00 PM Paul A Jungwirth
> <pj@illuminatedcomputing.com> wrote:
>>
>> Here is another round to fix a few rebase conflicts.
> 
> I realized we didn't have any tests for v18's new feature to say
> `UPDATE ... RETURNING OLD.foo, NEW.foo`. These patches add a small
> test for `RETURNING OLD.valid_at, NEW.valid_at` when you say `UPDATE
> FOR PORTION OF valid_at`. This seems worth testing since that column
> gets set in an automatic way, not via the normal SET syntax. No fixes
> were needed.
> 
> I also corrected the commit message, which still referred to the
> without_overlaps function that we renamed to
> {range,multirange}_minus_multi.
> 
> As far as I know nothing else here is waiting on me, but please
> correct me if I've overlooked something.
> 
> Rebased to 18bcdb75d1.

Hi Paul,

Review of v67-0001-Add-range_get_constructor2-to-lsyscache.patch and
v67-0002-Add-UPDATE-DELETE-FOR-PORTION-OF.patch:

1) In src/backend/parser/analyze.c, transformForPortionOfClause():

The variable range_name is not very useful; using
forPortionOf->range_name is clearer.

Try making the forPortionOf argument const.

The initialization strat = RTOverlapStrategyNumber; is confusing.

2) src/backend/parser/gram.y

Explain the %prec in opt_alias with a comment.  Maybe the production
name should be more specific if it's only applicable to some specific
statement.

3) In the test src/test/regress/expected/for_portion_of.out:

3.1)

Fix the wording of the error message here (remove "period", add back in 
later patch):

+ERROR:  column or period "invalid_at" of relation "for_portion_of_test" 
does not exist

3.2)

Could this error message have a location pointer?

+ERROR:  range lower bound must be less than or equal to range upper bound

3.3)

Improve wording of this error message:

+ERROR:  got a NULL FOR PORTION OF target

("FOR PORTION OF target was null"?)

3.4)

+-- Updating the non-range part of the PK:

This test updates the id column but the SELECT afterwards only shows
rows for the old id value.  The SELECT ought to use something like

WHERE id = '[1,2)' OR id = '[6,7)'

3.5)

-- UPDATE FOR PORTION OF in a CTE:

I think this test is meant to show that the UPDATE FOR PORTION OF
... RETURNING returns only the updated rows, not the inserted
"leftovers".

First, it would be good if the comment was more clear about what it's
trying to demonstrate.

And then, is there a reason for this behavior versus the alternatives?
SQL standard, other implementations?

3.6)

+-- Not visible to UPDATE:
+-- Tuples updated/inserted within the CTE are not visible to the main 
query yet,
+-- but neither are old tuples the CTE changed:

Is this behavior the same or different from the way normal queries
work?  Could be clarified in the comment either way.

3.7)

+-- UPDATE ... RETURNING returns only the updated values (not the 
inserted side values)

This test looks redundant with earlier tests.  Otherwise, maybe add a
comment about how it's different.

3.8)

+-- test that we run triggers on the UPDATE/DELETEd row and the INSERTed 
rows

Please indent the non-first rows of the CREATE TRIGGER statements.

4) In src/test/subscription/t/034_temporal.pl

+[4,5)|[2000-01-01,2010-01-01)|a}, 'replicated temporal_unique DEFAULT');

The change from FULL to DEFAULT seems wrong.

5) NULL bounds

A general comment: In particular after studying these tests in detail,
I'm suspicious that it's a good idea to interpret null bounds as
unbounded.  Expressions could return null for nested reasons, it would
be very hard to follow that.  Null values should mean "unknown",
unbounded should be explicit.  We have the keyword UNBOUNDED already,
maybe you could use that?  Or do you want to be able to return
unboundedness from an expression?