On Wed, Jan 9, 2019 at 2:04 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> I wrote:
> > John Naylor <john.nay...@2ndquadrant.com> writes:
> >> -There is a bit of a cognitive clash between $case_sensitive in
> >> gen_keywordlist.pl and $case_insensitive in PerfectHash.pm. They each
> >> make sense in their own file, but might it be worth using one or the
> >> other?

> Working on the fmgr-oid-lookup idea gave me the thought that
> PerfectHash.pm ought to support fixed-length keys.  Rather than start
> adding random parameters to the function, I borrowed an idea from
> PostgresNode.pm and made the options be keyword-style parameters.  Now
> the impedance mismatch about case sensitivity is handled with
>
> my $f = PerfectHash::generate_hash_function(\@keywords, $funcname,
>         case_insensitive => !$case_sensitive);
>
> which is at least a little clearer than before, though I'm not sure
> if it entirely solves the problem.

It's a bit clearer, but thinking about this some more, it makes sense
for gen_keywordlist.pl to use $case_insensitive, because right now
every instance of the var is "!$case_sensitive". In the attached (on
top of v4), I change the command line option to --citext, and add the
ability to negate it within the option, as '--no-citext'. It's kind of
a double negative for the C-keywords invocation, but we can have the
option for both cases, so we don't need to worry about what the
default is (which is case_insensitive=1).

--
John Naylor                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/common/Makefile b/src/common/Makefile
index d0c2b970eb..5f9327de59 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -124,7 +124,7 @@ libpgcommon_srv.a: $(OBJS_SRV)
 
 # generate SQL keyword lookup table to be included into keywords*.o.
 kwlist_d.h: $(top_srcdir)/src/include/parser/kwlist.h $(GEN_KEYWORDLIST_DEPS)
-	$(GEN_KEYWORDLIST) --extern $<
+	$(GEN_KEYWORDLIST) --extern --citext $<
 
 # Dependencies of keywords*.o need to be managed explicitly to make sure
 # that you don't get broken parsing code, even in a non-enable-depend build.
diff --git a/src/interfaces/ecpg/preproc/Makefile b/src/interfaces/ecpg/preproc/Makefile
index 6c02f97622..96dd76317f 100644
--- a/src/interfaces/ecpg/preproc/Makefile
+++ b/src/interfaces/ecpg/preproc/Makefile
@@ -60,10 +60,10 @@ preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.header ecpg.
 
 # generate keyword headers
 c_kwlist_d.h: c_kwlist.h $(GEN_KEYWORDLIST_DEPS)
-	$(GEN_KEYWORDLIST) --varname ScanCKeywords --case $<
+	$(GEN_KEYWORDLIST) --varname ScanCKeywords --no-citext  $<
 
 ecpg_kwlist_d.h: ecpg_kwlist.h $(GEN_KEYWORDLIST_DEPS)
-	$(GEN_KEYWORDLIST) --varname ScanECPGKeywords $<
+	$(GEN_KEYWORDLIST) --varname ScanECPGKeywords --citext $<
 
 # Force these dependencies to be known even without dependency info built:
 ecpg_keywords.o c_keywords.o keywords.o preproc.o pgc.o parser.o: preproc.h
diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 8a0f294323..bc4ee50682 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -80,10 +80,10 @@ plerrcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-plerrcodes.p
 
 # generate keyword headers for the scanner
 pl_reserved_kwlist_d.h: pl_reserved_kwlist.h $(GEN_KEYWORDLIST_DEPS)
-	$(GEN_KEYWORDLIST) --varname ReservedPLKeywords $<
+	$(GEN_KEYWORDLIST) --varname ReservedPLKeywords --citext $<
 
 pl_unreserved_kwlist_d.h: pl_unreserved_kwlist.h $(GEN_KEYWORDLIST_DEPS)
-	$(GEN_KEYWORDLIST) --varname UnreservedPLKeywords $<
+	$(GEN_KEYWORDLIST) --varname UnreservedPLKeywords --citext $<
 
 
 check: submake
diff --git a/src/tools/gen_keywordlist.pl b/src/tools/gen_keywordlist.pl
index 2744e1dc26..927511d7c4 100644
--- a/src/tools/gen_keywordlist.pl
+++ b/src/tools/gen_keywordlist.pl
@@ -35,13 +35,13 @@ use PerfectHash;
 
 my $output_path = '';
 my $extern = 0;
-my $case_sensitive = 0;
+my $case_insensitive = 1;
 my $varname = 'ScanKeywords';
 
 GetOptions(
 	'output:s'       => \$output_path,
 	'extern'         => \$extern,
-	'case-sensitive' => \$case_sensitive,
+	'citext!'        => \$case_insensitive,
 	'varname:s'      => \$varname) || usage();
 
 my $kw_input_file = shift @ARGV || die "No input file.\n";
@@ -97,7 +97,7 @@ while (<$kif>)
 }
 
 # When being case-insensitive, insist that the input be all-lower-case.
-if (!$case_sensitive)
+if ($case_insensitive)
 {
 	foreach my $kw (@keywords)
 	{
@@ -157,7 +157,7 @@ printf $kwdef "#define %s_NUM_KEYWORDS %d\n\n", uc $varname, scalar @keywords;
 my $funcname = $varname . "_hash_func";
 
 my $f = PerfectHash::generate_hash_function(\@keywords, $funcname,
-	case_insensitive => !$case_sensitive);
+	case_insensitive => $case_insensitive);
 
 printf $kwdef qq|static %s\n|, $f;
 
@@ -178,11 +178,11 @@ printf $kwdef "#endif\t\t\t\t\t\t\t/* %s_H */\n", uc $base_filename;
 sub usage
 {
 	die <<EOM;
-Usage: gen_keywordlist.pl [--output/-o <path>] [--varname/-v <varname>] [--extern/-e] input_file
+Usage: gen_keywordlist.pl [--output/-o <path>] [--varname/-v <varname>] [--extern/-e] [--(no-)citext ] input_file
     --output   Output directory (default '.')
     --varname  Name for ScanKeywordList variable (default 'ScanKeywords')
     --extern   Allow the ScanKeywordList variable to be globally visible
-    --case     Keyword matching is to be case-sensitive
+    --citext   Keyword matching is to be case-insensitive
 
 gen_keywordlist.pl transforms a list of keywords into a ScanKeywordList.
 The output filename is derived from the input file by inserting _d,

Reply via email to