From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Paul Jungwirth <pj(at)illuminatedcomputing(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: SQL:2011 application time |
Date: | 2024-05-08 13:51:28 |
Message-ID: | 731ef81e-d4c3-41c6-ba8f-f6131f4f6407@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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-b224859fe63e@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;
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2024-05-08 13:55:50 | Re: BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL); |
Previous Message | Masahiko Sawada | 2024-05-08 13:46:52 | Re: Fix parallel vacuum buffer usage reporting |