From 17cadb17279c802c79e4afa0d7fcd7f63ed38d03 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Thu, 15 Mar 2018 13:12:17 +0900 Subject: [PATCH] Autogenerating function to detect list-formatted GUC variables Some features require to know whether a GUC variables is list or not but the set of such functions can be modified through development. We tried to check it on-the-fly but found that that makes things more complex. Instead, this generates the list of such variables by scanning the whole source tree, instead of manually maintaining it. --- src/backend/Makefile | 4 +- src/backend/utils/adt/ruleutils.c | 4 +- src/bin/pg_dump/Makefile | 4 +- src/bin/pg_dump/dumputils.c | 5 +- src/bin/pg_dump/pg_dump.c | 4 +- src/include/Makefile | 7 +- src/include/gen_listvars.pl | 145 ++++++++++++++++++++++++++++++++++++ src/test/regress/expected/rules.out | 24 ++++++ src/test/regress/sql/rules.sql | 12 +++ 9 files changed, 197 insertions(+), 12 deletions(-) create mode 100755 src/include/gen_listvars.pl diff --git a/src/backend/Makefile b/src/backend/Makefile index 4a28267339..10656c53e6 100644 --- a/src/backend/Makefile +++ b/src/backend/Makefile @@ -169,7 +169,7 @@ submake-schemapg: .PHONY: generated-headers -generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/probes.h +generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/probes.h $(top_builddir)/src/include/common/check_listvars.h $(top_builddir)/src/include/parser/gram.h: parser/gram.h prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \ @@ -205,6 +205,8 @@ $(top_builddir)/src/include/utils/probes.h: utils/probes.h cd '$(dir $@)' && rm -f $(notdir $@) && \ $(LN_S) "../../../$(subdir)/utils/probes.h" . +$(top_builddir)/src/include/common/check_listvars.h: + $(MAKE) -C $(dir $@).. common/check_listvars.h utils/probes.o: utils/probes.d $(SUBDIROBJS) $(DTRACE) $(DTRACEFLAGS) -C -G -s $(call expand_subsys,$^) -o $@ diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index b58ee3c387..d2ad034e06 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -41,6 +41,7 @@ #include "catalog/pg_type.h" #include "commands/defrem.h" #include "commands/tablespace.h" +#include "common/check_listvars.h" #include "common/keywords.h" #include "executor/spi.h" #include "funcapi.h" @@ -2599,8 +2600,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS) * Some GUC variable names are 'LIST' type and hence must not * be quoted. */ - if (pg_strcasecmp(configitem, "DateStyle") == 0 - || pg_strcasecmp(configitem, "search_path") == 0) + if (IsConfigOptionAList(configitem)) appendStringInfoString(&buf, pos); else simple_quote_literal(&buf, pos); diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile index e3bfc95f16..d79ac49a83 100644 --- a/src/bin/pg_dump/Makefile +++ b/src/bin/pg_dump/Makefile @@ -25,13 +25,13 @@ OBJS= pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o \ all: pg_dump pg_restore pg_dumpall -pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils +pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils submake-generated-headers $(CC) $(CFLAGS) pg_dump.o common.o pg_dump_sort.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) pg_restore: pg_restore.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils $(CC) $(CFLAGS) pg_restore.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) -pg_dumpall: pg_dumpall.o dumputils.o | submake-libpq submake-libpgport submake-libpgfeutils +pg_dumpall: pg_dumpall.o dumputils.o | submake-libpq submake-libpgport submake-libpgfeutils submake-generated-headers $(CC) $(CFLAGS) pg_dumpall.o dumputils.o $(WIN32RES) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) install: all installdirs diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 7f5bb1343e..0c4d648969 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -14,6 +14,7 @@ */ #include "postgres_fe.h" +#include "common/check_listvars.h" #include "dumputils.h" #include "fe_utils/string_utils.h" @@ -888,10 +889,8 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem, /* * Some GUC variable names are 'LIST' type and hence must not be quoted. - * XXX this list is incomplete ... */ - if (pg_strcasecmp(mine, "DateStyle") == 0 - || pg_strcasecmp(mine, "search_path") == 0) + if (IsConfigOptionAList(mine)) appendPQExpBufferStr(buf, pos); else appendStringLiteralConn(buf, pos, conn); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 566cbf2cda..6446f0ced7 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -53,6 +53,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" +#include "common/check_listvars.h" #include "libpq/libpq-fs.h" #include "dumputils.h" @@ -11880,8 +11881,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo) * Some GUC variable names are 'LIST' type and hence must not be * quoted. */ - if (pg_strcasecmp(configitem, "DateStyle") == 0 - || pg_strcasecmp(configitem, "search_path") == 0) + if (IsConfigOptionAList(configitem)) appendPQExpBufferStr(q, pos); else appendStringLiteralAH(q, pos, fout); diff --git a/src/include/Makefile b/src/include/Makefile index a689d352b6..a05f4ca197 100644 --- a/src/include/Makefile +++ b/src/include/Makefile @@ -13,7 +13,7 @@ top_builddir = ../.. include $(top_builddir)/src/Makefile.global -all: pg_config.h pg_config_ext.h pg_config_os.h +all: pg_config.h pg_config_ext.h pg_config_os.h common/check_listvars.h # Subdirectories containing installable headers @@ -25,6 +25,9 @@ SUBDIRS = access bootstrap catalog commands common datatype \ port/win32_msvc/sys port/win32/arpa port/win32/netinet \ port/win32/sys portability +common/check_listvars.h: + ./gen_listvars.pl + # Install all headers install: all installdirs # These headers are needed by the public headers of the interfaces. @@ -73,7 +76,7 @@ uninstall: clean: - rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h parser/gram.h utils/probes.h catalog/schemapg.h + rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h parser/gram.h utils/probes.h catalog/schemapg.h common/check_listvars.h distclean maintainer-clean: clean rm -f pg_config.h pg_config_ext.h pg_config_os.h dynloader.h stamp-h stamp-ext-h diff --git a/src/include/gen_listvars.pl b/src/include/gen_listvars.pl new file mode 100755 index 0000000000..081d2ea855 --- /dev/null +++ b/src/include/gen_listvars.pl @@ -0,0 +1,145 @@ +#! /usr/bin/perl +# src/include/gen_listvars.pl +# generates a function definition to check whether a guc variable is +# of list type or not. +use strict; +use File::Find; + +my $output_file = "common/check_listvars.h"; +my @varlist; + +find(\&findproc, "../"); + +open my $out, '>', $output_file || + die "failed to open output file $output_file: $!"; +print $out make_func_def(@varlist); +close($out); +print "done. The definition is written as $output_file.\n"; +exit; + +############################################### +# findproc() +# callback function for find() to retrieve names of list variables +# from a file. The names are stored to @varlist. +sub findproc +{ + my $f = $_; + my @l; + my $prepend = 0; + + # we are interested only in .c files. + return if ($f !~ /\.c$/); + + my $content = load_file($f); + if ($f =~ /^guc.c$/) + { + # guc varialbes are listed first + @l = extract_gucdef($content); + $prepend = 1; + } + else + { + # others follow guc variables + @l = extract_customdef($content); + } + + @l = sort @l; + + if ($#l >= 0) + { + printf("%d list variables found in %s\n", $#l + 1, $f); + } + + if ($prepend) + { + @varlist = (@l, @varlist); + } + else + { + @varlist = (@varlist, @l); + } +} + +############################################### +# extract_gucdef($filecontent); +# collects the the name of a enbedded GUC variable with GUC_LIST_INPUT +sub extract_gucdef +{ + my ($str) = @_; + my @ret = (); + + while ($str =~ /{"([^"]+)" *, *PGC_[^}]*\s*GUC_LIST_INPUT\s*[^}]*},/g) + { + push (@ret, $1); + } + + return @ret; +} + +################################################################## +# extract_customdef($filecontent); +# collects the the name of a custom variable with GUC_LIST_INPUT +sub extract_customdef +{ + my ($str) = @_; + my @ret = (); + + while ($str =~ /DefineCustomStringVariable\("([^"]+)"\s*,(?:(?!\)\s*;).)*,\s*GUC_LIST_INPUT\s*,(?:(?!\)\s*;).)*\);/g) + { + push (@ret, $1); + } + + return @ret; +} + + +######################################################################## +# load_file($filename); +# returns the file content as a string in which line-feeds are removed +sub load_file +{ + my $f = $_[0]; + my $ret = ""; + + open my $in, '<', $f || die "failed to open file $f: $!"; + $ret = ""; + while (<$in>) + { + chomp; + $ret .= $_; + } + close $in; + + return $ret; +} + +############################################### +# make_func_def(@varnames); +# generates the function definition +sub make_func_def +{ + my @list = @_; + my $ret; + + $ret = + "/* $output_file. Generated by src/include/gen_listvars.pl */\n". + "/* \n". + " * This is generated automatically, but not maintained automatically.\n". + " * make clean will remove file and generated in the next build.\n". + "*/\n\n". + "inline bool\n". + "IsConfigOptionAList(const char *name)\n". + "{\n". + " if ("; + my $first = 1; + + foreach my $i (@list) + { + $ret .= " ||\n " if (!$first); + $first = 0; + $ret .= "pg_strcasecmp(name, \"$i\") == 0"; + } + $ret .= ")\n return true;\n return false;\n}\n"; + + return $ret; +} diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index d7eff6c0a7..ce79515ab6 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -3228,6 +3228,30 @@ SELECT pg_get_partkeydef(0); (1 row) +-- test for pg_get_functiondef with list parameters which should not be +-- quoted. +CREATE FUNCTION func_with_set_params() RETURNS integer + AS 'select 1;' + LANGUAGE SQL + SET search_path TO 'pg_catalog' + SET wal_consistency_checking TO heap, heap2 + SET session_preload_libraries TO foo, bar + IMMUTABLE STRICT; +SELECT pg_get_functiondef('func_with_set_params'::regproc); + pg_get_functiondef +---------------------------------------------------------- + CREATE OR REPLACE FUNCTION public.func_with_set_params()+ + RETURNS integer + + LANGUAGE sql + + IMMUTABLE STRICT + + SET search_path TO pg_catalog + + SET wal_consistency_checking TO heap, heap2 + + SET session_preload_libraries TO foo, bar + + AS $function$select 1;$function$ + + +(1 row) + +DROP FUNCTION func_with_set_params(); -- test rename for a rule defined on a partitioned table CREATE TABLE parted_table (a int) PARTITION BY LIST (a); CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1); diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 0823c02acf..0d4938d715 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1170,6 +1170,18 @@ SELECT pg_get_function_arg_default(0, 0); SELECT pg_get_function_arg_default('pg_class'::regclass, 0); SELECT pg_get_partkeydef(0); +-- test for pg_get_functiondef with list parameters which should not be +-- quoted. +CREATE FUNCTION func_with_set_params() RETURNS integer + AS 'select 1;' + LANGUAGE SQL + SET search_path TO 'pg_catalog' + SET wal_consistency_checking TO heap, heap2 + SET session_preload_libraries TO foo, bar + IMMUTABLE STRICT; +SELECT pg_get_functiondef('func_with_set_params'::regproc); +DROP FUNCTION func_with_set_params(); + -- test rename for a rule defined on a partitioned table CREATE TABLE parted_table (a int) PARTITION BY LIST (a); CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1); -- 2.16.2