PostgreSQL 9.3.23 commit log

Stamp 9.3.23.

commit   : ec8e7d60bac23b1b9102798afc30078aca9076ae    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 7 May 2018 16:59:47 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 7 May 2018 16:59:47 -0400    

Click here for diff

M configure
M configure.in
M doc/bug.template
M src/include/pg_config.h.win32
M src/interfaces/libpq/libpq.rc.in
M src/port/win32ver.rc

Last-minute updates for release notes.

commit   : 035524b11821343bf1236e8ea5b078b9508225df    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 7 May 2018 13:13:27 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 7 May 2018 13:13:27 -0400    

Click here for diff

The set of functions that need parallel-safety adjustments isn't the  
same in 9.6 as 10, so I shouldn't have blindly back-patched that list.  
Adjust as needed.  Also, provide examples of the commands to issue.  

M doc/src/sgml/release-9.3.sgml

Translation updates

commit   : d58aaf7bb5f6d15321471e38e7d25802726aff60    
  
author   : Peter Eisentraut <[email protected]>    
date     : Mon, 7 May 2018 11:46:06 -0400    
  
committer: Peter Eisentraut <[email protected]>    
date     : Mon, 7 May 2018 11:46:06 -0400    

Click here for diff

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

M src/backend/po/fr.po
M src/backend/po/ru.po
M src/bin/pg_basebackup/po/ru.po
M src/bin/pg_dump/po/de.po
M src/bin/pg_dump/po/ru.po
M src/bin/scripts/po/de.po
M src/bin/scripts/po/ru.po
M src/interfaces/ecpg/preproc/po/ru.po
M src/interfaces/libpq/po/ru.po
M src/pl/plpgsql/src/po/ru.po
M src/pl/plpython/po/ru.po

Release notes for 10.4, 9.6.9, 9.5.13, 9.4.18, 9.3.23.

commit   : cfdd753bd12676e9996f8482a14fb4ff0e7a64b8    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 6 May 2018 15:30:45 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 6 May 2018 15:30:45 -0400    

Click here for diff

M doc/src/sgml/release-9.3.sgml

Clear severity 5 perlcritic warnings from vcregress.pl

commit   : a75b01c613104b49e1e54694c6595eb4dc754e5d    
  
author   : Andrew Dunstan <[email protected]>    
date     : Sun, 6 May 2018 07:37:05 -0400    
  
committer: Andrew Dunstan <[email protected]>    
date     : Sun, 6 May 2018 07:37:05 -0400    

Click here for diff

My recent update for python3 support used some idioms that are  
unapproved. This fixes them. Backpatch to all live branches like the  
original.  

M src/tools/msvc/vcregress.pl

Tweak tests to support Python 3.7

commit   : e7f90471590025ccb522562bec8513238805395b    
  
author   : Peter Eisentraut <[email protected]>    
date     : Tue, 13 Feb 2018 16:13:20 -0500    
  
committer: Peter Eisentraut <[email protected]>    
date     : Tue, 13 Feb 2018 16:13:20 -0500    

Click here for diff

