Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add parse location to IndexElem.

  2. Reject opclass options in ON CONFLICT clause

  1. weird ON CONFLICT clauses

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-27T16:00:24Z

    Hi,

While reviewing a patch I noticed that we allow some extraneous items in
ON CONFLICT clauses -- for instance,

create table tab (a int unique, b int);
insert into tab values (1, 1) on conflict (a int4_ops (fillfactor=10)) do nothing;

Why do we accept reloptions there without complaint?  Should we tighten
this up a little bit, or maybe it makes sense to accept this for some
reason?  I suspect the reloptions were added to index_elems after the ON
CONFLICT clause was made to use that production, but I didn't check the
git history.

So what about the attached patch?  I ran all tests and everything seems
to work correctly.  (Maybe I'd add some tests to verify that this
new error is covered, as the ones just above.)  It would complain to the
above:

ERROR:  operator class options are not allowed in ON CONFLICT clause
LÍNEA 1: insert into tab values (1, 1) on conflict (a int4_ops (fillf...
                                                   ^


This is certainly not very critical.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"El Maquinismo fue proscrito so pena de cosquilleo hasta la muerte"
(Ijon Tichy en Viajes, Stanislaw Lem)

  2. Re: weird ON CONFLICT clauses

    Peter Geoghegan <pg@bowt.ie> — 2025-11-27T16:40:18Z

    On Thu, Nov 27, 2025 at 11:00 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> Why do we accept reloptions there without complaint?  Should we tighten
> this up a little bit, or maybe it makes sense to accept this for some
> reason?  I suspect the reloptions were added to index_elems after the ON
> CONFLICT clause was made to use that production, but I didn't check the
> git history.

index_elems is needed by ON CONFLICT so that the user can specify an
operator class and/or a collation. This is probably hardly ever used,
but it does have its place.

> So what about the attached patch?  I ran all tests and everything seems
> to work correctly.  (Maybe I'd add some tests to verify that this
> new error is covered, as the ones just above.)  It would complain to the
> above:

Seems reasonable to me.

-- 
Peter Geoghegan

  3. Re: weird ON CONFLICT clauses

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-12-12T13:33:50Z

    On 2025-Nov-27, Peter Geoghegan wrote:

> On Thu, Nov 27, 2025 at 11:00 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:

> index_elems is needed by ON CONFLICT so that the user can specify an
> operator class and/or a collation. This is probably hardly ever used,
> but it does have its place.

Right.

> > So what about the attached patch?  I ran all tests and everything seems
> > to work correctly.  (Maybe I'd add some tests to verify that this
> > new error is covered, as the ones just above.)  It would complain to the
> > above:
> 
> Seems reasonable to me.

Pushed, thanks for looking.

While looking at the test output, I wondered if it would be useful to
make the error cursor point to the bogus element itself rather than to
the overall InferClause.  At the moment it doesn't look terribly useful,
so I'm parking this patch here; but if somebody were to be motivated to,
say, patch ComputeIndexAttrs to have a ParseState, we could add error
location to the ereports there.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are."  -- Charles J. Sykes' advice to teenagers