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,
