Обсуждение: weird ON CONFLICT clauses
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)
Вложения
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
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