Re: SQL:2011 application time

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 ~{:-)
pj(at)illuminatedcomputing(dot)com