Clean up handling of COLLATE clauses in index column definitions.

Ensure that COLLATE at the top level of an index expression is treated the
same as a grammatically separate COLLATE.  Fix bogus reverse-parsing logic
in pg_get_indexdef.

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index dafa6d5efc16e424d4ee5de030899f6fb6cc682a..cfcce559675a177d9f9afb0f51c44797c506194e 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -837,49 +837,62 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
            attcollation = attform->attcollation;
            ReleaseSysCache(atttuple);
        }
-       else if (attribute->expr && IsA(attribute->expr, Var) &&
-                ((Var *) attribute->expr)->varattno != InvalidAttrNumber)
-       {
-           /* Tricky tricky, he wrote (column) ... treat as simple attr */
-           Var        *var = (Var *) attribute->expr;
-
-           indexInfo->ii_KeyAttrNumbers[attn] = var->varattno;
-           atttype = get_atttype(relId, var->varattno);
-           attcollation = var->varcollid;
-       }
        else
        {
            /* Index expression */
-           Assert(attribute->expr != NULL);
-           indexInfo->ii_KeyAttrNumbers[attn] = 0;     /* marks expression */
-           indexInfo->ii_Expressions = lappend(indexInfo->ii_Expressions,
-                                               attribute->expr);
-           atttype = exprType(attribute->expr);
-           attcollation = exprCollation(attribute->expr);
+           Node   *expr = attribute->expr;
 
-           /*
-            * We don't currently support generation of an actual query plan
-            * for an index expression, only simple scalar expressions; hence
-            * these restrictions.
-            */
-           if (contain_subplans(attribute->expr))
-               ereport(ERROR,
-                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("cannot use subquery in index expression")));
-           if (contain_agg_clause(attribute->expr))
-               ereport(ERROR,
-                       (errcode(ERRCODE_GROUPING_ERROR),
-               errmsg("cannot use aggregate function in index expression")));
+           Assert(expr != NULL);
+           atttype = exprType(expr);
+           attcollation = exprCollation(expr);
 
            /*
-            * A expression using mutable functions is probably wrong, since
-            * if you aren't going to get the same result for the same data
-            * every time, it's not clear what the index entries mean at all.
+            * Strip any top-level COLLATE clause.  This ensures that we treat
+            * "x COLLATE y" and "(x COLLATE y)" alike.
             */
-           if (CheckMutability((Expr *) attribute->expr))
-               ereport(ERROR,
-                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                        errmsg("functions in index expression must be marked IMMUTABLE")));
+           while (IsA(expr, CollateExpr))
+               expr = (Node *) ((CollateExpr *) expr)->arg;
+
+           if (IsA(expr, Var) &&
+               ((Var *) expr)->varattno != InvalidAttrNumber)
+           {
+               /*
+                * User wrote "(column)" or "(column COLLATE something)".
+                * Treat it like simple attribute anyway.
+                */
+               indexInfo->ii_KeyAttrNumbers[attn] = ((Var *) expr)->varattno;
+           }
+           else
+           {
+               indexInfo->ii_KeyAttrNumbers[attn] = 0; /* marks expression */
+               indexInfo->ii_Expressions = lappend(indexInfo->ii_Expressions,
+                                                   expr);
+
+               /*
+                * We don't currently support generation of an actual query
+                * plan for an index expression, only simple scalar
+                * expressions; hence these restrictions.
+                */
+               if (contain_subplans(expr))
+                   ereport(ERROR,
+                           (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                            errmsg("cannot use subquery in index expression")));
+               if (contain_agg_clause(expr))
+                   ereport(ERROR,
+                           (errcode(ERRCODE_GROUPING_ERROR),
+                            errmsg("cannot use aggregate function in index expression")));
+
+               /*
+                * A expression using mutable functions is probably wrong,
+                * since if you aren't going to get the same result for the
+                * same data every time, it's not clear what the index entries
+                * mean at all.
+                */
+               if (CheckMutability((Expr *) expr))
+                   ereport(ERROR,
+                           (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                            errmsg("functions in index expression must be marked IMMUTABLE")));
+           }
        }
 
        /*

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 573c8dd410482e5c14e56f57ccfc7d6616805baf..621f1eb24ae73293fa8197ef967e09a35257a43c 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -794,7 +794,6 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
    List       *context;
    Oid         indrelid;
    int         keyno;
-   Oid         keycoltype;
    Datum       indcollDatum;
    Datum       indclassDatum;
    Datum       indoptionDatum;
@@ -902,6 +901,8 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
    {
        AttrNumber  attnum = idxrec->indkey.values[keyno];
        int16       opt = indoption->values[keyno];
+       Oid         keycoltype;
+       Oid         keycolcollation;
 
        if (!colno)
            appendStringInfoString(&buf, sep);
@@ -916,6 +917,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
            if (!colno || colno == keyno + 1)
                appendStringInfoString(&buf, quote_identifier(attname));
            keycoltype = get_atttype(indrelid, attnum);
+           keycolcollation = get_attcollation(indrelid, attnum);
        }
        else
        {
@@ -939,16 +941,18 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
                    appendStringInfo(&buf, "(%s)", str);
            }
            keycoltype = exprType(indexkey);
+           keycolcollation = exprCollation(indexkey);
        }
 
        if (!attrsOnly && (!colno || colno == keyno + 1))
        {
-           Oid coll;
+           Oid     indcoll;
 
-           /* Add collation, if not default */
-           coll = indcollation->values[keyno];
-           if (coll && coll != DEFAULT_COLLATION_OID && coll != get_attcollation(indrelid, attnum))
-               appendStringInfo(&buf, " COLLATE %s", generate_collation_name((indcollation->values[keyno])));
+           /* Add collation, if not default for column */
+           indcoll = indcollation->values[keyno];
+           if (OidIsValid(indcoll) && indcoll != keycolcollation)
+               appendStringInfo(&buf, " COLLATE %s",
+                                generate_collation_name((indcoll)));
 
            /* Add the operator class name, if not default */
            get_opclass_name(indclass->values[keyno], keycoltype, &buf);
