On Mon, Jan 22, 2018 at 12:48:45PM +0100, Daniel Gustafsson wrote: > Gotcha. I’ve added some tests to the patch. The test for CREATE > FUNCTION was omitted for now awaiting the outcome of the discussion > around isStrict and isCachable.
That makes sense. > Not sure how much they’re worth in "make check” though as it’s quite > easy to add a new option checked with pg_strcasecmp which then isn’t > tested. Still, it might aid review so definitely worth it. Thanks for making those, this eases the review lookups. There are a couple of code paths that remained untested: - contrib/unaccent/ - contrib/dict_xsyn/ - contrib/dict_int/ - The combinations of toast reloptions is pretty particular as option namespaces also go through pg_strcasecmp, so I think that those should be tested as well (your patch includes a test for fillfactor, which is a good base by the way). - check_option for CREATE VIEW and ALTER VIEW SET. - ALTER TEXT SEARCH CONFIGURATION for copy/parser options. - CREATE TYPE AS RANGE with DefineRange(). There are as well two things I have spotted on the way: 1) fillRelOptions() can safely use strcmp. 2) indexam.sgml mentions using pg_strcasecmp() for consistency with the core code when defining amproperty for an index AM. Well, with this patch I think that for consistency with the core code that would involve using strcmp instead, extension developers can of course still use pg_strcasecmp. Those are changed as well in the attached, which applies on top of your v6. I have added as well in it the tests I spotted were missing. If this looks right to you, I am fine to switch this patch as ready for committer, without considering the issues with isCachable and isStrict in CREATE FUNCTION of course. -- Michael
diff --git a/contrib/dict_int/expected/dict_int.out b/contrib/dict_int/expected/dict_int.out
index 3b766ec52a..20d172c730 100644
--- a/contrib/dict_int/expected/dict_int.out
+++ b/contrib/dict_int/expected/dict_int.out
@@ -300,3 +300,6 @@ select ts_lexize('intdict', '314532610153');
{314532}
(1 row)
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY intdict ("Maxlen" = 4);
+ERROR: unrecognized intdict parameter: "Maxlen"
diff --git a/contrib/dict_int/sql/dict_int.sql b/contrib/dict_int/sql/dict_int.sql
index 8ffec6b770..05d2149101 100644
--- a/contrib/dict_int/sql/dict_int.sql
+++ b/contrib/dict_int/sql/dict_int.sql
@@ -51,3 +51,6 @@ select ts_lexize('intdict', '252281774');
select ts_lexize('intdict', '313425');
select ts_lexize('intdict', '641439323669');
select ts_lexize('intdict', '314532610153');
+
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY intdict ("Maxlen" = 4);
diff --git a/contrib/dict_xsyn/expected/dict_xsyn.out b/contrib/dict_xsyn/expected/dict_xsyn.out
index 9b95e13559..4cc42956c7 100644
--- a/contrib/dict_xsyn/expected/dict_xsyn.out
+++ b/contrib/dict_xsyn/expected/dict_xsyn.out
@@ -140,3 +140,6 @@ SELECT ts_lexize('xsyn', 'grb');
(1 row)
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY xsyn ("Matchorig" = false);
+ERROR: unrecognized xsyn parameter: "Matchorig"
diff --git a/contrib/dict_xsyn/sql/dict_xsyn.sql b/contrib/dict_xsyn/sql/dict_xsyn.sql
index 49511061d0..ab1380c0a2 100644
--- a/contrib/dict_xsyn/sql/dict_xsyn.sql
+++ b/contrib/dict_xsyn/sql/dict_xsyn.sql
@@ -43,3 +43,6 @@ ALTER TEXT SEARCH DICTIONARY xsyn (RULES='xsyn_sample', KEEPORIG=false, MATCHORI
SELECT ts_lexize('xsyn', 'supernova');
SELECT ts_lexize('xsyn', 'sn');
SELECT ts_lexize('xsyn', 'grb');
+
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY xsyn ("Matchorig" = false);
diff --git a/contrib/unaccent/expected/unaccent.out b/contrib/unaccent/expected/unaccent.out
index b93105e9c7..30c061fe51 100644
--- a/contrib/unaccent/expected/unaccent.out
+++ b/contrib/unaccent/expected/unaccent.out
@@ -61,3 +61,6 @@ SELECT ts_lexize('unaccent', '
{}
(1 row)
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY unaccent ("Rules" = 'my_rules');
+ERROR: unrecognized Unaccent parameter: "Rules"
diff --git a/contrib/unaccent/sql/unaccent.sql b/contrib/unaccent/sql/unaccent.sql
index 310213994f..f18f934377 100644
--- a/contrib/unaccent/sql/unaccent.sql
+++ b/contrib/unaccent/sql/unaccent.sql
@@ -16,3 +16,6 @@ SELECT unaccent('unaccent', '
SELECT ts_lexize('unaccent', 'foobar');
SELECT ts_lexize('unaccent', '');
SELECT ts_lexize('unaccent', '');
+
+-- invalid: non-lowercase quoted identifiers
+ALTER TEXT SEARCH DICTIONARY unaccent ("Rules" = 'my_rules');
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index a7f6c8dc6a..6a5d307c69 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -439,7 +439,7 @@ amproperty (Oid index_oid, int attno,
If the core code does not recognize the property name
then <parameter>prop</parameter> is <literal>AMPROP_UNKNOWN</literal>.
Access methods can define custom property names by
- checking <parameter>propname</parameter> for a match (use <function>pg_strcasecmp</function>
+ checking <parameter>propname</parameter> for a match (use <function>strcmp</function>
to match, for consistency with the core code); for names known to the core
code, it's better to inspect <parameter>prop</parameter>.
If the <structfield>amproperty</structfield> method returns <literal>true</literal> then
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index a6de891eec..c82c28d781 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1260,7 +1260,7 @@ fillRelOptions(void *rdopts, Size basesize,
for (j = 0; j < numelems; j++)
{
- if (pg_strcasecmp(options[i].gen->name, elems[j].optname) == 0)
+ if (strcmp(options[i].gen->name, elems[j].optname) == 0)
{
relopt_string *optstring;
char *itempos = ((char *) rdopts) + elems[j].offset;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 552d7b09dc..0a34eddac2 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -866,3 +866,14 @@ DROP TABLE parted_col_comment;
-- invalid: non-lowercase quoted reloptions identifiers
CREATE TABLE tas_case WITH ("Fillfactor" = 10) AS SELECT 1 a;
ERROR: unrecognized parameter "Fillfactor"
+CREATE TABLE tas_case (a text) WITH ("Oids" = true);
+ERROR: unrecognized parameter "Oids"
+-- options with namespaces used with non-lowercase and quotes, all should fail
+CREATE TABLE tas_case (a text) WITH ("Toast"."Autovacuum_vacuum_cost_limit" = 1);
+ERROR: unrecognized parameter namespace "Toast"
+CREATE TABLE tas_case (a text) WITH ("Toast.Autovacuum_vacuum_cost_limit" = 1);
+ERROR: unrecognized parameter "Toast.Autovacuum_vacuum_cost_limit"
+CREATE TABLE tas_case (a text) WITH ("Toast".autovacuum_vacuum_cost_limit = 1);
+ERROR: unrecognized parameter namespace "Toast"
+CREATE TABLE tas_case (a text) WITH (toast."Autovacuum_vacuum_cost_limit" = 1);
+ERROR: unrecognized parameter "Autovacuum_vacuum_cost_limit"
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index 2139860861..fd2a81a930 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -182,3 +182,5 @@ WARNING: type attribute "Passedbyvalue" not recognized
LINE 7: "Passedbyvalue"
^
ERROR: type input function must be specified
+CREATE TYPE int4_range AS RANGE ("Subtype" = int4, "Subtype_diff" = int4mi);
+ERROR: type attribute "Subtype" not recognized
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 4468c85d77..a3d8869908 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -1712,3 +1712,13 @@ DROP SCHEMA temp_view_test CASCADE;
NOTICE: drop cascades to 27 other objects
DROP SCHEMA testviewschm2 CASCADE;
NOTICE: drop cascades to 62 other objects
+-- invalid: non-lowercase quoted identifiers
+-- Check for both CREATE VIEW and ALTER VIEW SET
+CREATE TABLE tt24 (a int);
+CREATE VIEW tt24v WITH ("Check_option" = 'local') AS SELECT * FROM tt24;
+ERROR: unrecognized parameter "Check_option"
+CREATE VIEW tt24v WITH (check_option = 'local') AS SELECT * FROM tt24;
+ALTER VIEW tt24v SET "Check_option" = 'local';
+ERROR: syntax error at or near ""Check_option"" at character 22
+DROP TABLE tt24 CASCADE;
+NOTICE: drop cascades to view tt24v
diff --git a/src/test/regress/expected/tsdicts.out b/src/test/regress/expected/tsdicts.out
index e01402d42b..b3141ce59e 100644
--- a/src/test/regress/expected/tsdicts.out
+++ b/src/test/regress/expected/tsdicts.out
@@ -590,3 +590,5 @@ CREATE TEXT SEARCH DICTIONARY tsdict_case
ERROR: text search template is required
ALTER TEXT SEARCH DICTIONARY tsdict_case ("Dictfile" = ispell_sample);
ERROR: text search dictionary "tsdict_case" does not exist
+CREATE TEXT SEARCH CONFIGURATION thesaurus_tst ("Copy" = synonym_tst);
+ERROR: text search configuration parameter "Copy" not recognized
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 3142817b22..0f9f75fda6 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -710,3 +710,9 @@ DROP TABLE parted_col_comment;
-- invalid: non-lowercase quoted reloptions identifiers
CREATE TABLE tas_case WITH ("Fillfactor" = 10) AS SELECT 1 a;
+CREATE TABLE tas_case (a text) WITH ("Oids" = true);
+-- options with namespaces used with non-lowercase and quotes, all should fail
+CREATE TABLE tas_case (a text) WITH ("Toast"."Autovacuum_vacuum_cost_limit" = 1);
+CREATE TABLE tas_case (a text) WITH ("Toast.Autovacuum_vacuum_cost_limit" = 1);
+CREATE TABLE tas_case (a text) WITH ("Toast".autovacuum_vacuum_cost_limit = 1);
+CREATE TABLE tas_case (a text) WITH (toast."Autovacuum_vacuum_cost_limit" = 1);
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index ef9acdb4f6..c990813d1a 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -136,7 +136,6 @@ SELECT format_type(atttypid,atttypmod) FROM pg_attribute
WHERE attrelid = 'mytab'::regclass AND attnum > 0;
-- invalid: non-lowercase quoted identifiers
-
CREATE TYPE case_int42 (
"Internallength" = 4,
"Input" = int42_in,
@@ -145,3 +144,4 @@ CREATE TYPE case_int42 (
"Default" = 42,
"Passedbyvalue"
);
+CREATE TYPE int4_range AS RANGE ("Subtype" = int4, "Subtype_diff" = int4mi);
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index b4e7a8793c..ec4f5203ab 100644
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -584,3 +584,11 @@ select pg_get_ruledef(oid, true) from pg_rewrite
\set VERBOSITY terse \\ -- suppress cascade details
DROP SCHEMA temp_view_test CASCADE;
DROP SCHEMA testviewschm2 CASCADE;
+
+-- invalid: non-lowercase quoted identifiers
+-- Check for both CREATE VIEW and ALTER VIEW SET
+CREATE TABLE tt24 (a int);
+CREATE VIEW tt24v WITH ("Check_option" = 'local') AS SELECT * FROM tt24;
+CREATE VIEW tt24v WITH (check_option = 'local') AS SELECT * FROM tt24;
+ALTER VIEW tt24v SET "Check_option" = 'local';
+DROP TABLE tt24 CASCADE;
diff --git a/src/test/regress/sql/tsdicts.sql b/src/test/regress/sql/tsdicts.sql
index 77ba073cca..75513600de 100644
--- a/src/test/regress/sql/tsdicts.sql
+++ b/src/test/regress/sql/tsdicts.sql
@@ -198,3 +198,4 @@ CREATE TEXT SEARCH DICTIONARY tsdict_case
);
ALTER TEXT SEARCH DICTIONARY tsdict_case ("Dictfile" = ispell_sample);
+CREATE TEXT SEARCH CONFIGURATION thesaurus_tst ("Copy" = synonym_tst);
signature.asc
Description: PGP signature