Python 3.7 removes the trailing comma in the repr() of  
BaseException (see <https://bugs.python.org/issue30399>), leading to  
test output differences.  Work around that by composing the equivalent  
test output in a more manual way.  

M src/pl/plpython/expected/plpython_subtransaction.out
M src/pl/plpython/expected/plpython_subtransaction_0.out
M src/pl/plpython/expected/plpython_subtransaction_5.out
M src/pl/plpython/sql/plpython_subtransaction.sql

Remove extra newlines after PQerrorMessage()

commit   : 4f90e0ffe7a5c9c8b22a22e1caf05b6a3cbdbae6    
  
author   : Peter Eisentraut <[email protected]>    
date     : Sat, 5 May 2018 10:51:38 -0400    
  
committer: Peter Eisentraut <[email protected]>    
date     : Sat, 5 May 2018 10:51:38 -0400    

Click here for diff

M src/bin/pg_dump/pg_dumpall.c

Provide for testing on python3 modules when under MSVC

commit   : af39c1da7de3cad28fa2e5e08f076bff9b2d6aa7    
  
author   : Andrew Dunstan <[email protected]>    
date     : Fri, 4 May 2018 15:22:48 -0400    
  
committer: Andrew Dunstan <[email protected]>    
date     : Fri, 4 May 2018 15:22:48 -0400    

Click here for diff

This should have been done some years ago as promised in commit  
c4dcdd0c2. However, better late than never.  
  
Along the way do a little housekeeping, including using a simpler test  
for the python version being tested, and removing a redundant subroutine  
parameter. These changes only apply back to release 9.5.  
  
Backpatch to all live releases.  

M src/tools/msvc/vcregress.pl

Allow MSYS as well as MINGW in Msys uname

commit   : 594ff3d7d78c7acf73ae3ea47795dc3e07550227    
  
author   : Andrew Dunstan <[email protected]>    
date     : Fri, 4 May 2018 14:54:04 -0400    
  
committer: Andrew Dunstan <[email protected]>    
date     : Fri, 4 May 2018 14:54:04 -0400    

Click here for diff

Msys2's uname -s outputs a string beginning MSYS rather than MINGW as is  
output by Msys. Allow either in pg_upgrade's test.sh.  
  
Backpatch to all live branches.  

M contrib/pg_upgrade/test.sh

Sync our copy of the timezone library with IANA release tzcode2018e.

commit   : 9469ebc71bab7e3beca4390d395e0ee66d462cfb    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 4 May 2018 12:26:25 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 4 May 2018 12:26:25 -0400    

Click here for diff

The non-cosmetic changes involve teaching the "zic" tzdata compiler about  
negative DST.  While I'm not currently intending that we start using  
negative-DST data right away, it seems possible that somebody would try  
to use our copy of zic with bleeding-edge IANA data.  So we'd better be  
out in front of this change code-wise, even though it doesn't matter for  
the data file we're shipping.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/timezone/README
M src/timezone/localtime.c
M src/timezone/strftime.c
M src/timezone/zic.c

Add HOLD_INTERRUPTS section into FinishPreparedTransaction.

commit   : 540e7a6e535ebdb12d83e6b639916f9df4420ae7    
  
author   : Teodor Sigaev <[email protected]>    
date     : Thu, 3 May 2018 20:10:34 +0300    
  
committer: Teodor Sigaev <[email protected]>    
date     : Thu, 3 May 2018 20:10:34 +0300    

Click here for diff

If an interrupt arrives in the middle of FinishPreparedTransaction  
and any callback decide to call CHECK_FOR_INTERRUPTS (e.g.  
RemoveTwoPhaseFile can write a warning with ereport, which checks for  
interrupts) then it's possible to leave current GXact undeleted.  
  
Backpatch to all supported branches  
  
Stas Kelvich  
  
Discussion: ihttps://www.postgresql.org/message-id/[email protected]  

M src/backend/access/transam/twophase.c

Revert back-branch changes in power()'s behavior for NaN inputs.

commit   : 99095b6320949a0c065bfa381b2d8006ca13e1b5    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 2 May 2018 17:32:41 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 2 May 2018 17:32:41 -0400    

Click here for diff

Per discussion, the value of fixing these bugs in the back branches  
doesn't outweigh the downsides of changing corner-case behavior in  
a minor release.  Hence, revert commits 217d8f3a1 and 4d864de48 in  
the v10 branch and the corresponding commits in 9.3-9.6.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/utils/adt/float.c
M src/test/regress/expected/float8-exp-three-digits-win32.out
M src/test/regress/expected/float8-small-is-zero.out
M src/test/regress/expected/float8-small-is-zero_1.out
M src/test/regress/expected/float8.out
M src/test/regress/sql/float8.sql

Fix bogus list-iteration code in pg_regress.c, affecting ecpg tests only.

commit   : 3f91c4005f3dd55b8653ebc220861d5497a8d4d8    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 21:56:28 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 21:56:28 -0400    

Click here for diff

While looking at a recent buildfarm failure in the ecpg tests, I wondered  
why the pg_regress output claimed the stderr part of the test failed, when  
the regression diffs were clearly for the stdout part.  Looking into it,  
the reason is that pg_regress.c's logic for iterating over three parallel  
lists is wrong, and has been wrong since it was written: it advances the  
"tag" pointer at a different place in the loop than the other two pointers.  
Fix that.  

M src/test/regress/pg_regress.c

Avoid wrong results for power() with NaN input on more platforms.

commit   : d9c7f56da464e5d499a2dcd0a601f2ee68fee06b    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 18:15:16 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 18:15:16 -0400    

Click here for diff

Buildfarm results show that the modern POSIX rule that 1 ^ NaN = 1 is not  
honored on *BSD until relatively recently, and really old platforms don't  
believe that NaN ^ 0 = 1 either.  (This is unsurprising, perhaps, since  
SUSv2 doesn't require either behavior.)  In hopes of getting to platform  
independent behavior, let's deal with all the NaN-input cases explicitly  
in dpow().  
  
Note that numeric_power() doesn't know either of these special cases.  
But since that behavior is platform-independent, I think it should be  
addressed separately, and probably not back-patched.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/utils/adt/float.c
M src/test/regress/expected/float8-exp-three-digits-win32.out
M src/test/regress/expected/float8-small-is-zero.out
M src/test/regress/expected/float8-small-is-zero_1.out
M src/test/regress/expected/float8.out
M src/test/regress/sql/float8.sql

Update time zone data files to tzdata release 2018d.

commit   : adcd0c2beebf8c7ce9fbf598d34e6b91d5750122    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 15:50:08 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 15:50:08 -0400    

Click here for diff

DST law changes in Palestine and Antarctica (Casey Station).  Historical  
corrections for Portugal and its colonies, as well as Enderbury, Jamaica,  
Turks & Caicos Islands, and Uruguay.  

M src/timezone/data/tzdata.zi

Avoid wrong results for power() with NaN input on some platforms.

commit   : 534267e7f3ea03f67e22950f689ad71aaa1560ce    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 15:21:45 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 29 Apr 2018 15:21:45 -0400    

Click here for diff

Per spec, the result of power() should be NaN if either input is NaN.  
It appears that on some versions of Windows, the libc function does  
return NaN, but it also sets errno = EDOM, confusing our code that  
attempts to work around shortcomings of other platforms.  Hence, add  
guard tests to avoid substituting a wrong result for the right one.  
  
It's been like this for a long time (and the odd behavior only appears  
in older MSVC releases, too) so back-patch to all supported branches.  
  
Dang Minh Huong, reviewed by David Rowley  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/utils/adt/float.c
M src/test/regress/expected/float8-exp-three-digits-win32.out
M src/test/regress/expected/float8-small-is-zero.out
M src/test/regress/expected/float8-small-is-zero_1.out
M src/test/regress/expected/float8.out
M src/test/regress/sql/float8.sql

docs: remove "III" version text from pgAdmin link

commit   : 1c66ee81febec2f0aa7fd0c3f2d5f70995542d30    
  
author   : Bruce Momjian <[email protected]>    
date     : Thu, 26 Apr 2018 11:10:43 -0400    
  
committer: Bruce Momjian <[email protected]>    
date     : Thu, 26 Apr 2018 11:10:43 -0400    

Click here for diff

Reported-by: [email protected]  
  
Discussion: https://postgr.es/m/[email protected]  
  
Backpatch-through: 9.3  

M doc/src/sgml/external-projects.sgml

Change more places to be less trusting of RestrictInfo.is_pushed_down.

commit   : 9680c120e5b20898f563ccf18c39713d9b754292    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 20 Apr 2018 15:19:17 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 20 Apr 2018 15:19:17 -0400    

Click here for diff

On further reflection, commit e5d83995e didn't go far enough: pretty much  
everywhere in the planner that examines a clause's is_pushed_down flag  
ought to be changed to use the more complicated behavior where we also  
check the clause's required_relids.  Otherwise we could make incorrect  
decisions about whether, say, a clause is safe to use as a hash clause.  
  
Some (many?) of these places are safe as-is, either because they are  
never reached while considering a parameterized path, or because there  
are additional checks that would reject a pushed-down clause anyway.  
However, it seems smarter to just code them all the same way rather  
than rely on easily-broken reasoning of that sort.  
  
In support of that, invent a new macro RINFO_IS_PUSHED_DOWN that should  
be used in place of direct tests on the is_pushed_down flag.  
  
Like the previous patch, back-patch to all supported branches.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/optimizer/path/costsize.c
M src/backend/optimizer/path/joinpath.c
M src/backend/optimizer/path/joinrels.c
M src/backend/optimizer/plan/analyzejoins.c
M src/backend/optimizer/plan/initsplan.c
M src/backend/optimizer/util/restrictinfo.c
M src/include/nodes/relation.h

Fix incorrect handling of join clauses pushed into parameterized paths.

commit   : e1d4398c0a5bdb4a3ccbbb667a5d8c583288ac55    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 19 Apr 2018 15:49:12 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 19 Apr 2018 15:49:12 -0400    

Click here for diff

In some cases a clause attached to an outer join can be pushed down into  
the outer join's RHS even though the clause is not degenerate --- this  
can happen if we choose to make a parameterized path for the RHS.  If  
the clause ends up attached to a lower outer join, we'd misclassify it  
as being a "join filter" not a plain "filter" condition at that node,  
leading to wrong query results.  
  
To fix, teach extract_actual_join_clauses to examine each join clause's  
required_relids, not just its is_pushed_down flag.  (The latter now  
seems vestigial, or at least in need of rethinking, but we won't do  
anything so invasive as redefining it in a bug-fix patch.)  
  
This has been wrong since we introduced parameterized paths in 9.2,  
though it's evidently hard to hit given the lack of previous reports.  
The test case used here involves a lateral function call, and I think  
that a lateral reference may be required to get the planner to select  
a broken plan; though I wouldn't swear to that.  In any case, even if  
LATERAL is needed to trigger the bug, it still affects all supported  
branches, so back-patch to all.  
  
Per report from Andreas Karlsson.  Thanks to Andrew Gierth for  
preliminary investigation.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/optimizer/plan/createplan.c
M src/backend/optimizer/util/restrictinfo.c
M src/include/optimizer/restrictinfo.h
M src/test/regress/expected/join.out
M src/test/regress/sql/join.sql

Enlarge find_other_exec's meager fgets buffer

commit   : 070179a4a2f8ebf8aefc90603237a3e794d3a9cf    
  
author   : Alvaro Herrera <[email protected]>    
date     : Thu, 19 Apr 2018 10:45:15 -0300    
  
committer: Alvaro Herrera <[email protected]>    
date     : Thu, 19 Apr 2018 10:45:15 -0300    

Click here for diff

The buffer was 100 bytes long, which is barely sufficient when the  
version string gets longer (such as by configure --with-extra-version).  
Set it to MAXPGPATH.  
  
Author: Nikhil Sontakke  
Discussion: https://postgr.es/m/CAMGcDxfLfpYU_Jru++L6ARPCOyxr0W+2O3Q54TDi5XdYeU36ow@mail.gmail.com  

M src/port/exec.c

Fix broken collation-aware searches in SP-GiST text opclass.

commit   : cf73a5b34a907ae5dbc01b94c41cc976a3eb8b84    
  
author   : Tom Lane <[email protected]>    
date     : Mon, 16 Apr 2018 16:06:47 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Mon, 16 Apr 2018 16:06:47 -0400    

Click here for diff

spg_text_leaf_consistent() supposed that it should compare only  
Min(querylen, entrylen) bytes of the two strings, and then deal with  
any excess bytes in one string or the other by assuming the longer  
string is greater if the prefixes are equal.  Quite aside from the  
fact that that's just wrong in some locales (e.g., 'ch' is not less  
than 'd' in cs_CZ), it also risked passing incomplete multibyte  
characters to strcoll(), with ensuing bad results.  
  
