On 12/23/2015 04:37 PM, Michael Paquier wrote:
> On Thu, Dec 24, 2015 at 2:08 AM, Joe Conway <m...@joeconway.com> wrote:
>> On 12/23/2015 05:45 AM, Michael Paquier wrote:
>>>> Yeah, the last version of the patch dates of August, and there is
>>>> visibly agreement that the information of pg_controldata provided at
>>>> SQL level is useful while the data of pg_config is proving to be less
>>>> interesting for remote users. Could the patch be rebased and split as
>>>> suggested above?
>>>
>>> I am marking this patch as returned with feedback, there is not much 
>>> activity...
>>
>> I just dusted this off yesterday finally. Anyway, based on the
>> discussions I plan to:
>>
>> 1) split it into two separate patches, one for pg_config and one for
>>    pg_controldata.
>> 2) Change the pg_controldata to be a bunch of separate functions as
>>    suggested by Josh Berkus rather than one SRF.
> 
> This looks like a plan, thanks!

First installment -- pg_config function/view as a separate patch,
rebased to current master.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 536c805..a50ffe8 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*************** CREATE VIEW pg_timezone_abbrevs AS
*** 433,438 ****
--- 433,444 ----
  CREATE VIEW pg_timezone_names AS
      SELECT * FROM pg_timezone_names();
  
+ CREATE VIEW pg_config AS
+     SELECT * FROM pg_config();
+ 
+ REVOKE ALL on pg_config FROM PUBLIC;
+ REVOKE EXECUTE ON FUNCTION pg_config() FROM PUBLIC;
+ 
  -- Statistics views
  
  CREATE VIEW pg_stat_all_tables AS
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index 7889101..e0015d8 100644
*** a/src/backend/utils/misc/Makefile
--- b/src/backend/utils/misc/Makefile
*************** include $(top_builddir)/src/Makefile.glo
*** 14,21 ****
  
  override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
  
! OBJS = guc.o help_config.o pg_rusage.o ps_status.o rls.o \
!        sampling.o superuser.o timeout.o tzparser.o
  
  # This location might depend on the installation directories. Therefore
  # we can't subsitute it into pg_config.h.
--- 14,34 ----
  
  override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
  
! # don't include subdirectory-path-dependent -I and -L switches
! STD_CPPFLAGS := $(filter-out -I$(top_srcdir)/src/include -I$(top_builddir)/src/include,$(CPPFLAGS))
! STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/port,$(LDFLAGS))
! override CPPFLAGS += -DVAL_CONFIGURE="\"$(configure_args)\""
! override CPPFLAGS += -DVAL_CC="\"$(CC)\""
! override CPPFLAGS += -DVAL_CPPFLAGS="\"$(STD_CPPFLAGS)\""
! override CPPFLAGS += -DVAL_CFLAGS="\"$(CFLAGS)\""
! override CPPFLAGS += -DVAL_CFLAGS_SL="\"$(CFLAGS_SL)\""
! override CPPFLAGS += -DVAL_LDFLAGS="\"$(STD_LDFLAGS)\""
! override CPPFLAGS += -DVAL_LDFLAGS_EX="\"$(LDFLAGS_EX)\""
! override CPPFLAGS += -DVAL_LDFLAGS_SL="\"$(LDFLAGS_SL)\""
! override CPPFLAGS += -DVAL_LIBS="\"$(LIBS)\""
! 
! OBJS = guc.o help_config.o pg_config.o pg_rusage.o \
!        ps_status.o rls.o sampling.o superuser.o timeout.o tzparser.o
  
  # This location might depend on the installation directories. Therefore
  # we can't subsitute it into pg_config.h.
