PostgreSQL 9.6.1 commit log

commit   : 16e7c02c423449697b78ca857e5847981785e9ed    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 24 Oct 2016 16:08:51 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 24 Oct 2016 16:08:51 -0400    

commit   : 681fdbbff1cd44963883c27007f10e1a8c4e65de    
  
author   : Peter Eisentraut <[email protected]>    
date     : Mon, 24 Oct 2016 12:00:00 -0400    
  
committer: Peter Eisentraut <[email protected]>    
date     : Mon, 24 Oct 2016 12:00:00 -0400    

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git  
Source-Git-Hash: e4e428572533133cac4ecefd69d251a0b5674fa3  

commit   : b9a728ef45fcfd93aa2efafb6876d2a946793f95    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 24 Oct 2016 09:37:23 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 24 Oct 2016 09:37:23 -0400    

commit   : c8329f934e1bce2a47ca9127cb7cf704e070c278    
  
author   : Alvaro Herrera <[email protected]>    
date     : Mon, 24 Oct 2016 09:27:24 -0300    
  
committer: Alvaro Herrera <[email protected]>    
date     : Mon, 24 Oct 2016 09:27:24 -0300    

An oversight in setting the boundaries of known commit timestamps during  
startup caused old commit timestamps to become inaccessible after a  
server restart.  
  
Author and reporter: Julien Rouhaud  
Review, test code: Craig Ringer  

commit   : 6beb8c75ceed610a8e5d416882aad75fe722249e    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 22:13:28 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 22:13:28 -0400    

commit   : fdcee9f1fc12d6d2c20da8da632b2062d2ed6289    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 19:14:32 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 19:14:32 -0400    

INSERT ... ON CONFLICT (specifically ExecCheckHeapTupleVisible) contains  
another example of this unsafe coding practice.  It is much harder to get  
a failure out of it than the case fixed in commit 6292c2339, because in  
most scenarios any hint bits that could be set would have already been set  
earlier in the command.  However, Konstantin Knizhnik reported a failure  
with a custom transaction manager, and it's clearly possible to get a  
failure via a race condition in async-commit mode.  
  
For lack of a reproducible example, no regression test case in this  
commit.  
  
I did some testing with Asserts added to tqual.c's functions, and can say  
that running "make check-world" exposed these two bugs and no others.  
The Asserts are messy enough that I've not added them to the code for now.  
  
Report: <[email protected]>  
Related-Discussion: <CAO3NbwOycQjt2Oqy2VW-eLTq2M5uGMyHnGm=RNga4mjqcYD7gQ@mail.gmail.com>  

commit   : c4016fcb1fd2a20ef589a3a616b59381256a985a    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 18:36:13 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 18:36:13 -0400    

A transaction that conflicts against itself, for example  
	INSERT INTO t(pk) VALUES (1),(1) ON CONFLICT DO NOTHING;  
should behave the same regardless of isolation level.  It certainly  
shouldn't throw a serialization error, as retrying will not help.  
We got this wrong due to the ON CONFLICT logic not considering the case,  
as reported by Jason Dusek.  
  
Core of this patch is by Peter Geoghegan (based on an earlier patch by  
Thomas Munro), though I didn't take his proposed code refactoring for fear  
that it might have unexpected side-effects.  Test cases by Thomas Munro  
and myself.  
  
Report: <CAO3NbwOycQjt2Oqy2VW-eLTq2M5uGMyHnGm=RNga4mjqcYD7gQ@mail.gmail.com>  
Related-Discussion: <[email protected]>  

commit   : 5beb73b49598ff255c3d085d27d72bf28c8551b0    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 15:01:24 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 23 Oct 2016 15:01:24 -0400    

Despite the argumentation I wrote in commit 7a2fe85b0, it's unsafe to do  
this, because in corner cases it's possible for HeapTupleSatisfiesSelf  
to try to set hint bits on the target tuple; and at least since 8.2 we  
have required the buffer content lock to be held while setting hint bits.  
  
The added regression test exercises one such corner case.  Unpatched, it  
causes an assertion failure in assert-enabled builds, or otherwise would  
cause a hint bit change in a buffer we don't hold lock on, which given  
the right race condition could result in checksum failures or other data  
consistency problems.  The odds of a problem in the field are probably  
pretty small, but nonetheless back-patch to all supported branches.  
  
Report: <[email protected]>  

commit   : dc04be21d655c06ead19247c0240c684ce03b9d1    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 22 Oct 2016 14:04:51 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 22 Oct 2016 14:04:51 -0400    

Show how to get the system's huge page size, rather than misleadingly  
referring to PAGE_SIZE (which is usually understood to be the regular  
page size).  Show how to confirm whether huge pages have been allocated.  
Minor wordsmithing.  Back-patch to 9.4 where this section appeared.  

commit   : a92c0f695bdf4e38218e918bf0474c0d45b6ebd0    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 21 Oct 2016 11:01:35 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 21 Oct 2016 11:01:35 -0400    

Replace "Full path to ..." with "Full path name of ...".  At least one  
user has misinterpreted the existing wording as meaning "Directory  
containing ...".  

commit   : 0336665151f346737102cf27dbb682d6a526c8e4    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 17:17:50 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 17:17:50 -0400    

With track_io_timing = on, EXPLAIN (ANALYZE, BUFFERS) will emit fields  
named like "I/O Read Time".  The slash makes that invalid as an XML  
element name, so that adding FORMAT XML would produce invalid XML.  
  
We already have code in there to translate spaces to dashes, so let's  
generalize that to convert anything that isn't a valid XML name character,  
viz letters, digits, hyphens, underscores, and periods.  We could just  
reject slashes, which would run a bit faster.  But the fact that this went  
unnoticed for so long doesn't give me a warm feeling that we'd notice the  
next creative violation, so let's make it a permanent fix.  
  
Reported by Markus Winand, though this isn't his initial patch proposal.  
  