Instead, just pass the full strings to varstr_cmp, and let it decide  
what to do about unequal-length strings.  
  
Fortunately, this error doesn't imply any index corruption, it's just  
that searches might return the wrong set of entries.  
  
Per report from Emre Hasegeli, though this is not his patch.  
Thanks to Peter Geoghegan for review and discussion.  
  
This code was born broken, so back-patch to all supported branches.  
In HEAD, I failed to resist the temptation to do a bit of cosmetic  
cleanup/pgindent'ing on 710d90da1, too.  
  
Discussion: https://postgr.es/m/CAE2gYzzb6K51VnTq5i5p52z+j9p2duEa-K1T3RrC_GQEynAKEg@mail.gmail.com  

M src/backend/access/spgist/spgtextproc.c

Fix potentially-unportable code in contrib/adminpack.

commit   : 9ed1c8ca2594e8ab523181bd011402a031bdcaaa    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 15 Apr 2018 13:02:12 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 15 Apr 2018 13:02:12 -0400    

Click here for diff

Spelling access(2)'s second argument as "2" is just horrid.  
POSIX makes no promises as to the numeric values of W_OK and related  
macros.  Even if it accidentally works as intended on every supported  
platform, it's still unreadable and inconsistent with adjacent code.  
  
In passing, don't spell "NULL" as "0" either.  Yes, that's legal C;  
no, it's not project style.  
  
