Horton

Re: SQL:2011 application time

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Paul Jungwirth <pj@illuminatedcomputing.com>, jian he <jian.universality@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-03-11T07:46:16Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Rename gist stratnum support function

  2. Remove support for temporal RESTRICT foreign keys

  3. Cache NO ACTION foreign keys separately from RESTRICT foreign keys

  4. Fix NO ACTION temporal foreign keys when the referenced endpoints change

  5. Improve whitespace in without_overlaps test

  6. Tests for logical replication with temporal keys

  7. Support for GiST in get_equal_strategy_number()

  8. Make the conditions in IsIndexUsableForReplicaIdentityFull() more explicit

  9. Replace get_equal_strategy_number_for_am() by get_equal_strategy_number()

  10. Improve internal logical replication error for missing equality strategy

  11. Simplify IsIndexUsableForReplicaIdentityFull()

  12. Fix ALTER TABLE / REPLICA IDENTITY for temporal tables

  13. doc: Update pg_constraint.conexclop docs for WITHOUT OVERLAPS

  14. doc: Add PERIOD to ALTER TABLE reference docs

  15. doc: Add WITHOUT OVERLAPS to ALTER TABLE reference docs

  16. Add temporal FOREIGN KEY contraints

  17. Add temporal PRIMARY KEY and UNIQUE constraints

  18. Add stratnum GiST support function

  19. Avoid crashing when a JIT-inlined backend function throws an error.

  20. Revert temporal primary keys and foreign keys

  21. Fix ON CONFLICT DO NOTHING/UPDATE for temporal indexes

  22. Add test for REPLICA IDENTITY with a temporal key

  23. Use half-open interval notation in without_overlaps tests

  24. Use daterange and YMD in without_overlaps tests instead of tsrange.

  25. Rename pg_constraint.conwithoutoverlaps to conperiod

  26. Fix comment on gist_stratnum_btree

  27. Add missing TAP test name

  28. Improve error handling of HMAC computations

  29. Rename functions to avoid future conflicts

Attachments

On 01.03.24 22:56, Paul Jungwirth wrote:
> On 3/1/24 12:38, Paul Jungwirth wrote:
>> On 2/29/24 13:16, Paul Jungwirth wrote:
>> Here is a v26 patch series to fix a cfbot failure in sepgsql. Rebased 
>> to 655dc31046.
> 
> v27 attached, fixing some cfbot failures from 
> headerscheck+cpluspluscheck. Sorry for the noise!

I had committed v27-0001-Rename-conwithoutoverlaps-to-conperiod.patch a 
little while ago.

I have reviewed v27-0002 through 0004 now.  I have one semantic question 
below, and there are a few places where more clarification of the 
interfaces could help.  Other than that, I think this is pretty good.

Attached is a small patch that changes the PERIOD keyword to unreserved 
for this patch.  You had said earlier that this didn't work for you. 
The attached patch works for me when applied on top of 0003.


* v27-0002-Add-GiST-referencedagg-support-func.patch

You wrote:

 > I'm not sure how else to do it. The issue is that `range_agg` returns 
 > a multirange, so the result
 > type doesn't match the inputs. But other types will likely have the
 > same problem: to combine boxes
 > you may need a multibox. The combine mdranges you may need a
 > multimdrange.

Can we just hardcode the use of range_agg for this release?  Might be 
easier.  I don't see all this generality being useful in the near future.

 > Btw that part changed a bit since v24 because as jian he pointed out, 
 > our type system doesn't
 > support anyrange inputs and an anyrange[] output. So I changed the
 > support funcs to use SETOF.

I didn't see any SETOF stuff in the patch, or I didn't know where to look.

I'm not sure I follow all the details here.  So more explanations of any 
kind could be helpful.


* v27-0003-Refactor-FK-operator-lookup.patch

I suggest to skip this refactoring patch.  I don't think the way this is 
sliced up is all that great, and it doesn't actually help with the 
subsequent patches.


* v27-0004-Add-temporal-FOREIGN-KEYs.patch

- src/backend/catalog/pg_constraint.c

FindFKPeriodOpersAndProcs() could use a bit more top-level
documentation.  Where does the input opclass come from?  What are the
three output values?  What is the business with "symmetric types"?

- src/backend/commands/indexcmds.c

GetOperatorFromWellKnownStrategy() is apparently changed to accept
InvalidOid for rhstype, but the meaning of this is not explained in
the function header.  It's also not clear to me why an existing caller
is changed.  This should be explained more thoroughly.

- src/backend/commands/tablecmds.c

is_temporal and similar should be renamed to with_period or similar 
throughout this patch.

In transformFkeyGetPrimaryKey():

      * Now build the list of PK attributes from the indkey definition (we
-    * assume a primary key cannot have expressional elements)
+    * assume a primary key cannot have expressional elements, unless it
+    * has a PERIOD)

I think the original statement is still true even with PERIOD.  The 
expressional elements refer to expression indexes.  I don't think we can 
have a PERIOD marker on an expression?

- src/backend/utils/adt/ri_triggers.c

Please remove the separate trigger functions for the period case.  They 
are the same as the non-period ones, so we don't need separate ones. 
The difference is handled lower in the call stack, which I think is a 
good setup.  Removing the separate functions also removes a lot of extra 
code in other parts of the patch.

- src/include/catalog/pg_constraint.h

Should also update catalogs.sgml accordingly.

- src/test/regress/expected/without_overlaps.out
- src/test/regress/sql/without_overlaps.sql

A few general comments on the tests:

- In the INSERT commands, specify the column names explicitly.  This 
makes the tests easier to read (especially since the column order 
between the PK and the FK table is sometimes different).

- Let's try to make it so that the inserted literals match the values 
shown in the various error messages, so it's easier to match them up. 
So, change the int4range literals to half-open notation.  And also maybe 
change the date output format to ISO.

- In various comments, instead of test FK "child", maybe use 
"referencing table"?  Instead of "parent", use "referenced table" (or 
primary key table).  When I read child and parent I was looking for 
inheritance.

- Consider truncating the test tables before each major block of tests 
and refilling them with fresh data.  So it's easier to eyeball the 
tests.  Otherwise, there is too much dependency on what earlier tests 
left behind.

A specific question:

In this test, a PERIOD marker on the referenced site is automatically 
inferred from the primary key:

+-- with inferred PK on the referenced table:
+CREATE TABLE temporal_fk_rng2rng (
+   id int4range,
+   valid_at tsrange,
+   parent_id int4range,
+   CONSTRAINT temporal_fk_rng2rng_pk PRIMARY KEY (id, valid_at WITHOUT 
OVERLAPS),
+   CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD 
valid_at)
+       REFERENCES temporal_rng
+);

In your patch, this succeeds.  According to the SQL standard, it should 
not.  In subclause 11.8, syntax rule 4b:

"""
Otherwise, the table descriptor of the referenced table shall include a 
unique constraint UC that specifies PRIMARY KEY. The table constraint 
descriptor of UC shall not include an application time period name.
"""

So this case is apparently explicitly ruled out.

(It might be ok to make an extension here, but then we should be 
explicit about it.)