diff --git a/src/backend/utils/misc/pg_config.c b/src/backend/utils/misc/pg_config.c
index ...e29a706 .
*** a/src/backend/utils/misc/pg_config.c
--- b/src/backend/utils/misc/pg_config.c
***************
*** 0 ****
--- 1,266 ----
+ /*-------------------------------------------------------------------------
+  *
+  * pg_config.c
+  *		Expose same output as pg_config except as an SRF
+  *
+  * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  * IDENTIFICATION
+  *	  src/backend/utils/misc/pg_config.c
+  *
+  */
+ 
+ #include "postgres.h"
+ 
+ #include "funcapi.h"
+ #include "miscadmin.h"
+ #include "catalog/pg_type.h"
+ #include "utils/builtins.h"
+ #include "utils/elog.h"
+ #include "port.h"
+ 
+ static void get_configdata(void);
+ 
+ #ifdef PGDLLIMPORT
+ /* Postgres global */
+ extern PGDLLIMPORT char my_exec_path[];
+ #else
+ /* Postgres global */
+ extern DLLIMPORT char my_exec_path[];
+ #endif /* PGDLLIMPORT */
+ 
+ struct configdata
+ {
+ 	char	   *name;
+ 	char	   *setting;
+ };
+ 
+ static struct configdata ConfigData[] =
+ {
+ 	{"BINDIR", NULL},
+ 	{"DOCDIR", NULL},
+ 	{"HTMLDIR", NULL},
+ 	{"INCLUDEDIR", NULL},
+ 	{"PKGINCLUDEDIR", NULL},
+ 	{"INCLUDEDIR-SERVER", NULL},
+ 	{"LIBDIR", NULL},
+ 	{"PKGLIBDIR", NULL},
+ 	{"LOCALEDIR", NULL},
+ 	{"MANDIR", NULL},
+ 	{"SHAREDIR", NULL},
+ 	{"SYSCONFDIR", NULL},
+ 	{"PGXS", NULL},
+ 	{"CONFIGURE", NULL},
+ 	{"CC", NULL},
+ 	{"CPPFLAGS", NULL},
+ 	{"CFLAGS", NULL},
+ 	{"CFLAGS_SL", NULL},
+ 	{"LDFLAGS", NULL},
+ 	{"LDFLAGS_EX", NULL},
+ 	{"LDFLAGS_SL", NULL},
+ 	{"LIBS", NULL},
+ 	{"VERSION", NULL},
+ 	{NULL, NULL}
+ };
+ 
+ static void get_configdata(void);
+ 
+ Datum
+ pg_config(PG_FUNCTION_ARGS)
+ {
+ 	ReturnSetInfo	   *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ 	Tuplestorestate	   *tupstore;
+ 	HeapTuple			tuple;
+ 	TupleDesc			tupdesc;
+ 	AttInMetadata	   *attinmeta;
+ 	MemoryContext		per_query_ctx;
+ 	MemoryContext		oldcontext;
+ 	char			   *values[2];
+ 	int					i = 0;
+ 
+ 	/* check to see if caller supports us returning a tuplestore */
+ 	if (!rsinfo || !(rsinfo->allowedModes & SFRM_Materialize))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_SYNTAX_ERROR),
+ 				 errmsg("materialize mode required, but it is not "
+ 						"allowed in this context")));
+ 
+ 	per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
+ 	oldcontext = MemoryContextSwitchTo(per_query_ctx);
+ 
+ 	/* get the requested return tuple description */
+ 	tupdesc = CreateTupleDescCopy(rsinfo->expectedDesc);
+ 
+ 	/*
+ 	 * Check to make sure we have a reasonable tuple descriptor
+ 	 */
+ 	if (tupdesc->natts != 2 ||
+ 		tupdesc->attrs[0]->atttypid != TEXTOID ||
+ 		tupdesc->attrs[1]->atttypid != TEXTOID)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_SYNTAX_ERROR),
+ 				 errmsg("query-specified return tuple and "
+ 						"function return type are not compatible")));
+ 
+ 	/* OK to use it */
+ 	attinmeta = TupleDescGetAttInMetadata(tupdesc);
+ 
+ 	/* let the caller know we're sending back a tuplestore */
+ 	rsinfo->returnMode = SFRM_Materialize;
+ 
+ 	/* initialize our tuplestore */
+ 	tupstore = tuplestore_begin_heap(true, false, work_mem);
+ 
+ 	get_configdata();
+ 	while (ConfigData[i].name)
+ 	{
+ 		values[0] = ConfigData[i].name;
+ 		values[1] = ConfigData[i].setting;
+ 
+ 		tuple = BuildTupleFromCStrings(attinmeta, values);
+ 		tuplestore_puttuple(tupstore, tuple);
+ 		++i;
+ 	}
+ 	
+ 	/*
+ 	 * no longer need the tuple descriptor reference created by
+ 	 * TupleDescGetAttInMetadata()
+ 	 */
+ 	ReleaseTupleDesc(tupdesc);
+ 
+ 	tuplestore_donestoring(tupstore);
+ 	rsinfo->setResult = tupstore;
+ 
+ 	/*
+ 	 * SFRM_Materialize mode expects us to return a NULL Datum. The actual
+ 	 * tuples are in our tuplestore and passed back through
+ 	 * rsinfo->setResult. rsinfo->setDesc is set to the tuple description
+ 	 * that we actually used to build our tuples with, so the caller can
+ 	 * verify we did what it was expecting.
+ 	 */
+ 	rsinfo->setDesc = tupdesc;
+ 	MemoryContextSwitchTo(oldcontext);
+ 
+ 	return (Datum) 0;
+ }
+ 
+ 
+ static void
+ get_configdata(void)
+ {
+ 	char			path[MAXPGPATH];
+ 	char		   *lastsep;
+ 
+ 	strcpy(path, my_exec_path);
+ 	lastsep = strrchr(path, '/');
+ 	if (lastsep)
+ 		*lastsep = '\0';
+ 	cleanup_path(path);
+ 	ConfigData[0].setting = pstrdup(path);
+ 
+ 	get_doc_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[1].setting = pstrdup(path);
+ 
+ 	get_html_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[2].setting = pstrdup(path);
+ 
+ 	get_include_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[3].setting = pstrdup(path);
+ 
+ 	get_pkginclude_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[4].setting = pstrdup(path);
+ 
+ 	get_includeserver_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[5].setting = pstrdup(path);
+ 
+ 	get_lib_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[6].setting = pstrdup(path);
+ 
+ 	get_pkglib_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[7].setting = pstrdup(path);
+ 
+ 	get_locale_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[8].setting = pstrdup(path);
+ 
+ 	get_man_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[9].setting = pstrdup(path);
+ 
+ 	get_share_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[10].setting = pstrdup(path);
+ 
+ 	get_etc_path(my_exec_path, path);
+ 	cleanup_path(path);
+ 	ConfigData[11].setting = pstrdup(path);
+ 
+ 	get_pkglib_path(my_exec_path, path);
+ 	strlcat(path, "/pgxs/src/makefiles/pgxs.mk", sizeof(path));
+ 	cleanup_path(path);
+ 	ConfigData[12].setting = pstrdup(path);
+ 
+ #ifdef VAL_CONFIGURE
+ 	ConfigData[13].setting = pstrdup(VAL_CONFIGURE);
+ #else
+ 	ConfigData[13].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_CC
+ 	ConfigData[14].setting = pstrdup(VAL_CC);
+ #else
+ 	ConfigData[14].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_CPPFLAGS
+ 	ConfigData[15].setting = pstrdup(VAL_CPPFLAGS);
+ #else
+ 	ConfigData[15].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_CFLAGS
+ 	ConfigData[16].setting = pstrdup(VAL_CFLAGS);
+ #else
+ 	ConfigData[16].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_CFLAGS_SL
+ 	ConfigData[17].setting = pstrdup(VAL_CFLAGS_SL);
+ #else
+ 	ConfigData[17].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_LDFLAGS
+ 	ConfigData[18].setting = pstrdup(VAL_LDFLAGS);
+ #else
+ 	ConfigData[18].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_LDFLAGS_EX
+ 	ConfigData[19].setting = pstrdup(VAL_LDFLAGS_EX);
+ #else
+ 	ConfigData[19].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_LDFLAGS_SL
+ 	ConfigData[20].setting = pstrdup(VAL_LDFLAGS_SL);
+ #else
+ 	ConfigData[20].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ #ifdef VAL_LIBS
+ 	ConfigData[21].setting = pstrdup(VAL_LIBS);
+ #else
+ 	ConfigData[21].setting = pstrdup(_("not recorded"));
+ #endif
+ 
+ 	ConfigData[22].setting = pstrdup("PostgreSQL " PG_VERSION);
+ }
diff --git a/src/bin/pg_config/pg_config.c b/src/bin/pg_config/pg_config.c
index 9b1f95d..25efc71 100644
*** a/src/bin/pg_config/pg_config.c
--- b/src/bin/pg_config/pg_config.c
***************
*** 29,65 ****
  static const char *progname;
  static char mypath[MAXPGPATH];
  
- 
- /*
-  * This function cleans up the paths for use with either cmd.exe or Msys
-  * on Windows. We need them to use filenames without spaces, for which a
-  * short filename is the safest equivalent, eg:
-  *		C:/Progra~1/
-  */
- static void
- cleanup_path(char *path)
- {
- #ifdef WIN32
- 	char	   *ptr;
- 
- 	/*
- 	 * GetShortPathName() will fail if the path does not exist, or short names
- 	 * are disabled on this file system.  In both cases, we just return the
- 	 * original path.  This is particularly useful for --sysconfdir, which
- 	 * might not exist.
- 	 */
- 	GetShortPathName(path, path, MAXPGPATH - 1);
- 
- 	/* Replace '\' with '/' */
- 	for (ptr = path; *ptr; ptr++)
- 	{
- 		if (*ptr == '\\')
- 			*ptr = '/';
- 	}
- #endif
- }
- 
- 
  /*
   * For each piece of information known to pg_config, we define a subroutine
   * to print it.  This is probably overkill, but it avoids code duplication
--- 29,34 ----
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index d8640db..2e1a418 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DESCR("get an individual replication ori
*** 5337,5342 ****
--- 5337,5346 ----
  DATA(insert OID = 6014 ( pg_show_replication_origin_status PGNSP PGUID 12 1 100 0 0 f f f f f t v r 0 0 2249 "" "{26,25,3220,3220}" "{o,o,o,o}" "{local_id, external_id, remote_lsn, local_lsn}" _null_ _null_ pg_show_replication_origin_status _null_ _null_ _null_ ));
  DESCR("get progress for all replication origins");
  
+ /* pg_config */
+ DATA(insert OID = 3330 ( pg_config PGNSP PGUID 12 1 23 0 0 f f f f t t i r 0 0 2249 "" "{25,25}" "{o,o}" "{name,setting}" _null_ _null_ pg_config _null_ _null_ _null_ ));
+ DESCR("pg_config binary as a function");
+ 
  /*
   * Symbolic values for provolatile column: these indicate whether the result
   * of a function is dependent *only* on the values of its explicit arguments,
diff --git a/src/include/port.h b/src/include/port.h
index 3787cbf..ab38f62 100644
*** a/src/include/port.h
--- b/src/include/port.h
*************** extern void join_path_components(char *r
*** 42,47 ****
--- 42,48 ----
  					 const char *head, const char *tail);
  extern void canonicalize_path(char *path);
  extern void make_native_path(char *path);
+ extern void cleanup_path(char *path);
  extern bool path_contains_parent_reference(const char *path);
  extern bool path_is_relative_and_below_cwd(const char *path);
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index e610bf3..d0abb82 100644
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
*************** extern Datum set_config_by_name(PG_FUNCT
*** 1123,1128 ****
--- 1123,1131 ----
  extern Datum show_all_settings(PG_FUNCTION_ARGS);
  extern Datum show_all_file_settings(PG_FUNCTION_ARGS);
  
+ /* pg_config.c */
+ extern Datum pg_config(PG_FUNCTION_ARGS);
+ 
  /* rls.c */
  extern Datum row_security_active(PG_FUNCTION_ARGS);
  extern Datum row_security_active_name(PG_FUNCTION_ARGS);
diff --git a/src/port/path.c b/src/port/path.c
index d0f72df..7613666 100644
*** a/src/port/path.c
--- b/src/port/path.c
*************** make_native_path(char *filename)
*** 172,177 ****
--- 172,207 ----
  
  
  /*
+  * This function cleans up the paths for use with either cmd.exe or Msys
+  * on Windows. We need them to use filenames without spaces, for which a
+  * short filename is the safest equivalent, eg:
+  *		C:/Progra~1/
+  */
+ void
+ cleanup_path(char *path)
+ {
+ #ifdef WIN32
+ 	char	   *ptr;
+ 
+ 	/*
+ 	 * GetShortPathName() will fail if the path does not exist, or short names
+ 	 * are disabled on this file system.  In both cases, we just return the
+ 	 * original path.  This is particularly useful for --sysconfdir, which
+ 	 * might not exist.
+ 	 */
+ 	GetShortPathName(path, path, MAXPGPATH - 1);
+ 
+ 	/* Replace '\' with '/' */
+ 	for (ptr = path; *ptr; ptr++)
+ 	{
+ 		if (*ptr == '\\')
+ 			*ptr = '/';
+ 	}
+ #endif
+ }
+ 
+ 
+ /*
   * join_path_components - join two path components, inserting a slash
   *
   * We omit the slash if either given component is empty.
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 80374e4..151b8c1 100644
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** pg_available_extensions| SELECT e.name,
*** 1305,1310 ****
--- 1305,1313 ----
      e.comment
     FROM (pg_available_extensions() e(name, default_version, comment)
       LEFT JOIN pg_extension x ON ((e.name = x.extname)));
+ pg_config| SELECT pg_config.name,
+     pg_config.setting
+    FROM pg_config() pg_config(name, setting);
  pg_cursors| SELECT c.name,
      c.statement,
      c.is_holdable,

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to