Avoid unnecessary use of pg_strcasecmp for already-downcased identifiers

Enterprise / PostgreSQL - Tom Lane [sss.pgh.pa.us] - 26 January 2018 23:25 EST

We have a lot of code in which option names, which from the user's
viewpoint are logically keywords, are passed through the grammar as plain identifiers, and then matched to string literals during command execution. This approach avoids making words into lexer keywords unnecessarily. Some places matched these strings using plain strcmp, some using pg_strcasecmp. But the latter should be unnecessary since identifiers would have been downcased on their way through the parser. Aside from any efficiency concerns (probably not a big factor), the lack of consistency in this area creates a hazard of subtle bugs due to different places coming to different conclusions about whether two option names are the same or different. Hence, standardize on using strcmp() to match any option names that are expected to have been fed through the parser.

This does create a user-visible behavioral change, which is that while formerly all of these would work: alter table foo set (fillfactor = 50); alter table foo set (FillFactor = 50); alter table foo set ("fillfactor" = 50); alter table foo set ("FillFactor" = 50); now the last case will fail because that double-quoted identifier is different from the others. However, none of our documentation says that you can use a quoted identifier in such contexts at all, and we should discourage doing so since it would break if we ever decide to parse such constructs as true lexer keywords rather than poor man's substitutes. So this shouldn't create a significant compatibility issue for users.

Daniel Gustafsson, reviewed by Michael Paquier, small changes by me

Discussion: https://postgr.es/m/29405B24-564E-476B-98C0-677A29805B84@yesql.se

fb8697b31a Avoid unnecessary use of pg_strcasecmp for already-downcased identifiers.
contrib/dict_int/dict_int.c | 4 +-
contrib/dict_xsyn/dict_xsyn.c | 10 ++---
contrib/unaccent/unaccent.c | 2 +-
doc/src/sgml/textsearch.sgml | 1 +
src/backend/access/common/reloptions.c | 22 +++++-----
src/backend/commands/aggregatecmds.c | 56 +++++++++++++-------------
src/backend/commands/collationcmds.c | 12 +++---
src/backend/commands/operatorcmds.c | 44 ++++++++++----------
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/tsearchcmds.c | 23 +++++------
src/backend/commands/typecmds.c | 48 +++++++++++-----------
src/backend/commands/view.c | 6 +--
src/backend/parser/parse_clause.c | 2 +-
src/backend/snowball/dict_snowball.c | 4 +-
src/backend/tsearch/dict_ispell.c | 6 +--
src/backend/tsearch/dict_simple.c | 4 +-
src/backend/tsearch/dict_synonym.c | 4 +-
src/backend/tsearch/dict_thesaurus.c | 4 +-
src/include/access/reloptions.h | 2 +-
src/test/regress/expected/aggregates.out | 15 +++----
src/test/regress/expected/alter_generic.out | 6 +++
src/test/regress/expected/alter_operator.out | 3 ++
src/test/regress/expected/collate.out | 5 +++
src/test/regress/expected/create_aggregate.out | 30 ++++++++++++++
src/test/regress/expected/create_operator.out | 23 +++++++++++
src/test/regress/expected/create_table.out | 5 +++
src/test/regress/expected/create_type.out | 28 +++++++++++++
src/test/regress/expected/tsdicts.out | 8 ++++
src/test/regress/sql/aggregates.sql | 15 +++----
src/test/regress/sql/alter_generic.sql | 6 +++
src/test/regress/sql/alter_operator.sql | 3 ++
src/test/regress/sql/collate.sql | 2 +
src/test/regress/sql/create_aggregate.sql | 20 +++++++++
src/test/regress/sql/create_operator.sql | 14 +++++++
src/test/regress/sql/create_table.sql | 4 ++
src/test/regress/sql/create_type.sql | 10 +++++
src/test/regress/sql/tsdicts.sql | 8 ++++
37 files changed, 318 insertions(+), 143 deletions(-)

Upstream: git.postgresql.org


  • Share