public inbox for [email protected]
help / color / mirror / Atom feedRe: SQL:2011 application time
10+ messages / 4 participants
[nested] [flat]
* Re: SQL:2011 application time
@ 2024-05-09 04:24 Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-10 13:25 ` Re: SQL:2011 application time Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Paul Jungwirth @ 2024-05-09 04:24 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; +Cc: jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
Here are a couple new patches, rebased to e305f715, addressing Peter's feedback. I'm still working
on integrating jian he's suggestions for the last patch, so I've omitted that one here.
On 5/8/24 06:51, Peter Eisentraut wrote:
> About v2-0001-Fix-ON-CONFLICT-DO-NOTHING-UPDATE-for-temporal-in.patch, I think the
> ideas are right, but I wonder if we can fine-tune the new conditionals a bit.
>
> --- a/src/backend/executor/execIndexing.c
> +++ b/src/backend/executor/execIndexing.c
> @@ -210,7 +210,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative)
> * If the indexes are to be used for speculative insertion, add extra
> * information required by unique index entries.
> */
> - if (speculative && ii->ii_Unique)
> + if (speculative && ii->ii_Unique && !ii->ii_HasWithoutOverlaps)
> BuildSpeculativeIndexInfo(indexDesc, ii);
>
> Here, I think we could check !indexDesc->rd_index->indisexclusion instead. So we
> wouldn't need ii_HasWithoutOverlaps.
Okay.
> Or we could push this into BuildSpeculativeIndexInfo(); it could just skip the rest
> if an exclusion constraint is passed, on the theory that all the speculative index
> info is already present in that case.
I like how BuildSpeculativeIndexInfo starts with an Assert that it's given a unique index, so I've
left the check outside the function. This seems cleaner anyway: the function stays more focused.
> --- a/src/backend/optimizer/util/plancat.c
> +++ b/src/backend/optimizer/util/plancat.c
> @@ -815,7 +815,7 @@ infer_arbiter_indexes(PlannerInfo *root)
> */
> if (indexOidFromConstraint == idxForm->indexrelid)
> {
> - if (!idxForm->indisunique && onconflict->action == ONCONFLICT_UPDATE)
> + if ((!idxForm->indisunique || idxForm->indisexclusion) && onconflict->action ==
> ONCONFLICT_UPDATE)
> ereport(ERROR,
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints")));
>
> Shouldn't this use only idxForm->indisexclusion anyway? Like
>
> + if (idxForm->indisexclusion && onconflict->action == ONCONFLICT_UPDATE)
>
> That matches what the error message is reporting afterwards.
Agreed.
> * constraints), so index under consideration can be immediately
> * skipped if it's not unique
> */
> - if (!idxForm->indisunique)
> + if (!idxForm->indisunique || idxForm->indisexclusion)
> goto next;
>
> Maybe here we need a comment. Or make that a separate statement, like
Yes, that is nice. Done.
Yours,
--
Paul ~{:-)
[email protected]
Attachments:
[text/x-patch] v2-0001-Fix-ON-CONFLICT-DO-NOTHING-UPDATE-for-temporal-in.patch (21.5K, ../../[email protected]/2-v2-0001-Fix-ON-CONFLICT-DO-NOTHING-UPDATE-for-temporal-in.patch)
download | inline diff:
From 0ebe2e6d6cd8dc6f8120fe93b9024cf80472f8cc Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Sun, 24 Mar 2024 21:46:30 -0700
Subject: [PATCH v2 1/2] Fix ON CONFLICT DO NOTHING/UPDATE for temporal indexes
A PRIMARY KEY or UNIQUE constraint with WITHOUT OVERLAPS will be a GiST
index, not a B-Tree, but it will still have indisunique set. The code
for ON CONFLICT fails if it sees a non-btree index that has indisunique.
This commit fixes that and adds some tests. But now that we can't just
test indisunique, we also need some extra checks to prevent DO UPDATE
from running against a WITHOUT OVERLAPS constraint (because the conflict
could happen against more than one row, and we'd only update one).
---
src/backend/catalog/index.c | 1 +
src/backend/executor/execIndexing.c | 2 +-
src/backend/optimizer/util/plancat.c | 9 +-
src/include/nodes/execnodes.h | 1 +
.../regress/expected/without_overlaps.out | 176 ++++++++++++++++++
src/test/regress/sql/without_overlaps.sql | 113 +++++++++++
6 files changed, 300 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 5a8568c55c9..1fd543cc550 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2443,6 +2443,7 @@ BuildIndexInfo(Relation index)
&ii->ii_ExclusionOps,
&ii->ii_ExclusionProcs,
&ii->ii_ExclusionStrats);
+ ii->ii_HasWithoutOverlaps = ii->ii_Unique;
}
return ii;
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 9f05b3654c1..59acf67a36a 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -210,7 +210,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative)
* If the indexes are to be used for speculative insertion, add extra
* information required by unique index entries.
*/
- if (speculative && ii->ii_Unique)
+ if (speculative && ii->ii_Unique && !indexDesc->rd_index->indisexclusion)
BuildSpeculativeIndexInfo(indexDesc, ii);
relationDescs[i] = indexDesc;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 130f838629f..775c3e26cd8 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -815,7 +815,7 @@ infer_arbiter_indexes(PlannerInfo *root)
*/
if (indexOidFromConstraint == idxForm->indexrelid)
{
- if (!idxForm->indisunique && onconflict->action == ONCONFLICT_UPDATE)
+ if (idxForm->indisexclusion && onconflict->action == ONCONFLICT_UPDATE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints")));
@@ -840,6 +840,13 @@ infer_arbiter_indexes(PlannerInfo *root)
if (!idxForm->indisunique)
goto next;
+ /*
+ * So-called unique constraints with WITHOUT OVERLAPS are really
+ * exclusion constraints, so skip those too.
+ */
+ if (idxForm->indisexclusion)
+ goto next;
+
/* Build BMS representation of plain (non expression) index attrs */
indexedAttrs = NULL;
for (natt = 0; natt < idxForm->indnkeyatts; natt++)
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 8bc421e7c05..4fb7e3b284c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -204,6 +204,7 @@ typedef struct IndexInfo
bool ii_Summarizing;
int ii_ParallelWorkers;
Oid ii_Am;
+ bool ii_HasWithoutOverlaps;
void *ii_AmCache;
MemoryContext ii_Context;
} IndexInfo;
diff --git a/src/test/regress/expected/without_overlaps.out b/src/test/regress/expected/without_overlaps.out
index abc22d0113f..e2f2a1cbe20 100644
--- a/src/test/regress/expected/without_overlaps.out
+++ b/src/test/regress/expected/without_overlaps.out
@@ -453,6 +453,182 @@ DROP TABLE temporal_partitioned;
ALTER TABLE temporal_rng REPLICA IDENTITY USING INDEX temporal_rng_pk;
ERROR: cannot use non-unique index "temporal_rng_pk" as replica identity
--
+-- ON CONFLICT
+--
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+ [1,2) | [2010-01-01,2020-01-01)
+ [2,3) | [2005-01-01,2006-01-01)
+(3 rows)
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+ [1,2) | [2010-01-01,2020-01-01)
+ [2,3) | [2005-01-01,2006-01-01)
+(3 rows)
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+-- with a UNIQUE constraint:
+CREATE TABLE temporal3 (
+ id int4range,
+ valid_at daterange,
+ CONSTRAINT temporal3_uq UNIQUE (id, valid_at WITHOUT OVERLAPS)
+);
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+ [1,2) | [2010-01-01,2020-01-01)
+ [2,3) | [2005-01-01,2006-01-01)
+(3 rows)
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+ [1,2) | [2010-01-01,2020-01-01)
+ [2,3) | [2005-01-01,2006-01-01)
+(3 rows)
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+ id | valid_at
+-------+-------------------------
+ [1,2) | [2000-01-01,2010-01-01)
+(1 row)
+
+DROP TABLE temporal3;
+--
-- test FK dependencies
--
-- can't drop a range referenced by an FK, unless with CASCADE
diff --git a/src/test/regress/sql/without_overlaps.sql b/src/test/regress/sql/without_overlaps.sql
index d4ae03ae529..5d41a6bd628 100644
--- a/src/test/regress/sql/without_overlaps.sql
+++ b/src/test/regress/sql/without_overlaps.sql
@@ -319,6 +319,119 @@ DROP TABLE temporal_partitioned;
-- (should fail)
ALTER TABLE temporal_rng REPLICA IDENTITY USING INDEX temporal_rng_pk;
+--
+-- ON CONFLICT
+--
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO NOTHING;
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+
+TRUNCATE temporal_rng;
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+-- id matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+-- date matches but no conflict
+INSERT INTO temporal_rng (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal_rng_pk DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+SELECT * FROM temporal_rng ORDER BY id, valid_at;
+
+-- with a UNIQUE constraint:
+
+CREATE TABLE temporal3 (
+ id int4range,
+ valid_at daterange,
+ CONSTRAINT temporal3_uq UNIQUE (id, valid_at WITHOUT OVERLAPS)
+);
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT DO NOTHING;
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO NOTHING;
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO NOTHING;
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT (id, valid_at) DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+
+TRUNCATE temporal3;
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2000-01-01', '2010-01-01'));
+-- with a conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[2,3)';
+-- id matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[1,2)', daterange('2010-01-01', '2020-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[3,4)';
+-- date matches but no conflict
+INSERT INTO temporal3 (id, valid_at) VALUES ('[2,3)', daterange('2005-01-01', '2006-01-01')) ON CONFLICT ON CONSTRAINT temporal3_uq DO UPDATE SET id = EXCLUDED.id + '[4,5)';
+SELECT * FROM temporal3 ORDER BY id, valid_at;
+
+DROP TABLE temporal3;
+
--
-- test FK dependencies
--
--
2.45.0
[text/x-patch] v2-0002-Don-t-treat-WITHOUT-OVERLAPS-indexes-as-unique-in.patch (3.9K, ../../[email protected]/3-v2-0002-Don-t-treat-WITHOUT-OVERLAPS-indexes-as-unique-in.patch)
download | inline diff:
From edc3d4112fb60112a3375e9316e056e61581971f Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Tue, 2 Apr 2024 15:39:04 -0700
Subject: [PATCH v2 2/2] Don't treat WITHOUT OVERLAPS indexes as unique in
planner
Because the special rangetype 'empty' never overlaps another value, it
is possible for WITHOUT OVERLAPS tables to have two rows with the same
key, despite being indisunique, if the application-time range is
'empty'. So to be safe we should not treat WITHOUT OVERLAPS indexes as
unique in any proofs.
This still needs a test, but I'm having trouble finding a query that
gives wrong results.
---
src/backend/optimizer/path/indxpath.c | 5 +++--
src/backend/optimizer/plan/analyzejoins.c | 6 +++---
src/backend/optimizer/util/plancat.c | 1 +
src/include/nodes/pathnodes.h | 2 ++
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c0fcc7d78df..72346f78ebe 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3498,13 +3498,14 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
/*
* If the index is not unique, or not immediately enforced, or if it's
- * a partial index, it's useless here. We're unable to make use of
+ * a partial index, or if it's a WITHOUT OVERLAPS index (so not
+ * literally unique), it's useless here. We're unable to make use of
* predOK partial unique indexes due to the fact that
* check_index_predicates() also makes use of join predicates to
* determine if the partial index is usable. Here we need proofs that
* hold true before any joins are evaluated.
*/
- if (!ind->unique || !ind->immediate || ind->indpred != NIL)
+ if (!ind->unique || !ind->immediate || ind->indpred != NIL || ind->hasperiod)
continue;
/*
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index aa725925675..ebca049fd5b 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -800,8 +800,8 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel)
* For a plain relation, we only know how to prove uniqueness by
* reference to unique indexes. Make sure there's at least one
* suitable unique index. It must be immediately enforced, and not a
- * partial index. (Keep these conditions in sync with
- * relation_has_unique_index_for!)
+ * partial index, and not WITHOUT OVERLAPS (Keep these conditions
+ * in sync with relation_has_unique_index_for!)
*/
ListCell *lc;
@@ -809,7 +809,7 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel)
{
IndexOptInfo *ind = (IndexOptInfo *) lfirst(lc);
- if (ind->unique && ind->immediate && ind->indpred == NIL)
+ if (ind->unique && ind->immediate && ind->indpred == NIL && !ind->hasperiod)
return true;
}
}
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 775c3e26cd8..146029577bd 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -457,6 +457,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
info->predOK = false; /* set later, in indxpath.c */
info->unique = index->indisunique;
info->immediate = index->indimmediate;
+ info->hasperiod = index->indisunique && index->indisexclusion;
info->hypothetical = false;
/*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 14ef296ab72..e24a45f0cd5 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1176,6 +1176,8 @@ struct IndexOptInfo
bool unique;
/* is uniqueness enforced immediately? */
bool immediate;
+ /* true if index has WITHOUT OVERLAPS */
+ bool hasperiod;
/* true if index doesn't really exist */
bool hypothetical;
--
2.45.0
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
@ 2024-05-10 00:44 ` Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Matthias van de Meent @ 2024-05-10 00:44 UTC (permalink / raw)
To: Paul Jungwirth <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
I haven't really been following this thread, but after playing around
a bit with the feature I feel there are new gaps in error messages. I
also think there are gaps in the functionality regarding the (lack of)
support for CREATE UNIQUE INDEX, and attaching these indexes to
constraints.
pg=# CREATE TABLE temporal_testing (
pg(# id bigint NOT NULL
pg(# generated always as identity,
pg(# valid_during tstzrange
pg(# );
CREATE TABLE
pg=# ALTER TABLE temporal_testing
pg-# ADD CONSTRAINT temp_unique UNIQUE (id, valid_during WITHOUT OVERLAPS);
ALTER TABLE
pg=# \d+ temp_unique
Index "public.temp_unique"
Column | Type | Key? | Definition | Storage | Stats target
--------------+-------------+------+--------------+----------+--------------
id | gbtreekey16 | yes | id | plain |
valid_during | tstzrange | yes | valid_during | extended |
unique, gist, for table "public.temporal_testing"
-- ^^ note the "unique, gist"
pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
ERROR: access method "gist" does not support unique indexes
Here we obviously have a unique GIST index in the catalogs, but
they're "not supported" by GIST when we try to create such index
ourselves (!). Either the error message needs updating, or we need to
have a facility to actually support creating these unique indexes
outside constraints.
Additionally, because I can't create my own non-constraint-backing
unique GIST indexes, I can't pre-create my unique constraints
CONCURRENTLY as one could do for the non-temporal case: UNIQUE
constraints hold ownership of the index and would drop the index if
the constraint is dropped, too, and don't support a CONCURRENTLY
modifier, nor an INVALID modifier. This means temporal unique
constraints have much less administrative wiggle room than normal
unique constraints, and I think that's not great.
Kind regards,
Matthias van de Meent.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
@ 2024-05-12 03:26 ` Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Paul Jungwirth @ 2024-05-12 03:26 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On 5/9/24 17:44, Matthias van de Meent wrote:
> I haven't really been following this thread, but after playing around
> a bit with the feature I feel there are new gaps in error messages. I
> also think there are gaps in the functionality regarding the (lack of)
> support for CREATE UNIQUE INDEX, and attaching these indexes to
> constraints
Thank you for trying this out and sharing your thoughts! I think these are good points about CREATE
UNIQUE INDEX and then creating the constraint by handing it an existing index. This is something
that I am hoping to add, but it's not covered by the SQL:2011 standard, so I think it needs some
discussion, and I don't think it needs to go into v17.
For instance you are saying:
> pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
> ERROR: access method "gist" does not support unique indexes
To me that error message seems correct. The programmer hasn't said anything about the special
temporal behavior they are looking for. To get non-overlapping semantics from an index, this more
explicit syntax seems better, similar to PKs in the standard:
> pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during WITHOUT OVERLAPS);
> ERROR: access method "gist" does not support unique indexes
We could also support *non-temporal* unique GiST indexes, particularly now that we have the stratnum
support function. Those would use the syntax you gave, omitting WITHOUT OVERLAPS. But that seems
like a separate effort to me.
> Additionally, because I can't create my own non-constraint-backing
> unique GIST indexes, I can't pre-create my unique constraints
> CONCURRENTLY as one could do for the non-temporal case: UNIQUE
> constraints hold ownership of the index and would drop the index if
> the constraint is dropped, too, and don't support a CONCURRENTLY
> modifier, nor an INVALID modifier. This means temporal unique
> constraints have much less administrative wiggle room than normal
> unique constraints, and I think that's not great.
This is a great use-case for why we should support this eventually, even if it uses non-standard syntax.
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
@ 2024-05-12 12:55 ` Matthias van de Meent <[email protected]>
2024-05-12 15:51 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-06-05 20:57 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Matthias van de Meent @ 2024-05-12 12:55 UTC (permalink / raw)
To: Paul Jungwirth <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, 12 May 2024 at 05:26, Paul Jungwirth
<[email protected]> wrote:
> On 5/9/24 17:44, Matthias van de Meent wrote:
> > I haven't really been following this thread, but after playing around
> > a bit with the feature I feel there are new gaps in error messages. I
> > also think there are gaps in the functionality regarding the (lack of)
> > support for CREATE UNIQUE INDEX, and attaching these indexes to
> > constraints
> Thank you for trying this out and sharing your thoughts! I think these are good points about CREATE
> UNIQUE INDEX and then creating the constraint by handing it an existing index. This is something
> that I am hoping to add, but it's not covered by the SQL:2011 standard, so I think it needs some
> discussion, and I don't think it needs to go into v17.
Okay.
> For instance you are saying:
>
> > pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
> > ERROR: access method "gist" does not support unique indexes
>
> To me that error message seems correct. The programmer hasn't said anything about the special
> temporal behavior they are looking for.
But I showed that I had a GIST index that does have the indisunique
flag set, which shows that GIST does support indexes with unique
semantics.
That I can't use CREATE UNIQUE INDEX to create such an index doesn't
mean the feature doesn't exist, which is what the error message
implies.
> To get non-overlapping semantics from an index, this more
> explicit syntax seems better, similar to PKs in the standard:
Yes, agreed on that part.
> > pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during WITHOUT OVERLAPS);
> > ERROR: access method "gist" does not support unique indexes
>
> We could also support *non-temporal* unique GiST indexes, particularly now that we have the stratnum
> support function. Those would use the syntax you gave, omitting WITHOUT OVERLAPS. But that seems
> like a separate effort to me.
No objection on that.
Kind regards,
Matthias van de Meent
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
@ 2024-05-12 15:51 ` Paul Jungwirth <[email protected]>
2024-05-13 04:54 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Paul Jungwirth @ 2024-05-12 15:51 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On 5/12/24 05:55, Matthias van de Meent wrote:
>> > pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
>> > ERROR: access method "gist" does not support unique indexes
>>
>> To me that error message seems correct. The programmer hasn't said anything about the special
>> temporal behavior they are looking for.
>
> But I showed that I had a GIST index that does have the indisunique
> flag set, which shows that GIST does support indexes with unique
> semantics.
>
> That I can't use CREATE UNIQUE INDEX to create such an index doesn't
> mean the feature doesn't exist, which is what the error message
> implies.
True, the error message is not really telling the truth anymore. I do think most people who hit this
error are not thinking about temporal constraints at all though, and for non-temporal constraints it
is still true. It's also true for CREATE INDEX, since WITHOUT OVERLAPS is only available on the
*constraint*. So how about adding a hint, something like this?:
ERROR: access method "gist" does not support unique indexes
HINT: To create a unique constraint with non-overlap behavior, use ADD CONSTRAINT ... WITHOUT OVERLAPS.
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 15:51 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
@ 2024-05-13 04:54 ` Paul Jungwirth <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Paul Jungwirth @ 2024-05-13 04:54 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On 5/12/24 08:51, Paul Jungwirth wrote:
> On 5/12/24 05:55, Matthias van de Meent wrote:
>>> > pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
>>> > ERROR: access method "gist" does not support unique indexes
>>>
>>> To me that error message seems correct. The programmer hasn't said anything about the special
>>> temporal behavior they are looking for.
>>
>> But I showed that I had a GIST index that does have the indisunique
>> flag set, which shows that GIST does support indexes with unique
>> semantics.
>>
>> That I can't use CREATE UNIQUE INDEX to create such an index doesn't
>> mean the feature doesn't exist, which is what the error message
>> implies.
>
> True, the error message is not really telling the truth anymore. I do think most people who hit this
> error are not thinking about temporal constraints at all though, and for non-temporal constraints it
> is still true. It's also true for CREATE INDEX, since WITHOUT OVERLAPS is only available on the
> *constraint*. So how about adding a hint, something like this?:
>
> ERROR: access method "gist" does not support unique indexes
> HINT: To create a unique constraint with non-overlap behavior, use ADD CONSTRAINT ... WITHOUT OVERLAPS.
I thought a little more about eventually implementing WITHOUT OVERLAPS support for CREATE INDEX, and
how it relates to this error message in particular. Even when that is done, it will still depend on
the stratnum support function for the keys' opclasses, so the GiST AM itself will still have false
amcanunique, I believe. Probably the existing error message is still the right one. The hint won't
need to mention ADD CONSTRAINT anymore. It should still point users to WITHOUT OVERLAPS, and
possibly the stratnum support function too. I think what we are doing for v17 is all compatible with
that plan.
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
@ 2024-06-05 20:57 ` Paul Jungwirth <[email protected]>
2024-06-12 15:48 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Paul Jungwirth @ 2024-06-05 20:57 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, May 9, 2024 at 5:44 PM Matthias van de Meent <[email protected]> wrote:
> Additionally, because I can't create my own non-constraint-backing
> unique GIST indexes, I can't pre-create my unique constraints
> CONCURRENTLY as one could do for the non-temporal case
We talked about this a bit at pgconf.dev. I would like to implement it, since I agree it is an
important workflow to support. Here are some thoughts about what would need to be done.
First we could take a small step: allow non-temporal UNIQUE GiST indexes. This is possible according
to [1], but in the past we had no way of knowing which strategy number an opclass was using for
equality. With the stratnum support proc introduced by 6db4598fcb (reverted for v17), we could
change amcanunique to true for the GiST AM handler. If the index's opclasses had that sproc and it
gave non-zero for RTEqualStrategyNumber, we would have a reliable "definition of uniqueness". UNIQUE
GiST indexes would raise an error if they detected a duplicate record.
Incidentally, this would also let us correct the error message about GiST not supporting unique,
fixing the problem you raised here:
On Sun, May 12, 2024 at 8:51 AM Paul Jungwirth <[email protected]> wrote:
>
> On 5/12/24 05:55, Matthias van de Meent wrote:
> >> > pg=# CREATE UNIQUE INDEX ON temporal_testing USING gist (id, valid_during);
> >> > ERROR: access method "gist" does not support unique indexes
> >>
> >> To me that error message seems correct. The programmer hasn't said anything about the special
> >> temporal behavior they are looking for.
> >
> > But I showed that I had a GIST index that does have the indisunique
> > flag set, which shows that GIST does support indexes with unique
> > semantics.
> >
> > That I can't use CREATE UNIQUE INDEX to create such an index doesn't
> > mean the feature doesn't exist, which is what the error message
> > implies.
>
> True, the error message is not really telling the truth anymore.
But that is just regular non-temporal indexes. To avoid a long table lock you'd need a way to build
the index that is not just unique, but also does exclusion based on &&. We could borrow syntax from
SQL:2011 and allow `CREATE INDEX idx ON t (id, valid_at WITHOUT OVERLAPS)`. But since CREATE INDEX
is a lower-level concept than a constraint, it'd be better to do something more general. You can
already give opclasses for each indexed column. How about allowing operators as well? For instance
`CREATE UNIQUE INDEX idx ON t (id WITH =, valid_at WITH &&)`? Then the index would know to enforce
those rules. This is the same data we store today in pg_constraint.conexclops. So that would get
moved/copied to pg_index (probably moved).
Then when you add the constraint, what is the syntax? Today when you say PRIMARY KEY/UNIQUE USING
INDEX, you don't give the column names. So how do we know it's WITHOUT OVERLAPS? I guess if the
underlying index has (foo WITH = [, bar WITH =], baz WITH &&) we just assume the user wants WITHOUT
OVERLAPS, and otherwise they want a regular PK/UQ constraint?
In addition this workflow only works if you can CREATE INDEX CONCURRENTLY. I'm not sure yet if we'll
have problems there. I noticed that for REINDEX at least, there were plans in 2012 to support
exclusion-constraint indexes,[2] but when the patch was committed in 2019 they had been dropped,
with plans to add support eventually.[3] Today they are still not supported. Maybe whatever caused
problems for REINDEX isn't an issue for just INDEX, but it would take more research to find out.
[1] https://dsf.berkeley.edu/papers/sigmod97-gist.pdf
[2] Original patch thread from 2012:
https://www.postgresql.org/message-id/flat/CAB7nPqS%2BWYN021oQHd9GPe_5dSVcVXMvEBW_E2AV9OOEwggMHw%40m...
[3] Revised patch thread, committed in 2019:
https://www.postgresql.org/message-id/flat/60052986-956b-4478-45ed-8bd119e9b9cf%402ndquadrant.com#74...
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Re: SQL:2011 application time Matthias van de Meent <[email protected]>
2024-06-05 20:57 ` Re: SQL:2011 application time Paul Jungwirth <[email protected]>
@ 2024-06-12 15:48 ` Matthias van de Meent <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Matthias van de Meent @ 2024-06-12 15:48 UTC (permalink / raw)
To: Paul Jungwirth <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, 5 Jun 2024 at 22:57, Paul Jungwirth <[email protected]> wrote:
>
> On Thu, May 9, 2024 at 5:44 PM Matthias van de Meent <[email protected]> wrote:
> > Additionally, because I can't create my own non-constraint-backing
> > unique GIST indexes, I can't pre-create my unique constraints
> > CONCURRENTLY as one could do for the non-temporal case
>
> We talked about this a bit at pgconf.dev. I would like to implement it, since I agree it is an
> important workflow to support. Here are some thoughts about what would need to be done.
>
> First we could take a small step: allow non-temporal UNIQUE GiST indexes. This is possible according
> to [1], but in the past we had no way of knowing which strategy number an opclass was using for
> equality. With the stratnum support proc introduced by 6db4598fcb (reverted for v17), we could
> change amcanunique to true for the GiST AM handler. If the index's opclasses had that sproc and it
> gave non-zero for RTEqualStrategyNumber, we would have a reliable "definition of uniqueness". UNIQUE
> GiST indexes would raise an error if they detected a duplicate record.
Cool.
> But that is just regular non-temporal indexes. To avoid a long table lock you'd need a way to build
> the index that is not just unique, but also does exclusion based on &&. We could borrow syntax from
> SQL:2011 and allow `CREATE INDEX idx ON t (id, valid_at WITHOUT OVERLAPS)`. But since CREATE INDEX
> is a lower-level concept than a constraint, it'd be better to do something more general. You can
> already give opclasses for each indexed column. How about allowing operators as well? For instance
> `CREATE UNIQUE INDEX idx ON t (id WITH =, valid_at WITH &&)`? Then the index would know to enforce
> those rules.
I think this looks fine. I'd like it even better if we could default
to the equality operator that's used by the type's default btree
opclass in this syntax; that'd make CREATE UNIQUE INDEX much less
awkward for e.g. hash indexes.
> This is the same data we store today in pg_constraint.conexclops. So that would get
> moved/copied to pg_index (probably moved).
I'd keep the pg_constraint.conexclops around: People are inevitably
going to want to keep the current exclusion constraints' handling of
duplicate empty ranges, which is different from expectations we see
for UNIQUE INDEX's handling.
> Then when you add the constraint, what is the syntax? Today when you say PRIMARY KEY/UNIQUE USING
> INDEX, you don't give the column names. So how do we know it's WITHOUT OVERLAPS? I guess if the
> underlying index has (foo WITH = [, bar WITH =], baz WITH &&) we just assume the user wants WITHOUT
> OVERLAPS, and otherwise they want a regular PK/UQ constraint?
Presumably you would know this based on the pg_index.indisunique flag?
> In addition this workflow only works if you can CREATE INDEX CONCURRENTLY. I'm not sure yet if we'll
> have problems there. I noticed that for REINDEX at least, there were plans in 2012 to support
> exclusion-constraint indexes,[2] but when the patch was committed in 2019 they had been dropped,
> with plans to add support eventually.[3] Today they are still not supported. Maybe whatever caused
> problems for REINDEX isn't an issue for just INDEX, but it would take more research to find out.
I don't quite see where exclusion constraints get into the picture?
Isn't this about unique indexes, not exclusion constraints? I
understand exclusion constraints are backed by indexes, but that
doesn't have to make it a unique index, right? I mean, currently, you
can write an exclusion constraint that makes sure that all rows with a
certain prefix have the same suffix columns (given a btree-esque index
type with <> -operator support), which seems exactly opposite of what
unique indexes should do.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: SQL:2011 application time
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
@ 2024-05-10 13:25 ` Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Peter Eisentraut @ 2024-05-10 13:25 UTC (permalink / raw)
To: Paul Jungwirth <[email protected]>; Robert Haas <[email protected]>; +Cc: jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
I have committed the
v2-0001-Fix-ON-CONFLICT-DO-NOTHING-UPDATE-for-temporal-in.patch from
this (confusingly, there was also a v2 earlier in this thread), and I'll
continue working on the remaining items.
On 09.05.24 06:24, Paul Jungwirth wrote:
> Here are a couple new patches, rebased to e305f715, addressing Peter's
> feedback. I'm still working on integrating jian he's suggestions for the
> last patch, so I've omitted that one here.
>
> On 5/8/24 06:51, Peter Eisentraut wrote:
>> About v2-0001-Fix-ON-CONFLICT-DO-NOTHING-UPDATE-for-temporal-in.patch,
>> I think the
>> ideas are right, but I wonder if we can fine-tune the new conditionals
>> a bit.
>>
>> --- a/src/backend/executor/execIndexing.c
>> +++ b/src/backend/executor/execIndexing.c
>> @@ -210,7 +210,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo, bool
>> speculative)
>> * If the indexes are to be used for speculative
>> insertion, add extra
>> * information required by unique index entries.
>> */
>> - if (speculative && ii->ii_Unique)
>> + if (speculative && ii->ii_Unique &&
>> !ii->ii_HasWithoutOverlaps)
>> BuildSpeculativeIndexInfo(indexDesc, ii);
>>
>> Here, I think we could check !indexDesc->rd_index->indisexclusion
>> instead. So we
>> wouldn't need ii_HasWithoutOverlaps.
>
> Okay.
>
>> Or we could push this into BuildSpeculativeIndexInfo(); it could just
>> skip the rest
>> if an exclusion constraint is passed, on the theory that all the
>> speculative index
>> info is already present in that case.
>
> I like how BuildSpeculativeIndexInfo starts with an Assert that it's
> given a unique index, so I've left the check outside the function. This
> seems cleaner anyway: the function stays more focused.
>
>> --- a/src/backend/optimizer/util/plancat.c
>> +++ b/src/backend/optimizer/util/plancat.c
>> @@ -815,7 +815,7 @@ infer_arbiter_indexes(PlannerInfo *root)
>> */
>> if (indexOidFromConstraint == idxForm->indexrelid)
>> {
>> - if (!idxForm->indisunique && onconflict->action ==
>> ONCONFLICT_UPDATE)
>> + if ((!idxForm->indisunique || idxForm->indisexclusion) &&
>> onconflict->action == ONCONFLICT_UPDATE)
>> ereport(ERROR,
>> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>> errmsg("ON CONFLICT DO UPDATE not supported
>> with exclusion constraints")));
>>
>> Shouldn't this use only idxForm->indisexclusion anyway? Like
>>
>> + if (idxForm->indisexclusion && onconflict->action ==
>> ONCONFLICT_UPDATE)
>>
>> That matches what the error message is reporting afterwards.
>
> Agreed.
>
>> * constraints), so index under consideration can be immediately
>> * skipped if it's not unique
>> */
>> - if (!idxForm->indisunique)
>> + if (!idxForm->indisunique || idxForm->indisexclusion)
>> goto next;
>>
>> Maybe here we need a comment. Or make that a separate statement, like
>
> Yes, that is nice. Done.
>
> Yours,
>
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v46 5/7] Fix a few problems in index build progress reporting.
@ 2026-03-27 15:50 Álvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Álvaro Herrera @ 2026-03-27 15:50 UTC (permalink / raw)
First, index_build() should not update the progress when being driven by
REPACK, because the progress reporting infractructure cannot handle status of
two commands at the same time. So far, REPACK with the CONCURRENTLY option
neglected this problem altogether, but even the existing REPACK wasn't
consistent enough: even if the 'progress' variable in repack_index() was
false, it didn't pass the value to index_build().
Second, REPACK (CONCURRENTLY) should not set PROGRESS_REPACK_PHASE to
PROGRESS_REPACK_PHASE_FINAL_CLEANUP in rebuild_relation() because it calls
finish_heap_swap() anyway (via rebuild_relation_finish_concurrent()), which
does the same thing.
---
src/backend/bootstrap/bootstrap.c | 2 +-
src/backend/catalog/heap.c | 3 ++-
src/backend/catalog/index.c | 22 ++++++++++++++++++----
src/backend/catalog/toasting.c | 3 ++-
src/backend/commands/indexcmds.c | 1 +
src/include/catalog/index.h | 4 +++-
6 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 68a42de0889..1c8226c6d67 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -1181,7 +1181,7 @@ build_indices(void)
heap = table_open(ILHead->il_heap, NoLock);
ind = index_open(ILHead->il_ind, NoLock);
- index_build(heap, ind, ILHead->il_info, false, false);
+ index_build(heap, ind, ILHead->il_info, false, false, false);
index_close(ind, NoLock);
table_close(heap, NoLock);
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 5748aa9a1a9..ae6b7cda3dd 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3570,7 +3570,8 @@ RelationTruncateIndexes(Relation heapRelation)
/* Initialize the index and rebuild */
/* Note: we do not need to re-establish pkey setting */
- index_build(heapRelation, currentIndex, indexInfo, true, false);
+ index_build(heapRelation, currentIndex, indexInfo, true, false,
+ true);
/* We're done with this index */
index_close(currentIndex, NoLock);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index de7182a85a9..cfd33f73f48 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -715,6 +715,9 @@ UpdateIndexRelation(Oid indexoid,
* already exists.
* INDEX_CREATE_PARTITIONED:
* create a partitioned index (table must be partitioned)
+ * INDEX_CREATE_REPORT_PROGRESS:
+ * update the backend's progress information during index build.
+
* constr_flags: flags passed to index_constraint_create
* (only if INDEX_CREATE_ADD_CONSTRAINT is set)
* allow_system_table_mods: allow table to be a system catalog
@@ -760,6 +763,7 @@ index_create(Relation heapRelation,
bool invalid = (flags & INDEX_CREATE_INVALID) != 0;
bool concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
bool partitioned = (flags & INDEX_CREATE_PARTITIONED) != 0;
+ bool progress = (flags & INDEX_CREATE_REPORT_PROGRESS) != 0;
char relkind;
TransactionId relfrozenxid;
MultiXactId relminmxid;
@@ -1276,7 +1280,8 @@ index_create(Relation heapRelation,
}
else
{
- index_build(heapRelation, indexRelation, indexInfo, false, true);
+ index_build(heapRelation, indexRelation, indexInfo, false, true,
+ progress);
}
/*
@@ -1438,6 +1443,12 @@ index_create_copy(Relation heapRelation, bool concurrently,
stattargets[i].isnull = isnull;
}
+ /*
+ * Note: The current callers do not need INDEX_CREATE_REPORT_PROGRESS. If
+ * 'concurrently' is true, there is no build at all. Otherwise the index
+ * build is a sub-command of REPACK. The current infrastructure does not
+ * allow two commands to report their progress at the same time.
+ */
if (concurrently)
flags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
@@ -1528,7 +1539,7 @@ index_concurrently_build(Oid heapRelationId,
indexInfo->ii_BrokenHotChain = false;
/* Now build the index */
- index_build(heapRel, indexRelation, indexInfo, false, true);
+ index_build(heapRel, indexRelation, indexInfo, false, true, true);
/* Roll back any GUC changes executed by index functions */
AtEOXact_GUC(false, save_nestlevel);
@@ -3001,6 +3012,7 @@ index_update_stats(Relation rel,
*
* isreindex indicates we are recreating a previously-existing index.
* parallel indicates if parallelism may be useful.
+ * progress indicates if the backend should update its progress info.
*
* Note: before Postgres 8.2, the passed-in heap and index Relations
* were automatically closed by this routine. This is no longer the case.
@@ -3011,7 +3023,8 @@ index_build(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool isreindex,
- bool parallel)
+ bool parallel,
+ bool progress)
{
IndexBuildResult *stats;
Oid save_userid;
@@ -3062,6 +3075,7 @@ index_build(Relation heapRelation,
RestrictSearchPath();
/* Set up initial progress report status */
+ if (progress)
{
const int progress_index[] = {
PROGRESS_CREATEIDX_PHASE,
@@ -3819,7 +3833,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
/* Initialize the index and rebuild */
/* Note: we do not need to re-establish pkey setting */
- index_build(heapRelation, iRel, indexInfo, true, true);
+ index_build(heapRelation, iRel, indexInfo, true, true, progress);
/* Re-allow use of target index */
ResetReindexProcessing();
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 078a1cf5127..73c41360ae9 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -331,7 +331,8 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_REPORT_PROGRESS, 0,
+ true, true, NULL);
table_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 83edab38760..c9e22e95515 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1233,6 +1233,7 @@ DefineIndex(ParseState *pstate,
flags |= INDEX_CREATE_PARTITIONED;
if (stmt->primary)
flags |= INDEX_CREATE_IS_PRIMARY;
+ flags |= INDEX_CREATE_REPORT_PROGRESS;
/*
* If the table is partitioned, and recursion was declined but partitions
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 56a064ef444..a03238304a0 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -71,6 +71,7 @@ extern void index_check_primary_key(Relation heapRel,
#define INDEX_CREATE_IF_NOT_EXISTS (1 << 4)
#define INDEX_CREATE_PARTITIONED (1 << 5)
#define INDEX_CREATE_INVALID (1 << 6)
+#define INDEX_CREATE_REPORT_PROGRESS (1 << 7)
extern Oid index_create(Relation heapRelation,
const char *indexRelationName,
@@ -148,7 +149,8 @@ extern void index_build(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool isreindex,
- bool parallel);
+ bool parallel,
+ bool progress);
extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
--
2.47.3
--mdsv5bdgfjjj6d77
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v46-0006-Error-out-any-process-that-would-block-at-REPACK.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-03-27 15:50 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-05-09 04:24 Re: SQL:2011 application time Paul Jungwirth <[email protected]>
2024-05-10 00:44 ` Matthias van de Meent <[email protected]>
2024-05-12 03:26 ` Paul Jungwirth <[email protected]>
2024-05-12 12:55 ` Matthias van de Meent <[email protected]>
2024-05-12 15:51 ` Paul Jungwirth <[email protected]>
2024-05-13 04:54 ` Paul Jungwirth <[email protected]>
2024-06-05 20:57 ` Paul Jungwirth <[email protected]>
2024-06-12 15:48 ` Matthias van de Meent <[email protected]>
2024-05-10 13:25 ` Peter Eisentraut <[email protected]>
2026-03-27 15:50 [PATCH v46 5/7] Fix a few problems in index build progress reporting. Álvaro Herrera <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox