On 30.04.24 18:39, Paul Jungwirth wrote:
On 4/30/24 09:24, Robert Haas wrote:
Peter, could you have a look at
http://postgr.es/m/47550967-260b-4180-9791-b224859fe...@illuminatedcomputing.com
and express an opinion about whether each of those proposals are (a)
good or bad ideas and (b) whether they need to be fixed for the
current release?
Here are the same patches but rebased.
I have committed
v2-0002-Add-test-for-REPLICA-IDENTITY-with-a-temporal-key.patch.
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.
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.
--- 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.
* 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
/* not supported yet etc. */
if (idxForm->indixexclusion)
next;
