Reject opclass options in ON CONFLICT clause

It's as pointless as ASC/DESC and NULLS FIRST/LAST are, so reject all of
them in the same way.  While at it, normalize the others' error messages
to have less translatable strings.  Add tests for these errors.

Noticed while reviewing recent INSERT ON CONFLICT patches.

Author: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/202511271516.oiefpvn3z27m@alvherre.pgsql

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 944482207f3857684dae1a9d68f82b0c905865c6..57609e2d55c4033c71e02bafe665417b4d9ee93b 100644 (file)
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3277,24 +3277,32 @@ resolve_unique_index_expr(ParseState *pstate, InferClause *infer,
         * Raw grammar re-uses CREATE INDEX infrastructure for unique index
         * inference clause, and so will accept opclasses by name and so on.
         *
         * Raw grammar re-uses CREATE INDEX infrastructure for unique index
         * inference clause, and so will accept opclasses by name and so on.
         *

-        * Make no attempt to match ASC or DESC ordering or NULLS FIRST/NULLS
-        * LAST ordering, since those are not significant for inference
-        * purposes (any unique index matching the inference specification in
-        * other regards is accepted indifferently).  Actively reject this as
-        * wrong-headed.
+        * Make no attempt to match ASC or DESC ordering, NULLS FIRST/NULLS
+        * LAST ordering or opclass options, since those are not significant
+        * for inference purposes (any unique index matching the inference
+        * specification in other regards is accepted indifferently). Actively
+        * reject this as wrong-headed.

         */
        if (ielem->ordering != SORTBY_DEFAULT)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
         */
        if (ielem->ordering != SORTBY_DEFAULT)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),

-                    errmsg("ASC/DESC is not allowed in ON CONFLICT clause"),
+                    errmsg("%s is not allowed in ON CONFLICT clause",
+                           "ASC/DESC"),

                     parser_errposition(pstate,
                                        exprLocation((Node *) infer))));
        if (ielem->nulls_ordering != SORTBY_NULLS_DEFAULT)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
                     parser_errposition(pstate,
                                        exprLocation((Node *) infer))));
        if (ielem->nulls_ordering != SORTBY_NULLS_DEFAULT)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),

-                    errmsg("NULLS FIRST/LAST is not allowed in ON CONFLICT clause"),
+                    errmsg("%s is not allowed in ON CONFLICT clause",
+                           "NULLS FIRST/LAST"),

                     parser_errposition(pstate,
                                        exprLocation((Node *) infer))));
                     parser_errposition(pstate,
                                        exprLocation((Node *) infer))));

+       if (ielem->opclassopts)
+           ereport(ERROR,
+                   errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+                   errmsg("operator class options are not allowed in ON CONFLICT clause"),
+                   parser_errposition(pstate,
+                                      exprLocation((Node *) infer)));

 
        if (!ielem->expr)
        {
 
        if (!ielem->expr)
        {

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index db6684746842fec5be8e53767bfb57bb3b465ca7..91fbe91844d2a38915e5c788a652b882a09b3830 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -2,6 +2,19 @@
 -- insert...on conflict do unique index inference
 --
 create table insertconflicttest(key int4, fruit text);
 -- insert...on conflict do unique index inference
 --
 create table insertconflicttest(key int4, fruit text);

+-- invalid clauses
+insert into insertconflicttest values (1) on conflict (key int4_ops (fillfactor=10)) do nothing;
+ERROR:  operator class options are not allowed in ON CONFLICT clause
+LINE 1: ...rt into insertconflicttest values (1) on conflict (key int4_...
+                                                             ^
+insert into insertconflicttest values (1) on conflict (key asc) do nothing;
+ERROR:  ASC/DESC is not allowed in ON CONFLICT clause
+LINE 1: ...rt into insertconflicttest values (1) on conflict (key asc) ...
+                                                             ^
+insert into insertconflicttest values (1) on conflict (key nulls last) do nothing;
+ERROR:  NULLS FIRST/LAST is not allowed in ON CONFLICT clause
+LINE 1: ...rt into insertconflicttest values (1) on conflict (key nulls...
+                                                             ^

 -- These things should work through a view, as well
 create view insertconflictview as select * from insertconflicttest;
 --
 -- These things should work through a view, as well
 create view insertconflictview as select * from insertconflicttest;
 --

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 549c46452ec0967f4c2bc290f05375c98e0f910d..03b1f0e44b057617f08a821433f55535487c2549 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -3,6 +3,11 @@
 --
 create table insertconflicttest(key int4, fruit text);
 
 --
 create table insertconflicttest(key int4, fruit text);
 

+-- invalid clauses
+insert into insertconflicttest values (1) on conflict (key int4_ops (fillfactor=10)) do nothing;
+insert into insertconflicttest values (1) on conflict (key asc) do nothing;
+insert into insertconflicttest values (1) on conflict (key nulls last) do nothing;
+

 -- These things should work through a view, as well
 create view insertconflictview as select * from insertconflicttest;
 
 -- These things should work through a view, as well
 create view insertconflictview as select * from insertconflicttest;