Back-patch, just in case the unportability is real and not theoretical.  
(Most likely, even if a platform had different bit assignments for  
access()'s modes, there'd not be an observable behavior difference  
here; but I'm being paranoid today.)  

M contrib/adminpack/adminpack.c

In libpq, free any partial query result before collecting a server error.

commit   : bbec33c2d0263e367f7ba7d7add768d16861d8a7    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 13 Apr 2018 12:53:46 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 13 Apr 2018 12:53:46 -0400    

Click here for diff

We'd throw away the partial result anyway after parsing the error message.  
Throwing it away beforehand costs nothing and reduces the risk of  
out-of-memory failure.  Also, at least in systems that behave like  
glibc/Linux, if the partial result was very large then the error PGresult  
would get allocated at high heap addresses, preventing the heap storage  
used by the partial result from being released to the OS until the error  
PGresult is freed.  
  
In psql >= 9.6, we hold onto the error PGresult until another error is  
received (for \errverbose), so that this behavior causes a seeming  
memory leak to persist for awhile, as in a recent complaint from  
Darafei Praliaskouski.  This is a potential performance regression from  
older versions, justifying back-patching at least that far.  But similar  
behavior may occur in other client applications, so it seems worth just  
back-patching to all supported branches.  
  
Discussion: https://postgr.es/m/CAC8Q8tJ=7cOkPePyAbJE_Pf691t8nDFhJp0KZxHvnq_uicfyVg@mail.gmail.com  

M src/interfaces/libpq/fe-protocol2.c
M src/interfaces/libpq/fe-protocol3.c

Fix bogus affix-merging code.

commit   : ac8ea0f27f3f1c3f537b8a4d136fa7f76bc520dc    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 12 Apr 2018 18:39:52 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 12 Apr 2018 18:39:52 -0400    

Click here for diff

NISortAffixes() compared successive compound affixes incorrectly,  
thus possibly failing to merge identical affixes, or (less likely)  
merging ones that shouldn't be merged.  The user-visible effects  
of this are unclear, to me anyway.  
  
Per bug #15150 from Alexander Lakhin.  It's been broken for a long time,  
so back-patch to all supported branches.  
  
Arthur Zakirov  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/tsearch/spell.c

Ignore nextOid when replaying an ONLINE checkpoint.

commit   : 66d4b6bb8065368f313507e94a0003ae48d96405    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 11 Apr 2018 18:11:30 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 11 Apr 2018 18:11:30 -0400    

Click here for diff

The nextOid value is from the start of the checkpoint and may well be stale  
compared to values from more recent XLOG_NEXTOID records.  Previously, we  
adopted it anyway, allowing the OID counter to go backwards during a crash.  
While this should be harmless, it contributed to the severity of the bug  
fixed in commit 0408e1ed5, by allowing duplicate TOAST OIDs to be assigned  
immediately following a crash.  Without this error, that issue would only  
have arisen when TOAST objects just younger than a multiple of 2^32 OIDs  
were deleted and then not vacuumed in time to avoid a conflict.  
  
Pavan Deolasee  
  
Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com  

M src/backend/access/transam/xlog.c

Do not select new object OIDs that match recently-dead entries.

commit   : 7448e7e237996d51e575e97160d3b07daf4d7b89    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 11 Apr 2018 17:41:10 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 11 Apr 2018 17:41:10 -0400    

Click here for diff

When selecting a new OID, we take care to avoid picking one that's already  
in use in the target table, so as not to create duplicates after the OID  
counter has wrapped around.  However, up to now we used SnapshotDirty when  
scanning for pre-existing entries.  That ignores committed-dead rows, so  
that we could select an OID matching a deleted-but-not-yet-vacuumed row.  
While that mostly worked, it has two problems:  
  
* If recently deleted, the dead row might still be visible to MVCC  
snapshots, creating a risk for duplicate OIDs when examining the catalogs  
within our own transaction.  Such duplication couldn't be visible outside  
the object-creating transaction, though, and we've heard few if any field  
reports corresponding to such a symptom.  
  
* When selecting a TOAST OID, deleted toast rows definitely *are* visible  
to SnapshotToast, and will remain so until vacuumed away.  This leads to  
a conflict that will manifest in errors like "unexpected chunk number 0  
(expected 1) for toast value nnnnn".  We've been seeing reports of such  
errors from the field for years, but the cause was unclear before.  
  
The fix is simple: just use SnapshotAny to search for conflicting rows.  
This results in a slightly longer window before object OIDs can be  
recycled, but that seems unlikely to create any large problems.  
  
Pavan Deolasee  
  
Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com  

M src/backend/access/heap/tuptoaster.c
M src/backend/catalog/catalog.c

Make local copy of client hostnames in backend status array.

commit   : dfc383cf3975ccb1053ef7431d1dfdcff1dfefea    
  
author   : Heikki Linnakangas <[email protected]>    
date     : Wed, 11 Apr 2018 23:39:48 +0300    
  
committer: Heikki Linnakangas <[email protected]>    
date     : Wed, 11 Apr 2018 23:39:48 +0300    

Click here for diff

The other strings, application_name and query string, were snapshotted to  
local memory in pgstat_read_current_status(), but we forgot to do that for  
client hostnames. As a result, the client hostname would appear to change in  
the local copy, if the client disconnected.  
  
Backpatch to all supported versions.  
  
Author: Edmund Horner  
Reviewed-by: Michael Paquier  
Discussion: https://www.postgresql.org/message-id/CAMyN-kA7aOJzBmrYFdXcc7Z0NmW%2B5jBaf_m%3D_-77uRNyKC9r%3DA%40mail.gmail.com  

M src/backend/postmaster/pgstat.c

Doc: clarify explanation of pg_dump usage.

commit   : bd213fd66a9250f801608c104a262bd3d71154a2    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 8 Apr 2018 16:35:43 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 8 Apr 2018 16:35:43 -0400    

Click here for diff

This section confusingly used both "infile" and "outfile" to refer  
to the same file, i.e. the textual output of pg_dump.  Use "dumpfile"  
for both cases, per suggestion from Jonathan Katz.  
  
Discussion: https://postgr.es/m/[email protected]  

M doc/src/sgml/backup.sgml

doc: remove mention of the DMOZ catalog in ltree docs

commit   : d2c9737bc8bbde3f6d19333f9db0727a4e93f44d    
  
author   : Bruce Momjian <[email protected]>    
date     : Thu, 5 Apr 2018 15:55:41 -0400    
  
committer: Bruce Momjian <[email protected]>    
date     : Thu, 5 Apr 2018 15:55:41 -0400    

Click here for diff

Discussion: https://postgr.es/m/CAF4Au4xYem_W3KOuxcKct7=G4j8Z3uO9j3DUKTFJqUsfp_9pQg@mail.gmail.com  
  
Author: Oleg Bartunov  
  
Backpatch-through: 9.3  

M doc/src/sgml/ltree.sgml

docs: update ltree URL for the DMOZ catalog

commit   : 4b9b3f5c437ecd544a2b8eac2835546039e2aa38    
  
author   : Bruce Momjian <[email protected]>    
date     : Wed, 4 Apr 2018 15:06:21 -0400    
  
committer: Bruce Momjian <[email protected]>    
date     : Wed, 4 Apr 2018 15:06:21 -0400    

Click here for diff

Reported-by: [email protected]  
  
Discussion: https://postgr.es/m/[email protected]  
  
Author: Oleg Bartunov  
  
Backpatch-through: 9.3  

M doc/src/sgml/ltree.sgml

doc: document "IS NOT DOCUMENT"

commit   : 6e9c8d5a2fedff5f33d0c820da69a65769f4ee39    
  
author   : Bruce Momjian <[email protected]>    
date     : Mon, 2 Apr 2018 16:41:46 -0400    
  
committer: Bruce Momjian <[email protected]>    
date     : Mon, 2 Apr 2018 16:41:46 -0400    

Click here for diff

Reported-by: [email protected]  
  
Discussion: https://postgr.es/m/[email protected]  
  
Author: Euler Taveira  
  
Backpatch-through: 9.3  

M doc/src/sgml/func.sgml

Fix bogus provolatile/proparallel markings on a few built-in functions.

commit   : 485857d447c96ac93d255c8c54a5b373a90af4e1    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 30 Mar 2018 18:14:51 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 30 Mar 2018 18:14:51 -0400    

Click here for diff

Richard Yen reported that pg_upgrade failed if the target cluster had  
force_parallel_mode = on, because binary_upgrade_create_empty_extension()  
is marked parallel restricted, allowing it to be executed in parallel  
mode, which complains because it tries to acquire an XID.  
  
In general, no function that might try to modify database data should  
be considered parallel safe or restricted, since execution of it might  
force XID acquisition.  We found several other examples of this mistake.  
  
Furthermore, functions that execute user-supplied SQL queries or query  
fragments, or pull data from user-supplied cursors, had better be marked  
both volatile and parallel unsafe, because we don't know what the supplied  
query or cursor might try to do.  There were several tsquery and XML  
functions that had the wrong proparallel marking for this, and some of  
them were even mislabeled as to volatility.  
  
All these bugs are old, dating back to 9.6 for the proparallel mistakes  
and much further for the provolatile mistakes.  We can't force a  
catversion bump in the back branches, but we can at least ensure that  
installations initdb'd in future have the right values.  
  
Thomas Munro and Tom Lane  
  
Discussion: https://postgr.es/m/CAEepm=2sNDScSLTfyMYu32Q=ob98ZGW-vM_2oLxinzSABGQ6VA@mail.gmail.com  

M src/include/catalog/pg_proc.h

docs: add parameter with brackets around varbit()

commit   : 10fbb0eabb4e39030c65d8f7bba7c15bc492eeea    
  
author   : Bruce Momjian <[email protected]>    
date     : Fri, 30 Mar 2018 13:34:12 -0400    
  
committer: Bruce Momjian <[email protected]>    
date     : Fri, 30 Mar 2018 13:34:12 -0400    

Click here for diff

Reported-by: [email protected]  
  
Discussion: https://postgr.es/m/[email protected]  
  
Author: Euler Taveira  
  
Backpatch-through: 9.3  

M doc/src/sgml/datatype.sgml

Doc: add example of type resolution in nested UNIONs.

commit   : b666c55b2caee0e0c1f9afc404dd00f79c98bc3d    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 25 Mar 2018 16:15:16 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 25 Mar 2018 16:15:16 -0400    

Click here for diff

Section 10.5 didn't say explicitly that multiple UNIONs are resolved  
pairwise.  Since the resolution algorithm is described as taking any  
number of inputs, readers might well think that a query like  
"select x union select y union select z" would be resolved by  
considering x, y, and z in one resolution step.  But that's not what  
happens (and I think that behavior is per SQL spec).  Add an example  
clarifying this point.  
  
Per bug #15129 from Philippe Beaudoin.  
  
Discussion: https://postgr.es/m/[email protected]  

M doc/src/sgml/typeconv.sgml

Doc: remove extra comma in syntax summary for array_fill().

commit   : 675001d729621bdd49482a71d7676e953ccc40de    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 25 Mar 2018 12:38:21 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 25 Mar 2018 12:38:21 -0400    

Click here for diff

Noted by Scott Ure.  Back-patch to all supported branches.  
  
Discussion: https://postgr.es/m/[email protected]  

M doc/src/sgml/func.sgml

Don't qualify type pg_catalog.text in extend-extensions-example.

commit   : 9dbcbd1e35c34e4c09c8204c0377fa0747141715    
  
author   : Noah Misch <[email protected]>    
date     : Fri, 23 Mar 2018 20:31:03 -0700    
  
committer: Noah Misch <[email protected]>    
date     : Fri, 23 Mar 2018 20:31:03 -0700    

Click here for diff

Extension scripts begin execution with pg_catalog at the front of the  
search path, so type names reliably refer to pg_catalog.  Remove these  
superfluous qualifications.  Earlier <programlisting> of this <sect1>  
already omitted them.  Back-patch to 9.3 (all supported versions).  

M doc/src/sgml/extend.sgml

Fix make rules that generate multiple output files.

commit   : 3bd5009033555ca8085dae45ec166c33c5d60b87    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 23 Mar 2018 13:45:38 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 23 Mar 2018 13:45:38 -0400    

Click here for diff

For years, our makefiles have correctly observed that "there is no correct  
way to write a rule that generates two files".  However, what we did is to  
provide empty rules that "generate" the secondary output files from the  
primary one, and that's not right either.  Depending on the details of  
the creating process, the primary file might end up timestamped later than  
one or more secondary files, causing subsequent make runs to consider the  
secondary file(s) out of date.  That's harmless in a plain build, since  
make will just re-execute the empty rule and nothing happens.  But it's  
fatal in a VPATH build, since make will expect the secondary file to be  
rebuilt in the build directory.  This would manifest as "file not found"  
failures during VPATH builds from tarballs, if we were ever unlucky enough  
to ship a tarball with apparently out-of-date secondary files.  (It's not  
clear whether that has ever actually happened, but it definitely could.)  
  
To ensure that secondary output files have timestamps >= their primary's,  
change our makefile convention to be that we provide a "touch $@" action  
not an empty rule.  Also, make sure that this rule actually gets invoked  
during a distprep run, else the hazard remains.  
  
It's been like this a long time, so back-patch to all supported branches.  
  
In HEAD, I skipped the changes in src/backend/catalog/Makefile, because  
those rules are due to get replaced soon in the bootstrap data format  
patch, and there seems no need to create a merge issue for that patch.  
If for some reason we fail to land that patch in v11, we'll need to  
back-fill the changes in that one makefile from v10.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/Makefile.shlib
M src/backend/Makefile
M src/backend/catalog/Makefile
M src/backend/parser/Makefile
M src/backend/utils/Makefile
M src/bin/psql/Makefile
M src/interfaces/ecpg/preproc/Makefile
M src/pl/plpgsql/src/Makefile
M src/test/isolation/Makefile

Fix tuple counting in SP-GiST index build.

commit   : 46f80803a1273836b818a60127ae01a891c42c85    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 22 Mar 2018 13:23:48 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 22 Mar 2018 13:23:48 -0400    

Click here for diff

Count the number of tuples in the index honestly, instead of assuming  
that it's the same as the number of tuples in the heap.  (It might be  
different if the index is partial.)  
  