Back-patch to 9.2 where track_io_timing was added.  The problem is only  
latent in 9.1, so I don't feel a need to fix it there.  
  
Discussion: <[email protected]>  

commit   : 845a8ea3e144c3a9ca2c95fd70de89c26422fb35    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 15:40:07 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 15:40:07 -0400    

This absorbs a fix for a symlink-manipulation bug in zic that was  
introduced in 2016g.  It probably isn't interesting for our use-case,  
but I'm not quite sure, so let's update while we're at it.  

commit   : 80ba149b057418b61d123d9428144406d2d6d238    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 15:20:11 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 20 Oct 2016 15:20:11 -0400    

(Didn't I just do this?  Oh well.)  
  
DST law changes in Palestine.  Historical corrections for Turkey.  
Switch to numeric abbreviations for Asia/Colombo.  

commit   : 9937088b26eef7bbe27a9bb5f497aa6307fded5c    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 23:32:08 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 23:32:08 -0400    

clang points out that SIZE_MAX wouldn't fit into an int, which means  
this comparison is pretty useless.  Per report from Thomas Munro.  

commit   : 9ee22f1ff66ae3632a4cd0391c487a327ef401ca    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 19:28:11 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 19:28:11 -0400    

Per buildfarm.  

commit   : 7fec5e101a12acbabea6ecef3e7101c5de81e609    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 18:55:52 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 18:55:52 -0400    

This is mostly to absorb some corner-case fixes in zic for year-2037  
timestamps.  The other changes that have been made are unlikely to affect  
our usage, but nonetheless we may as well take 'em.  

commit   : 1b145a700158ab48c387d38f4830136d307a0e87    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 18:11:49 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 18:11:49 -0400    

IANA got rid of the really silly "abbreviation" and replaced it with one  
that's only moderately silly.  But it's still pointless, so keep on not  
showing it.  

commit   : 0c2f4c54c57cbc7283606137dba5c658511e6c1b    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 17:56:38 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 19 Oct 2016 17:56:38 -0400    

DST law changes in Turkey.  Historical corrections for America/Los_Angeles,  
Europe/Kirov, Europe/Moscow, Europe/Samara, and Europe/Ulyanovsk.  
Rename Asia/Rangoon to Asia/Yangon, with a backward compatibility link.  
  
The IANA crew continue their campaign to replace invented time zone  
abbrevations with numeric GMT offsets.  This update changes numerous zones  
in Antarctica and the former Soviet Union, for instance Antarctica/Casey  
now reports "+08" not "AWST" in the pg_timezone_names view.  I kept these  
abbreviations in the tznames/ data files, however, so that we will still  
accept them for input.  (We may want to start trimming those files someday,  
but today is not that day.)  
  
An exception is that since IANA no longer claims that "AMT" is in use  
in Armenia for GMT+4, I replaced it in the Default file with GMT-4,  
corresponding to Amazon Time which is in use in South America.  It may be  
that that meaning is also invented and IANA will drop it in a future  
update; but for now, it seems silly to give pride of place to a meaning  
not traceable to IANA over one that is.  

commit   : 142530ef0e683af4f063e4efd3b322cd3e9ea87e    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Wed, 19 Oct 2016 14:43:34 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Wed, 19 Oct 2016 14:43:34 +0300    

When a relation is truncated, it is important that the FSM is truncated as  
well. Otherwise, after recovery, the FSM can return a page that has been  
truncated away, leading to errors like:  
  
ERROR:  could not read block 28991 in file "base/16390/572026": read only 0  
of 8192 bytes  
  
We were using MarkBufferDirtyHint() to dirty the buffer holding the last  
remaining page of the FSM, but during recovery, that might in fact not  
dirty the page, and the FSM update might be lost.  
  
To fix, use the stronger MarkBufferDirty() function. MarkBufferDirty()  
requires us to do WAL-logging ourselves, to protect from a torn page, if  
checksumming is enabled.  
  
Also fix an oversight in visibilitymap_truncate: it also needs to WAL-log  
when checksumming is enabled.  
  
Analysis by Pavan Deolasee.  
  
Discussion: <CABOikdNr5vKucqyZH9s1Mh0XebLs_jRhKv6eJfNnD2wxTn=_9A@mail.gmail.com>  
  
Backpatch to 9.3, where we got data checksums.  

commit   : cb6ab91797b98d1498adcea906fbddffec8c8f9c    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 18 Oct 2016 12:24:46 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 18 Oct 2016 12:24:46 -0400    

CommandId is declared as uint32, and values up to 4G are indeed legal.  
cidout() handles them properly by treating the value as unsigned int.  
But cidin() was just using atoi(), which has platform-dependent behavior  
for values outside the range of signed int, as reported by Bart Lengkeek  
in bug #14379.  Use strtoul() instead, as xidin() does.  
  
In passing, make some purely cosmetic changes to make xidin/xidout  
look more like cidin/cidout; the former didn't have a monopoly on  
best practice IMO.  
  
Neither xidin nor cidin make any attempt to throw error for invalid input.  
I didn't change that here, and am not sure it's worth worrying about  
since neither is really a user-facing type.  The point is just to ensure  
that indubitably-valid inputs work as expected.  
  
It's been like this for a long time, so back-patch to all supported  
branches.  
  
Report: <[email protected]>  

commit   : a5f0bd77a2fab60a52dc335a63efc21abc806aa7    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Mon, 17 Oct 2016 12:13:16 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Mon, 17 Oct 2016 12:13:16 +0300    

Have tuplesort_gettupleslot() copy the contents of its current table slot  
as needed. This is based on an approach taken by tuplestore_gettupleslot().  
In the future, tuplesort_gettupleslot() may also be taught to avoid copying  
the tuple where caller can determine that that is safe (the  
tuplestore_gettupleslot() interface already offers this option to callers).  
  
Patch by Peter Geoghegan. Fixes bug #14344, reported by Regina Obe.  
  
Report: <[email protected]>  
  
Backpatch-through: 9.6  

commit   : db4b54f1550aa830890ce190e5d196de8d17db33    
  
author   : Robert Haas <[email protected]>    
date     : Wed, 12 Oct 2016 17:01:19 -0700    
  
committer: Tatsuo Ishii <[email protected]>    
date     : Wed, 12 Oct 2016 17:01:19 -0700    

Tatsuo Ishii  

commit   : 4338ac8e78e95dcc327e86fc9944bdae8c551c27    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 14 Oct 2016 16:28:34 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 14 Oct 2016 16:28:34 -0400    

bitshiftright() and bitshiftleft() would recursively call each other  
infinitely if the user passed INT_MIN for the shift amount, due to integer  
overflow in negating the shift amount.  To fix, clamp to -VARBITMAXLEN.  
That doesn't change the results since any shift distance larger than the  
input bit string's length produces an all-zeroes result.  
  
Also fix some places that seemed inadequately paranoid about input typmods  
exceeding VARBITMAXLEN.  While a typmod accepted by anybit_typmodin() will  
certainly be much less than that, at least some of these spots are  
reachable with user-chosen integer values.  
  
Andreas Seltenreich and Tom Lane  
  
Discussion: <[email protected]>  

commit   : a209d6dc8cce69bf4d305486816845be3febacba    
  
author   : Tatsuo Ishii <[email protected]>    
date     : Fri, 14 Oct 2016 09:03:25 +0900    
  
committer: Tatsuo Ishii <[email protected]>    
date     : Fri, 14 Oct 2016 09:03:25 +0900    

Confirmed by Michael Paquier.  

commit   : 03f2bf70a3e6fb57f29e41a86cf8f9a14b082616    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 19:45:58 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 19:45:58 -0400    

pgstat_twophase_postcommit is supposed to duplicate the math in  
AtEOXact_PgStat, but it had missed out the bit about clearing  
t_delta_live_tuples/t_delta_dead_tuples for a TRUNCATE.  
  
It's harder than you might think to replicate the issue here, because  
those counters would only be nonzero when a previous transaction in  
the same backend had added/deleted tuples in the truncated table,  
and those counts hadn't been sent to the stats collector yet.  
  
Evident oversight in commit d42358efb.  I've not added a regression  
test for this; we tried to add one in d42358efb, and had to revert it  
because it was too timing-sensitive for the buildfarm.  
  
Back-patch to 9.5 where d42358efb came in.  
  
Stas Kelvich  
  
Discussion: <[email protected]>  

commit   : b8850031ccc51401c256203b7dc9029a1160479e    
  
author   : Tatsuo Ishii <[email protected]>    
date     : Fri, 14 Oct 2016 07:45:25 +0900    
  
committer: Tatsuo Ishii <[email protected]>    
date     : Fri, 14 Oct 2016 07:45:25 +0900    

Confirmed by Tom Lane.  

commit   : f9e8b05e5f1c5eeaa963a9550ba6a592bdcb9a97    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 17:05:14 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 17:05:14 -0400    

It's not good for an inherited child constraint to be marked connoinherit;  
that would result in the constraint not propagating to grandchild tables,  
if any are created later.  The code mostly prevented this from happening  
but there was one case that was missed.  
  
This is somewhat related to commit e55a946a8, which also tightened checks  
on constraint merging.  Hence, back-patch to 9.2 like that one.  This isn't  
so much because there's a concrete feature-related reason to stop there,  
as to avoid having more distinct behaviors than we have to in this area.  
  
Amit Langote  
  
Discussion: <[email protected]>  

commit   : 2dd9e315d461b81258f4a575c916aba69c1d69c9    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 15:06:46 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 15:06:46 -0400    

Even if Linux's mmap() is okay with a partial-hugepage request, munmap()  
is not, as reported by Chris Richards.  Therefore it behooves us to try  
a bit harder to find out the actual hugepage size, instead of assuming  
that we can skate by with a guess.  
  
For the moment, just look into /proc/meminfo to find out the default  
hugepage size, and use that.  Later, on kernels that support requests  
for nondefault sizes, we might try to consider other alternatives.  
But that smells more like a new feature than a bug fix, especially if  
we want to provide any way for the DBA to control it, so leave it for  
another day.  
  
I set this up to allow easy addition of platform-specific code for  
non-Linux platforms, if needed; but right now there are no reports  
suggesting that we need to work harder on other platforms.  
  
Back-patch to 9.4 where hugepage support was introduced.  
  
Discussion: <[email protected]>  

commit   : 9a9771c50e8100deb31058fcbb35a5bfd9e1e064    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 13:59:56 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 13:59:56 -0400    

Fix detaching of the mmap'd segment to have its own on_shmem_exit callback,  
rather than piggybacking on the one for detaching from the SysV segment.  
That was confusing, and given the distance between the two attach calls,  
it was trouble waiting to happen.  
  
Make the detaching calls idempotent by clearing AnonymousShmem to show  
we've already unmapped.  I spent quite a bit of time yesterday trying  
to find a path that would allow the munmap()'s to be done twice, and  
while I did not succeed, it seems silly that there's even a question.  
  
Make the #ifdef logic less confusing by separating "do we want to use  
anonymous shmem" from EXEC_BACKEND.  Even though there's no current  
scenario where those conditions are different, it is not helpful for  
different places in the same file to be testing EXEC_BACKEND for what  
are fundamentally different reasons.  
  
Don't do on_exit_reset() in StartBackgroundWorker().  At best that's  
useless (InitPostmasterChild would have done it already) and at worst  
it could zap some callback that's unrelated to shared memory.  
  
Improve comments, and simplify the huge_pages enablement logic slightly.  
  
Back-patch to 9.4 where hugepage support was introduced.  
Arguably this should go into 9.3 as well, but the code looks  
significantly different there, and I doubt it's worth the  
trouble of adapting the patch given I can't show a live bug.  

commit   : 0e9e64c07ed93fb2be4583adaea28205d3188a0e    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 00:25:28 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 13 Oct 2016 00:25:28 -0400    

Commit 0b62fd036 did a fairly sloppy job of refactoring setPath()  
to support jsonb_insert() along with jsonb_set().  In its defense,  
though, there was no regression test case exercising the case of  
replacing an existing element in a jsonb array.  
  
Per bug #14366 from Peng Sun.  Back-patch to 9.6 where bug was introduced.  
  
Report: <[email protected]>  

commit   : 490734c588c4e70d46950c4f6c64dfd6e592cdcc    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 12 Oct 2016 18:01:43 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 12 Oct 2016 18:01:43 -0400    

This turns out not to be as harmless as I thought: MSVC will complain  
if it sees an "extern" declaration without PGDLLEXPORT and then one with.  
(Seems fairly silly, given that this can be changed after the fact by the  
linker, but there you have it.)  Therefore, contrib modules that have  
extern's for V1 functions in header files are falling over in the  
buildfarm, since none of those externs are marked PGDLLEXPORT.  
  
We might or might not conclude that we're willing to plaster those  
declarations with PGDLLEXPORT in HEAD, but in any case there's no way we're  
going to ship this change in the back branches.  Third-party authors would  
not thank us for breaking their code in a minor release.  Hence, revert  
the addition of PGDLLEXPORT (but let's keep the extra info in the comment).  
If we do the other changes we can revert this commit in HEAD.  
  
Per buildfarm.  

commit   : c4a2ffefbf74bae049c95339fb733955d2b12c55    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 12 Oct 2016 12:45:50 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 12 Oct 2016 12:45:50 -0400    

This isn't really necessary for our own code, because we use a .DEF file  
in MSVC builds (see gendef.pl), or --export-all-symbols in MinGW and  
Cygwin builds, to ensure that all global symbols in loadable modules  
will be exported on Windows.  However, third-party authors might use  
different build processes that need this marker, and it's harmless  
enough for our own builds.  
  
To some extent, this is an oversight in commit e7128e8db, so back-patch  
to 9.4 where that was added.  
  
Laurenz Albe  
  
Discussion: <A737B7A37273E048B164557ADEF4A58B539300BD@ntex2010a.host.magwien.gv.at>  

commit   : 3ccc15312b23fe283c7b63b3109229c16166745e    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Wed, 12 Oct 2016 12:07:54 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Wed, 12 Oct 2016 12:07:54 +0300    

Amit Langote  

commit   : 1fd64a9277439c94b8e102da6ced534f91807e57    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 11 Oct 2016 10:33:59 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 11 Oct 2016 10:33:59 -0400    

Fix poor grammar introduced in 741ccd501.  

commit   : cdc9d712f56bd3a7f40dbb9135992eb6a7a65e2f    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 11 Oct 2016 10:08:45 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 11 Oct 2016 10:08:45 -0400    

It was perhaps not entirely clear that internal self-references shouldn't  
be schema-qualified even if the view name is written with a schema.  
Spell it out.  
  
Discussion: <[email protected]>  

commit   : bb211b66f2934fbfe85c091dbff8db9ac520ceb5    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 10 Oct 2016 10:35:58 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 10 Oct 2016 10:35:58 -0400    

Ordinarily there would not be an async result sitting around at this  
point, but it appears that in corner cases there can be.  Considering  
all the work we're about to launch, it's hardly going to cost anything  
noticeable to check.  
  
It's been like this forever, so back-patch to all supported branches.  
  
Report: <CAD-Qf1eLUtBOTPXyFQGW-4eEsop31tVVdZPu4kL9pbQ6tJPO8g@mail.gmail.com>  

commit   : dca25c2562199ce1e7e26367613912a8eadbbde8    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 9 Oct 2016 12:49:37 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 9 Oct 2016 12:49:37 -0400    

The transfunction was told that its first argument and result were  
of the window function output type, not the aggregate state type.  
This'd only matter if the transfunction consults get_fn_expr_argtype,  
which typically only polymorphic functions would do.  
  
Although we have several regression tests around polymorphic aggs,  
none of them detected this mistake --- in fact, they still didn't  
fail when I injected the same mistake into nodeAgg.c.  So add some  
more tests covering both plain agg and window-function-agg cases.  
  
Per report from Sebastian Luque.  Back-patch to 9.6 where the error  
was introduced (by sloppy refactoring in commit 804163bc2, looks like).  
  
Report: <[email protected]>  

commit   : b605aeba0e117e4b8087fc95eba4cce07bb185ef    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 8 Oct 2016 19:29:27 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 8 Oct 2016 19:29:27 -0400    

Historically, we've allowed users to add a CHECK constraint to a child  
table and then add an identical CHECK constraint to the parent.  This  
results in "merging" the two constraints so that the pre-existing  
child constraint ends up with both conislocal = true and coninhcount > 0.  
However, if you tried to do it in the other order, you got a duplicate  
constraint error.  This is problematic for pg_dump, which needs to issue  
separated ADD CONSTRAINT commands in some cases, but has no good way to  
ensure that the constraints will be added in the required order.  
And it's more than a bit arbitrary, too.  The goal of complaining about  
duplicated ADD CONSTRAINT commands can be served if we reject the case of  
adding a constraint when the existing one already has conislocal = true;  
but if it has conislocal = false, let's just make the ADD CONSTRAINT set  
conislocal = true.  In this way, either order of adding the constraints  
has the same end result.  
  
Another problem was that the code allowed creation of a parent constraint  
marked convalidated that is merged with a child constraint that is  
!convalidated.  In this case, an inheritance scan of the parent table could  
emit some rows violating the constraint condition, which would be an  
unexpected result given the marking of the parent constraint as validated.  
Hence, forbid merging of constraints in this case.  (Note: valid child and  
not-valid parent seems fine, so continue to allow that.)  
  
Per report from Benedikt Grundmann.  Back-patch to 9.2 where we introduced  
possibly-not-valid check constraints.  The second bug obviously doesn't  
apply before that, and I think the first doesn't either, because pg_dump  
only gets into this situation when dealing with not-valid constraints.  
  
Report: <CADbMkNPT-Jz5PRSQ4RbUASYAjocV_KHUWapR%2Bg8fNvhUAyRpxA%40mail.gmail.com>  
Discussion: <[email protected]>  

commit   : b89f07b592210be3906d8201f03f0cf569f0dd83    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 8 Oct 2016 18:43:01 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 8 Oct 2016 18:43:01 -0400    

Back-patch commit 0dba54f1666ead71c54ce100b39efda67596d297 into the older  
branches.  This test is almost as much of a patching hazard there as it is  
in HEAD, and it has no more reason to be needed than it does in HEAD.  
  
I went back as far as 9.2; I judged 9.1 not worth the trouble since  
it's on the verge of being EOL'd.  

commit   : f812b20ba57cd753a5290967db3496790a2dff0f    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 7 Oct 2016 21:12:25 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 7 Oct 2016 21:12:25 -0400    

The need for this was previously obscured even on picky platforms  
by the hack we used to support direct cross-module references in  
the transforms contrib modules.  Now that that hack is gone, the  
undefined symbol is exposed, as reported by Robert Haas.  
  
Back-patch to 9.5 where we started to use -Wl,-undefined,dynamic_lookup.  
I'm a bit surprised that the older branches don't seem to contain  
any gettext references in this module, but since they don't fail  
at build time, they must not.  (We might be able to get away with  
leaving this alone in 9.5/9.6, but I think it's cleaner if the  
reference gets resolved at link time.)  
  
Report: <CA+TgmoaHJKU5kcWZcYduATYVT7Mnx+8jUnycaYYL7OtCwCigug@mail.gmail.com>  

commit   : a4b25cd9cba99b2bf1363f6b4fb9ced84b46d796    
  
author   : Andres Freund <[email protected]>    
date     : Fri, 7 Oct 2016 16:55:15 -0700    
  
committer: Andres Freund <[email protected]>    
date     : Fri, 7 Oct 2016 16:55:15 -0700    

I somehow had assumed that in the spinlock (in turn possibly using  
semaphores) based fallback atomics implementation 32 bit writes could be  
done without a lock. As far as the write goes that's correct, since  
postgres supports only platforms with single-copy atomicity for aligned  
32bit writes.  But writing without holding the spinlock breaks  
read-modify-write operations like pg_atomic_compare_exchange_u32(),  
since they'll potentially "miss" a concurrent write, which can't happen  
in actual hardware implementations.  
  
In 9.6+ when using the fallback atomics implementation this could lead  
to buffer header locks not being properly marked as released, and  
potentially some related state corruption.  I don't see a related danger  
in 9.5 (earliest release with the API), because pg_atomic_write_u32()  
wasn't used in a concurrent manner there.  
  
The state variable of local buffers, before this change, were  
manipulated using pg_atomic_write_u32(), to avoid unnecessary  
synchronization overhead. As that'd not be the case anymore, introduce  
and use pg_atomic_unlocked_write_u32(), which does not correctly  
interact with RMW operations.  
  
This bug only caused issues when postgres is compiled on platforms  
without atomics support (i.e. no common new platform), or when compiled  
with --disable-atomics, which explains why this wasn't noticed in  
testing.  
  
Reported-By: Tom Lane  
Discussion: <[email protected]>  
Backpatch: 9.5-, where the atomic operations API was introduced.  

commit   : 2efed55db1cc8e40d74242e61d7c741cccb4a50d    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 21:48:21 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 21:48:21 +0300    

Recent Perl and/or new Linux distributions are starting to remove "." from  
the @INC list by default. That breaks pg_rewind and ssl test suites, which  
use helper perl modules that reside in the same directory. To fix, add the  
current source directory explicitly to prove's include dir.  
  
The vcregress.pl script probably also needs something like this, but I  
wasn't able to remove '.' from @INC on Windows to test this, and don't want  
to try doing that blindly.  
  
Discussion: <[email protected]>  

commit   : 1749332ec9e5017ca7ebfa245597cc765b4553fd    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 7 Oct 2016 09:51:18 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 7 Oct 2016 09:51:18 -0400    

getBlobs' queries for pre-9.0 servers were broken in two ways:  
the 7.x/8.x query uses DISTINCT so it can't have unspecified-type  
NULLs in the target list, and both that query and the 7.0 one  
failed to provide the correct output column labels, so that the  
subsequent code to extract data from the PGresult would fail.  
  
Back-patch to 9.6 where the breakage was introduced (by commit 23f34fa4b).  
  
Amit Langote and Tom Lane  
  
Discussion: <[email protected]>  

commit   : 2933ed0362ae5fbc9e0c28035286318b4d299492    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 14:35:17 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 14:35:17 +0300    

They are supposed to be mutually exclusive, but there was no check for  
that.  
  
Michael Banck  
  
Discussion: <[email protected]>  

commit   : 4d3ecbfee9dbe34fe20991a5a402b1f949814928    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 12:53:40 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 12:53:40 +0300    

Leaving the error in the error queue used to be harmless, because the  
X509_STORE_load_locations() call used to be the last step in  
initialize_SSL(), and we would clear the queue before the next  
SSL_connect() call. But previous commit moved things around. The symptom  
was that if a CRL file was not found, and one of the subsequent  
initialization steps, like loading the client certificate or private key,  
failed, we would incorrectly print the "no such file" error message from  
the earlier X509_STORE_load_locations() call as the reason.  
  
Backpatch to all supported versions, like the previous patch.  

commit   : 341acf235943c15aa56103ef7b66ecec625b4af0    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 12:20:39 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Fri, 7 Oct 2016 12:20:39 +0300    

There were several issues with the old coding:  
  
1. There was a race condition, if two threads opened a connection at the  
   same time. We used a mutex around SSL_CTX_* calls, but that was not  
   enough, e.g. if one thread SSL_CTX_load_verify_locations() with one  
   path, and another thread set it with a different path, before the first  
   thread got to establish the connection.  
  
2. Opening two different connections, with different sslrootcert settings,  
   seemed to fail outright with "SSL error: block type is not 01". Not sure  
   why.  
  
3. We created the SSL object, before calling SSL_CTX_load_verify_locations  
   and SSL_CTX_use_certificate_chain_file on the SSL context. That was  
   wrong, because the options set on the SSL context are propagated to the  
   SSL object, when the SSL object is created. If they are set after the  
   SSL object has already been created, they won't take effect until the  
   next connection. (This is bug #14329)  
  
At least some of these could've been fixed while still using a shared  
context, but it would've been more complicated and error-prone. To keep  
things simple, let's just use a separate SSL context for each connection,  
and accept the overhead.  
  
Backpatch to all supported versions.  
  
Report, analysis and test case by Kacper Zuk.  
  
Discussion: <[email protected]>  

commit   : aab809664813380b3cd6c94a2791ffddc656b1f1    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Thu, 6 Oct 2016 13:24:46 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Thu, 6 Oct 2016 13:24:46 +0300    

If you point pg_rewind to a server that is using synchronous replication,  
with "pg_rewind --source-server=...", and the replication is not working  
for some reason, pg_rewind will get stuck because it creates a temporary  
table, which needs to be replicated. You could call broken replication a  
pilot error, but pg_rewind is often used in special circumstances, when  
there are changes to the replication setup.  
  
We don't do any "real" updates, and we don't care about fsyncing or  
replicating the operations on the temporary tables, so fix that by  
setting synchronous_commit off.  
  
Michael Banck, Michael Paquier. Backpatch to 9.5, where pg_rewind was  
introduced.  
  
Discussion: <[email protected]>  

commit   : bfcd07b4444d8fdf1ddaba375aa5b3aea419a898    
  
author   : Robert Haas <[email protected]>    
date     : Wed, 5 Oct 2016 13:08:48 -0400    
  
committer: Robert Haas <[email protected]>    
date     : Wed, 5 Oct 2016 13:08:48 -0400    

Loose ends from commit 2a0f89cd717ce6d49cdc47850577823682167e87.  
  
Daniel Gustafsson  

commit   : 76c0b73df6c481a29cf6e45bebeafd6d71957f08    
  
author   : Andres Freund <[email protected]>    
date     : Mon, 3 Oct 2016 22:11:36 -0700    
  
committer: Andres Freund <[email protected]>    
date     : Mon, 3 Oct 2016 22:11:36 -0700    

Before initializing iteration over a subtransaction's changes, the last  
few changes were not spilled to disk. That's correct if the transaction  
didn't spill to disk, but otherwise... This bug can lead to missed or  
misorderd subtransaction contents when they were spilled to disk.  
  
Move spilling of the remaining in-memory changes to  
ReorderBufferIterTXNInit(), where it can easily be applied to the top  
transaction and, if present, subtransactions.  
  
Since this code had too many bugs already, noticeably increase test  
coverage.  
  
Fixes: #14319  
Reported-By: Huan Ruan  
Discussion: <[email protected]>  
Backport: 9,4-, where logical decoding was added  

commit   : 993d94c59038fb80068a2b7780a1062dcaa2bb4f    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 3 Oct 2016 16:40:06 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 3 Oct 2016 16:40:06 -0400    

Commit 88e982302 invented GUC_UNIT_XSEGS for min_wal_size and max_wal_size,  
but neglected to make it display sensibly in pg_settings.unit (by adding a  
case to the switch in GetConfigOptionByNum).  Fix that, and adjust said  
switch to throw a run-time error the next time somebody forgets.  
  
In passing, avoid using a static buffer for the output string --- the rest  
of this function pstrdup's from a local buffer, and I see no very good  
reason why the units code should do it differently and less safely.  
  
Per report from Otar Shavadze.  Back-patch to 9.5 where the new unit type  
was added.  
  
Report: <CAG-jOyA=iNFhN+yB4vfvqh688B7Tr5SArbYcFUAjZi=0Exp-Lg@mail.gmail.com>  

commit   : 190765a059cc5e8226cdb18b9715741b32b12255    
  
author   : Stephen Frost <[email protected]>    
date     : Mon, 3 Oct 2016 16:23:02 -0400    
  
committer: Stephen Frost <[email protected]>    
date     : Mon, 3 Oct 2016 16:23:02 -0400    

Attempting to COPY a subset of columns from a table with RLS enabled  
would fail due to an invalid query being constructed (using a single  
ColumnRef with the list of fields to exact in 'fields', but that's for  
the different levels of an indirection for a single column, not for  
specifying multiple columns).  
  
Correct by building a ColumnRef and then RestTarget for each column  
being requested and then adding those to the targetList for the select  
query.  Include regression tests to hopefully catch if this is broken  
again in the future.  
  
Patch-By: Adam Brightwell  
Reviewed-By: Michael Paquier  

commit   : bac56dbe090670408551bd68690e91f7b0207ccf    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 3 Oct 2016 10:07:39 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 3 Oct 2016 10:07:39 -0400    

pg_upgrade checks whether all the shared libraries used in the old cluster  
are also available in the new one by issuing LOAD for each library name.  
Previously, it cared not what order it did the LOADs in.  Ideally it  
should not have to care, but currently the transform modules in contrib  
fail unless both the language and datatype modules they depend on are  
loaded first.  A backend-side solution for that looks possible but  
probably not back-patchable, so as a stopgap measure, let's do the LOAD  
tests in order by library name length.  That should fix the problem for  
reasonably-named transform modules, eg "hstore_plpython" will be loaded  
after both "hstore" and "plpython".  (Yeah, it's a hack.)  
  
In a larger sense, having a predictable order of these probes is a good  
thing, since it will make upgrades predictably work or not work in the  
face of inter-library dependencies.  Also, this patch replaces O(N^2)  
de-duplication logic with O(N log N) logic, which could matter in  
installations with very many databases.  So I don't foresee reverting this  
even after we have a proper fix for the library-dependency problem.  
  
In passing, improve a couple of SQL queries used here.  
  
Per complaint from Andrew Dunstan that pg_upgrade'ing the transform contrib  
modules failed.  Back-patch to 9.5 where transform modules were introduced.  
  
Discussion: <[email protected]>  

commit   : f40334b85cf89f1eafd04d31665704fd173e317c    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 2 Oct 2016 14:31:28 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 2 Oct 2016 14:31:28 -0400    

Without this, an extension containing an access method is not properly  
dumped/restored during pg_upgrade --- the AM ends up not being a member  
of the extension after upgrading.  
  
Another oversight in commit 473b93287, reported by Andrew Dunstan.  
  
Report: <[email protected]>  

commit   : 4677fe9fb0aa534b8611c29075fb57b3573612eb    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 17:15:10 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 17:15:10 -0400    

In standard Unix builds, postmaster child processes do ClosePostmasterPorts  
immediately after InitPostmasterChild, that is almost immediately after  
being spawned.  This is important because we don't want children holding  
open the postmaster's end of the postmaster death watch pipe.  
  
However, in EXEC_BACKEND builds, SubPostmasterMain was postponing this  
responsibility significantly, in order to make it slightly more convenient  
to pass the right flag value to ClosePostmasterPorts.  This is bad,  
particularly seeing that process_shared_preload_libraries() might invoke  
nearly-arbitrary code.  Rearrange so that we do it as soon as we've  
fetched the socket FDs via read_backend_variables().  
  
Also move the comment explaining about randomize_va_space to before the  
call of PGSharedMemoryReAttach, which is where it's relevant.  The old  
placement was appropriate when the reattach happened inside  
CreateSharedMemoryAndSemaphores, but that was a long time ago.  
  
Back-patch to 9.3; the patch doesn't apply cleanly before that, and  
it doesn't seem worth a lot of effort given that we've had no actual  
field complaints traceable to this.  
  
Discussion: <[email protected]>  

commit   : f4e787c826446102eb38ffb5f398bea43b44cacf    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 16:32:55 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 16:32:55 -0400    

collect_corrupt_items() failed to initialize tuple.t_self.  While  
HeapTupleSatisfiesVacuum() doesn't actually use that value, it does  
Assert that it's valid, so that the code would dump core if ip_posid  
chanced to be zero.  (That's somewhat unlikely, which probably explains  
how this got missed.  In any case it wouldn't matter for field use.)  
  
Also, collect_corrupt_items was returning the wrong TIDs, that is the  
contents of t_ctid rather than the tuple's own location.  This would  
be the same thing in simple cases, but it could be wrong if, for  
example, a past update attempt had been rolled back, leaving a live  
tuple whose t_ctid doesn't point at itself.  
  
Also, in pg_visibility(), guard against trying to read a page past  
the end of the rel.  The VM code handles inquiries beyond the end  
of the map by silently returning zeroes, and it seems like we should  
do the same thing here.  
  
I ran into the assertion failure while using pg_visibility to check  
pg_upgrade's behavior, and then noted the other problems while  
reading the code.  
  
Report: <[email protected]>  

commit   : b01f9ed25950f26f85c44e31b2a97d6f1ba3534e    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 15:32:53 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 15:32:53 -0400    

Add omitted names for some function parameters.  
Fix some minor grammatical issues.  

commit   : 581f431f7be3c335f11dfd29a2498ad96d8a08ce    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 13:45:16 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 13:45:16 -0400    

There is no need for "all: all-lib" to be placed before inclusion of  
Makefile.shlib.  Makefile.global is what ensures that "all" is the  
default target, and we already document that that has to be included  
first.  Per comment from Pavel Raiskup.  
  
Discussion: <[email protected]>  

commit   : 68fb75e103e78478ef45a71e1a25055751e789f2    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 13:35:13 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 1 Oct 2016 13:35:13 -0400    

The sequence "configure; cd src/pl/plpython; make -j" failed due to  
trying to compile plpython's .o files before the generated headers  
finished building.  (This is an important real-world case, since it's  
the typical second step when building both plpython2 and plpython3.)  
This happens because the submake-generated-headers target is not  
placed in a way to make it a prerequisite to compiling the .o files.  
Fix that.  
  
Checking other uses of submake-generated-headers, I noted that the one  
attached to pg_regress was similarly misplaced; but it's actually not  
needed at all for pg_regress.o, rather regress.o, so move it to be a  
prerequisite of that.  
  
Back-patch to 9.6 where submake-generated-headers was introduced  
(by commit 548af97fc).  It's not immediately clear to me why the  
previous coding didn't have the same issue; but since we've not  
had field reports of plpython make failing, leave it alone in the  
older branches.  
  
Pavel Raiskup and Tom Lane  
  
Discussion: <[email protected]>  

commit   : 53fbeed40702e49c02a85a31f52a1683abe57a4e    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 30 Sep 2016 20:40:27 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 30 Sep 2016 20:40:27 -0400    

The previous design for this had copyFile(), linkFile(), and  
rewriteVisibilityMap() returning strerror strings, with the caller  
producing one-size-fits-all error messages based on that.  This made it  
impossible to produce messages that described the failures with any degree  
of precision, especially not short-read problems since those don't set  
errno at all.  
  
Since pg_upgrade has no intention of continuing after any error in this  
area, let's fix this by just letting these functions call pg_fatal() for  
themselves, making it easy for each point of failure to have a suitable  
error message.  Taking this approach also allows dropping cleanup code  
that was unnecessary and was often rather sloppy about preserving errno.  
To not lose relevant info that was reported before, pass in the schema name  
and table name of the current table so that they can be included in the  
error reports.  
  
An additional problem was the use of getErrorText(), which was flat out  
wrong for all but a couple of call sites, because it unconditionally did  
"_dosmaperr(GetLastError())" on Windows.  That's only appropriate when  
reporting an error from a Windows-native API, which only a couple of  
the callers were actually doing.  Thus, even the reported strerror string  
would be unrelated to the actual failure in many cases on Windows.  
To fix, get rid of getErrorText() altogether, and just have call sites  
do strerror(errno) instead, since that's the way all the rest of our  
frontend programs do it.  Add back the _dosmaperr() calls in the two  
places where that's actually appropriate.  
  
In passing, make assorted messages hew more closely to project style  
guidelines, notably by removing initial capitals in not-complete-sentence  
primary error messages.  (I didn't make any effort to clean up places  
I didn't have another reason to touch, though.)  
  
Per discussion of a report from Thomas Kellerer.  Back-patch to 9.6,  
but no further; given the relative infrequency of reports of problems  
here, it's not clear it's worth adapting the patch to older branches.  
  
Patch by me, but with credit to Alvaro Herrera for spotting the issue  
with getErrorText's misuse of _dosmaperr().  
  
Discussion: <[email protected]>  

commit   : b6d90607364c4ad9315e28f76c836de4751beca5    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 30 Sep 2016 20:39:06 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 30 Sep 2016 20:39:06 -0400    

This is new code in 9.6, and evidently we missed out testing it as  
thoroughly as it should have been.  Bugs fixed here:  
  
1. Use binary not text mode to open the files on Windows.  Before, if  
the visibility map chanced to contain two bytes that looked like \r\n,  
Windows' read() would convert that to \n, which both corrupts the map  
data and causes the file to look shorter than it should.  Unless you  
were *very* unlucky and had an exact multiple of 8K such occurrences  
in each VM file, this would cause pg_upgrade to report a failure,  
though with a rather obscure error message.  
  
2. The code for copying rebuilt bytes into the output was simply wrong.  
It chanced to work okay on little-endian machines but would emit the  
bytes in the wrong order on big-endian, leading to silent corruption  
of the visibility map data.  
  
3. The code was careless about alignment of the working buffers.  Given  
all three of an alignment-picky architecture, a compiler that chooses  
to put the new_vmbuf[] local variable at an odd starting address, and  
a checksum-enabled database, pg_upgrade would dump core.  
  
Point one was reported by Thomas Kellerer, the other two detected by  
code-reading.  
  
Point two is much the nastiest of these issues from an impact standpoint,  
though fortunately it affects only a minority of users.  The Windows issue  
will definitely bite people, but it seems quite unlikely that there would  
be undetected corruption from that.  
  
In addition, I failed to resist the temptation to do some minor cosmetic  
adjustments, mostly improving the comments.  
  
It would be a good idea to try to improve the error reporting here, but  
that seems like material for a separate patch.  
  
Discussion: <[email protected]>  

commit   : 41d58e97afe4c24398a5469f79468a50895b3ab0    
  
author   : Magnus Hagander <[email protected]>    
date     : Fri, 30 Sep 2016 11:19:30 +0200    
  
committer: Magnus Hagander <[email protected]>    
date     : Fri, 30 Sep 2016 11:19:30 +0200    

There is a small window between when the server closes out the existing  
segment and the new one is created. Put a loop around the open call in  
this case to make sure we wait for the new file to actually appear.  

commit   : 7341c2830cf3d06a96d97f0e6a2e85bf808faae4    
  
author   : Alvaro Herrera <[email protected]>    
date     : Wed, 28 Sep 2016 19:31:58 -0300    
  
committer: Alvaro Herrera <[email protected]>    
date     : Wed, 28 Sep 2016 19:31:58 -0300    

Reported by Peter Eisentraut.  Coding suggested by Tom Lane.  

commit   : f0476e226f1bc3aaaa4185ace3a09bc586b42cde    
  
author   : Robert Haas <[email protected]>    
date     : Wed, 28 Sep 2016 12:38:33 -0400    
  
committer: Robert Haas <[email protected]>    
date     : Wed, 28 Sep 2016 12:38:33 -0400    

Without this, statistics changes accumulated by the worker never get  
reported to the stats collector, which is bad.  
  
Julien Rouhaud  

commit   : 32841fa325de298e0bad771c588354fd59866e42    
  
author   : Robert Haas <[email protected]>    
date     : Wed, 28 Sep 2016 11:19:46 -0400    
  
committer: Robert Haas <[email protected]>    
date     : Wed, 28 Sep 2016 11:19:46 -0400    

Commit 3fe3511d05127cc024b221040db2eeb352e7d716 introduced a new  
case into this function, but neglected to ensure that the "ondisk"  
pointer got updated after a possible reallocation as the code does  
in other cases.  
  
Stas Kelvich, per diagnosis by Konstantin Knizhnik.  

commit   : f0631bda9bf729161490da76414648dafcc0adbe    
  
author   : Alvaro Herrera <[email protected]>    
date     : Tue, 27 Sep 2016 01:05:21 -0300    
  
committer: Alvaro Herrera <[email protected]>    
date     : Tue, 27 Sep 2016 01:05:21 -0300    

<sys/select.h> is required by POSIX.1-2001 to get the prototype of  
select(2), but nearly no systems enforce that because older standards  
let you get away with including some other headers.  Recent OpenBSD  
hacking has removed that frail touch of friendliness, however, which  
broke some compiles; fix all the way back to 9.1 by adding the required  
standard.  Only vacuumdb.c was reported to fail, but it seems easier to  
fix the whole lot in a fell swoop.  
  
Per bug #14334 by Sean Farrell.