@@ -6646,7 +6650,8 @@ get_from_clause_coldeflist(List *names, List *types, List *typmods, List *collat
                         quote_identifier(attname),
                         format_type_with_typemod(atttypid, atttypmod));
        if (attcollation && attcollation != DEFAULT_COLLATION_OID)
-           appendStringInfo(buf, " COLLATE %s", generate_collation_name(attcollation));
+           appendStringInfo(buf, " COLLATE %s",
+                            generate_collation_name(attcollation));
        i++;
    }
 

diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out
index b03395b35a9a7f6e76ec4504f5c8de2335749068..879f97327d154fab76544bbc7fa465b9295782f5 100644 (file)
--- a/src/test/regress/expected/collate.linux.utf8.out
+++ b/src/test/regress/expected/collate.linux.utf8.out
@@ -747,19 +747,21 @@ SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2;
 CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
 CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
 CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
-CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
+CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
+CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "C"); -- fail
 ERROR:  collations are not supported by type integer
-CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
+CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C")); -- fail
 ERROR:  collations are not supported by type integer
-LINE 1: ...ATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C...
+LINE 1: ...ATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C...
                                                              ^
-SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
-      relname       |                                       pg_get_indexdef                                        
---------------------+----------------------------------------------------------------------------------------------
+SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
+      relname       |                                           pg_get_indexdef                                           
+--------------------+-----------------------------------------------------------------------------------------------------
  collate_test1_idx1 | CREATE INDEX collate_test1_idx1 ON collate_test1 USING btree (b)
  collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b COLLATE "C")
- collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (((b COLLATE "C")) COLLATE "C")
-(3 rows)
+ collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (b COLLATE "C")
+ collate_test1_idx4 | CREATE INDEX collate_test1_idx4 ON collate_test1 USING btree (((b || 'foo'::text)) COLLATE "POSIX")
+(4 rows)
 
 -- schema manipulation commands
 CREATE ROLE regress_test_role;

diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out
index 0ed9601bf2147448b365d0c2249fc68989c77198..f1a025ae3731485dff7644c43fadaf2f3ccf74e1 100644 (file)
--- a/src/test/regress/expected/collate.out
+++ b/src/test/regress/expected/collate.out
@@ -524,21 +524,23 @@ SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2;
 
 -- indexes
 CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
-CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
-CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
-CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
+CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "POSIX");
+CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "POSIX")); -- this is different grammatically
+CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
+CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "POSIX"); -- fail
 ERROR:  collations are not supported by type integer
-CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
+CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "POSIX")); -- fail
 ERROR:  collations are not supported by type integer
-LINE 1: ...ATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C...
+LINE 1: ...ATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "P...
                                                              ^
-SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
-      relname       |                                       pg_get_indexdef                                        
---------------------+----------------------------------------------------------------------------------------------
+SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
+      relname       |                                           pg_get_indexdef                                           
+--------------------+-----------------------------------------------------------------------------------------------------
  collate_test1_idx1 | CREATE INDEX collate_test1_idx1 ON collate_test1 USING btree (b)
- collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b)
- collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (((b COLLATE "C")) COLLATE "C")
-(3 rows)
+ collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b COLLATE "POSIX")
+ collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (b COLLATE "POSIX")
+ collate_test1_idx4 | CREATE INDEX collate_test1_idx4 ON collate_test1 USING btree (((b || 'foo'::text)) COLLATE "POSIX")
+(4 rows)
 
 --
 -- Clean up.  Many of these table names will be re-used if the user is

diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql
index f5c5d802b293bef5e93262e079f69f18ff69376f..4aec27d88013a826525e5732f545b7d058d5925f 100644 (file)
--- a/src/test/regress/sql/collate.linux.utf8.sql
+++ b/src/test/regress/sql/collate.linux.utf8.sql
@@ -231,11 +231,12 @@ SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2;
 CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
 CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
 CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
+CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
 
-CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
-CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
+CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "C"); -- fail
+CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C")); -- fail
 
-SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
+SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
 
 
 -- schema manipulation commands

diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql
index c7e26d8577bc3b98d0474972ec46d22f2d1a1e1b..c904aa77f37f19b49b0507387d00b6f79cf1c218 100644 (file)
--- a/src/test/regress/sql/collate.sql
+++ b/src/test/regress/sql/collate.sql
@@ -178,13 +178,14 @@ SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2;
 -- indexes
 
 CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
-CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
-CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
+CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "POSIX");
+CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "POSIX")); -- this is different grammatically
+CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
 
-CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
-CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
+CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "POSIX"); -- fail
+CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "POSIX")); -- fail
 
-SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
+SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
 
 --
 -- Clean up.  Many of these table names will be re-used if the user is