Back-patch to all supported versions.  
  
Tomas Vondra  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/access/spgist/spginsert.c

Fix mishandling of quoted-list GUC values in pg_dump and ruleutils.c.

commit   : be677bb5aa792dfb1d108dff445c484baf7f6f5b    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 21 Mar 2018 20:03:29 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 21 Mar 2018 20:03:29 -0400    

Click here for diff

Code that prints out the contents of setconfig or proconfig arrays in  
SQL format needs to handle GUC_LIST_QUOTE variables differently from  
other ones, because for those variables, flatten_set_variable_args()  
already applied a layer of quoting.  The value can therefore safely  
be printed as-is, and indeed must be, or flatten_set_variable_args()  
will muck it up completely on reload.  For all other GUC variables,  
it's necessary and sufficient to quote the value as a SQL literal.  
  
We'd recognized the need for this long ago, but mis-analyzed the  
need slightly, thinking that all GUC_LIST_INPUT variables needed  
the special treatment.  That's actually wrong, since a valid value  
of a LIST variable might include characters that need quoting,  
although no existing variables accept such values.  
  
More to the point, we hadn't made any particular effort to keep the  
various places that deal with this up-to-date with the set of variables  
that actually need special treatment, meaning that we'd do the wrong  
thing with, for example, temp_tablespaces values.  This affects dumping  
of SET clauses attached to functions, as well as ALTER DATABASE/ROLE SET  
commands.  
  
In ruleutils.c we can fix it reasonably honestly by exporting a guc.c  
function that allows discovering the flags for a given GUC variable.  
But pg_dump doesn't have easy access to that, so continue the old method  
of having a hard-wired list of affected variable names.  At least we can  
fix it to have just one list not two, and update the list to match  
current reality.  
  
A remaining problem with this is that it only works for built-in  
GUC variables.  pg_dump's list obvious knows nothing of third-party  
extensions, and even the "ask guc.c" method isn't bulletproof since  
the relevant extension might not be loaded.  There's no obvious  
solution to that, so for now, we'll just have to discourage extension  
authors from inventing custom GUCs that need GUC_LIST_QUOTE.  
  
This has been busted for a long time, so back-patch to all supported  
branches.  
  
Michael Paquier and Tom Lane, reviewed by Kyotaro Horiguchi and  
Pavel Stehule  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/utils/adt/ruleutils.c
M src/backend/utils/misc/guc.c
M src/bin/pg_dump/dumputils.c
M src/bin/pg_dump/dumputils.h
M src/bin/pg_dump/pg_dump.c
M src/bin/pg_dump/pg_dumpall.c
M src/include/utils/guc.h
M src/test/regress/expected/rules.out
M src/test/regress/sql/rules.sql

Fix typo.

commit   : 9bf3458a957a54dd76c339aad446a3aaaf85699f    
  
author   : Tatsuo Ishii <[email protected]>    
date     : Wed, 21 Mar 2018 23:08:43 +0900    
  
committer: Tatsuo Ishii <[email protected]>    
date     : Wed, 21 Mar 2018 23:08:43 +0900    

Click here for diff

Patch by me.  

M doc/src/sgml/protocol.sgml

Doc: note that statement-level view triggers require an INSTEAD OF trigger.

commit   : 60c23ff76b1fdabd7c1adcdb00adfb11c77b04dc    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 18 Mar 2018 15:10:28 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 18 Mar 2018 15:10:28 -0400    

Click here for diff

If a view lacks an INSTEAD OF trigger, DML on it can only work by rewriting  
the command into a command on the underlying base table(s).  Then we will  
fire triggers attached to those table(s), not those for the view.  This  
seems appropriate from a consistency standpoint, but nowhere was the  
behavior explicitly documented, so let's do that.  
  
There was some discussion of throwing an error or warning if a statement  
trigger is created on a view without creating a row INSTEAD OF trigger.  
But a simple implementation of that would result in dump/restore ordering  
hazards.  Given that it's been like this all along, and we hadn't heard  
a complaint till now, a documentation improvement seems sufficient.  
  
Per bug #15106 from Pu Qun.  Back-patch to all supported branches.  
  
Discussion: https://postgr.es/m/[email protected]  

M doc/src/sgml/ref/create_trigger.sgml
M doc/src/sgml/trigger.sgml

Fix overflow handling in plpgsql's integer FOR loops.

commit   : f1f7a85d8c3ed914f4d0df93e40f0144931af315    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 17 Mar 2018 15:38:15 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 17 Mar 2018 15:38:15 -0400    

Click here for diff

The test to exit the loop if the integer control value would overflow  
an int32 turns out not to work on some ICC versions, as it's dependent  
on the assumption that the compiler will execute the code as written  
rather than "optimize" it.  ICC lacks any equivalent of gcc's -fwrapv  
switch, so it was optimizing on the assumption of no integer overflow,  
and that breaks this.  Rewrite into a form that in fact does not  
do any overflowing computations.  
  
Per Tomas Vondra and buildfarm member fulmar.  It's been like this  
for a long time, although it was not till we added a regression test  
case covering the behavior (in commit dd2243f2a) that the problem  
became apparent.  Back-patch to all supported versions.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/pl/plpgsql/src/pl_exec.c

Fix WHERE CURRENT OF when the referenced cursor uses an index-only scan.

commit   : 5b77c11da24a4dc14d24d833bbb81390c72f52fe    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 17 Mar 2018 14:59:31 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 17 Mar 2018 14:59:31 -0400    

Click here for diff

"UPDATE/DELETE WHERE CURRENT OF cursor_name" failed, with an error message  
like "cannot extract system attribute from virtual tuple", if the cursor  
was using a index-only scan for the target table.  Fix it by digging the  
current TID out of the indexscan state.  
  
It seems likely that the same failure could occur for CustomScan plans  
and perhaps some FDW plan types, so that leaving this to be treated as an  
internal error with an obscure message isn't as good an idea as it first  
seemed.  Hence, add a bit of heaptuple.c infrastructure to let us deliver  
a more on-topic message.  I chose to make the message match what you get  
for the case where execCurrentOf can't identify the target scan node at  
all, "cursor "foo" is not a simply updatable scan of table "bar"".  
Perhaps it should be different, but we can always adjust that later.  
  
In the future, it might be nice to provide hooks that would let custom  
scan providers and/or FDWs deal with this in other ways; but that's  
not a suitable topic for a back-patchable bug fix.  
  
It's been like this all along, so back-patch to all supported branches.  
  
Yugo Nagata and Tom Lane  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/access/common/heaptuple.c
M src/backend/executor/execCurrent.c
M src/include/executor/tuptable.h
M src/test/regress/expected/portals.out
M src/test/regress/sql/portals.sql

Fix query-lifespan memory leakage in repeatedly executed hash joins.

commit   : 574386ddca69b4126e77f993d3d770c85f99d209    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 16 Mar 2018 16:03:45 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 16 Mar 2018 16:03:45 -0400    

Click here for diff

ExecHashTableCreate allocated some memory that wasn't freed by  
ExecHashTableDestroy, specifically the per-hash-key function information.  
That's not a huge amount of data, but if one runs a query that repeats  
a hash join enough times, it builds up.  Fix by arranging for the data  
in question to be kept in the hashtable's hashCxt instead of leaving it  
"loose" in the query-lifespan executor context.  (This ensures that we'll  
also clean up anything that the hash functions allocate in fn_mcxt.)  
  
Per report from Amit Khandekar.  It's been like this forever, so back-patch  
to all supported branches.  
  
Discussion: https://postgr.es/m/CAJ3gD9cFofAWGvcxLOxDHC=B0hjtW8yGmUsF2hdGh97CM38=7g@mail.gmail.com  

M src/backend/executor/nodeHash.c

Doc: explicitly point out that enum values can't be dropped.

commit   : a9d0c862d53db353e9879acbeaa75a1c9fe39123    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 16 Mar 2018 13:44:34 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 16 Mar 2018 13:44:34 -0400    

Click here for diff

This was not stated in so many words anywhere.  Document it to make  
clear that it's a design limitation and not just an oversight or  
documentation omission.  
  
Discussion: https://postgr.es/m/[email protected]  

M doc/src/sgml/datatype.sgml

Fix double frees in ecpg.

commit   : 09f4ca92bbe5a68b228d6a3251e4d2be31bb6377    
  
author   : Michael Meskes <[email protected]>    
date     : Wed, 14 Mar 2018 00:47:49 +0100    
  
committer: Michael Meskes <[email protected]>    
date     : Wed, 14 Mar 2018 00:47:49 +0100    

Click here for diff

Patch by Patrick Krecker <[email protected]>  

M src/interfaces/ecpg/preproc/ecpg.c

When updating reltuples after ANALYZE, just extrapolate from our sample.

commit   : d44ce7b1afdbc8e0540086e4fc94a540924bf3b6    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 13 Mar 2018 13:24:27 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 13 Mar 2018 13:24:27 -0400    

Click here for diff

The existing logic for updating pg_class.reltuples trusted the sampling  
results only for the pages ANALYZE actually visited, preferring to  
believe the previous tuple density estimate for all the unvisited pages.  
While there's some rationale for doing that for VACUUM (first that  
VACUUM is likely to visit a very nonrandom subset of pages, and second  
that we know for sure that the unvisited pages did not change), there's  
no such rationale for ANALYZE: by assumption, it's looked at an unbiased  
random sample of the table's pages.  Furthermore, in a very large table  
ANALYZE will have examined only a tiny fraction of the table's pages,  
meaning it cannot slew the overall density estimate very far at all.  
In a table that is physically growing, this causes reltuples to increase  
nearly proportionally to the change in relpages, regardless of what is  
actually happening in the table.  This has been observed to cause reltuples  
to become so much larger than reality that it effectively shuts off  
autovacuum, whose threshold for doing anything is a fraction of reltuples.  
(Getting to the point where that would happen seems to require some  
additional, not well understood, conditions.  But it's undeniable that if  
reltuples is seriously off in a large table, ANALYZE alone will not fix it  
in any reasonable number of iterations, especially not if the table is  
continuing to grow.)  
  
Hence, restrict the use of vac_estimate_reltuples() to VACUUM alone,  
and in ANALYZE, just extrapolate from the sample pages on the assumption  
that they provide an accurate model of the whole table.  If, by very bad  
luck, they don't, at least another ANALYZE will fix it; in the old logic  
a single bad estimate could cause problems indefinitely.  
  
In HEAD, let's remove vac_estimate_reltuples' is_analyze argument  
altogether; it was never used for anything and now it's totally pointless.  
But keep it in the back branches, in case any third-party code is calling  
this function.  
  
Per bug #15005.  Back-patch to all supported branches.  
  
David Gould, reviewed by Alexander Kuzmenkov, cosmetic changes by me  
  
Discussion: https://postgr.es/m/20180117164916.3fdcf2e9@engels  

M src/backend/commands/analyze.c
M src/backend/commands/vacuum.c

Avoid holding AutovacuumScheduleLock while rechecking table statistics.

commit   : 5328b6135756822d19288b3ee366eb5e7cea0426    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 13 Mar 2018 12:28:16 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 13 Mar 2018 12:28:16 -0400    

Click here for diff

In databases with many tables, re-fetching the statistics takes some time,  
so that this behavior seriously decreases the available concurrency for  
multiple autovac workers.  There's discussion afoot about more complete  
fixes, but a simple and back-patchable amelioration is to claim the table  
and release the lock before rechecking stats.  If we find out there's no  
longer a reason to process the table, re-taking the lock to un-claim the  
table is cheap enough.  
  
