public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Paul Jungwirth <[email protected]>
To: Robert Haas <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 application time
Date: Wed, 8 May 2024 15:51:28 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com>
	<[email protected]>
	<CACJufxF8HyERm7SKBJ+duxAP9sTgVhaW6b-oWADANGgn-o4wPA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CACJufxHsYDfHK62FTHdLsgDenvJd_bEuxCiyH62Lacj7NvzX4w@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CA+TgmoaH-2fAttd24pA3p45TUHbM9hRU6ud=brhKsp9ENHXWDg@mail.gmail.com>
	<[email protected]>
	<CA+TgmobfDJuXJATNUco+g6AiUb3WS2Pi9FrJe9hKAHGw37RX-g@mail.gmail.com>
	<[email protected]>

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/[email protected]
>> 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;







view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: SQL:2011 application time
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox