Re: [HACKERS] Sanity checking for ./configure options?

[Peter Eisentraut]

On 2/22/16 6:24 PM, Jim Nasby wrote:
> On 2/5/16 10:08 AM, David Fetter wrote:
>> On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote:
>>> I just discovered that ./configure will happily accept
>>> '--with-pgport=' (I
>>> was actually doing =$PGPORT, and didn't realize $PGPORT was empty).
>>> What you
>>> end up with is a compile error in guc.c, with no idea why it's
>>> broken. Any
>>> reason not to have configure or at least make puke if pgport isn't
>>> valid?
>>
>> That seems like a good idea.
> 
> Patch attached. I've verified it with --with-pgport=, =0, =7 and =1.
> It catches what you'd expect it to.

Your code and comments suggest that you can specify the port to
configure by setting PGPORT, but that is not the case.

test == is not portable (bashism).

Error messages should have consistent capitalization.

Indentation in configure is two spaces.

> As the comment states, it doesn't catch things like --with-pgport=1a in
> configure, but the compile error you get with that isn't too hard to
> figure out, so I think it's OK.

Passing a non-integer as argument will produce an error message like
(depending on shell)

./configure: line 3107: test: 11a: integer expression expected

but will not actually abort configure.

It would work more robustly if you did something like this

elif test "$default_port" -ge "1" -a "$default_port" -le "65535"; then
  :
else
  AC_MSG_ERROR([port must be between 1 and 65535])
fi

but that still leaks the shell's error message.

There is also the risk of someone specifying a number with a leading
zero, which C would interpret as octal but the shell would not.

To make this really robust, you might need to do pattern matching on the
value.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] syslog configurable line splitting behavior

[Peter Eisentraut]

Writing log messages to syslog caters to ancient syslog implementations
in two ways:

- sequence numbers
- line splitting

While these are arguably reasonable defaults, I would like a way to turn
them off, because they get in the way of doing more interesting things
with syslog (e.g., logging somewhere that is not just a text file).

So I propose the two attached patches that introduce new configuration
Boolean parameters syslog_sequence_numbers and syslog_split_lines that
can toggle these behaviors.
From e6a17750956e3e6950683bad397a74adb30f30a2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 26 Feb 2016 22:34:30 -0500
Subject: [PATCH 1/2] Add syslog_sequence_numbers parameter

---
 doc/src/sgml/config.sgml  | 28 +++
 src/backend/utils/error/elog.c| 12 ++--
 src/backend/utils/misc/guc.c  | 10 ++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/utils/elog.h  |  1 +
 5 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a09ceb2..0d1ae4b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4218,6 +4218,34 @@ Where To Log

   
 
+  
+   syslog_sequence_numbers (boolean)
+
+ syslog_sequence_numbers configuration parameter
+
+   
+
+   
+
+ When logging to syslog and this is on (the
+ default), then each message will be prefixed by an increasing
+ sequence number (such as [2]).  This circumvents
+ the --- last message repeated N times --- suppression
+ that many syslog implementations perform by default.  In more modern
+ syslog implementations, repeat message suppression can be configured
+ (for example, $RepeatedMsgReduction
+ in rsyslog), so this might not be
+ necessary.  Also, you could turn this off if you actually want to
+ suppress repeated messages.
+
+
+
+ This parameter can only be set in the postgresql.conf
+ file or on the server command line.
+
+  
+ 
+
  
   event_source (string)
   
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 9005b26..0bc96b4 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -106,6 +106,7 @@ int			Log_error_verbosity = PGERROR_VERBOSE;
 char	   *Log_line_prefix = NULL;		/* format for extra log line info */
 int			Log_destination = LOG_DESTINATION_STDERR;
 char	   *Log_destination_string = NULL;
+bool		syslog_sequence_numbers = true;
 
 #ifdef HAVE_SYSLOG
 
@@ -2008,7 +2009,11 @@ write_syslog(int level, const char *line)
 
 			chunk_nr++;
 
-			syslog(level, "[%lu-%d] %s", seq, chunk_nr, buf);
+			if (syslog_sequence_numbers)
+syslog(level, "[%lu-%d] %s", seq, chunk_nr, buf);
+			else
+syslog(level, "[%d] %s", chunk_nr, buf);
+
 			line += buflen;
 			len -= buflen;
 		}
@@ -2016,7 +2021,10 @@ write_syslog(int level, const char *line)
 	else
 	{
 		/* message short enough */
-		syslog(level, "[%lu] %s", seq, line);
+		if (syslog_sequence_numbers)
+			syslog(level, "[%lu] %s", seq, line);
+		else
+			syslog(level, "%s", line);
 	}
 }
 #endif   /* HAVE_SYSLOG */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ea5a09a..bc8faa9 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1632,6 +1632,16 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"syslog_sequence_numbers", PGC_SIGHUP, LOGGING_WHERE,
+			gettext_noop("Add sequence number to syslog messags to avoid duplicate suppression."),
+			NULL
+		},
+		&syslog_sequence_numbers,
+		true,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee3d378..a85ba36 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -358,6 +358,7 @@
 # These are relevant when logging to syslog:
 #syslog_facility = 'LOCAL0'
 #syslog_ident = 'postgres'
+#syslog_sequence_numbers = true
 
 # This is only relevant when logging to eventlog (win32):
 #event_source = 'PostgreSQL'
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 326896f..bfbcf96 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -396,6 +396,7 @@ extern int	Log_error_verbosity;
 extern char *Log_line_prefix;
 extern int	Log_destination;
 extern char *Log_destination_string;
+extern bool syslog_sequence_numbers;
 
 /* Log destination bitmap */
 #define LOG_DESTINATION_STDERR	 1
-- 
2.7.2

From 72ea7dc222f41ab8246c0

Re: [HACKERS] pg_ctl promote wait

[Peter Eisentraut]

On 2/19/16 3:09 PM, Tom Lane wrote:
> I see no need for an additional mechanism.  Just watch pg_control until
> you see DB_IN_PRODUCTION state there, then switch over to the same
> connection probing that "pg_ctl start -w" uses.

Here is a patch set around that idea.

The subsequent discussion mentioned that there might still be a window
between end of waiting and when read-write queries would be accepted.  I
don't know how big that window would be in practice and would be
interested in some testing and feedback.
From cb5d4a63620636d4043d1a85acf7fcfdace73b1d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 28 Feb 2016 20:21:54 -0500
Subject: [PATCH 1/3] pg_ctl: Add tests for promote action

---
 src/bin/pg_ctl/t/003_promote.pl | 62 +
 src/test/perl/TestLib.pm| 11 
 2 files changed, 73 insertions(+)
 create mode 100644 src/bin/pg_ctl/t/003_promote.pl

diff --git a/src/bin/pg_ctl/t/003_promote.pl b/src/bin/pg_ctl/t/003_promote.pl
new file mode 100644
index 000..64d72e0
--- /dev/null
+++ b/src/bin/pg_ctl/t/003_promote.pl
@@ -0,0 +1,62 @@
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 13;
+
+my $tempdir   = TestLib::tempdir;
+#my $tempdir_short = TestLib::tempdir_short;
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', "$tempdir/nonexistent" ],
+   qr/directory .* does not exist/,
+   'pg_ctl promote with nonexistent directory');
+
+my $node_primary = get_new_node('primary');
+$node_primary->init;
+$node_primary->append_conf(
+	"postgresql.conf", qq(
+wal_level = hot_standby
+max_wal_senders = 2
+wal_keep_segments = 20
+hot_standby = on
+)
+	);
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', $node_primary->data_dir ],
+   qr/PID file .* does not exist/,
+   'pg_ctl promote of not running instance fails');
+
+$node_primary->start;
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', $node_primary->data_dir ],
+   qr/not in standby mode/,
+   'pg_ctl promote of primary instance fails');
+
+my $node_standby = get_new_node('standby');
+$node_primary->backup('my_backup');
+$node_standby->init_from_backup($node_primary, 'my_backup');
+my $connstr_primary = $node_primary->connstr('postgres');
+
+$node_standby->append_conf(
+	"recovery.conf", qq(
+primary_conninfo='$connstr_primary'
+standby_mode=on
+recovery_target_timeline='latest'
+)
+	);
+
+$node_standby->start;
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+			qr/^t$/,
+			'standby is in recovery');
+
+command_ok([ 'pg_ctl', 'promote', '-D', $node_standby->data_dir ],
+		   'pg_ctl promote of standby runs');
+
+sleep 3;  # needs a moment to react
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+			qr/^f$/,
+			'promoted standby is not in recovery');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 3d11cbb..dd275cf 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -34,6 +34,7 @@ our @EXPORT = qw(
   program_version_ok
   program_options_handling_ok
   command_like
+  command_fails_like
 
   $windows_os
 );
@@ -262,4 +263,14 @@ sub command_like
 	like($stdout, $expected_stdout, "$test_name: matches");
 }
 
+sub command_fails_like
+{
+	my ($cmd, $expected_stderr, $test_name) = @_;
+	my ($stdout, $stderr);
+	print("# Running: " . join(" ", @{$cmd}) . "\n");
+	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+	ok(!$result, "@$cmd exit code not 0");
+	like($stderr, $expected_stderr, "$test_name: matches");
+}
+
 1;
-- 
2.7.2

From 7c1b6e94f5e3beb9e558d2af7098940d4475fe11 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 28 Feb 2016 20:21:54 -0500
Subject: [PATCH 2/3] pg_ctl: Detect current standby state from pg_control

pg_ctl used to determine whether a server was in standby mode by looking
for a recovery.conf file.  With this change, it instead looks into
pg_control, which is potentially more accurate.  There are also
occasional discussions about removing recovery.conf, so this removes one
dependency.
---
 src/bin/pg_ctl/pg_ctl.c | 60 ++---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index bae6c22..c38c479 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -19,6 +19,7 @@
 
 #in

[HACKERS] pg_resetxlog reference page reorganization

[Peter Eisentraut]

The pg_resetxlog reference page has grown over the years into an
unnavigable jungle, so here is a patch that reorganizes it to be more in
the style of the other ref pages, with a normal options list.
From a9024195e9f7a0b47e592f39938bdc9743152a70 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 29 Feb 2016 18:48:34 -0500
Subject: [PATCH] doc: Reorganize pg_resetxlog reference page

The pg_resetxlog reference page didn't have a proper options list, only
running text listing the options and some explanations of them.  This
might have worked when there were only a few options, but the list has
grown over the releases, and now it's hard to find an option and its
associated explanation.  So write out the options list as on other
reference pages.
---
 doc/src/sgml/ref/pg_resetxlog.sgml | 222 -
 1 file changed, 144 insertions(+), 78 deletions(-)

diff --git a/doc/src/sgml/ref/pg_resetxlog.sgml b/doc/src/sgml/ref/pg_resetxlog.sgml
index 1bcc5a7..fd9d0be 100644
--- a/doc/src/sgml/ref/pg_resetxlog.sgml
+++ b/doc/src/sgml/ref/pg_resetxlog.sgml
@@ -22,15 +22,9 @@
  
   
pg_resetxlog
-   -c xid,xid
-f
-n
-   -o oid
-   -x xid
-   -e xid_epoch
-   -m mxid,mxid
-   -O mxoff
-   -l xlogfile
+   option
-D datadir
   
  
@@ -76,78 +70,108 @@ Description
execute any data-modifying operations in the database before you dump,
as any such action is likely to make the corruption worse.
   
+ 
 
-  
-   The -o, -x, -e,
-   -m, -O,
-   -c
-   and -l
-   options allow the next OID, next transaction ID, next transaction ID's
-   epoch, next and oldest multitransaction ID, next multitransaction offset,
-   oldest and newest transaction IDs for which the commit time can be retrieved,
-   and WAL
-   starting address values to be set manually.  These are only needed when
-   pg_resetxlog is unable to determine appropriate values
-   by reading pg_control.  Safe values can be determined as
-   follows:
+ 
+  Options
 
-   
+  
+   
+-f
 
  
-  A safe value for the next transaction ID (-x)
-  can be determined by looking for the numerically largest
-  file name in the directory pg_clog under the data directory,
-  adding one,
-  and then multiplying by 1048576.  Note that the file names are in
-  hexadecimal.  It is usually easiest to specify the option value in
-  hexadecimal too. For example, if 0011 is the largest entry
-  in pg_clog, -x 0x120 will work (five
-  trailing zeroes provide the proper multiplier).
+  Force pg_resetxlog to proceed even if it cannot determine
+  valid data for pg_control, as explained above.
  
 
+   
 
+   
+-n
 
  
-  A safe value for the next multitransaction ID (first part of -m)
-  can be determined by looking for the numerically largest
-  file name in the directory pg_multixact/offsets under the
-  data directory, adding one, and then multiplying by 65536.
-  Conversely, a safe value for the oldest multitransaction ID (second part of
-  -m)
-  can be determined by looking for the numerically smallest
-  file name in the same directory and multiplying by 65536.
-  As above, the file names are in hexadecimal, so the easiest way to do
-  this is to specify the option value in hexadecimal and append four zeroes.
+  The -n (no operation) option instructs
+  pg_resetxlog to print the values reconstructed from
+  pg_control and values about to be changed, and then exit
+  without modifying anything. This is mainly a debugging tool, but can be
+  useful as a sanity check before allowing pg_resetxlog
+  to proceed for real.
  
 
+   
 
+   
+-V
+--version
+Display version information, then exit.
+   
+
+   
+-?
+--help
+Show help, then exit.
+   
+  
+
+  
+   The following options are only needed when
+   pg_resetxlog is unable to determine appropriate values
+   by reading pg_control.  Safe values can be determined as
+   described below.  For values that take numeric arguments, hexadecimal
+   values can be specified by using the prefix 0x.
+  
+
+  
+   
+-c xid,xid
 
  
-  A safe value for the next multitransaction offset (-O)
-  can be determined by looking for the numerically largest
-  file name in the directory pg_multixact/members under the
-  data directory, adding one, and then multiplying by 52352.  As above,
-  the file names are in hexadecimal.  There is no simple recipe such as
-  the ones above of appending zeroes.
+  Manually set the oldest and newest transaction IDs for which the commit
+  time can be retrieved.
  
-
 
-
  
   A safe value for the oldest transaction ID for which the commit time can
-  be retrieved (first part of -c) can be determined by looking
+  be retrieved (first part) can be determined by looking
   for the numerically smallest f

Re: [HACKERS] remove wal_level archive

[Peter Eisentraut]

On 2/8/16 9:36 AM, David Steele wrote:
> -#define XLogIsNeeded() (wal_level >= WAL_LEVEL_ARCHIVE)
> +#define XLogIsNeeded() (wal_level >= WAL_LEVEL_REPLICA)
> <...>
> -#define XLogStandbyInfoActive() (wal_level >= WAL_LEVEL_HOT_STANDBY)
> +#define XLogStandbyInfoActive() (wal_level >= WAL_LEVEL_REPLICA)
> 
> Since these are identical now shouldn't one be removed?  I searched the
> code and I couldn't find anything that looked dead (i.e. XLogIsNeeded()
> && !XLogStandbyInfoActive()) but it still seems like having both could
> cause confusion.

I think this should eventually be cleaned up, but it doesn't seem
necessary in the first patch.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] remove wal_level archive

[Peter Eisentraut]

On 2/8/16 7:34 AM, Michael Paquier wrote:
> Shouldn't backup.sgml be updated as well? Here is the portion that I
> am referring to:
> To enable WAL archiving, set the 
> configuration parameter to archive or higher,
>  to on,
> 
>  But minimal WAL does not contain enough information to reconstruct 
> the
> -data from a base backup and the WAL logs, so archive or
> +data from a base backup and the WAL logs, so replica or
>  higher must be used to enable WAL archiving
>  () and streaming replication.
> 

Checked for leftovers again and fixed one.

> 
> -In hot_standby level, the same information is logged as
> -with archive, plus information needed to reconstruct
> -the status of running transactions from the WAL. To enable read-only
> As the paragraph about the difference between hot_standby and archive
> is removed, I think that it would be better to mention that setting
> wal_level to replica allows to reconstruct data from a base backup and
> the WAL logs, *and* to run read-only queries when hot_standby is
> enabled.

Well, I think that is really only of historical interest.  The
assumption is, as long as hot_standby = on, you can run read-only
queries.  The WAL level is taken completely out of the mental
consideration, because if you have replicate at all, it's good enough.
That is part of the point of this patch.

> 
> -   if (ControlFile->wal_level < WAL_LEVEL_HOT_STANDBY)
> +   if (ControlFile->wal_level < WAL_LEVEL_REPLICA)
> Upthread it was mentioned that switching to an approach where enum
> values are directly listed would be better. The target of an extra
> patch on top of this one?

I'm not sure what is meant by that.

> 
> -   if (wal_level < WAL_LEVEL_ARCHIVE)
> -   ereport(ERROR,
> -
> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -errmsg("replication slots can only be
> used if wal_level >= archive")));
> We should still forbid the creation of replication slots if wal_level = 
> minimal.

I think I took this out because you actually can't get to that check,
but I put it back in because it seems better for clarity.

From 574dd447b4a077267200d2ca9b8b4e185d4bb052 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 29 Feb 2016 20:01:54 -0500
Subject: [PATCH] Merge wal_level "archive" and "hot_standby" into new name
 "replica"

The distinction between "archive" and "hot_standby" existed only because
at the time "hot_standby" was added, there was some uncertainty about
stability.  This is now a long time ago.  We would like to move forward
with simplifying the replication configuration, but this distinction is
in the way, because a primary server cannot tell (without asking a
standby or predicting the future) which one of these would be the
appropriate level.

Pick a new name for the combined setting to make it clearer that it
covers all (non-logical) backup and replication uses.  The old values
are still accepted but are converted internally.
---
 doc/src/sgml/backup.sgml  |  4 ++--
 doc/src/sgml/config.sgml  | 30 +++
 doc/src/sgml/high-availability.sgml   |  2 +-
 doc/src/sgml/ref/alter_system.sgml|  2 +-
 doc/src/sgml/ref/pgupgrade.sgml   |  2 +-
 src/backend/access/rmgrdesc/xlogdesc.c|  5 +++--
 src/backend/access/transam/xact.c |  2 +-
 src/backend/access/transam/xlog.c | 20 --
 src/backend/access/transam/xlogfuncs.c|  2 +-
 src/backend/postmaster/postmaster.c   |  2 +-
 src/backend/replication/slot.c|  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |  2 +-
 src/bin/pg_controldata/pg_controldata.c   |  6 ++
 src/include/access/xlog.h | 11 +-
 src/include/catalog/pg_control.h  |  2 +-
 src/test/perl/PostgresNode.pm |  2 +-
 17 files changed, 44 insertions(+), 54 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 7413666..9092cf8 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -592,7 +592,7 @@ Setting Up WAL Archiving
 

 To enable WAL archiving, set the 
-configuration parameter to archive or higher,
+configuration parameter to replica or higher,
  to on,
 and specify the shell command to use in the  configuration parameter.  In practice
@@ -1285,7 +1285,7 @@ Standalone Hot Backups
   If more flexibility in copying the backup files is needed, a lower
   level process can be used for standalone hot backups a

Re: [HACKERS] GinPageIs* don't actually return a boolean

[Peter Eisentraut]

On 2/11/16 9:30 PM, Michael Paquier wrote:
>> Well, Yury was saying upthread that some MSVC versions have a problem
>> with the existing coding, which would be a reason to back-patch ...
>> but I'd like to see a failing buildfarm member first.  Don't particularly
>> want to promise to support compilers not represented in the farm.
> 
> Grmbl. Forgot to attach the rebased patch upthread. Here is it now.
> 
> As of now the only complain has been related to MS2015 and MS2013. If
> we follow the pattern of cec8394b and [1], support to compile on newer
> versions of MSVC would be master and REL9_5_STABLE, but MS2013 is
> supported down to 9.3. Based on this reason, we would want to
> backpatch down to 9.3 the patch of this thread.

After reviewing this thread and relevant internet lore, I think this
might be the wrong way to address this problem.  It is in general not
guaranteed in C that a Boolean-sounding function or macro returns 0 or
1.  Prime examples are ctype.h functions such as isupper().  This is
normally not a problem because built-in conditionals such as if, &&, ||
handle this just fine.  So code like

-   Assert(!create || !!txn);
+   Assert(!create || txn != NULL);

is arguably silly either way.  There is no risk in writing just

Assert(!create || txn);

The problem only happens if you compare two "Boolean" values directly
with each other; and so maybe you shouldn't do that, or at least place
the extra care there instead, instead of fighting a permanent battle
with the APIs you're using.  (This isn't an outrageous requirement: You
can't compare floats or strings either without extra care.)

A quick look through the code based on the provided patch shows that
approximately the only place affected by this is

if (isLeaf != GinPageIsLeaf(page) || isData != GinPageIsData(page))
elog(ERROR, "right sibling of GIN page is of different type");

and that's not actually a problem because isLeaf and isData are earlier
populated by the same macros.  It would still be worth fixing, but a
localized fix seems better.


Now on the matter of stdbool, I tried putting an #include 
near the top of c.h and compile that to see what would happen.  This is
the first warning I see:

ginlogic.c: In function 'shimTriConsistentFn':
ginlogic.c:171:24: error: comparison of constant '2' with boolean
expression is always false [-Werror=bool-compare]
   if (key->entryRes[i] == GIN_MAYBE)
^

and then later on something related:

../../../../src/include/tsearch/ts_utils.h:107:13: note: expected '_Bool
(*)(void *, QueryOperand *) {aka _Bool (*)(void *, struct 
*)}' but argument is of type 'GinTernaryValue (*)(void *, QueryOperand
*) {aka char (*)(void *, struct  *)}'

So the compiler is actually potentially helpful, but as it stands,
PostgreSQL code is liable to break if you end up with stdbool.h somehow.

(plperl also fails to compile because of a hot-potato game about who is
actually responsible for defining bool.)

So one idea would be to actually get ahead of the game, include
stdbool.h if available, fix the mentioned issues, and maybe get more
robust code that way.

But the lore on the internet casts some doubt on that: There is no
guarantee that bool is 1 byte, that bool can be passed around like char,
or even that bool arrays are laid out like char arrays.  Maybe this all
works out okay, just like it has worked out so far that int is 4 bytes,
but we don't know enough about it.  We could probably add some configure
tests around that.

We could also go the other way and forcibly undefine an existing bool
type (since stdbool.h is supposed to use macros, not typedefs).  But
that might not work well if a header that is included later pulls in
stdbool.h on top of that.


My proposal on this particular patch is to do nothing.  The stdbool
issues should be looked into, for the sake of Windows and other
future-proofness.  But that will likely be an entirely different patch.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Fix for OpenSSL error queue bug

[Peter Eisentraut]

On 2/5/16 5:04 AM, Peter Geoghegan wrote:
> As Heikki goes into on that thread, the appropriate action seems to be
> to constantly reset the error queue, and to make sure that we
> ourselves clear the queue consistently. (Note that we might not have
> consistently called ERR_get_error() in the event of an OOM within
> SSLerrmessage(), for example). I have not changed backend code in the
> patch, though; I felt that we had enough control of the general
> situation there for it to be unnecessary to lock everything down.

I think clearing the error after a call is not necessary.  The API
clearly requires that you should clear the error queue before a call, so
clearing it afterwards does not accomplish anything, except maybe make
broken code work sometimes, for a while.  Also, there is nothing that
says that an error produces exactly one entry in the error queue; it
could be multiple.  Or that errors couldn't arise at random times
between the reset and whatever happens next.

I think this is analogous to clearing errno before a C library call.
You could clear it afterwards as well, to be nice to the next guy, but
the next guy should really take care of that themselves, and we can't
rely on what happens in between anyway.

The places that you identified for change look correct as far as libpq
goes.  I do think that the backend should be updated in the same way,
because it's a) correct, b) easy enough, and c) there could well be
interactions with postgres_fdw, plproxy, plperl, or who knows what.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Fix handling of invalid sockets returned by PQsocket()

[Peter Eisentraut]

On 2/17/16 10:52 PM, Michael Paquier wrote:
> On Thu, Feb 18, 2016 at 1:58 AM, Alvaro Herrera
>  wrote:
>> Michael Paquier wrote:
>>> Hi all,
>>>
>>> After looking at Alvaro's message mentioning the handling of
>>> PQsocket() for invalid sockets, I just had a look by curiosity at
>>> other calls of this routine, and found a couple of issues:
>>> 1) In vacuumdb.c, init_slot() does not check for the return value of 
>>> PQsocket():
>>> slot->sock = PQsocket(conn);
>>> 2) In isolationtester.c, try_complete_step() should do the same.
>>> 3) In pg_recvlogical.c for StreamLogicalLog() I am spotting the same 
>>> problem.
>>> I guess those ones should be fixed as well, no?
>>
>> I patched pgbench to use PQerrorMessage rather than strerror(errno).  I
>> think your patch should do the same.
> 
> OK, this looks like a good idea. I would suggest doing the same in
> receivelog.c then.

Let's make the error messages consistent as "invalid socket".  "bad
socket" isn't really our style, and pg_basebackup saying "socket not
open" is just plain incorrect.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GCC 6 warning fixes

[Peter Eisentraut]

On 3/8/16 4:44 PM, Robert Haas wrote:
> On Mon, Feb 29, 2016 at 4:50 PM, Thomas Munro
>  wrote:
>> On Sat, Feb 20, 2016 at 5:14 PM, Peter Eisentraut  wrote:
>>> Here are three patches to fix new warnings in GCC 6.
>>>
>>> 0001 is apparently a typo.
>>
>> Right, looks like it.  Builds and tests OK with this change (though I
>> didn't get any warning from GCC6.0.0 -Wall for this one).
>>
>>> 0002 was just (my?) stupid code to begin with.
>>
>> Right, it makes sense to define QL_HELP in just one translation unit
>> with external linkage.  Builds and works fine.  I got the 'defined but
>> not used' warning from GCC6 and it went away with this patch.
>>
>>> 0003 is more of a workaround.  There could be other ways address this, too.
>>
>> This way seems fine to me (you probably want the function to continue
>> to exist rather than, say, becoming a macro evaluating to false on
>> non-WIN32, if this gets backpatched).  I got this warning from GCC6
>> and it went away with this patch.
> 
> Peter, are you going to commit this?

done



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] syslog configurable line splitting behavior

[Peter Eisentraut]

On 3/4/16 11:01 AM, Alexander Korotkov wrote:
> On Sat, Feb 27, 2016 at 6:49 AM, Peter Eisentraut  <mailto:pete...@gmx.net>> wrote:
> 
> Writing log messages to syslog caters to ancient syslog implementations
> in two ways:
> 
> - sequence numbers
> - line splitting
> 
> While these are arguably reasonable defaults, I would like a way to turn
> them off, because they get in the way of doing more interesting things
> with syslog (e.g., logging somewhere that is not just a text file).
> 
> So I propose the two attached patches that introduce new configuration
> Boolean parameters syslog_sequence_numbers and syslog_split_lines that
> can toggle these behaviors.
> 
> 
> Would it have any usage if we make PG_SYSLOG_LIMIT configurable (-1 for
> disable) instead of introducing boolean?

That would work, too.  But then we'd need another setting to disable
splitting on newlines.  That way we'd have more settings, but they
actually mirror the corresponding settings on the rsyslogd side better.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] syslog configurable line splitting behavior

[Peter Eisentraut]

On 3/8/16 9:12 PM, Andreas Karlsson wrote:
> As someone who uses syslog for my servers I find both of these GUCs
> useful, especially when used in combination, and I do not think a
> compile time option like suggest by Alexander would be suitable
> substitute because then I would need a custom build of PostgreSQL just
> to change this which seems too much effort just for this.

I think he was suggesting to take the existing compile-time constant and
make a run-time setting out of it.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Crash with old Windows on new CPU

[Peter Eisentraut]

On 2/12/16 11:24 AM, Christian Ullrich wrote:
> Otherwise, it may be time to update the manual (15.6 Supported
> Platforms) where it says PostgreSQL "can be expected to work on these
> operating systems: [...] Windows (Win2000 SP4 and later), [...]".
> Perhaps we could add "except Windows before 7 SP1/2008R2 SP1 when
> running in x64 mode on Intel CPUs introduced after May 2013 (Haswell and
> later)"?

Wouldn't the fix be for users to upgrade their service packs?


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Fix for OpenSSL error queue bug

[Peter Eisentraut]

On 3/10/16 6:10 PM, Peter Geoghegan wrote:
> On Thu, Mar 10, 2016 at 3:09 PM, Peter Geoghegan  wrote:
>> Getting to it very soon. Just really busy right this moment.
> 
> That said, I agree with Peter's remarks about doing this frontend and
> backend. So, while I'm not sure, I think we're in agreement on all
> issues. I would have no problem with Peter E following through with
> final steps + commit as Robert outlined, if that works for him.

My proposal is the attached patch.


From 697b4f75fccbb5eda530211f3ef58c2b226c5461 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 10 Mar 2016 20:59:30 -0500
Subject: [PATCH] Clear OpenSSL error queue before OpenSSL calls

OpenSSL requires that the error queue be cleared before certain OpenSSL
API calls, so that you can be sure that the error you are checking
afterwards actually come from you and was not left over from other
activity.  We had never done that, which appears to have worked as long
as we are the only users of OpenSSL in the process.  But if a process
using libpq or a backend plugin uses OpenSSL as well, this can lead to
confusion and crashes.

see bug #12799 and https://bugs.php.net/bug.php?id=68276

based on patches by Dave Vitek and Peter Geoghegan
---
 src/backend/libpq/be-secure-openssl.c| 3 +++
 src/interfaces/libpq/fe-secure-openssl.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 1e3dfb6..be337f5 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -353,6 +353,7 @@ be_tls_open_server(Port *port)
 	port->ssl_in_use = true;
 
 aloop:
+	ERR_clear_error();
 	r = SSL_accept(port->ssl);
 	if (r <= 0)
 	{
@@ -501,6 +502,7 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
 	int			err;
 
 	errno = 0;
+	ERR_clear_error();
 	n = SSL_read(port->ssl, ptr, len);
 	err = SSL_get_error(port->ssl, n);
 	switch (err)
@@ -558,6 +560,7 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
 	int			err;
 
 	errno = 0;
+	ERR_clear_error();
 	n = SSL_write(port->ssl, ptr, len);
 	err = SSL_get_error(port->ssl, n);
 	switch (err)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 133546b..0535338 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -212,6 +212,7 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)
 
 rloop:
 	SOCK_ERRNO_SET(0);
+	ERR_clear_error();
 	n = SSL_read(conn->ssl, ptr, len);
 	err = SSL_get_error(conn->ssl, n);
 	switch (err)
@@ -320,6 +321,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
 	int			err;
 
 	SOCK_ERRNO_SET(0);
+	ERR_clear_error();
 	n = SSL_write(conn->ssl, ptr, len);
 	err = SSL_get_error(conn->ssl, n);
 	switch (err)
@@ -1327,6 +1329,7 @@ open_client_SSL(PGconn *conn)
 {
 	int			r;
 
+	ERR_clear_error();
 	r = SSL_connect(conn->ssl);
 	if (r <= 0)
 	{
-- 
2.7.2


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Relaxing SSL key permission checks

[Peter Eisentraut]

On 3/4/16 3:55 PM, Alvaro Herrera wrote:
> * it failed to check for S_IXUSR, so permissions 0700 were okay, in
> contradiction with what the error message indicates.  This is a
> preexisting bug actually.  Do we want to fix it by preventing a
> user-executable file (possibly breaking compability with existing
> executable key files), or do we want to document what the restriction
> really is?

I think we should not check for S_IXUSR.  There is no reason for doing that.

I can imagine that key files are sometimes copied around using USB
drives with FAT file systems or other means of that sort where
permissions can scrambled.  While I hate gratuitous executable bits as
much as the next person, insisting here would just create annoyances in
practice.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Fix for OpenSSL error queue bug

[Peter Eisentraut]

On 3/10/16 9:38 PM, Peter Geoghegan wrote:
> Looked at your proposed patch. Will respond to your original mail on the 
> matter.
> 
> On Thu, Mar 3, 2016 at 4:15 PM, Peter Eisentraut  wrote:
>> I think clearing the error after a call is not necessary.  The API
>> clearly requires that you should clear the error queue before a call, so
>> clearing it afterwards does not accomplish anything, except maybe make
>> broken code work sometimes, for a while.
> 
> Uh, if it's so clear, then why haven't we been doing it all along?

The issue only happens if two interleaving trains of execution, one of
which is libpq, use OpenSSL.  Not many applications do that.  And you
also need to provoke the errors in a certain order.  And even then, in
some cases you might just see a false positive error, rather than a
crash.  So it's an edge case.

> Part of the problem is that various scripting language OpenSSL
> wrappers are only very thin wrappers. They effectively pass the buck
> on to PHP and Ruby devs. If we cannot get it right, what chance have
> they? I've personally experienced a bit uptick in complaints about
> this recently. I think there are 3 separate groups within Heroku that
> regularly ask me how this patch is doing.

I think they have been getting away with it for so long for the same
reasons.

Arguably, if everyone followed "my" approach, this should be very easy
to fix everywhere.  Instead, reading through the PHP bug report, they
are coming up with a fairly complex solution for clearing the error
queue afterwards so as to not leave "landmines" for the next guy.  But
their code will (AFAICT) still be wrong because they are not clearing
the error *before* the API calls where it is required per documentation.
 So "everyone" (sample of 2) is scrambling to clean up for the next guy
instead of doing the straightforward fix of following the API
documentation and cleaning up before their own calls.

I also see the clean-up-afterwards approach in the Python ssl module.  I
fear there is de facto a second API specification that requires you to
clean up errors after yourself and gives an implicit guarantee that the
error queue is empty whenever you want to make any SSL calls.  I don't
think this actually works in all cases, but maybe if everyone else is
convinced of that (in plain violation of the published OpenSSL
documentation, AFAICT) we need to get on board with that for
interoperability.

>> Also, there is nothing that
>> says that an error produces exactly one entry in the error queue; it
>> could be multiple.  Or that errors couldn't arise at random times
>> between the reset and whatever happens next.
> 
> I think that it's kind of implied, since calling ERR_get_error() pops
> the stack. But even if that isn't so, it might be worth preventing bad
> things from happening to client applications only sometimes.

The lore on the internet suggests that multiple errors could definitely
happen.  So popping one error afterwards isn't going to fix it, it just
moves the edge case around.  At least what we should do is clear the
entire queue afterwards instead of just the first error.

> doesn't it give you pause? Heikki seemed to think that clearing our
> own queue was important when he looked at this a year ago:
> 
> http://www.postgresql.org/message-id/54edd30d.5050...@vmware.com

I think that message suggests that we should clear the queue before each
call, not after.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: BSD Authentication support

[Peter Eisentraut]

On 1/7/16 9:40 PM, Marisa Emerson wrote:
> There's a port for PAM, but we would prefer to use BSD Auth as its quite
> a lot cleaner and is standard on OpenBSD.
> 
> I've attached an updated patch that includes documentation. It has been
> tested against OpenBSD 5.8. I'll add this thread to the commitfest.

(Not a BSD user, just reviewing the code.)

configure.in has "build with BSD support", which should be "build with
BSD Authentication support".

There should be some documentation of the new configure option in
installation.sgml.

The documentation in client-auth.sgml speaks of a postgresql user and an
auth group.  Maybe that's clear to users of BSD, but I don't know
whether these are OS entities or groups that I need to create or what.

The auth_userokay() call hardcodes a "type" of "pg-auth".  That seems
important and should probably be documented.  Extrapolating from PAM, I
think that should perhaps be an option in pg_hba.conf.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: BSD Authentication support

[Peter Eisentraut]

On 3/11/16 4:38 PM, Thomas Munro wrote:
> It looks like this needs review from an OpenBSD user specifically.
> FreeBSD and NetBSD use PAM instead of BSD auth.

FreeBSD has man pages for this stuff, so maybe they also have it now.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: FW: [HACKERS] [NOVICE] WHERE clause not used when index is used

[Peter Eisentraut]

On 3/15/16 2:28 PM, Jernigan, Kevin wrote:
> I recently joined the product management team for AWS RDS Postgres
> (after years at Oracle in their database team), and we are very
> interested in confirming (or not) that the fix for the problem below
> will be included in 9.5.2, and in the community’s plans (likely date)
> for releasing 9.5.2.

The patch was reverted in the 9.5 branch, so assuming that that is the
end of this investigation (which it appears to be), then it will be part
of the 9.5.2 release.

> Is there an email list other than hackers where we can follow
> discussions on release plans for 9.5.2 (and future releases)?

This is a good list to follow to know about release schedules.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pam auth - add rhost item

[Peter Eisentraut]

On 3/10/16 8:11 AM, Grzegorz Sampolski wrote:
> In attchment new patch with updated documentation and with small change
> to coding style as you suggested.

This patch seems fine.  I'm not sure about the name "pamusedns" for the
option, since we use the OS resolver, which might not actually use DNS.
 Maybe something like "pam_use_hostname"?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] insufficient qualification of some objects in dump files

[Peter Eisentraut]

On 2/26/16 1:30 AM, Tom Lane wrote:
> As the patch is presented, I agree with Peter that it does not really
> need a format number bump.  The question that has to be answered is
> whether this solution is good enough?  You could not trust it for
> automated processing of tags --- it's easy to think of cases in which the
> schema/object name separation would be ambiguous.  So is the tag really
> "strictly for human consumption"?  I'm not sure about that.

Well what are those tags for?  They are not used by pg_restore, so they
are for users.  My understanding is that the tags help in editing a TOC
list for use by pg_restore.  What pg_restore actually reads are the
OIDs, but the tags are there so users can edit the files.  The tags can
also be used for ad hoc automatic processing.  They are not sufficiently
delimited and escaped for robustness in all cases, but it can be done if
you control the inputs and know what to expect.  But this is the same
problem before and after my patch.

Both of these cases are helped by my patch, and both of these cases were
pretty broken (for the object classes in question) before my patch.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Peter Eisentraut]

On 2/5/16 3:09 AM, Kyotaro HORIGUCHI wrote:
> I considered how to make tab-completion robust for syntactical
> noises, in other words, optional words in syntax. Typically "IF
> (NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
> further completion.

To repeat the question I raised in the previous commit fest about tab
completion: Why do you want tab completion for IF NOT EXISTS?  When you
tab complete, the completion mechanism will show you whether the item in
question exists.  What is the use case?




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Relaxing SSL key permission checks

[Peter Eisentraut]

On 3/10/16 9:20 PM, Peter Eisentraut wrote:
> On 3/4/16 3:55 PM, Alvaro Herrera wrote:
>> * it failed to check for S_IXUSR, so permissions 0700 were okay, in
>> contradiction with what the error message indicates.  This is a
>> preexisting bug actually.  Do we want to fix it by preventing a
>> user-executable file (possibly breaking compability with existing
>> executable key files), or do we want to document what the restriction
>> really is?
> 
> I think we should not check for S_IXUSR.  There is no reason for doing that.
> 
> I can imagine that key files are sometimes copied around using USB
> drives with FAT file systems or other means of that sort where
> permissions can scrambled.  While I hate gratuitous executable bits as
> much as the next person, insisting here would just create annoyances in
> practice.

I'm happy with this patch except this minor point.  Any final comments?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] syslog configurable line splitting behavior

[Peter Eisentraut]

On 3/8/16 9:12 PM, Andreas Karlsson wrote:
> I have one nitpick: why is one of the variables "true" while the other
> is "on" in the example? I think both should be "on".
> 
> #syslog_sequence_numbers = true
> #syslog_split_lines = on
> 
> Another possible improvement would be to change "Split messages sent to
> syslog." to something more verbose like "Split messages sent to syslog,
> by lines and to fit in 1024 bytes.".

Updated patches with your suggestions.  I also renamed
syslog_split_lines to syslog_split_messages, which I think is more accurate.


From 70bacecba46eb38c02c43957c2f1812faf5684df Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 26 Feb 2016 22:34:30 -0500
Subject: [PATCH 1/2] Add syslog_sequence_numbers parameter

---
 doc/src/sgml/config.sgml  | 28 +++
 src/backend/utils/error/elog.c| 12 ++--
 src/backend/utils/misc/guc.c  | 10 ++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/utils/elog.h  |  1 +
 5 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6c73fb4..bbe87ce 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4305,6 +4305,34 @@ Where To Log

   
 
+  
+   syslog_sequence_numbers (boolean)
+
+ syslog_sequence_numbers configuration parameter
+
+   
+
+   
+
+ When logging to syslog and this is on (the
+ default), then each message will be prefixed by an increasing
+ sequence number (such as [2]).  This circumvents
+ the --- last message repeated N times --- suppression
+ that many syslog implementations perform by default.  In more modern
+ syslog implementations, repeat message suppression can be configured
+ (for example, $RepeatedMsgReduction
+ in rsyslog), so this might not be
+ necessary.  Also, you could turn this off if you actually want to
+ suppress repeated messages.
+
+
+
+ This parameter can only be set in the postgresql.conf
+ file or on the server command line.
+
+  
+ 
+
  
   event_source (string)
   
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 5b7554b..88421c7 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -106,6 +106,7 @@ int			Log_error_verbosity = PGERROR_VERBOSE;
 char	   *Log_line_prefix = NULL;		/* format for extra log line info */
 int			Log_destination = LOG_DESTINATION_STDERR;
 char	   *Log_destination_string = NULL;
+bool		syslog_sequence_numbers = true;
 
 #ifdef HAVE_SYSLOG
 
@@ -2018,7 +2019,11 @@ write_syslog(int level, const char *line)
 
 			chunk_nr++;
 
-			syslog(level, "[%lu-%d] %s", seq, chunk_nr, buf);
+			if (syslog_sequence_numbers)
+syslog(level, "[%lu-%d] %s", seq, chunk_nr, buf);
+			else
+syslog(level, "[%d] %s", chunk_nr, buf);
+
 			line += buflen;
 			len -= buflen;
 		}
@@ -2026,7 +2031,10 @@ write_syslog(int level, const char *line)
 	else
 	{
 		/* message short enough */
-		syslog(level, "[%lu] %s", seq, line);
+		if (syslog_sequence_numbers)
+			syslog(level, "[%lu] %s", seq, line);
+		else
+			syslog(level, "%s", line);
 	}
 }
 #endif   /* HAVE_SYSLOG */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f0d4ec1..3ef432a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1632,6 +1632,16 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"syslog_sequence_numbers", PGC_SIGHUP, LOGGING_WHERE,
+			gettext_noop("Add sequence number to syslog messags to avoid duplicate suppression."),
+			NULL
+		},
+		&syslog_sequence_numbers,
+		true,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee3d378..b72ea6d 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -358,6 +358,7 @@
 # These are relevant when logging to syslog:
 #syslog_facility = 'LOCAL0'
 #syslog_ident = 'postgres'
+#syslog_sequence_numbers = on
 
 # This is only relevant when logging to eventlog (win32):
 #event_source = 'PostgreSQL'
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7d338dd..e245b2e 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -397,6 +397,7 @@ extern int	Log_error_verbosity;
 extern char *Log_line_prefix;
 extern int	Log_destination;
 extern char *Log_destination_string;
+extern

Re: [HACKERS] Relaxing SSL key permission checks

[Peter Eisentraut]

Committed with the discussed adjustment and documentation update.

On 3/18/16 2:26 PM, Christoph Berg wrote:
> Re: Peter Eisentraut 2016-03-16 <56e8c221.1050...@gmx.net>
>>>> * it failed to check for S_IXUSR, so permissions 0700 were okay, in
>>>> contradiction with what the error message indicates.  This is a
>>>> preexisting bug actually.  Do we want to fix it by preventing a
>>>> user-executable file (possibly breaking compability with existing
>>>> executable key files), or do we want to document what the restriction
>>>> really is?
>>>
>>> I think we should not check for S_IXUSR.  There is no reason for doing that.
>>>
>>> I can imagine that key files are sometimes copied around using USB
>>> drives with FAT file systems or other means of that sort where
>>> permissions can scrambled.  While I hate gratuitous executable bits as
>>> much as the next person, insisting here would just create annoyances in
>>> practice.
>>
>> I'm happy with this patch except this minor point.  Any final comments?
> 
> I'm fine with that change.
> 
> Do you want me to update the patch or do you already have a new
> version, given it's marked as Ready for Committer?
> 
> Christoph
> 



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Relaxing SSL key permission checks

[Peter Eisentraut]

Committed with the discussed adjustment and documentation update.

On 3/18/16 2:26 PM, Christoph Berg wrote:
> Re: Peter Eisentraut 2016-03-16 <56e8c221.1050...@gmx.net>
>>>> * it failed to check for S_IXUSR, so permissions 0700 were okay, in
>>>> contradiction with what the error message indicates.  This is a
>>>> preexisting bug actually.  Do we want to fix it by preventing a
>>>> user-executable file (possibly breaking compability with existing
>>>> executable key files), or do we want to document what the restriction
>>>> really is?
>>>
>>> I think we should not check for S_IXUSR.  There is no reason for doing that.
>>>
>>> I can imagine that key files are sometimes copied around using USB
>>> drives with FAT file systems or other means of that sort where
>>> permissions can scrambled.  While I hate gratuitous executable bits as
>>> much as the next person, insisting here would just create annoyances in
>>> practice.
>>
>> I'm happy with this patch except this minor point.  Any final comments?
> 
> I'm fine with that change.
> 
> Do you want me to update the patch or do you already have a new
> version, given it's marked as Ready for Committer?
> 
> Christoph
> 



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some bugs in psql_complete of psql

[Peter Eisentraut]

On 12/22/15 4:44 AM, Kyotaro HORIGUCHI wrote:
> 1. 0001-Fix-tab-complete-of-CREATE-INDEX.patch
> 
>   Fixes completion for CREATE INDEX in ordinary way.

This part has been fixed in another thread.  Please check whether that
satisfies all your issues.

> 3. 0002-Fix-tab-completion-for-DROP-INDEX.patch
> 
>   Fix of DROP INDEX completion in the type-2 way.

I agree that we could use completion support for DROP INDEX
CONCURRENTLY, but I would rather not throw IF NOT EXISTS into the same
patch.  We don't have support for IF NOT EXISTS anywhere else.  If you
think about, it's rather unnecessary, because tab completion will
determine for you whether an object exists.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some bugs in psql_complete of psql

[Peter Eisentraut]

On 1/12/16 9:46 PM, Peter Eisentraut wrote:
> On 12/22/15 4:44 AM, Kyotaro HORIGUCHI wrote:
>> 1. 0001-Fix-tab-complete-of-CREATE-INDEX.patch
>>
>>   Fixes completion for CREATE INDEX in ordinary way.
> 
> This part has been fixed in another thread.  Please check whether that
> satisfies all your issues.
> 
>> 3. 0002-Fix-tab-completion-for-DROP-INDEX.patch
>>
>>   Fix of DROP INDEX completion in the type-2 way.
> 
> I agree that we could use completion support for DROP INDEX
> CONCURRENTLY, but I would rather not throw IF NOT EXISTS into the same
> patch.  We don't have support for IF NOT EXISTS anywhere else.  If you
> think about, it's rather unnecessary, because tab completion will
> determine for you whether an object exists.

I have applied a reduced version of the DROP INDEX patch.  I think that
covers everything in your submission, but please check.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Releasing in September

[Peter Eisentraut]

On 1/20/16 1:44 PM, Robert Haas wrote:
> And, you know, I did my time fighting major wars to try to compress
> the release schedule, and honestly, it wasn't that much fun.  The
> process we have now is imperfect in many ways, but I no longer have
> abuse heaped on me for wanting to boot a patch out of a CommitFest.
> That may be bad for the project, but it's spared me a lot of grief
> personally.  I think that many other people are in the same situation.
> Everybody would like somebody else to be the schedule enforcer ...
> unless they have something that they still want to get in, in which
> case they would like nobody to do it.

Yeah, a lot of the ideas in this thread, while reasonable, are of the
sort "We need to be better about ..." which sounds a lot like "I need to
be better about exercising".  A system based purely on distributed
willpower isn't going to last very long, as we are finding out.

Sure, I'd like more than one party to review my patches, and I'd like to
spend more time doing additional reviews on other people's patches.  But
I can barely keep up as it is.  I know we need code reviews, but I think
it's never going to work well to the point that we are satisfied with
it.  Just look around the world, in software, open or commercial, or
even academics, and tell me who has peer reviews figured out.

My feeble attempts at alleviating this problem a bit are adding more
testing and removing outdated functionality, but both of those are also
incredibly abuse-prone activities.

FWIW, I do apologize for proposing the fifth commitfest (PGCon 2014).



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] COPY (... tab completion

[Peter Eisentraut]

On 1/19/16 6:00 AM, Andreas Karlsson wrote:
> On 01/19/2016 07:55 AM, Michael Paquier wrote:
>> +/* If we have COPY BINARY, compelete with list of tables */
>> s/compelete/complete
> 
> Fixed.
> 
>> +else if (TailMatches2("COPY|\\copy", "("))
>> +COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
>> "UPDATE", "DELETE", "WITH");
>> This one should be Matches, no?
> 
> Yep, fixed.

Committed v4, thanks.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/18/16 10:58 AM, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
>> I have written a couple of patches to improve the integration of the
>> postgres daemon with systemd.
> 
> Great.  Is anything happening with these patches, or?  They've been
> inactive for quite a while now.

Well, they are waiting for someone to review them.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Improved tab completion for FDW DDL

[Peter Eisentraut]

On 1/18/16 8:36 PM, Andreas Karlsson wrote:
> On 01/11/2016 02:39 AM, Peter Eisentraut wrote:
>> The second part is not necessary, because there is already code that
>> completes FDW names after "FOREIGN DATA WRAPPER".  So this already works.
> 
> Good spot, thanks. I have no idea why I did not think it worked. Maybe
> it just did not work in 9.2 and I failed when trying to reproduce it on
> master.
> 
>> - Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER.
> 
> Done.
> 
>> - Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands.
> 
> Done.

committed



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: Trigonometric functions in degrees

[Peter Eisentraut]

On 1/23/16 12:08 PM, Tom Lane wrote:
> So I pushed that, and tern/sungazer are still failing.  Noah, could you
> trace through that and see exactly where it's going off the rails?

I'm still getting a failure in float8 on OS X after commit 73193d8:

@@ -490,9 +490,9 @@
   x   | asind | acosd | atand
 --+---+---+---
-1 |   -90 |   180 |   -45
- -0.5 |   -30 |   120 |
+ -0.5 |   |   120 |
 0 | 0 |90 | 0
-  0.5 |30 |60 |
+  0.5 |   |   |
 1 |90 | 0 |45



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: Trigonometric functions in degrees

[Peter Eisentraut]

On 1/23/16 3:05 PM, Tom Lane wrote:
> Peter Eisentraut  writes:
>> On 1/23/16 12:08 PM, Tom Lane wrote:
>>> So I pushed that, and tern/sungazer are still failing.  Noah, could you
>>> trace through that and see exactly where it's going off the rails?
> 
>> I'm still getting a failure in float8 on OS X after commit 73193d8:
> 
> Weird, because my OS X critters are all happy.  Which OS X and compiler
> version, exactly?  Any special compile flags?

I'm using gcc 4.8.  It passes with the system clang.  So the explanation
is probably along the lines of what Noah has described.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: Trigonometric functions in degrees

[Peter Eisentraut]

On 1/23/16 4:18 PM, Tom Lane wrote:
> Peter Eisentraut  writes:
>> On 1/23/16 3:05 PM, Tom Lane wrote:
>>> Peter Eisentraut  writes:
>>>> I'm still getting a failure in float8 on OS X after commit 73193d8:
> 
>>> Weird, because my OS X critters are all happy.  Which OS X and compiler
>>> version, exactly?  Any special compile flags?
> 
>> I'm using gcc 4.8.  It passes with the system clang.  So the explanation
>> is probably along the lines of what Noah has described.
> 
> Ah.  Please see if what I just pushed fixes it.

Works now.  Thanks.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Releasing in September

[Peter Eisentraut]

On 1/25/16 2:48 AM, Torsten Zühlsdorff wrote:
> Nobody, but there are different solutions. And the same solutions works
> different in quality and quantity in the different projects.
> In FreeBSD for example there is an online tool for review
> (http://review.freebsd.org) which was opened to public. There you can
> review any code, left comments in the parts you wanted, submit different
> users to it etc.
> It is not perfect, but a huge step forward for the project. And
> something i misses here often.
> But as stated earlier in another thread: for a not-so-deep-involved
> volunteer, it is often unclear *what* to review. The threads are long
> and often there is no final description about how the patch is supposed
> to work. That make testing quite hard and time consuming.

I agree better code review tooling could help a bit.  The URL you post
above doesn't work at the moment (for me), though.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] remove wal_level archive

[Peter Eisentraut]

On 1/26/16 10:56 AM, Simon Riggs wrote:
> Removing one of "archive" or "hot standby" will just cause confusion and
> breakage, so neither is a good choice for removal.

I'm pretty sure nothing would break, but I do agree that it could be
confusing.

> What we should do is 
> 1. Map "archive" and "hot_standby" to one level with a new name that
> indicates that it can be used for both/either backup or replication.
>   (My suggested name for the new level is "replica"...)

I have been leaning toward making up a new name, too, but hadn't found a
good one.  I tend to like "replica", though.

> 2. Deprecate "archive" and "hot_standby" so that those will be removed
> in a later release.

If we do 1, then we might as well get rid of the old names right away.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/27/16 7:02 AM, Pavel Stehule wrote:
> The issues:
> 
> 1. configure missing systemd integration test, compilation fails:
> 
> postmaster.o postmaster.c
> postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
> directory

Updated patch attached that fixes this by adding additional checking in
configure.

> 3. PostgreSQL is able to write to systemd log, but multiline entry was
> stored with different priorities

Yeah, as Tom had already pointed out, this doesn't work as I had
imagined it.  I'm withdrawing this part of the patch for now.  I'll come
back to it later.

> Second issue:
> 
> Mapping of levels between pg and journal levels is moved by1

This is the same as how the "syslog" destination works.

From 607323c95ca62d74668992219569c7cff470b63d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 1/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 38ba82f..b8d34b5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, &stat_buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+	 progname,
+	 configdir,
+	 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, &stat_buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 	 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.7.0

From 339836d39d8566ed794f6e1d56384fd93073d5bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 2/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure   | 38 +
 configure.in|  9 +
 doc/src/sgml/installation.sgml  | 16 
 doc/src/sgml/runtime.sgml   | 24 +++
 src/Makefile.global.in  |  1 +
 src/backend/Makefile|  4 
 src/backend/postmaster/postmaster.c | 21 
 src/include/pg_config.h.in  |  3 +++
 8 files changed, 116 insertions(+)

diff --git a/configure b/configure
index 3dd1b15..5c2e767 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
   --with-selinux  build with SELinux support
+  --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
   prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-systemd option" "$LINENO&q

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/28/16 4:00 AM, Pavel Stehule wrote:
> Hi
> 
> 2016-01-28 3:50 GMT+01:00 Peter Eisentraut  <mailto:pete...@gmx.net>>:
> 
> On 1/27/16 7:02 AM, Pavel Stehule wrote:
> > The issues:
> >
> > 1. configure missing systemd integration test, compilation fails:
> >
> > postmaster.o postmaster.c
> > postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
> > directory
> 
> Updated patch attached that fixes this by adding additional checking in
> configure.
> 
> 
> 
> You sent only rebased code of previous version. I didn't find additional
> checks.

Oops.  Here is the actual new code.

From 607323c95ca62d74668992219569c7cff470b63d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 1/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 38ba82f..b8d34b5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, &stat_buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+	 progname,
+	 configdir,
+	 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, &stat_buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 	 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.7.0

From 8a2156bd3add4c8427d216df5757554a421573e7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 2/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure   | 49 +
 configure.in| 13 ++
 doc/src/sgml/installation.sgml  | 16 
 doc/src/sgml/runtime.sgml   | 24 ++
 src/Makefile.global.in  |  1 +
 src/backend/Makefile|  4 +++
 src/backend/postmaster/postmaster.c | 21 
 src/include/pg_config.h.in  |  3 +++
 8 files changed, 131 insertions(+)

diff --git a/configure b/configure
index 3dd1b15..b3f3abe 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
   --with-selinux  build with SELinux support
+  --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
   prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-systemd option" "$LINENO" 5
+  ;;
+  esac
+
+else
+  with_systemd=no
+
+fi
+
+
+

[HACKERS] insufficient qualification of some objects in dump files

[Peter Eisentraut]

Some dump objects whose names are not unique on a schema level have
insufficient details in the dump TOC.  For example, a column default
might have a TOC entry like this:

2153; 2604 39696 DEFAULT public a rolename

Note that this only contains the schema name and the column name, but
not the table name.  So this makes it impossible to filter the TOC by
text search in situations like this.

It looks like other object types such as triggers have the same problem.

I think we should amend the archive tag for these kinds of objects to
include the table name, so it might look like:

2153; 2604 39696 DEFAULT public test a rolename

Comments?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/28/16 9:46 AM, Christoph Berg wrote:
> If a cluster is configured for non-hot-standby replication, the
> READY=1 seems to never happen. Did you check if that doesn't trigger
> any timeouts with would make the unit "fail" or the like?

As Pavel showed, it doesn't work for that.  I'll look into that.

> Also, I'm wondering how hard it would be to get socket activation work
> with that? (I wouldn't necessarily recommend that for production use,
> but on my desktop it would certainly be helpful not to have all those
> 8.4/9.0/.../9.6 clusters running all the time doing nothing.)

I had looked into socket activation, and it looks feasible, but it's a
separate feature.  I couldn't really think of a strong use case, but
what you describe makes sense.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/28/16 10:08 AM, Alvaro Herrera wrote:
> I wonder if instead of HAVE_SYSTEMD at each callsite we shouldn't
> instead have a pg_sd_notify() call that's a no-op when not systemd.

We do this for other optional features as well, and I think it keeps the
code clearest, especially if the ifdef'ed sections are short.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/29/16 4:15 PM, Pavel Stehule wrote:
> Hi
> 
> >
> >
> > You sent only rebased code of previous version. I didn't find additional
> > checks.
> 
> Oops.  Here is the actual new code.
> 
> 
> New test is working as expected
> 
> I did lot of tests - and this code works perfect in single server mode,
> and with slave hot-standby mode.
> 
> It doesn't work with only standby mode

Yeah, I hadn't though of that.  How about this change in addition:

diff --git a/src/backend/postmaster/postmaster.c 
b/src/backend/postmaster/postmaster.c
index 2e7f1d7..d983a50 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4933,6 +4933,11 @@ sigusr1_handler(SIGNAL_ARGS)
if (XLogArchivingAlways())
PgArchPID = pgarch_start();

+#ifdef USE_SYSTEMD
+   if (!EnableHotStandby)
+   sd_notify(0, "READY=1");
+#endif
+
pmState = PM_RECOVERY;
}
if (CheckPostmasterSignal(PMSIGNAL_BEGIN_HOT_STANDBY) &&

> Default timeout on FC is 90 sec - it is should not to be enough for
> large servers with large shared buffers and high checkpoint segments. It
> should be mentioned in service file.

Good point.  I think we should set TimeoutSec=0 in the suggested service file.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] statistics for shared catalogs not updated when autovacuum is off

[Peter Eisentraut]

When autovacuum is off, the statistics in pg_stat_sys_tables for shared
catalogs (e.g., pg_authid, pg_database) never update.  So seq_scan
doesn't update when you read the table, last_analyze doesn't update when
you run analyze, etc.

But when you shut down the server and restart it with autovacuum on, the
updated statistics magically appear right away.  So seq_scan is updated
with the number of reads you did before the shutdown, last_analyze
updates with the time of the analyze you did before the shutdown, etc.
So the data is saved, just not propagated to the view properly.

I can reproduce this back to 9.3, but not 9.2.  Any ideas?


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] statistics for shared catalogs not updated when autovacuum is off

[Peter Eisentraut]

On 2/1/16 9:49 AM, Jim Nasby wrote:
> On 1/30/16 5:05 PM, Peter Eisentraut wrote:
>> When autovacuum is off, the statistics in pg_stat_sys_tables for shared
>> catalogs (e.g., pg_authid, pg_database) never update.  So seq_scan
>> doesn't update when you read the table, last_analyze doesn't update when
>> you run analyze, etc.
>>
>> But when you shut down the server and restart it with autovacuum on, the
> 
> What about with autovacuum still off?

nothing

>> updated statistics magically appear right away.  So seq_scan is updated
>> with the number of reads you did before the shutdown, last_analyze
>> updates with the time of the analyze you did before the shutdown, etc.
>> So the data is saved, just not propagated to the view properly.
>>
>> I can reproduce this back to 9.3, but not 9.2.  Any ideas?
> 
> ISTR that there's some code in the autovac launcher that ensures certain
> stats have been loaded from the file into memory; I'm guessing that the
> functions implementing the shared catalog views need something similar.

That's probably right.  Even with autovacuum on, the statistics for
shared catalogs do not appear as updated right away.  That is, if you
run VACUUM and then look at pg_stat_sys_tables right away, you will see
the stats for shared catalogs to be slightly out of date until the
minutely autovacuum check causes them to update.

So the problem exists in general, but the autovacuum launcher papers
over it every minute.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] better systemd integration

[Peter Eisentraut]

I've committed this.  Thanks for checking.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: multiple psql option -c

[Peter Eisentraut]

On 12/29/15 10:38 AM, Bruce Momjian wrote:
> On Thu, Dec 10, 2015 at 11:10:55AM -0500, Robert Haas wrote:
>> On Wed, Dec 9, 2015 at 12:15 AM, Pavel Stehule  
>> wrote:
 On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule 
 wrote:
> should be noted, recorded somewhere so this introduce possible
> compatibility
> issue - due default processing .psqlrc.

 That's written in the commit log, so I guess that's fine.
>>>
>>> ook
>>
>> Bruce uses the commit log to prepare the release notes, so I guess
>> he'll make mention of this.
> 
> Yes, I will certainly see it.

I generally use the master branch psql for normal work, and this change
has caused massive breakage for me.  It's straightforward to fix, but in
some cases the breakage is silent, for example if you do
something=$(psql -c ...) and the .psqlrc processing causes additional
output.  I'm not sure what to make of it yet, but I want to mention it,
because I fear there will be heartache.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] remove wal_level archive

[Peter Eisentraut]

On 1/26/16 10:56 AM, Simon Riggs wrote:
> Removing one of "archive" or "hot standby" will just cause confusion and
> breakage, so neither is a good choice for removal.
> 
> What we should do is 
> 1. Map "archive" and "hot_standby" to one level with a new name that
> indicates that it can be used for both/either backup or replication.
>   (My suggested name for the new level is "replica"...)
> 2. Deprecate "archive" and "hot_standby" so that those will be removed
> in a later release.

Updated patch to reflect these suggestions.

From a1df946bc77cb51ca149a52276175a001942d8d0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 7 Feb 2016 10:09:05 -0500
Subject: [PATCH v2] Merge wal_level "archive" and "hot_standby" into new name
 "replica"

The distinction between "archive" and "hot_standby" existed only because
at the time "hot_standby" was added, there was some uncertainty about
stability.  This is now a long time ago.  We would like to move forward
with simplifying the replication configuration, but this distinction is
in the way, because a primary server cannot tell (without asking a
standby or predicting the future) which one of these would be the
appropriate level.

Pick a new name for the combined setting to make it clearer that it
covers all (non-logical) backup and replication uses.  The old values
are still accepted but are converted internally.
---
 doc/src/sgml/backup.sgml  |  2 +-
 doc/src/sgml/config.sgml  | 30 +++
 doc/src/sgml/high-availability.sgml   |  2 +-
 doc/src/sgml/ref/alter_system.sgml|  2 +-
 doc/src/sgml/ref/pgupgrade.sgml   |  2 +-
 src/backend/access/rmgrdesc/xlogdesc.c|  5 +++--
 src/backend/access/transam/xact.c |  2 +-
 src/backend/access/transam/xlog.c | 20 --
 src/backend/access/transam/xlogfuncs.c|  2 +-
 src/backend/postmaster/postmaster.c   |  2 +-
 src/backend/replication/slot.c|  5 -
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |  2 +-
 src/bin/pg_controldata/pg_controldata.c   |  6 ++
 src/bin/pg_rewind/RewindTest.pm   |  2 +-
 src/include/access/xlog.h | 11 +-
 src/include/catalog/pg_control.h  |  2 +-
 17 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 7413666..46017a6 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1285,7 +1285,7 @@ Standalone Hot Backups
   If more flexibility in copying the backup files is needed, a lower
   level process can be used for standalone hot backups as well.
   To prepare for low level standalone hot backups, set wal_level to
-  archive or higher, archive_mode to
+  replica or higher, archive_mode to
   on, and set up an archive_command that performs
   archiving only when a switch file exists.  For example:
 
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 392eb70..bed7436 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1971,9 +1971,9 @@ Settings
 wal_level determines how much information is written
 to the WAL. The default value is minimal, which writes
 only the information needed to recover from a crash or immediate
-shutdown. archive adds logging required for WAL archiving;
-hot_standby further adds information required to run
-read-only queries on a standby server; and, finally
+shutdown. replica adds logging required for WAL
+archiving as well as information required to run
+read-only queries on a standby server.  Finally,
 logical adds information necessary to support logical
 decoding.  Each level includes the information logged at all lower
 levels.  This parameter can only be set at server start.
@@ -1991,30 +1991,24 @@ Settings
  transaction
 
 But minimal WAL does not contain enough information to reconstruct the
-data from a base backup and the WAL logs, so archive or
+data from a base backup and the WAL logs, so replica or
 higher must be used to enable WAL archiving
 () and streaming replication.


-In hot_standby level, the same information is logged as
-with archive, plus information needed to reconstruct
-the status of running transactions from the WAL. To enable read-only
-queries on a standby server, wal_level must be set to
-hot_standby or higher on the primary, and
- must be enabled in the standby. It is
-thought that there is little measura

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Peter Eisentraut]

On 2/12/16 8:20 AM, Eugene Kazakov wrote:
> TAP-tests need two Perl modules: Test::More and IPC::Run.
> 
> The patch adds checking of modules in configure.in and configure.

I think those modules are part of a standard Perl installation.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Peter Eisentraut]

On 2/15/16 8:57 AM, Teodor Sigaev wrote:
>> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
>>> TAP-tests need two Perl modules: Test::More and IPC::Run.
>> I think those modules are part of a standard Perl installation.
> 
> IPC::Run is not. At least for perl from ports tree in FreeBSD.

Right, that's why we added the configure option.  But Test::More is
standard.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] Proposal for \crosstabview in psql

[Peter Eisentraut]

On 2/9/16 11:21 AM, Daniel Verite wrote:
> Note that NULL values in the column that pivots are discarded
> by \crosstabview, because NULL as the name of a column does not
> make sense.

Why not?

All you're doing is printing it out, and psql is quite capable of
printing a null value.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 1/31/16 7:34 AM, Michael Paquier wrote:
> I am marking this patch as returned with feedback for now, not all the
> issues have been fixed yet, and there are still no docs (the
> conclusion being that people would like to have this stuff, right?).
> Feel free to move it to the next CF should a new version be written.

I think we still don't have a real use case for this feature, and a
couple of points against it.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Figures in docs

[Peter Eisentraut]

On 2/16/16 8:17 PM, Tatsuo Ishii wrote:
> It seems there's no figures/diagrams in our docs. I vaguely recall that
> we used to have a few diagrams in our docs. If so, was there any
> technical reason to remove them?

Because no one has been able to propose a good format for storing and
editing pictures.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 2/17/16 12:15 PM, Joe Conway wrote:
> On 02/17/2016 02:32 AM, Michael Paquier wrote:
>> Actually, having second-thoughts on the matter, why is is that
>> necessary to document the function pg_config()? The functions wrapping
>> a system view just have the view documented, take for example
>> pg_show_all_file_settings, pg_show_all_settings,
>> pg_stat_get_wal_receiver, etc.
> 
> Ok, removed the documentation on the function pg_config() and pushed.

I still have my serious doubts about this, especially not even requiring
superuser access for this information.  Could someone explain why we
need this?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 2/17/16 5:20 PM, Josh berkus wrote:
> I have a use-case for this feature, at part of it containerized
> PostgreSQL. Right now, there is certain diagnostic information (like
> timeline) which is exposed ONLY in pg_controldata.

I'm talking about the pg_config() function, not pg_controldata.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Figures in docs

[Peter Eisentraut]

On 2/17/16 8:23 PM, Tatsuo Ishii wrote:
> Do we really need git history for each figure? It seems we are waiting
> for a solution which will never realize.

What we need is tooling around a new file format that is similar or
analogous to our existing tooling, so that it is easy to edit, easy to
review, easy to build, easy to test, and so on.  Otherwise, the image
files will not get maintained properly.

As an example, if an image contains text (e.g., a flow chart or
architecture diagram), then I expect that git grep will find it there,
so that I know to update it when I rename or augment something.

The above is all solvable.  There are certainly image formats that fit
those descriptions.

Personally, I'd want to know specifically what images people would want,
so we could discuss or prototype something around that.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 2/17/16 9:08 PM, Michael Paquier wrote:
> On Thu, Feb 18, 2016 at 11:02 AM, Peter Eisentraut  wrote:
>> On 2/17/16 5:20 PM, Josh berkus wrote:
>>> I have a use-case for this feature, at part of it containerized
>>> PostgreSQL. Right now, there is certain diagnostic information (like
>>> timeline) which is exposed ONLY in pg_controldata.
>>
>> I'm talking about the pg_config() function, not pg_controldata.
> 
> Andrew has mentioned a use case he had at the beginning of this thread
> to enhance a bit the regression tests related to libxml.

While that idea was useful, I think we had concluded that there are
better ways to do this and that this way probably wouldn't even work
(Windows?).


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] pg_ctl promote wait

[Peter Eisentraut]

It would be nice if pg_ctl promote supported the -w (wait) option.

How could pg_ctl determine when the promotion has finished?

We could query pg_is_in_recovery(), but that would require database
access, which we got rid of in pg_ctl.

We could check when recovery.conf is gone or recovery.done appears.
This could work, but I think some people are trying to get rid of these
files, so building a feature on that might be a problem?  (I think the
latest news on that was that the files might be different in this
hypothetical future but there would still be a file to prevent
re-recovery on restart.)

Other ideas?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Relaxing SSL key permission checks

[Peter Eisentraut]

On 2/18/16 10:17 AM, Tom Lane wrote:
> Christoph Berg  writes:
>> Currently the server insists on ssl_key_file's permissions to be 0600
>> or less, and be owned by the database user. Debian has been patching
>> be-secure.c since forever (the git history goes back to 8.2beta1) to
>> relax that to 0640 or less, and owned by root or the database user.
> 
> Debian can do that if they like, but it's entirely unacceptable as an
> across-the-board patch.  Not all systems treat groups as being narrow
> domains in which it's okay to assume that group-readable files are
> secure enough to be keys.  As an example, on OS X user files tend to be
> group "staff" or "admin" which'd be close enough to world readable.
> 
> We could allow group-readable if we had some way to know whether to
> trust the specific group, but I don't think there's any practical
> way to do that.  System conventions vary too much.

Wouldn't POSIX ACLs bypass this anyway?


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 2/18/16 3:30 AM, Andres Freund wrote:
> I don't understand why you're so opposed to this. Several people said
> that they're interested in this information in the current discussion
> and it has been requested repeatedly over the years.

I think no one except Andrew Dunstan has requested this, and his use
case is disputed.  Everyone else is either confusing this with the
pg_controldata part or is just transitively claiming that someone else
wanted it.

I don't have a problem with the implementation, but I don't understand
what this feature is meant for.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Peter Eisentraut]

On 2/18/16 12:20 PM, Joe Conway wrote:
> Just to be clear, AFAIK there is no issue with the committed changes on
> Windows. If there is please provide a concrete example that we can discuss.

Originally it was mentioned that this feature could be used in the
regression tests by making certain tests conditional on configure
options.  Which presumably won't work if the build was on Windows.

I don't doubt that your code actually works on Windows.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_ctl promote wait

[Peter Eisentraut]

On 2/18/16 3:33 AM, Andres Freund wrote:
> Hi,
> 
> On 2016-02-17 21:47:50 -0500, Peter Eisentraut wrote:
>> It would be nice if pg_ctl promote supported the -w (wait) option.
>>
>> How could pg_ctl determine when the promotion has finished?
> 
> How about looking into pg_control? ControlFileData->state ought to have
> the correct information.

Is it safe to read pg_control externally without a lock?  pg_controldata
will just report a CRC error and proceed, and if you're not sure you can
just run it again.  But if a promotion fails every so often because of
concurrent pg_control writes, that would make this feature annoying.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_ctl promote wait

[Peter Eisentraut]

On 2/19/16 10:06 AM, Fujii Masao wrote:
> One concern is that there can be a "time" after the pg_control's state
> is changed to DB_IN_PRODUCTION and before the server is able to
> start accepting normal (not read-only) connections. So if users try to
> start write transaction just after pg_ctl promote -w ends, they might
> get an error because the server is still in recovery, i.e., the startup
> process is running.

I think that window would be acceptable.

If not, then the way to deal with it would seem to be to create an
entirely new mechanism to communicate with pg_ctl (e.g., a file) and
call that at the very end of StartupXLOG().  I'm not sure that that is
worth it.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] FDW: should GetFdwRoutine be called when drop table?

[Peter Eisentraut]

On 2/19/16 12:21 PM, Feng Tian wrote:
> I have an fdw that each foreign table will acquire some persisted resource.
> In my case, some files in file system.   To drop the table cleanly, I
> have written
> an object_access_hook that remove those files.  The hook is installed in
> _PG_init.  
> 
> It all worked well except one case. Suppose a user login, the very first
> command is 
> drop foreign table.  Drop foreign table will not load the module, so
> that the hook 
> is not installed and the files are not properly cleaned up.

You could load your library with one of the *_library_preload settings
to make sure the hook is always present.

But foreign data wrappers are meant to be wrappers around data managed
elsewhere, not their own storage managers (although that is clearly
tempting), so there might well be other places where this breaks down.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] GCC 6 warning fixes

[Peter Eisentraut]

Here are three patches to fix new warnings in GCC 6.

0001 is apparently a typo.

0002 was just (my?) stupid code to begin with.

0003 is more of a workaround.  There could be other ways address this, too.
From 1e5bf0bdcd86b807d881ea82245275389083ec75 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 19 Feb 2016 23:07:46 -0500
Subject: [PATCH 1/3] ecpg: Fix typo

GCC 6 points out the redundant conditions, which were apparently typos.
---
 src/interfaces/ecpg/test/compat_informix/describe.pgc| 2 +-
 src/interfaces/ecpg/test/expected/compat_informix-describe.c | 2 +-
 src/interfaces/ecpg/test/expected/sql-describe.c | 2 +-
 src/interfaces/ecpg/test/sql/describe.pgc| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/interfaces/ecpg/test/compat_informix/describe.pgc b/src/interfaces/ecpg/test/compat_informix/describe.pgc
index 1836ac3..6f6 100644
--- a/src/interfaces/ecpg/test/compat_informix/describe.pgc
+++ b/src/interfaces/ecpg/test/compat_informix/describe.pgc
@@ -150,7 +150,7 @@ exec sql end declare section;
 	exec sql describe st_id2 using descriptor sqlda2;
 	exec sql describe st_id2 into sqlda3;
 
-	if (sqlda1 == NULL || sqlda1 == NULL || sqlda2 == NULL)
+	if (sqlda1 == NULL || sqlda2 == NULL || sqlda3 == NULL)
 		exit(1);
 
 	strcpy(msg, "get descriptor");
diff --git a/src/interfaces/ecpg/test/expected/compat_informix-describe.c b/src/interfaces/ecpg/test/expected/compat_informix-describe.c
index 5951ae6..9eb176e 100644
--- a/src/interfaces/ecpg/test/expected/compat_informix-describe.c
+++ b/src/interfaces/ecpg/test/expected/compat_informix-describe.c
@@ -362,7 +362,7 @@ if (sqlca.sqlcode < 0) exit (1);}
 #line 151 "describe.pgc"
 
 
-	if (sqlda1 == NULL || sqlda1 == NULL || sqlda2 == NULL)
+	if (sqlda1 == NULL || sqlda2 == NULL || sqlda3 == NULL)
 		exit(1);
 
 	strcpy(msg, "get descriptor");
diff --git a/src/interfaces/ecpg/test/expected/sql-describe.c b/src/interfaces/ecpg/test/expected/sql-describe.c
index 356f587..d0e39e9 100644
--- a/src/interfaces/ecpg/test/expected/sql-describe.c
+++ b/src/interfaces/ecpg/test/expected/sql-describe.c
@@ -360,7 +360,7 @@ if (sqlca.sqlcode < 0) exit (1);}
 #line 151 "describe.pgc"
 
 
-	if (sqlda1 == NULL || sqlda1 == NULL || sqlda2 == NULL)
+	if (sqlda1 == NULL || sqlda2 == NULL || sqlda3 == NULL)
 		exit(1);
 
 	strcpy(msg, "get descriptor");
diff --git a/src/interfaces/ecpg/test/sql/describe.pgc b/src/interfaces/ecpg/test/sql/describe.pgc
index cd52c82..b95ab35 100644
--- a/src/interfaces/ecpg/test/sql/describe.pgc
+++ b/src/interfaces/ecpg/test/sql/describe.pgc
@@ -150,7 +150,7 @@ exec sql end declare section;
 	exec sql describe st_id2 using descriptor sqlda2;
 	exec sql describe st_id2 into sqlda3;
 
-	if (sqlda1 == NULL || sqlda1 == NULL || sqlda2 == NULL)
+	if (sqlda1 == NULL || sqlda2 == NULL || sqlda3 == NULL)
 		exit(1);
 
 	strcpy(msg, "get descriptor");
-- 
2.7.1

From 996a5d27a2bc79e1e0b2ee0aa39cfdc7615e874c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 19 Feb 2016 23:07:46 -0500
Subject: [PATCH 2/3] psql: Fix some strange code in SQL help creation

Struct QL_HELP used to be defined as static in the sql_help.h header
file, which is included in sql_help.c and help.c, thus creating two
separate instances of the struct.  This causes a warning from GCC 6,
because the struct is not used in sql_help.c.

Instead, declare the struct as extern in the header file and define it
in sql_help.c.  This also allows making a bunch of functions static
because they are no longer needed outside of sql_help.c.
---
 src/bin/psql/create_help.pl | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/bin/psql/create_help.pl b/src/bin/psql/create_help.pl
index fedcc47..b9b8e87 100644
--- a/src/bin/psql/create_help.pl
+++ b/src/bin/psql/create_help.pl
@@ -59,8 +59,6 @@
 #ifndef $define
 #define $define
 
-#define N_(x) (x)/* gettext noop */
-
 #include \"postgres_fe.h\"
 #include \"pqexpbuffer.h\"
 
@@ -72,6 +70,7 @@
 	intnl_count;	/* number of newlines in syntax (for pager) */
 };
 
+extern const struct _helpStruct QL_HELP[];
 ";
 
 print CFILE "/*
@@ -83,6 +82,8 @@
  *
  */
 
+#define N_(x) (x)/* gettext noop */
+
 #include \"$hfile\"
 
 ";
@@ -170,8 +171,7 @@
 	$synopsis =~ s/\\n/\\n"\n$prefix"/g;
 	my @args =
 	  ("buf", $synopsis, map("_(\"$_\")", @{ $entries{$_}{params} }));
-	print HFILE "extern void sql_help_$id(PQExpBuffer buf);\n";
-	print CFILE "void
+	print CFILE "static void
 sql_help_$id(PQExpBuffer buf)
 {
 \tappendPQExpBuffer(" . join(",\n$prefix", @args) . ");
@@ -180,15 +180,14 @@
 ";
 }
 
-print HFILE "
-
-static const struct _helpStruct QL_HELP[] = {
+print CFILE "
+const struct _helpStruct QL_HELP[] 

Re: [HACKERS] Declarative partitioning

[Peter Eisentraut]

On 2/16/16 9:56 PM, Amit Langote wrote:
> From now on, instead of attaching multiple files like in the previous
> message, I will send a single tar.gz which will contain patches created by
> git-format-patch.

Please don't do that.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

[Peter Eisentraut]

On 1/18/16 4:21 PM, Robert Haas wrote:
> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> then you want to make BAR an array of that type rather than a scalar,
> why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> natural to me.

Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types.  Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.

> I think the part of this patch that makes %TYPE work for more kinds of
> types is probably a good idea, although I haven't carefully studied
> exactly what it does.

I agree that this should be more general.  For instance, this patch
would allow you to get the element type of an array-typed variable, but
there is no way to get the element type of just another type.  If we
could do something like

DECLARE
  var ELEMENT OF point;

(not necessary that syntax)

then

DECLARE
  var ELEMENT OF othervar%TYPE;

should just fall into place.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive

[Peter Eisentraut]

Could you enhance the documentation about the difference between "wait
event type name" and "wait event name" (examples?)?  This is likely to
be quite confusing for users who are used to just the plain "waiting"
column.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] insufficient qualification of some objects in dump files

[Peter Eisentraut]

On 1/29/16 1:24 AM, Michael Paquier wrote:
>> I think we should amend the archive tag for these kinds of objects to
>> > include the table name, so it might look like:
>> >
>> > 2153; 2604 39696 DEFAULT public test a rolename
>> >
>> > Comments?
> +1. I noticed that this limitation is present for triggers (as you
> mentioned), constraints, fk constraints and RLS policies which should
> be completed with a table name.

Thank you for collecting this list.  Attached is a patch for this.

Tom thought this might require an archive version dump, but I'm not
sure.  The tags are more of an informational string for human
consumption, not strictly part of the archive format.

From 44a23b4e960040f1e5de7a0ae0ecb6de432c4027 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 25 Feb 2016 18:56:37 -0500
Subject: [PATCH] pg_dump: Add table qualifications to some tags

Some object types have names that are only unique for one table.  But
for those we generally didn't put the table name into the dump TOC tag.
So it was impossible to identify these objects if the same name was used
for multiple tables.  This affects policies, column defaults,
constraints, triggers, and rules.

Fix by adding the table name to the TOC tag, so that it now reads
"$schema $table $object".
---
 src/bin/pg_dump/pg_dump.c | 42 ++
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 64c2673..8bb134d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3057,6 +3057,7 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
 	PQExpBuffer query;
 	PQExpBuffer delqry;
 	const char *cmd;
+	char	   *tag;
 
 	if (dopt->dataOnly)
 		return;
@@ -3124,8 +3125,10 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
 	appendPQExpBuffer(delqry, "DROP POLICY %s", fmtId(polinfo->polname));
 	appendPQExpBuffer(delqry, " ON %s;\n", fmtId(tbinfo->dobj.name));
 
+	tag = psprintf("%s %s", tbinfo->dobj.name, polinfo->dobj.name);
+
 	ArchiveEntry(fout, polinfo->dobj.catId, polinfo->dobj.dumpId,
- polinfo->dobj.name,
+ tag,
  polinfo->dobj.namespace->dobj.name,
  NULL,
  tbinfo->rolname, false,
@@ -3134,6 +3137,7 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
  NULL, 0,
  NULL, NULL);
 
+	free(tag);
 	destroyPQExpBuffer(query);
 	destroyPQExpBuffer(delqry);
 }
@@ -14626,6 +14630,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
 	int			adnum = adinfo->adnum;
 	PQExpBuffer q;
 	PQExpBuffer delq;
+	char	   *tag;
 
 	/* Skip if table definition not to be dumped */
 	if (!tbinfo->dobj.dump || dopt->dataOnly)
@@ -14654,8 +14659,10 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
 	appendPQExpBuffer(delq, "ALTER COLUMN %s DROP DEFAULT;\n",
 	  fmtId(tbinfo->attnames[adnum - 1]));
 
+	tag = psprintf("%s %s", tbinfo->dobj.name, tbinfo->attnames[adnum - 1]);
+
 	ArchiveEntry(fout, adinfo->dobj.catId, adinfo->dobj.dumpId,
- tbinfo->attnames[adnum - 1],
+ tag,
  tbinfo->dobj.namespace->dobj.name,
  NULL,
  tbinfo->rolname,
@@ -14664,6 +14671,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
  NULL, 0,
  NULL, NULL);
 
+	free(tag);
 	destroyPQExpBuffer(q);
 	destroyPQExpBuffer(delq);
 }
@@ -14804,6 +14812,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 	TableInfo  *tbinfo = coninfo->contable;
 	PQExpBuffer q;
 	PQExpBuffer delq;
+	char	   *tag = NULL;
 
 	/* Skip if not to be dumped */
 	if (!coninfo->dobj.dump || dopt->dataOnly)
@@ -14897,8 +14906,10 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 		appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
 		  fmtId(coninfo->dobj.name));
 
+		tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
+
 		ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
-	 coninfo->dobj.name,
+	 tag,
 	 tbinfo->dobj.namespace->dobj.name,
 	 indxinfo->tablespace,
 	 tbinfo->rolname, false,
@@ -14930,8 +14941,10 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 		appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
 		  fmtId(coninfo->dobj.name));
 
+		tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
+
 		ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
-	 coninfo->dobj.name,
+	 tag,
 	 tbinfo->dobj.namespace->dobj.name,
 	 NULL,
 	 tbinfo->rolname, false,
@@ -14965,8 +14978,10 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 			appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
 			  fmtId(coninfo->dobj.name));
 
+			tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
+
 			ArchiveEntry(f

[HACKERS] drop src/backend/port/darwin/system.c ?

[Peter Eisentraut]

/* only needed in OS X 10.1 and possibly early 10.2 releases */

Maybe it's time to let it go?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Add -c to rsync commands on SR tutorial wiki page

[Peter Eisentraut]

On 8/17/16 5:34 PM, Jim Nasby wrote:
> https://wiki.postgresql.org/wiki/Binary_Replication_Tutorial does not 
> specify -c for any of the rsync commands. That's maybe safe for WAL, but 
> I don't think it's safe for any of the other uses, right? I'd like 
> someone to confirm before I just change the page... my intention is to 
> just stick -c in all the commands.

Why do you think that?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Making pg_hba.conf case-insensitive

[Peter Eisentraut]

On 8/18/16 1:59 PM, Bruce Momjian wrote:
> o  compares words in columns that can only support keywords as
> case-insensitive, double-quoted or not
> 
> o  compares words in columns that can contain user/db names or keywords
> as case-sensitive if double-quoted, case-insensitive if not

I can maybe see the case of the second one, but the first one doesn't
make sense to me.

We've in the past had discussions like this about whether command line
arguments of tools should be case insensitive like SQL, and we had
resolved that since the shell is not SQL, it shouldn't work like that.
pg_hba.conf is also not SQL, and neither, for that matter, is
pg_ident.conf and postgresql.conf.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure)

[Peter Eisentraut]

On 3/22/16 9:27 AM, Aleksander Alekseev wrote:
> diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
> index 28f9fb5..45aa802 100644
> --- a/src/backend/libpq/hba.c
> +++ b/src/backend/libpq/hba.c
> @@ -1008,14 +1008,9 @@ parse_hba_line(List *line, int line_num, char 
> *raw_line)
>   *cidr_slash = '\0';
>  
>   /* Get the IP address either way */
> + memset(&hints, 0, sizeof(hints));
>   hints.ai_flags = AI_NUMERICHOST;
>   hints.ai_family = AF_UNSPEC;
> - hints.ai_socktype = 0;
> - hints.ai_protocol = 0;
> - hints.ai_addrlen = 0;
> - hints.ai_canonname = NULL;
> - hints.ai_addr = NULL;
> - hints.ai_next = NULL;
>  
>   ret = pg_getaddrinfo_all(str, NULL, &hints, 
> &gai_result);
>   if (ret == 0 && gai_result)

In addition to what Heikki wrote, I think the above is not necessary.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: About CMake v2

[Peter Eisentraut]

On 8/18/16 4:23 PM, Christian Convey wrote:
> What standard would you suggest for those platforms which don't have
> an obvious default version of CMake?

In the olden days, when many platforms we supported didn't come with GNU
make or other GNU tools or even a compiler, we collected a lot of
practical information about where to get these things.  You can see most
of that at
<https://www.postgresql.org/docs/devel/static/installation-platform-notes.html>.

That's at least the spirit of things.  I wouldn't worry about this so
much right now.  Get it working first.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_basebackup wish list

[Peter Eisentraut]

On 7/12/16 9:55 PM, Masahiko Sawada wrote:
> And what I think is pg_baseback never remove the directory specified
> by -D option even if execution is failed. initdb command behaves so.
> I think it's helpful for backup operation.

This has been bothering me as well.

How about the attached patch as a start?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 910310b7eab88af8972906307a55e02e64618da7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 18 Aug 2016 12:00:00 -0400
Subject: [PATCH] pg_basebackup: Clean created directories on failure

Like initdb, clean up created data and xlog directories, unless the new
-n/--noclean option is specified.

Tablespace directories are not cleaned up, but a message is written
about that.
---
 doc/src/sgml/ref/pg_basebackup.sgml  | 18 +
 src/bin/pg_basebackup/pg_basebackup.c| 98 ++--
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 10 ++-
 3 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 03615da..9f1eae1 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -399,6 +399,24 @@ Options
  
 
  
+  -n
+  --noclean
+  
+   
+By default, when pg_basebackup aborts with an
+error, it removes any directories it might have created before
+discovering that it cannot finish the job (for example, data directory
+and transaction log directory). This option inhibits tidying-up and is
+thus useful for debugging.
+   
+
+   
+Note that tablespace directories are not cleaned up either way.
+   
+  
+ 
+
+ 
   -P
   --progress
   
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index ed41db8..d13a9a3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -57,6 +57,7 @@ static TablespaceList tablespace_dirs = {NULL, NULL};
 static char *xlog_dir = "";
 static char format = 'p';		/* p(lain)/t(ar) */
 static char *label = "pg_basebackup base backup";
+static bool noclean = false;
 static bool showprogress = false;
 static int	verbose = 0;
 static int	compresslevel = 0;
@@ -68,6 +69,13 @@ static int	standby_message_timeout = 10 * 1000;		/* 10 sec = default */
 static pg_time_t last_progress_report = 0;
 static int32 maxrate = 0;		/* no limit by default */
 
+static bool success = false;
+static bool made_new_pgdata = false;
+static bool found_existing_pgdata = false;
+static bool made_new_xlogdir = false;
+static bool found_existing_xlogdir = false;
+static bool made_tablespace_dirs = false;
+static bool found_tablespace_dirs = false;
 
 /* Progress counters */
 static uint64 totalsize;
@@ -81,6 +89,7 @@ static int	bgpipe[2] = {-1, -1};
 
 /* Handle to child process */
 static pid_t bgchild = -1;
+static bool in_log_streamer = false;
 
 /* End position for xlog streaming, empty string if unknown yet */
 static XLogRecPtr xlogendptr;
@@ -97,7 +106,7 @@ static PQExpBuffer recoveryconfcontents = NULL;
 /* Function headers */
 static void usage(void);
 static void disconnect_and_exit(int code);
-static void verify_dir_is_empty_or_create(char *dirname);
+static void verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found);
 static void progress_report(int tablespacenum, const char *filename, bool force);
 
 static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
@@ -114,6 +123,69 @@ static void tablespace_list_append(const char *arg);
 
 
 static void