(This patch is quite old, but got lost amongst a discussion of more  
aggressive fixes.  It's not clear when or if such a fix will be  
accepted, but in any case it'd be unlikely to get back-patched.  
Let's do this now so we have some improvement for the back branches.)  
  
In passing, make the normal un-claim step take AutovacuumScheduleLock  
not AutovacuumLock, since that is what is documented to protect the  
wi_tableoid field.  This wasn't an actual bug in view of the fact that  
readers of that field hold both locks, but it creates some concurrency  
penalty against operations that need only AutovacuumLock.  
  
Back-patch to all supported versions.  
  
Jeff Janes  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/postmaster/autovacuum.c

Set connection back to NULL after freeing it.

commit   : 042badc3778ade0f430049eb7dac03972e544e5a    
  
author   : Michael Meskes <[email protected]>    
date     : Mon, 12 Mar 2018 23:52:08 +0100    
  
committer: Michael Meskes <[email protected]>    
date     : Mon, 12 Mar 2018 23:52:08 +0100    

Click here for diff

Patch by Jeevan Ladhe <[email protected]>  

M src/interfaces/ecpg/preproc/output.c

Fix improper uses of canonicalize_qual().

commit   : 925581d89cee624ec16b1cd201a4e1de27f2d652    
  
author   : Tom Lane <[email protected]>    
date     : Sun, 11 Mar 2018 18:10:43 -0400    
  
committer: Tom Lane <[email protected]>    
date     : Sun, 11 Mar 2018 18:10:43 -0400    

Click here for diff

One of the things canonicalize_qual() does is to remove constant-NULL  
subexpressions of top-level AND/OR clauses.  It does that on the assumption  
that what it's given is a top-level WHERE clause, so that NULL can be  
treated like FALSE.  Although this is documented down inside a subroutine  
of canonicalize_qual(), it wasn't mentioned in the documentation of that  
function itself, and some callers hadn't gotten that memo.  
  
Notably, commit d007a9505 caused get_relation_constraints() to apply  
canonicalize_qual() to CHECK constraints.  That allowed constraint  
exclusion to misoptimize situations in which a CHECK constraint had a  
provably-NULL subclause, as seen in the regression test case added here,  
in which a child table that should be scanned is not.  (Although this  
thinko is ancient, the test case doesn't fail before 9.2, for reasons  
I've not bothered to track down in detail.  There may be related cases  
that do fail before that.)  
  
More recently, commit f0e44751d added an independent bug by applying  
canonicalize_qual() to index expressions, which is even sillier since  
those might not even be boolean.  If they are, though, I think this  
could lead to making incorrect index entries for affected index  
expressions in v10.  I haven't attempted to prove that though.  
  
To fix, add an "is_check" parameter to canonicalize_qual() to specify  
whether it should assume WHERE or CHECK semantics, and make it perform  
NULL-elimination accordingly.  Adjust the callers to apply the right  
semantics, or remove the call entirely in cases where it's not known  
that the expression has one or the other semantics.  I also removed  
the call in some cases involving partition expressions, where it should  
be a no-op because such expressions should be canonical already ...  
and was a no-op, independently of whether it could in principle have  
done something, because it was being handed the qual in implicit-AND  
format which isn't what it expects.  In HEAD, add an Assert to catch  
that type of mistake in future.  
  
This represents an API break for external callers of canonicalize_qual().  
While that's intentional in HEAD to make such callers think about which  
case applies to them, it seems like something we probably wouldn't be  
thanked for in released branches.  Hence, in released branches, the  
extra parameter is added to a new function canonicalize_qual_ext(),  
and canonicalize_qual() is a wrapper that retains its old behavior.  
  
Patch by me with suggestions from Dean Rasheed.  Back-patch to all  
supported branches.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/optimizer/plan/planner.c
M src/backend/optimizer/plan/subselect.c
M src/backend/optimizer/prep/prepqual.c
M src/backend/optimizer/util/plancat.c
M src/backend/utils/cache/relcache.c
M src/include/optimizer/prep.h
M src/test/regress/expected/inherit.out
M src/test/regress/sql/inherit.sql

Fix assorted issues in convert_to_scalar().

commit   : 0bea99bd929ace532d96dbb00841c30449327ad4    
  
author   : Tom Lane <[email protected]>    
date     : Sat, 3 Mar 2018 20:31:35 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Sat, 3 Mar 2018 20:31:35 -0500    

Click here for diff

If convert_to_scalar is passed a pair of datatypes it can't cope with,  
its former behavior was just to elog(ERROR).  While this is OK so far as  
the core code is concerned, there's extension code that would like to use  
scalarltsel/scalargtsel/etc as selectivity estimators for operators that  
work on non-core datatypes, and this behavior is a show-stopper for that  
use-case.  If we simply allow convert_to_scalar to return FALSE instead of  
outright failing, then the main logic of scalarltsel/scalargtsel will work  
fine for any operator that behaves like a scalar inequality comparison.  
The lack of conversion capability will mean that we can't estimate to  
better than histogram-bin-width precision, since the code will effectively  
assume that the comparison constant falls at the middle of its bin.  But  
that's still a lot better than nothing.  (Someday we should provide a way  
for extension code to supply a custom version of convert_to_scalar, but  
today is not that day.)  
  
While poking at this issue, we noted that the existing code for handling  
type bytea in convert_to_scalar is several bricks shy of a load.  
It assumes without checking that if the comparison value is type bytea,  
the bounds values are too; in the worst case this could lead to a crash.  
It also fails to detoast the input values, so that the comparison result is  
complete garbage if any input is toasted out-of-line, compressed, or even  
just short-header.  I'm not sure how often such cases actually occur ---  
the bounds values, at least, are probably safe since they are elements of  
an array and hence can't be toasted.  But that doesn't make this code OK.  
  
Back-patch to all supported branches, partly because author requested that,  
but mostly because of the bytea bugs.  The change in API for the exposed  
routine convert_network_to_scalar() is theoretically a back-patch hazard,  
but it seems pretty unlikely that any third-party code is calling that  
function directly.  
  
Tomas Vondra, with some adjustments by me  
  
Discussion: https://postgr.es/m/[email protected]  

M contrib/btree_gist/btree_inet.c
M src/backend/utils/adt/network.c
M src/backend/utils/adt/selfuncs.c
M src/include/utils/builtins.h

Make gistvacuumcleanup() count the actual number of index tuples.

commit   : 6b56f07525a04a22260d3f9650b02c26d2d6c366    
  
author   : Tom Lane <[email protected]>    
date     : Fri, 2 Mar 2018 11:22:42 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Fri, 2 Mar 2018 11:22:42 -0500    

Click here for diff

Previously, it just returned the heap tuple count, which might be only an  
estimate, and would be completely the wrong thing if the index is partial.  
Since this function scans every index page anyway to find free pages,  
it's practically free to count the surviving index tuples.  Let's do that  
and return an accurate count.  
  
This is easily visible as a wrong reltuples value for a partial GiST  
index following VACUUM, so back-patch to all supported branches.  
  
Andrey Borodin, reviewed by Michail Nikolaev  
  
Discussion: https://postgr.es/m/[email protected]  

M src/backend/access/gist/gistvacuum.c

Use ereport not elog for some corrupt-HOT-chain reports.

commit   : 341b73448009dbc1d9020a1221326af67c716b14    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 1 Mar 2018 16:23:30 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 1 Mar 2018 16:23:30 -0500    

Click here for diff

These errors have been seen in the field in corrupted-data situations.  
It seems worthwhile to report them with ERRCODE_DATA_CORRUPTED, rather  
than the generic ERRCODE_INTERNAL_ERROR, for the benefit of log monitoring  
and tools like amcheck.  However, use errmsg_internal so that the text  
strings still aren't translated; it seems unlikely to be worth  
translators' time to do so.  
  
Back-patch to 9.3, like the predecessor commit d70cf811f that introduced  
these elog calls originally (replacing Asserts).  
  
Peter Geoghegan  
  
Discussion: https://postgr.es/m/CAH2-Wzmn4-Pg-UGFwyuyK-wiTih9j32pwg_7T9iwqXpAUZr=Mg@mail.gmail.com  

M src/backend/catalog/index.c

Relax overly strict sanity check for upgraded ancient databases

commit   : 650f3863f45304934b3d98aa1b4175b5ddf1b092    
  
author   : Alvaro Herrera <[email protected]>    
date     : Thu, 1 Mar 2018 18:07:46 -0300    
  
