Skip to content

Commit 3bba9ce

Browse files
tglsfdctglsfdc
committed
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.
1 parent 472671e commit 3bba9ce

File tree

6 files changed

+94
-70
lines changed

6 files changed

+94
-70
lines changed

‎src/backend/commands/indexcmds.c

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -837,49 +837,62 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
837837
attcollation = attform->attcollation;
838838
ReleaseSysCache(atttuple);
839839
}
840-
else if (attribute->expr && IsA(attribute->expr, Var) &&
841-
((Var *) attribute->expr)->varattno != InvalidAttrNumber)
842-
{
843-
/* Tricky tricky, he wrote (column) ... treat as simple attr */
844-
Var *var = (Var *) attribute->expr;
845-
846-
indexInfo->ii_KeyAttrNumbers[attn] = var->varattno;
847-
atttype = get_atttype(relId, var->varattno);
848-
attcollation = var->varcollid;
849-
}
850840
else
851841
{
852842
/* Index expression */
853-
Assert(attribute->expr != NULL);
854-
indexInfo->ii_KeyAttrNumbers[attn] = 0; /* marks expression */
855-
indexInfo->ii_Expressions = lappend(indexInfo->ii_Expressions,
856-
attribute->expr);
857-
atttype = exprType(attribute->expr);
858-
attcollation = exprCollation(attribute->expr);
843+
Node *expr = attribute->expr;
859844

860-
/*
861-
* We don't currently support generation of an actual query plan
862-
* for an index expression, only simple scalar expressions; hence
863-
* these restrictions.
864-
*/
865-
if (contain_subplans(attribute->expr))
866-
ereport(ERROR,
867-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
868-
errmsg("cannot use subquery in index expression")));
869-
if (contain_agg_clause(attribute->expr))
870-
ereport(ERROR,
871-
(errcode(ERRCODE_GROUPING_ERROR),
872-
errmsg("cannot use aggregate function in index expression")));
845+
Assert(expr != NULL);
846+
atttype = exprType(expr);
847+
attcollation = exprCollation(expr);
873848

874849
/*
875-
* A expression using mutable functions is probably wrong, since
876-
* if you aren't going to get the same result for the same data
877-
* every time, it's not clear what the index entries mean at all.
850+
* Strip any top-level COLLATE clause. This ensures that we treat
851+
* "x COLLATE y" and "(x COLLATE y)" alike.
878852
*/
879-
if (CheckMutability((Expr *) attribute->expr))
880-
ereport(ERROR,
881-
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
882-
errmsg("functions in index expression must be marked IMMUTABLE")));
853+
while (IsA(expr, CollateExpr))
854+
expr = (Node *) ((CollateExpr *) expr)->arg;
855+
856+
if (IsA(expr, Var) &&
857+
((Var *) expr)->varattno != InvalidAttrNumber)
858+
{
859+
/*
860+
* User wrote "(column)" or "(column COLLATE something)".
861+
* Treat it like simple attribute anyway.
862+
*/
863+
indexInfo->ii_KeyAttrNumbers[attn] = ((Var *) expr)->varattno;
864+
}
865+
else
866+
{
867+
indexInfo->ii_KeyAttrNumbers[attn] = 0; /* marks expression */
868+
indexInfo->ii_Expressions = lappend(indexInfo->ii_Expressions,
869+
expr);
870+
871+
/*
872+
* We don't currently support generation of an actual query
873+
* plan for an index expression, only simple scalar
874+
* expressions; hence these restrictions.
875+
*/
876+
if (contain_subplans(expr))
877+
ereport(ERROR,
878+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
879+
errmsg("cannot use subquery in index expression")));
880+
if (contain_agg_clause(expr))
881+
ereport(ERROR,
882+
(errcode(ERRCODE_GROUPING_ERROR),
883+
errmsg("cannot use aggregate function in index expression")));
884+
885+
/*
886+
* A expression using mutable functions is probably wrong,
887+
* since if you aren't going to get the same result for the
888+
* same data every time, it's not clear what the index entries
889+
* mean at all.
890+
*/
891+
if (CheckMutability((Expr *) expr))
892+
ereport(ERROR,
893+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
894+
errmsg("functions in index expression must be marked IMMUTABLE")));
895+
}
883896
}
884897

885898
/*

‎src/backend/utils/adt/ruleutils.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,6 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
794794
List *context;
795795
Oid indrelid;
796796
int keyno;
797-
Oid keycoltype;
798797
Datum indcollDatum;
799798
Datum indclassDatum;
800799
Datum indoptionDatum;
@@ -902,6 +901,8 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
902901
{
903902
AttrNumber attnum = idxrec->indkey.values[keyno];
904903
int16 opt = indoption->values[keyno];
904+
Oid keycoltype;
905+
Oid keycolcollation;
905906

906907
if (!colno)
907908
appendStringInfoString(&buf, sep);
@@ -916,6 +917,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
916917
if (!colno || colno == keyno + 1)
917918
appendStringInfoString(&buf, quote_identifier(attname));
918919
keycoltype = get_atttype(indrelid, attnum);
920+
keycolcollation = get_attcollation(indrelid, attnum);
919921
}
920922
else
921923
{
@@ -939,16 +941,18 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
939941
appendStringInfo(&buf, "(%s)", str);
940942
}
941943
keycoltype = exprType(indexkey);
944+
keycolcollation = exprCollation(indexkey);
942945
}
943946

944947
if (!attrsOnly && (!colno || colno == keyno + 1))
945948
{
946-
Oid coll;
949+
Oid indcoll;
947950

948-
/* Add collation, if not default */
949-
coll = indcollation->values[keyno];
950-
if (coll && coll != DEFAULT_COLLATION_OID && coll != get_attcollation(indrelid, attnum))
951-
appendStringInfo(&buf, " COLLATE %s", generate_collation_name((indcollation->values[keyno])));
951+
/* Add collation, if not default for column */
952+
indcoll = indcollation->values[keyno];
953+
if (OidIsValid(indcoll) && indcoll != keycolcollation)
954+
appendStringInfo(&buf, " COLLATE %s",
955+
generate_collation_name((indcoll)));
952956