+cleanup_directories_atexit(void)
+{
+	if (success || in_log_streamer)
+		return;
+
+	if (!noclean)
+	{
+		if (made_new_pgdata)
+		{
+			fprintf(stderr, _("%s: removing data directory \"%s\"\n"),
+	progname, basedir);
+			if (!rmtree(basedir, true))
+fprintf(stderr, _("%s: failed to remove data directory\n"),
+		progname);
+		}
+		else if (found_existing_pgdata)
+		{
+			fprintf(stderr,
+	_("%s: removing contents of data directory \"%s\"\n"),
+	progname, basedir);
+			if (!rmtree(basedir, false))
+fprintf(stderr, _("%s: failed to remove contents of data directory\n"),
+		progname);
+		}
+
+		if (made_new_xlogdir)
+		{
+			fprintf(stderr, _("%s: removing transaction log directory \"%s\"\n"),
+	progname, xlog_dir);
+			if (!rmtree(xlog_dir, true))
+fprintf(stderr, _("%s: failed to remove transaction log directory\n"),
+		progname);
+		}
+		else if (found_existing_xlogdir)
+		{
+			fprintf(stderr,
+	_("%s: removing contents of transaction log directory \"%s\"\n"),
+	progname, xlog_dir);
+			if (!rmtree(xlog_dir, false))
+fprintf(stderr, _("%s:

Re: [HACKERS] drop src/backend/port/darwin/system.c ?

[Peter Eisentraut]

On 8/17/16 12:29 PM, Tom Lane wrote:
> Also, the early releases of OS X were rough enough that it's pretty hard
> to believe anyone is still using them anywhere (certainly the buildfarm
> isn't).  So the odds of anyone caring if we remove this file seem
> negligible.  Let's nuke it.

done

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid

[Peter Eisentraut]

On 8/18/16 9:20 PM, Craig Ringer wrote:
> On 19 August 2016 at 02:35, Jim Nasby  <mailto:jim.na...@bluetreble.com>> wrote:
> I think we need to either add real types for handling XID/epoch/TXID
> or finally create uint types. It's *way* too easy to screw things up
> the way they are today.
> 
> Hm. Large scope increase there. Especially introducing unsigned types.
> There's a reason that hasn't been done already - it's not just copying a
> whole pile of code, it's also defining all the signed/unsigned
> interactions and conversions carefully.

https://github.com/petere/pguint ;-)

> I'm not against adding a 'bigxid' or 'epoch_xid' or something,
> internally a uint64. It wouldn't need all the opclasses, casts, function
> overloads, etc that uint8 would.

That sounds much better.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Make better use of existing enums in plpgsql

[Peter Eisentraut]

plpgsql.h defines a number of enums, but most of the code passes them
around as ints.  The attached patch updates structs and function
prototypes to take enum types instead.  This clarifies the struct
definitions in plpgsql.h in particular.

I didn't deal with the PLPGSQL_RC_* symbols, since they are only used in
pl_exec.c (could be moved there?), and it would bloat this patch.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 59f9df74fdf1da8ef5f426fabf0815d4e4423f12 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 18 Aug 2016 12:00:00 -0400
Subject: [PATCH] Make better use of existing enums in plpgsql

plpgsql.h defines a number of enums, but most of the code passes them
around as ints.  Update structs and function prototypes to take enum
types instead.  This clarifies the struct definitions in plpgsql.h in
particular.
---
 src/pl/plpgsql/src/pl_comp.c  |   6 +--
 src/pl/plpgsql/src/pl_exec.c  |   2 +-
 src/pl/plpgsql/src/pl_funcs.c |  12 ++---
 src/pl/plpgsql/src/plpgsql.h  | 114 +-
 4 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b628c28..ef3f264 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -93,7 +93,7 @@ static PLpgSQL_function *do_compile(FunctionCallInfo fcinfo,
 		   PLpgSQL_func_hashkey *hashkey,
 		   bool forValidator);
 static void plpgsql_compile_error_callback(void *arg);
-static void add_parameter_name(int itemtype, int itemno, const char *name);
+static void add_parameter_name(PLpgSQL_nsitem_type itemtype, int itemno, const char *name);
 static void add_dummy_return(PLpgSQL_function *function);
 static Node *plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref);
 static Node *plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var);
@@ -412,7 +412,7 @@ do_compile(FunctionCallInfo fcinfo,
 char		argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
 PLpgSQL_type *argdtype;
 PLpgSQL_variable *argvariable;
-int			argitemtype;
+PLpgSQL_nsitem_type argitemtype;
 
 /* Create $n name for variable */
 snprintf(buf, sizeof(buf), "$%d", i + 1);
@@ -950,7 +950,7 @@ plpgsql_compile_error_callback(void *arg)
  * Add a name for a function parameter to the function's namespace
  */
 static void
-add_parameter_name(int itemtype, int itemno, const char *name)
+add_parameter_name(PLpgSQL_nsitem_type itemtype, int itemno, const char *name)
 {
 	/*
 	 * Before adding the name, check for duplicates.  We need this even though
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index f9b3b22..2a0617d 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -1559,7 +1559,7 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
 
 	CHECK_FOR_INTERRUPTS();
 
-	switch ((enum PLpgSQL_stmt_types) stmt->cmd_type)
+	switch (stmt->cmd_type)
 	{
 		case PLPGSQL_STMT_BLOCK:
 			rc = exec_stmt_block(estate, (PLpgSQL_stmt_block *) stmt);
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 27ebebc..e3cd9c0 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -51,7 +51,7 @@ plpgsql_ns_init(void)
  * --
  */
 void
-plpgsql_ns_push(const char *label, enum PLpgSQL_label_types label_type)
+plpgsql_ns_push(const char *label, PLpgSQL_label_type label_type)
 {
 	if (label == NULL)
 		label = "";
@@ -89,7 +89,7 @@ plpgsql_ns_top(void)
  * --
  */
 void
-plpgsql_ns_additem(int itemtype, int itemno, const char *name)
+plpgsql_ns_additem(PLpgSQL_nsitem_type itemtype, int itemno, const char *name)
 {
 	PLpgSQL_nsitem *nse;
 
@@ -231,7 +231,7 @@ plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur)
 const char *
 plpgsql_stmt_typename(PLpgSQL_stmt *stmt)
 {
-	switch ((enum PLpgSQL_stmt_types) stmt->cmd_type)
+	switch (stmt->cmd_type)
 	{
 		case PLPGSQL_STMT_BLOCK:
 			return _("statement block");
@@ -291,7 +291,7 @@ plpgsql_stmt_typename(PLpgSQL_stmt *stmt)
  * GET DIAGNOSTICS item name as a string, for use in error messages etc.
  */
 const char *
-plpgsql_getdiag_kindname(int kind)
+plpgsql_getdiag_kindname(PLpgSQL_getdiag_kind kind)
 {
 	switch (kind)
 	{
@@ -367,7 +367,7 @@ static void free_expr(PLpgSQL_expr *expr);
 static void
 free_stmt(PLpgSQL_stmt *stmt)
 {
-	switch ((enum PLpgSQL_stmt_types) stmt->cmd_type)
+	switch (stmt->cmd_type)
 	{
 		case PLPGSQL_STMT_BLOCK:
 			free_block((PLpgSQL_stmt_block *) stmt);
@@ -791,7 +791,7 @@ static void
 dump_stmt(PLpgSQL_stmt *stmt)
 {
 	printf("%3d:", stmt->lineno);
-	switch ((enum PLpgSQL_stmt_types) stmt->cmd_type)
+	switch (stmt->cmd_type)
 	{
 		case PLPGSQL_STMT_BLOCK:
 			dump_block((PLpgSQL_stmt_block *) stmt);
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
i

Re: [HACKERS] UTF-8 docs?

[Peter Eisentraut]

On 8/22/16 1:16 AM, Tatsuo Ishii wrote:
> Just out of curiopusity, I wonder why we can't make the encoding of
> SGML docs to be UTF-8, rather than current ISO-8859-1.

Encoding handling in DocBook SGML is weird, and making it work robustly
will either fail or might be more work than just completing the
conversion to XML.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] LSN as a recovery target

[Peter Eisentraut]

On 8/22/16 8:28 AM, Michael Paquier wrote:
> Thinking a bit wider than that, we may want to know such context for
> normal GUC parameters as well, and that's not the case now. Perhaps
> there is actually a reason why that's not done for GUCs, but it seems
> that it would be useful there as well.

GUC parsing generally needs, or used to need, to work under more
constrained circumstances, e.g., no full memory management.  That's not
a reason not to try this, but there might be non-obvious problems.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] UTF-8 docs?

[Peter Eisentraut]

On 8/22/16 9:32 AM, Tatsuo Ishii wrote:
> I don't know what kind of problem you are seeing with encoding
> handling, but at least UTF-8 is working for Japanese, French and
> Russian.

Those translations are using DocBook XML.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Showing parallel status in \df+

[Peter Eisentraut]

On 8/22/16 1:52 PM, Pavel Stehule wrote:
> If I understand to purpose of this patch - it is compromise - PL source
> is removed from table, but it is printed in result.

What does it do if you are displaying more than one function?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/25/16 10:39 PM, Michael Paquier wrote:
> I am relaunching $subject as 10 development will begin soon. As far as
> I know, there is agreement that we can do something here. Among the
> different proposals I have found:
> - pg_clog renamed to pg_commit_status, pg_xact or pg_commit
> - pg_xlog renamed to pg_xjournal, pg_wal or pg_journal

If we're going to do some renaming, then I suggest we do a
mini-file-system structure under $PGDATA, like

$PGDATA/etc
$PGDATA/log
$PGDATA/run (lock files etc.)
$PGDATA/tmp
$PGDATA/var

The names of all the things under "var" could still be refined, but it's
much less likely that users will confuse data with configuration or
plain logs under that scheme.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Unsupported feature F867: WITH TIES

[Peter Eisentraut]

On 8/26/16 9:06 AM, Jürgen Purtz wrote:
> Actually we don't support the SQL feature F867 "FETCH FIRST clause: WITH 
> TIES option". On the other side we support the window function "rank()" 
> which acts like the "WITH TIES option". My questions are: Is it hard to 
> implement the "WITH TIES option"? Are there plans for a realization / 
> how do we decide in general what to do next? Differs the semantic of the 
> "WITH TIES option" significantly from the "rank()" window function or 
> can we treat it as some kind of 'syntactical sugar'?

LIMIT/FETCH FIRST works at a level that is quite far removed from data
type semantics, which is what you'd need to have handy to compute ties.
LIMIT basically just tells the executor to stop after getting a certain
number of rows.

So implementing WITH TIES would be very difficult in the current setup.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] OpenSSL 1.1 breaks configure and more

[Peter Eisentraut]

On 8/26/16 5:31 AM, Heikki Linnakangas wrote:
> I think now would be a good time to drop support for OpenSSL versions 
> older than 0.9.8. OpenSSL don't even support 0.9.8 anymore, although 
> there are probably distributions out there that still provide patches 
> for it. But OpenSSL 0.9.7 and older are really not interesting for 
> PostgreSQL 10 anymore, I think.

CentOS 5 currently ships 0.9.8e.  That's usually the oldest OS we want
to support eagerly.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] old_snapshot_threshold documentation

[Peter Eisentraut]

I doubt the documentation for old_snapshot_threshold is going to be
understood by many ordinary users.  What is a "snapshot", first of all?
Why would a snapshot be old?  Why is that a problem?  What can I do to
avoid it?  What are the circumstances in practice where this issue would
arise, and what are the trade-offs?  Where can I see existing snapshots
and how old they are?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/26/16 11:58 AM, Magnus Hagander wrote:
> $PGDATA/etc
>> $PGDATA/log
>> $PGDATA/run (lock files etc.)
>> $PGDATA/tmp
>> $PGDATA/var
>>
>> The names of all the things under "var" could still be refined, but it's
>> much less likely that users will confuse data with configuration or
>> plain logs under that scheme
> 
> Interesting idea. I worry a bit that this might encourage distributions
> to split it up into different places though, and I'm not sure we want to
> encourage that..

They already do this.  This would just formalize where we think
appropriate boundaries between the pieces are.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/26/16 12:28 PM, Tom Lane wrote:
> Also, I'd just as soon not move/rename things
> that don't really need it.

I'm just as happy with not changing anything.  But if we're going to
rename stuff, let's at least think about something slightly more
comprehensive.  Any rename is going to break a bunch of stuff.  But if
we break it in a way that reduces the need for future discussion or
changes, it would at least be a small win in the long run.

> If, for example, we decide to move
> postgresql.conf to etc/postgresql.conf, that is going to break a metric
> ton of stuff that doesn't need to get broken AFAICS.

Sure, I'd be content with leaving those in the top-level instead of say
"etc".

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/26/16 4:02 PM, Joshua D. Drake wrote:
> Although... wouldn't run be under var?

Traditionally yes, but things are changing in this area, if you consider
the top-level file system of a modern Linux distribution.  One reason is
that "run" is/can be blown away at reboot.  This wouldn't be an entirely
useless feature for postgres: Can you tell otherwise which of the
various pid/lock/opts files you can delete if you have killed the
processes and want to eliminate any left-over state?  Is postmaster.opts
a configuration file or a state file?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/26/16 5:20 PM, Andres Freund wrote:
> I do think there's an order of magnitude between the impact between
> moving some and moving everything. And that's going to impact
> cost/benefit calculations.
> 
> Moving e.g. all ephemeral files into a (possibly configurable) directory
> is going to hardly impact anyone.  Renaming pg_logical into something
> different (FWIW, it was originally named differently...) will hopefully
> impact nobody, excepting some out of date file exclusion lists possibly.
> 
> But moving config files, and even pg_xlog (which we document to be
> symlinkable somewhere else) imo is different.

I agree with all that.  But the subject line is specifically about
moving pg_xlog.  So if your opinion is that we shouldn't move pg_xlog,
then that is noted.  But if we were to move it, we can think about a
good place to move it to.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming some binaries

[Peter Eisentraut]

On 8/26/16 12:26 PM, Euler Taveira wrote:
> initdb: we already have 'pg_ctl init' (since 9.0) and could remove initdb.

I have a concern specifically about pg_ctl.  Depending on how your
PostgreSQL is packaged, not all of the pg_ctl actions are safe or
sensible to run.  For example, if you are using systemd, then using
pg_ctl restart will probably not do the right thing.  And depending on
SELinux (maybe), running initdb directly might also not do the right
thing.  In most cases, you can just not use pg_ctl and do all these
things differently, but for example pg_ctl promote is the only
documented way to do that thing.

Until we have a better way to figure that out, I wouldn't want to put
more things into pg_ctl, especially as the only way.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] OpenSSL 1.1 breaks configure and more

[Peter Eisentraut]

On 8/26/16 9:26 PM, Andreas Karlsson wrote:
> I have attached a patch which removes the < 0.9.8 compatibility code. 
> Should we also add a version check to configure? We do not have any such 
> check currently.

I think that is not necessary.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/29/16 12:54 PM, Robert Haas wrote:
> As for the new names, how about pg_wal and
> pg_xact?  I don't think "pg_trans" is as good; is it transactional or
> transient or transport or transitive or what?

pg_transaction_status? (or pg_xact_status if you want to keep it short)

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[Peter Eisentraut]

On 8/28/16 8:45 PM, Jim Nasby wrote:
> People accidentally blowing away pg_clog or pg_xlog is a pretty common 
> occurrence, and I don't think there's all that many tools that reference 
> them. I think it's well worth renaming them.

I think a related problem is that the default log directory is called
"pg_log", which is very similar to those other two.  There is no quick
way to tell their meaning apart.

I would consider changing the default from "pg_log" to "log".  Then we'd
also be at the point where we can say, everything starting with "pg_" is
internal, don't touch it.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] sequences and pg_upgrade

[Peter Eisentraut]

I was toying with a couple of ideas that would involve changing the
storage of sequences.  (Say, for the sake of discussion, removing the
problematic/useless sequence_name field.)  This would cause problems for
pg_upgrade, because pg_upgrade copies the "heap" storage of sequences
like it does for normal tables, and we have no facilities for effecting
any changes during that.

There was a previous discussion in the early days of pg_migrator, which
resulted in the current behavior:
https://www.postgresql.org/message-id/flat/20090713220112.GF7933%40klana.box

This also alluded to what I think was the last change in the sequence
storage format (10a3471bed7b57fb986a5be8afdee5f0dda419de) between
versions 8.3 and 8.4.  How did pg_upgrade handle that?

I think the other solution mentioned in that thread would also work:
Have pg_upgrade treat sequences more like system catalogs, whose format
changes between major releases, and transferred them via the
dump/restore route.  So instead of copying the disk files, issue a
setval call, and the sequence should be all set up.

Am I missing anything?

Attached is a rough patch set that would implement that.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0c8f9bb630f48e83dc4dbe36e742db8e20f6b523 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 23 Aug 2016 12:00:00 -0400
Subject: [PATCH 1/3] pg_dump: Separate table data and sequence data object
 types

---
 src/bin/pg_dump/pg_dump.c  | 11 +++
 src/bin/pg_dump/pg_dump.h  |  1 +
 src/bin/pg_dump/pg_dump_sort.c |  7 +++
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a5c2d09..160bc41 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2133,6 +2133,8 @@ makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo, bool oids)
 
 	if (tbinfo->relkind == RELKIND_MATVIEW)
 		tdinfo->dobj.objType = DO_REFRESH_MATVIEW;
+	else if (tbinfo->relkind == RELKIND_SEQUENCE)
+		tdinfo->dobj.objType = DO_SEQUENCE_DATA;
 	else
 		tdinfo->dobj.objType = DO_TABLE_DATA;
 
@@ -9382,11 +9384,11 @@ dumpDumpableObject(Archive *fout, DumpableObject *dobj)
 		case DO_TRANSFORM:
 			dumpTransform(fout, (TransformInfo *) dobj);
 			break;
+		case DO_SEQUENCE_DATA:
+			dumpSequenceData(fout, (TableDataInfo *) dobj);
+			break;
 		case DO_TABLE_DATA:
-			if (((TableDataInfo *) dobj)->tdtable->relkind == RELKIND_SEQUENCE)
-dumpSequenceData(fout, (TableDataInfo *) dobj);
-			else
-dumpTableData(fout, (TableDataInfo *) dobj);
+			dumpTableData(fout, (TableDataInfo *) dobj);
 			break;
 		case DO_DUMMY_TYPE:
 			/* table rowtypes and array types are never dumped separately */
@@ -17482,6 +17484,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
 addObjectDependency(preDataBound, dobj->dumpId);
 break;
 			case DO_TABLE_DATA:
+			case DO_SEQUENCE_DATA:
 			case DO_BLOB_DATA:
 /* Data objects: must come between the boundaries */
 addObjectDependency(dobj, preDataBound->dumpId);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 2bfa2d9..6cc78d1 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -63,6 +63,7 @@ typedef enum
 	DO_PROCLANG,
 	DO_CAST,
 	DO_TABLE_DATA,
+	DO_SEQUENCE_DATA,
 	DO_DUMMY_TYPE,
 	DO_TSPARSER,
 	DO_TSDICT,
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index d87f08d..9ca3d2b 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -60,6 +60,7 @@ static const int oldObjectTypePriority[] =
 	2,			/* DO_PROCLANG */
 	2,			/* DO_CAST */
 	11,			/* DO_TABLE_DATA */
+	11,			/* DO_SEQUENCE_DATA */
 	7,			/* DO_DUMMY_TYPE */
 	4,			/* DO_TSPARSER */
 	4,			/* DO_TSDICT */
@@ -111,6 +112,7 @@ static const int newObjectTypePriority[] =
 	2,			/* DO_PROCLANG */
 	10,			/* DO_CAST */
 	23,			/* DO_TABLE_DATA */
+	23,			/* DO_SEQUENCE_DATA */
 	19,			/* DO_DUMMY_TYPE */
 	12,			/* DO_TSPARSER */
 	14,			/* DO_TSDICT */
@@ -1433,6 +1435,11 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
 	 "TABLE DATA %s  (ID %d OID %u)",
 	 obj->name, obj->dumpId, obj->catId.oid);
 			return;
+		case DO_SEQUENCE_DATA:
+			snprintf(buf, bufsize,
+	 "SEQUENCE DATA %s  (ID %d OID %u)",
+	 obj->name, obj->dumpId, obj->catId.oid);
+			return;
 		case DO_DUMMY_TYPE:
 			snprintf(buf, bufsize,
 	 "DUMMY TYPE %s  (ID %d OID %u)",
-- 
2.9.3

From 26325789ef3cb0e898d94f06b395ae4c64e3b2e9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 23 Aug 2016 12:00:00 -0400
Subject: [PATCH 2/3] pg_dump: Add --sequence-data option

---
 src/bin/pg_dump/pg_backup.h  |  2 ++
 src/bin/pg_dump/pg_backup_archiver.c |  6 +-
 src/bin/pg_du

[HACKERS] autonomous transactions

[Peter Eisentraut]

hardcoded knowledge of all relevant
background worker types.  So I tried a more general solution, with a hook.

- I added new test files in the plpgsql directory.  The main test for
plpgsql runs as part of the main test suite.  Maybe we want to move that
to the plpgsql directory as well.

- More guidance for using some of the background worker and shared
memory queue facilities.  For example, I don't know what a good queue
size would be.

- Both PL/pgSQL and PL/Python expose some details of SPI in ways that
make it difficult to run some things not through SPI.  For example,
return codes are exposed directly by PL/Python.  PL/pgSQL is heavily
tied to the API flow of SPI.  It's fixable, but it will be some work.  I
had originally wanted to hide the autonomous session API inside SPI or
make it fully compatible with SPI, but that was quickly thrown out.
PL/Python now contains some ugly code to make certain things match up so
that existing code can be used.  It's not always pretty.

- The patch "Set log_line_prefix and application name in test drivers"
(https://commitfest.postgresql.org/10/717/) is helpful in testing and
debugging this.


[0]:
https://www.postgresql.org/message-id/flat/CA+Tgmoam66dTzCP8N2cRcS6S6dBMFX+JMba+mDf68H=kakn...@mail.gmail.com

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index cec37ce..892d807 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -114,7 +114,7 @@ PrepareQuery(PrepareStmt *stmt, const char *queryString)
 	 */
 	query = parse_analyze_varparams((Node *) copyObject(stmt->query),
 	queryString,
-	&argtypes, &nargs);
+	&argtypes, &nargs, NULL);
 
 	/*
 	 * Check that all parameter types were determined.
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index defafa5..a522c69 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -674,12 +674,17 @@ show_random_seed(void)
  * SET CLIENT_ENCODING
  */
 
+void (*check_client_encoding_hook)(void);
+
 bool
 check_client_encoding(char **newval, void **extra, GucSource source)
 {
 	int			encoding;
 	const char *canonical_name;
 
+	if (check_client_encoding_hook)
+		check_client_encoding_hook();
+
 	/* Look up the encoding by name */
 	encoding = pg_valid_client_encoding(*newval);
 	if (encoding < 0)
diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c
index bfe66c6..7c7dc92 100644
--- a/src/backend/libpq/pqmq.c
+++ b/src/backend/libpq/pqmq.c
@@ -47,6 +47,11 @@ static PQcommMethods PqCommMqMethods = {
 	mq_endcopyout
 };
 
+static PQcommMethods *save_PqCommMethods;
+static CommandDest save_whereToSendOutput;
+static ProtocolVersion save_FrontendProtocol;
+static dsm_segment *save_seg;
+
 /*
  * Arrange to redirect frontend/backend protocol messages to a shared-memory
  * message queue.
@@ -54,12 +59,30 @@ static PQcommMethods PqCommMqMethods = {
 void
 pq_redirect_to_shm_mq(dsm_segment *seg, shm_mq_handle *mqh)
 {
+	save_PqCommMethods = PqCommMethods;
+	save_whereToSendOutput = whereToSendOutput;
+	save_FrontendProtocol = FrontendProtocol;
+
 	PqCommMethods = &PqCommMqMethods;
 	pq_mq = shm_mq_get_queue(mqh);
 	pq_mq_handle = mqh;
 	whereToSendOutput = DestRemote;
 	FrontendProtocol = PG_PROTOCOL_LATEST;
 	on_dsm_detach(seg, pq_cleanup_redirect_to_shm_mq, (Datum) 0);
+
+	save_seg = seg;
+}
+
+void
+pq_stop_redirect_to_shm_mq(void)
+{
+	cancel_on_dsm_detach(save_seg, pq_cleanup_redirect_to_shm_mq, (Datum) 0);
+	PqCommMethods = save_PqCommMethods;
+	whereToSendOutput = save_whereToSendOutput;
+	FrontendProtocol = save_FrontendProtocol;
+	pq_mq = NULL;
+	pq_mq_handle = NULL;
+	save_seg = NULL;
 }
 
 /*
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index eac86cc..5b94d85 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -124,7 +124,7 @@ parse_analyze(Node *parseTree, const char *sourceText,
  */
 Query *
 parse_analyze_varparams(Node *parseTree, const char *sourceText,
-		Oid **paramTypes, int *numParams)
+		Oid **paramTypes, int *numParams, const char *paramNames[])
 {
 	ParseState *pstate = make_parsestate(NULL);
 	Query	   *query;
@@ -133,7 +133,7 @@ parse_analyze_varparams(Node *parseTree, const char *sourceText,
 
 	pstate->p_sourcetext = sourceText;
 
-	parse_variable_parameters(pstate, paramTypes, numParams);
+	parse_variable_parameters(pstate, paramTypes, numParams, paramNames);
 
 	query = transformTopLevelStmt(pstate, parseTree);
 
diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index b402843..c459c00 100644
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -49,8 +49,10 @@ typedef struct VarParamState
 {
 	Oid		  **paramTypes;		/* array of parameter type OIDs

[HACKERS] ICU integration

[Peter Eisentraut]

Here is a patch I've been working on to allow the use of ICU for sorting
and other locale things.

This is mostly complementary to the existing FreeBSD ICU patch, most
recently discussed in [0].  While that patch removes the POSIX locale
use and replaces it with ICU, my interest was on allowing the use of
both.  I think that is necessary for upgrading, compatibility, and maybe
because someone likes it.

What I have done is extend collation objects with a collprovider column
that tells whether the collation is using POSIX (appropriate name?) or
ICU facilities.  The pg_locale_t type is changed to a struct that
contains the provider-specific locale handles.  Users of locale
information are changed to look into that struct for the appropriate
handle to use.

In initdb, I initialize the default collation set as before from the
`locale -a` output, but also add all available ICU locales with a "%icu"
appended (so "fr_FR%icu").  I suppose one could create a configuration
option perhaps in initdb to change the default so that, say, "fr_FR"
uses ICU and "fr_FR%posix" uses the old stuff.

That all works well enough for named collations and for sorting.  The
thread about the FreeBSD ICU patch discusses some details of how to best
use the ICU APIs to do various aspects of the sorting, so I didn't focus
on that too much.  I took the existing collate.linux.utf8.sql test and
ported it to the ICU setup, and it passes except for the case noted below.

I'm not sure how well it will work to replace all the bits of LIKE and
regular expressions with ICU API calls.  One problem is that ICU likes
to do case folding as a whole string, not by character.  I need to do
more research about that.  Another problem, which was also previously
discussed is that ICU does case folding in a locale-agnostic manner, so
it does not consider things such as the Turkish special cases.  This is
per Unicode standard modulo weasel wording, but it breaks existing tests
at least.

So right now the entries in collcollate and collctype need to be valid
for ICU *and* POSIX for everything to work.

Also note that ICU locales are encoding-independent and don't support a
separate collcollate and collctype, so the existing catalog structure is
not optimal.

Where it gets really interesting is what to do with the database
locales.  They just set the global process locale.  So in order to port
that to ICU we'd need to check every implicit use of the process locale
and tweak it.  We could add a datcollprovider column or something.  But
we also rely on the datctype setting to validate the encoding of the
database.  Maybe we wouldn't need that anymore, but it sounds risky.

We could have a datcollation column that by OID references a collation
defined inside the database.  With a background worker, we can log into
the database as it is being created and make adjustments, including
defining or adjusting collation definitions.  This would open up
interesting new possibilities.

What is a way to go forward here?  What's a minimal useful feature that
is future-proof?  Just allow named collations referencing ICU for now?
Is throwing out POSIX locales even for the process locale reasonable?

Oh, that case folding code in formatting.c needs some refactoring.
There are so many ifdefs there and it's repeated almost identically
three times, it's crazy to work in that.


[0]:
https://www.postgresql.org/message-id/flat/789A2F56-0E42-409D-A840-6AF5110D6085%40pingpong.net


-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/aclocal.m4 b/aclocal.m4
index 6f930b6..5ca902b 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -7,6 +7,7 @@ m4_include([config/docbook.m4])
 m4_include([config/general.m4])
 m4_include([config/libtool.m4])
 m4_include([config/perl.m4])
+m4_include([config/pkg.m4])
 m4_include([config/programs.m4])
 m4_include([config/python.m4])
 m4_include([config/tcl.m4])
diff --git a/config/pkg.m4 b/config/pkg.m4
new file mode 100644
index 000..82bea96
--- /dev/null
+++ b/config/pkg.m4
@@ -0,0 +1,275 @@
+dnl pkg.m4 - Macros to locate and utilise pkg-config.   -*- Autoconf -*-
+dnl serial 11 (pkg-config-0.29.1)
+dnl
+dnl Copyright © 2004 Scott James Remnant .
+dnl Copyright © 2012-2015 Dan Nicholson 
+dnl
+dnl This program is free software; you can redistribute it and/or modify
+dnl it under the terms of the GNU General Public License as published by
+dnl the Free Software Foundation; either version 2 of the License, or
+dnl (at your option) any later version.
+dnl
+dnl This program is distributed in the hope that it will be useful, but
+dnl WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+dnl General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU General Public License
+

[HACKERS] identity columns

[Peter Eisentraut]

Here is another attempt to implement identity columns.  This is a
standard-conforming variant of PostgreSQL's serial columns.  It also
fixes a few usability issues that serial columns have:

- need to set permissions on sequence in addition to table (*)
- CREATE TABLE / LIKE copies default but refers to same sequence
- cannot add/drop serialness with ALTER TABLE
- dropping default does not drop sequence
- slight weirdnesses because serial is some kind of special macro

(*) Not actually implemented yet, because I wanted to make use of the
NEXT VALUE FOR stuff I had previously posted, but I have more work to do
there.

There have been previous attempts at this between 2003 and 2007.  The
latest effort failed because it tried to implement identity columns and
generated columns in one go, but then discovered that they have wildly
different semantics.  But AFAICT, there weren't any fundamental issues
with the identity parts of the patch.

Here some interesting threads of old:

https://www.postgresql.org/message-id/flat/xzp1xw2x5jo.fsf%40dwp.des.no#xzp1xw2x5jo@dwp.des.no
https://www.postgresql.org/message-id/flat/1146360362.839.104.camel%40home#1146360362.839.104.camel@home
https://www.postgresql.org/message-id/23436.1149629297%40sss.pgh.pa.us
https://www.postgresql.org/message-id/flat/448C9B7A.601%40dunaweb.hu#448c9b7a.6010...@dunaweb.hu
https://www.postgresql.org/message-id/flat/45DACD1E.2000207%40dunaweb.hu#45dacd1e.2000...@dunaweb.hu
https://www.postgresql.org/message-id/flat/18812.1178572575%40sss.pgh.pa.us

Some comments on the implementation, and where it differs from previous
patches:

- The new attidentity column stores whether a column is an identity
column and when it is generated (always/by default).  I kept this
independent from atthasdef mainly for he benefit of existing (client)
code querying those catalogs, but I kept confusing myself by this, so
I'm not sure about that choice.  Basically we need to store four
distinct states (nothing, default, identity always, identity by default)
somehow.

- The way attidentity is initialized in some places is a bit hackish.  I
haven't focused on that so much without having a clear resolution to the
previous question.

- One previous patch managed to make GENERATED an unreserved key word.
I have it reserved right now, but perhaps with a bit more trying it can
be unreserved.

- I did not implement the restriction of one identity column per table.
That didn't seem necessary.

- I did not implement an override clause on COPY.  If COPY supplies a
value, it is always used.

- pg_dump is as always a mess.  Some of that is because of the way
pg_upgrade works: It only gives out one OID at a time, so you need to
create the table and sequences in separate entries.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 4e09e06..0879d12 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1095,6 +1095,17 @@ pg_attribute Columns
  
 
  
+  attidentity
+  char
+  
+  
+   If a space character, then not an identity column.  Otherwise,
+   a = generated always, d =
+   generated by default.
+  
+ 
+
+ 
   attisdropped
   bool
   
diff --git a/doc/src/sgml/information_schema.sgml b/doc/src/sgml/information_schema.sgml
index c43e325..8ece439 100644
--- a/doc/src/sgml/information_schema.sgml
+++ b/doc/src/sgml/information_schema.sgml
@@ -1583,13 +1583,20 @@ columns Columns
  
   is_identity
   yes_or_no
-  Applies to a feature not available in PostgreSQL
+  
+   If the column is an identity column, then YES,
+   else NO.
+  
  
 
  
   identity_generation
   character_data
-  Applies to a feature not available in PostgreSQL
+  
+   If the column is an identity column, then ALWAYS
+   or BY DEFAULT, reflecting the definition of the
+   column.
+  
  
 
  
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 6f51cbc..969ce45 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -42,6 +42,8 @@
 ALTER [ COLUMN ] column_name SET DEFAULT expression
 ALTER [ COLUMN ] column_name DROP DEFAULT
 ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL
+ALTER [ COLUMN ] column_name ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY [ ( sequence_options ) ]
+ALTER [ COLUMN ] column_name DROP IDENTITY
 ALTER [ COLUMN ] column_name SET STATISTICS integer
 ALTER [ COLUMN ] column_name SET ( attribute_option = value [, ... ] )
 ALTER [ COLUMN ] column_name RESET ( attribute_option [, ... ] )
@@ -170,6 +172,15 @@ Description

 

+ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY/DROP IDENTITY
+
+ 
+  These forms change wh

Re: [HACKERS] ICU integration

[Peter Eisentraut]

On 8/30/16 11:27 PM, Craig Ringer wrote:
> Speaking of which, have you had a chance to try it on Windows yet?

nope

> How stable are the UCU locales? Most importantly, does ICU offer any
> way to "pin" a locale version, so we can say "we want de_DE as it was
> in ICU 4.6" and get consistent behaviour when the user sets up a
> replica on some other system with ICU 4.8? Even if the German
> government has changed its mind (again) about some details of the
> language and 4.8 knows about the changes but 4.4 doesn't?

I forgot to mention this, but the patch adds a collversion column that
stores the collation version (provided by ICU).  And then when you
upgrade ICU to something incompatible you get

+   if (numversion != collform->collversion)
+   ereport(WARNING,
+   (errmsg("ICU collator version mismatch"),
+errdetail("The database was created using
version 0x%08X, the library provides version 0x%08X.",
+  (uint32) collform->collversion,
(uint32) numversion),
+errhint("Rebuild affected indexes, or build
PostgreSQL with the right version of ICU.")));

So you still need to manage this carefully, but at least you have a
chance to learn about it.

Suggestions for refining this are welcome.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] sequence data type

[Peter Eisentraut]

Here is a patch that adds the notion of a data type to a sequence.  So
it might be CREATE SEQUENCE foo AS integer.  The types are restricted to
int{2,4,8} as now.

The main point of this is to make monitoring sequences less complicated.
 Right now, a serial column creates an int4 column but creates the
sequence with a max value for int8.  So in order to correctly answer the
question, is the sequence about to run out, you need to look not only at
the sequence but also any columns it is associated with.  check_postgres
figures this out, but it's complicated and slow, and not easy to do
manually.

If you tell the sequence the data type you have in mind, it
automatically sets appropriate min and max values.  Serial columns also
make use of this, so the sequence type automatically matches the column
type.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 97246197cfe3a69d14af1eb98f894946a2b8122d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 23 Aug 2016 12:00:00 -0400
Subject: [PATCH] Add CREATE SEQUENCE AS  clause

This stores a data type, required to be an integer type, with the
sequence.  The sequences min and max values default to the range
supported by the type, and they cannot be set to values exceeding that
range.  The internal implementation of the sequence is not affected.

Change the serial types to create sequences of the appropriate type.
This makes sure that the min and max values of the sequence for a serial
column match the range of values supported by the table column.  So the
sequence can no longer overflow the table column.

This also makes monitoring for sequence exhaustion/wraparound easier,
which currently requires various contortions to cross-reference the
sequences with the table columns they are used with.
---
 doc/src/sgml/information_schema.sgml  |  4 +-
 doc/src/sgml/ref/create_sequence.sgml | 37 +++
 src/backend/catalog/information_schema.sql|  4 +-
 src/backend/commands/sequence.c   | 92 --
 src/backend/parser/gram.y |  6 +-
 src/backend/parser/parse_utilcmd.c|  2 +-
 src/bin/pg_dump/pg_dump.c | 94 +--
 src/bin/pg_dump/t/002_pg_dump.pl  |  2 +
 src/include/catalog/pg_proc.h |  2 +-
 src/include/commands/sequence.h   |  6 +-
 src/include/pg_config_manual.h|  6 --
 src/test/modules/test_pg_dump/t/001_base.pl   |  1 +
 src/test/regress/expected/sequence.out| 45 +
 src/test/regress/expected/sequence_1.out  | 45 +
 src/test/regress/expected/updatable_views.out |  3 +-
 src/test/regress/sql/sequence.sql | 20 +-
 16 files changed, 269 insertions(+), 100 deletions(-)

diff --git a/doc/src/sgml/information_schema.sgml b/doc/src/sgml/information_schema.sgml
index c43e325..a3a19ce 100644
--- a/doc/src/sgml/information_schema.sgml
+++ b/doc/src/sgml/information_schema.sgml
@@ -4653,9 +4653,7 @@ sequences Columns
   data_type
   character_data
   
-   The data type of the sequence.  In
-   PostgreSQL, this is currently always
-   bigint.
+   The data type of the sequence.
   
  
 
diff --git a/doc/src/sgml/ref/create_sequence.sgml b/doc/src/sgml/ref/create_sequence.sgml
index c959146..f31b595 100644
--- a/doc/src/sgml/ref/create_sequence.sgml
+++ b/doc/src/sgml/ref/create_sequence.sgml
@@ -21,7 +21,9 @@
 
  
 
-CREATE [ TEMPORARY | TEMP ] SEQUENCE [ IF NOT EXISTS ] name [ INCREMENT [ BY ] increment ]
+CREATE [ TEMPORARY | TEMP ] SEQUENCE [ IF NOT EXISTS ] name
+[ AS data_type ]
+[ INCREMENT [ BY ] increment ]
 [ MINVALUE minvalue | NO MINVALUE ] [ MAXVALUE maxvalue | NO MAXVALUE ]
 [ START [ WITH ] start ] [ CACHE cache ] [ [ NO ] CYCLE ]
 [ OWNED BY { table_name.column_name | NONE } ]
@@ -111,6 +113,21 @@ Parameters

 

+data_type
+
+ 
+  The optional
+  clause AS data_type
+  specifies the data type of the sequence.  Valid types are
+  are smallint, integer,
+  and bigint.  bigint is the
+  default.  The data type determines the default minimum and maximum
+  values of the sequence.
+ 
+
+   
+
+   
 increment
 
  
@@ -132,9 +149,9 @@ Parameters
   class="parameter">minvalue determines
   the minimum value a sequence can generate. If this clause is not
   supplied or NO MINVALUE is specified, then
-  defaults will be used.  The defaults are 1 and
-  -263-1 for ascending and descending sequences,
-  respectively.
+  defaults will be used.  The default for an ascending sequence is 1.  The
+  default for a descending sequence is the minimum value of the data type
+  plus 1.
  
 

@@ -148,9 +165,9 @@ Parameters
   class="parameter">maxvalue dete