committer: Alvaro Herrera <[email protected]>    
date     : Thu, 1 Mar 2018 18:07:46 -0300    

Click here for diff

Commit 4800f16a7ad0 added some sanity checks to ensure we don't  
accidentally corrupt data, but in one of them we failed to consider the  
effects of a database upgraded from 9.2 or earlier, where a tuple  
exclusively locked prior to the upgrade has a slightly different bit  
pattern.  Fix that by using the macro that we fixed in commit  
74ebba84aeb6 for similar situations.  
  
Reported-by: Alexandre Garcia  
Reviewed-by: Andres Freund  
Discussion: https://postgr.es/m/CAPYLKR6yxV4=pfW0Gwij7aPNiiPx+3ib4USVYnbuQdUtmkMaEA@mail.gmail.com  
  
Andres suspects that this bug may have wider ranging consequences, but I  
couldn't find anything.  

M src/backend/access/heap/heapam.c

Fix build failure on MSVC.

commit   : f0fb93a266f4e80bd5236f4725025565f237285b    
  
author   : Tom Lane <[email protected]>    
date     : Thu, 1 Mar 2018 13:27:44 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Thu, 1 Mar 2018 13:27:44 -0500    

Click here for diff

Commit 824cceded introduced use of pg_malloc and pg_realloc into  
specscanner.l, but it isn't working in 9.3 on MSVC.  Evidently we  
added the infrastructure for that in 9.4.  Since the chance of an  
actual OOM here is tiny, and the consequences would only be an  
isolation test failure, and we have unchecked OOM hazards elsewhere  
in this file in 9.3, it's not worth sweating over.  Just replace  
the calls with malloc and realloc.  
  
Per buildfarm.  

M src/test/isolation/specscanner.l

Rename base64 routines to avoid conflict with Solaris built-in functions.

commit   : 10102c91ea13028ac5bb5a362ee42e4af14ca624    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 28 Feb 2018 18:33:45 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 28 Feb 2018 18:33:45 -0500    

Click here for diff

Solaris 11.4 has built-in functions named b64_encode and b64_decode.  
Rename ours to something else to avoid the conflict (fortunately,  
ours are static so the impact is limited).  
  
One could wish for less duplication of code in this area, but that  
would be a larger patch and not very suitable for back-patching.  
Since this is a portability fix, we want to put it into all supported  
branches.  
  
Report and initial patch by Rainer Orth, reviewed and adjusted a bit  
by Michael Paquier  
  
Discussion: https://postgr.es/m/[email protected]  

M contrib/pgcrypto/pgp-armor.c
M src/backend/utils/adt/encode.c

Remove restriction on SQL block length in isolationtester scanner.

commit   : 824cceded4095eb3b26d19b1a91d01eaf37c0baf    
  
author   : Tom Lane <[email protected]>    
date     : Wed, 28 Feb 2018 16:57:38 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Wed, 28 Feb 2018 16:57:38 -0500    

Click here for diff

specscanner.l had a fixed limit of 1024 bytes on the length of  
individual SQL stanzas in an isolation test script.  People are  
starting to run into that, so fix it by making the buffer resizable.  
  
Once we allow this in HEAD, it seems inevitable that somebody will  
try to back-patch a test that exceeds the old limit, so back-patch  
this change as a preventive measure.  
  
Daniel Gustafsson  
  
Discussion: https://postgr.es/m/[email protected]  

M src/test/isolation/specscanner.l

Fix up ecpg's configuration so it handles "long long int" in MSVC builds.

commit   : 87b7e1e887727869053fd1e58cfa5a1284491e81    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 16:46:52 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 16:46:52 -0500    

Click here for diff

Although configure-based builds correctly define HAVE_LONG_LONG_INT when  
appropriate (in both pg_config.h and ecpg_config.h), builds using the MSVC  
scripts failed to do so.  This currently has no impact on the backend,  
since it uses that symbol nowhere; but it does prevent ecpg from  
supporting "long long int".  Fix that.  
  
Also, adjust Solution.pm so that in the constructed ecpg_config.h file,  
the "#if (_MSC_VER > 1200)" covers only the LONG_LONG_INT-related  
#defines, not the whole file.  AFAICS this was a thinko on somebody's  
part: ENABLE_THREAD_SAFETY should always be defined in Windows builds,  
and in branches using USE_INTEGER_DATETIMES, the setting of that shouldn't  
depend on the compiler version either.  If I'm wrong, I imagine the  
buildfarm will say so.  
  
Per bug #15080 from Jonathan Allen; issue diagnosed by Michael Meskes  
and Andrew Gierth.  Back-patch to all supported branches.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/include/pg_config.h.win32
M src/tools/msvc/Solution.pm

Remove regression tests' CREATE FUNCTION commands for unused C functions.

commit   : 0c4af129941b50d2cff65780ccf82c1e4b075a2d    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 15:04:21 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 15:04:21 -0500    

Click here for diff

I removed these functions altogether in HEAD, in commit db3af9feb, and  
it emerges that that causes trouble for cross-branch upgrade testing.  
We could put back stub functions but that seems pretty silly.  Instead,  
back-patch a minimal subset of db3af9feb, namely just removing the  
CREATE FUNCTION commands.  
  
Discussion: https://postgr.es/m/[email protected]  

M src/test/regress/input/create_function_1.source
M src/test/regress/input/create_function_2.source
M src/test/regress/output/create_function_1.source
M src/test/regress/output/create_function_2.source

Prevent dangling-pointer access when update trigger returns old tuple.

commit   : 9bc33ef5ece99faec588b93e5ebda0448ab2ece5    
  
author   : Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 13:27:38 -0500    
  
committer: Tom Lane <[email protected]>    
date     : Tue, 27 Feb 2018 13:27:38 -0500    

Click here for diff

A before-update row trigger may choose to return the "new" or "old" tuple  
unmodified.  ExecBRUpdateTriggers failed to consider the second  
possibility, and would proceed to free the "old" tuple even if it was the  
one returned, leading to subsequent access to already-deallocated memory.  
In debug builds this reliably leads to an "invalid memory alloc request  
size" failure; in production builds it might accidentally work, but data  
corruption is also possible.  
  
This is a very old bug.  There are probably a couple of reasons it hasn't  
been noticed up to now.  It would be more usual to return NULL if one  
wanted to suppress the update action; returning "old" is significantly less  
efficient since the update will occur anyway.  Also, none of the standard  
PLs would ever cause this because they all returned freshly-manufactured  
tuples even if they were just copying "old".  But commit 4b93f5799 changed  
that for plpgsql, making it possible to see the bug with a plpgsql trigger.  
Still, this is certainly legal behavior for a trigger function, so it's  
ExecBRUpdateTriggers's fault not plpgsql's.  
  
It seems worth creating a test case that exercises returning "old" directly  
with a C-language trigger; testing this through plpgsql seems unreliable  
because its behavior might change again.  
  
Report and fix by Rushabh Lathia; regression test case by me.  
Back-patch to all supported branches.  
  
Discussion: https://postgr.es/m/CAGPqQf1P4pjiNPrMof=P_16E-DFjt457j+nH2ex3=nBTew7tXw@mail.gmail.com  

M src/backend/commands/trigger.c
M src/test/regress/expected/triggers.out
M src/test/regress/input/create_function_1.source
M src/test/regress/output/create_function_1.source
M src/test/regress/regress.c
M src/test/regress/sql/triggers.sql