953957
/* Add the operator class name, if not default */
954958
get_opclass_name(indclass->values[keyno], keycoltype, &buf);
@@ -6646,7 +6650,8 @@ get_from_clause_coldeflist(List *names, List *types, List *typmods, List *collat
66466650
quote_identifier(attname),
66476651
format_type_with_typemod(atttypid, atttypmod));
66486652
if (attcollation && attcollation != DEFAULT_COLLATION_OID)
6649-
appendStringInfo(buf, " COLLATE %s", generate_collation_name(attcollation));
6653+
appendStringInfo(buf, " COLLATE %s",
6654+
generate_collation_name(attcollation));
66506655
i++;
66516656
}
66526657

‎src/test/regress/expected/collate.linux.utf8.out

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -747,19 +747,21 @@ SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2;
747747
CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
748748
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
749749
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
750-
CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
750+
CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
751+
CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "C"); -- fail
751752
ERROR: collations are not supported by type integer
752-
CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
753+
CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C")); -- fail
753754
ERROR: collations are not supported by type integer
754-
LINE 1: ...ATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C...
755+
LINE 1: ...ATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C...
755756
^
756-
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
757-
relname | pg_get_indexdef
758-
--------------------+----------------------------------------------------------------------------------------------
757+
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
758+
relname | pg_get_indexdef
759+
--------------------+-----------------------------------------------------------------------------------------------------
759760
collate_test1_idx1 | CREATE INDEX collate_test1_idx1 ON collate_test1 USING btree (b)
760761
collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b COLLATE "C")
761-
collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (((b COLLATE "C")) COLLATE "C")
762-
(3 rows)
762+
collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (b COLLATE "C")
763+
collate_test1_idx4 | CREATE INDEX collate_test1_idx4 ON collate_test1 USING btree (((b || 'foo'::text)) COLLATE "POSIX")
764+
(4 rows)
763765

764766
-- schema manipulation commands
765767
CREATE ROLE regress_test_role;

‎src/test/regress/expected/collate.out

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -524,21 +524,23 @@ SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2;
524524

525525
-- indexes
526526
CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
527-
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
528-
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
529-
CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
527+
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "POSIX");
528+
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "POSIX")); -- this is different grammatically
529+
CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
530+
CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "POSIX"); -- fail
530531
ERROR: collations are not supported by type integer
531-
CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
532+
CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "POSIX")); -- fail
532533
ERROR: collations are not supported by type integer
533-
LINE 1: ...ATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C...
534+
LINE 1: ...ATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "P...
534535
^
535-
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
536-
relname | pg_get_indexdef
537-
--------------------+----------------------------------------------------------------------------------------------
536+
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
537+
relname | pg_get_indexdef
538+
--------------------+-----------------------------------------------------------------------------------------------------
538539
collate_test1_idx1 | CREATE INDEX collate_test1_idx1 ON collate_test1 USING btree (b)
539-
collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b)
540-
collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (((b COLLATE "C")) COLLATE "C")
541-
(3 rows)
540+
collate_test1_idx2 | CREATE INDEX collate_test1_idx2 ON collate_test1 USING btree (b COLLATE "POSIX")
541+
collate_test1_idx3 | CREATE INDEX collate_test1_idx3 ON collate_test1 USING btree (b COLLATE "POSIX")
542+
collate_test1_idx4 | CREATE INDEX collate_test1_idx4 ON collate_test1 USING btree (((b || 'foo'::text)) COLLATE "POSIX")
543+
(4 rows)
542544

543545
--
544546
-- Clean up. Many of these table names will be re-used if the user is

‎src/test/regress/sql/collate.linux.utf8.sql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,12 @@ SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2;
231231
CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
232232
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
233233
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
234+
CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
234235

235-
CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
236-
CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
236+
CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "C"); -- fail
237+
CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "C")); -- fail
237238

238-
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
239+
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
239240

240241

241242
-- schema manipulation commands

‎src/test/regress/sql/collate.sql

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,13 +178,14 @@ SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2;
178178
-- indexes
179179

180180
CREATE INDEX collate_test1_idx1 ON collate_test1 (b);
181-
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "C");
182-
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "C")); -- this is different grammatically
181+
CREATE INDEX collate_test1_idx2 ON collate_test1 (b COLLATE "POSIX");
182+
CREATE INDEX collate_test1_idx3 ON collate_test1 ((b COLLATE "POSIX")); -- this is different grammatically
183+
CREATE INDEX collate_test1_idx4 ON collate_test1 (((b||'foo') COLLATE "POSIX"));
183184

184-
CREATE INDEX collate_test1_idx4 ON collate_test1 (a COLLATE "C"); -- fail
185-
CREATE INDEX collate_test1_idx5 ON collate_test1 ((a COLLATE "C")); -- fail
185+
CREATE INDEX collate_test1_idx5 ON collate_test1 (a COLLATE "POSIX"); -- fail
186+
CREATE INDEX collate_test1_idx6 ON collate_test1 ((a COLLATE "POSIX")); -- fail
186187

187-
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%';
188+
SELECT relname, pg_get_indexdef(oid) FROM pg_class WHERE relname LIKE 'collate_test%_idx%' ORDER BY 1;
188189

189190
--
190191
-- Clean up. Many of these table names will be re-used if the user is

0 commit comments

Comments
 (0)