From 3bba9ce945a702ab116fcedb9c0b970ecd69c9dd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 24 Mar 2011 15:29:52 -0400 Subject: [PATCH] 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. --- src/backend/commands/indexcmds.c | 85 ++++++++++++++---------- src/backend/utils/adt/ruleutils.c | 19 ++++-- src/test/regress/expected/collate.linux.utf8.out | 18 ++--- src/test/regress/expected/collate.out | 24 ++++--- src/test/regress/sql/collate.linux.utf8.sql | 7 +- src/test/regress/sql/collate.sql | 11 +-- 6 files changed, 94 insertions(+), 70 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index dafa6d5efc..cfcce55967 100644 --- 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 573c8dd410..621f1eb24a 100644 --- 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 b03395b35a..879f97327d 100644 --- 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 0ed9601bf2..f1a025ae37 100644 --- 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 f5c5d802b2..4aec27d880 100644 --- 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 c7e26d8577..c904aa77f3 100644 --- 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 -- 2.11.0