postgresql.git
8 years agoClarify pg_dump -b documentation
Stephen Frost [Tue, 29 Nov 2016 15:35:12 +0000 (10:35 -0500)]
Clarify pg_dump -b documentation

The documentation around the -b/--blobs option to pg_dump seemed to
imply that it might be possible to add blobs to a "schema-only" dump or
similar.  Clarify that blobs are data and therefore will only be
included in dumps where data is being included, even when -b is used to
request blobs be included.

The -b option has been around since before 9.2, so back-patch to all
supported branches.

Discussion: https://postgr.es/m/20161119173316[email protected]

8 years agoMention server start requirement for ssl parameters
Magnus Hagander [Sun, 27 Nov 2016 16:10:02 +0000 (17:10 +0100)]
Mention server start requirement for ssl parameters

Fix that the documentation for three ssl related parameters did not
specify that they can only be changed at server start.

Michael Paquier

8 years agoFix test about ignoring extension dependencies during extension scripts.
Tom Lane [Sat, 26 Nov 2016 18:31:35 +0000 (13:31 -0500)]
Fix test about ignoring extension dependencies during extension scripts.

Commit 08dd23cec introduced an exception to the rule that extension member
objects can only be dropped as part of dropping the whole extension,
intending to allow such drops while running the extension's own creation or
update scripts.  However, the exception was only applied at the outermost
recursion level, because it was modeled on a pre-existing check to ignore
dependencies on objects listed in pendingObjects.  Bug #14434 from Philippe
Beaudoin shows that this is inadequate: in some cases we can reach an
extension member object by recursion from another one.  (The bug concerns
the serial-sequence case; I'm not sure if there are other cases, but there
might well be.)

To fix, revert 08dd23cec's changes to findDependentObjects() and instead
apply the creating_extension exception regardless of stack level.

Having seen this example, I'm a bit suspicious that the pendingObjects
logic is also wrong and such cases should likewise be allowed at any
recursion level.  However, changing that would interact in subtle ways
with the recursion logic (at least it would need to be moved to after the
recursing-from check).  Given that the code's been like that a long time,
I'll refrain from touching it without a clear example showing it's wrong.

Back-patch to all active branches.  In HEAD and 9.6, where suitable
test infrastructure exists, add a regression test case based on the
bug report.

Report: <20161125151448[email protected]>
Discussion: <13224.1480177514@sss.pgh.pa.us>

8 years agoCheck for pending trigger events on far end when dropping an FK constraint.
Tom Lane [Fri, 25 Nov 2016 18:44:48 +0000 (13:44 -0500)]
Check for pending trigger events on far end when dropping an FK constraint.

When dropping a foreign key constraint with ALTER TABLE DROP CONSTRAINT,
we refuse the drop if there are any pending trigger events on the named
table; this ensures that we won't remove the pg_trigger row that will be
consulted by those events.  But we should make the same check for the
referenced relation, else we might remove a due-to-be-referenced pg_trigger
row for that relation too, resulting in "could not find trigger NNN" or
"relation NNN has no triggers" errors at commit.  Per bug #14431 from
Benjie Gillam.  Back-patch to all supported branches.

Report: <20161124114911[email protected]>

8 years agoMake sure ALTER TABLE preserves index tablespaces.
Tom Lane [Wed, 23 Nov 2016 18:45:56 +0000 (13:45 -0500)]
Make sure ALTER TABLE preserves index tablespaces.

When rebuilding an existing index, ALTER TABLE correctly kept the
physical file in the same tablespace, but it messed up the pg_class
entry if the index had been in the database's default tablespace
and "default_tablespace" was set to some non-default tablespace.
This led to an inaccessible index.

Fix by fixing pg_get_indexdef_string() to always include a tablespace
clause, whether or not the index is in the default tablespace.  The
previous behavior was installed in commit 537e92e41, and I think it just
wasn't thought through very clearly; certainly the possible effect of
default_tablespace wasn't considered.  There's some risk in changing the
behavior of this function, but there are no other call sites in the core
code.  Even if it's being used by some third party extension, it's fairly
hard to envision a usage that is okay with a tablespace clause being
appended some of the time but can't handle it being appended all the time.

Back-patch to all supported versions.

Code fix by me, investigation and test cases by Michael Paquier.

Discussion: <1479294998857-5930602[email protected]>

8 years agoDoc: improve documentation about composite-value usage.
Tom Lane [Tue, 22 Nov 2016 22:56:16 +0000 (17:56 -0500)]
Doc: improve documentation about composite-value usage.

Create a section specifically for the syntactic rules around whole-row
variable usage, such as expansion of "foo.*".  This was previously
documented only haphazardly, with some critical info buried in
unexpected places like xfunc-sql-composite-functions.  Per repeated
questions in different mailing lists.

Discussion: <16288.1479610770@sss.pgh.pa.us>

8 years agoDoc: add a section in Part II concerning RETURNING.
Tom Lane [Tue, 22 Nov 2016 19:02:52 +0000 (14:02 -0500)]
Doc: add a section in Part II concerning RETURNING.

There are assorted references to RETURNING in Part II, but nothing
that would qualify as an explanation of the feature, which seems
like an oversight considering how useful it is.  Add something.

Noted while looking for a place to point a cross-reference to ...

8 years agoMake contrib/test_decoding regression tests safe for CZ locale.
Tom Lane [Tue, 22 Nov 2016 01:39:28 +0000 (20:39 -0500)]
Make contrib/test_decoding regression tests safe for CZ locale.

A little COLLATE "C" goes a long way.

Pavel Stehule, per suggestion from Craig Ringer

Discussion: 

8 years agoFix PGLC_localeconv() to handle errors better.
Tom Lane [Mon, 21 Nov 2016 23:21:56 +0000 (18:21 -0500)]
Fix PGLC_localeconv() to handle errors better.

The code was intentionally not very careful about leaking strdup'd
strings in case of an error.  That was forgivable probably, but it
also failed to notice strdup() failures, which could lead to subsequent
null-pointer-dereference crashes, since many callers unsurprisingly
didn't check for null pointers in the struct lconv fields.  An even
worse problem is that it could throw error while we were setlocale'd
to a non-C locale, causing unwanted behavior in subsequent libc calls.

Rewrite to ensure that we cannot throw elog(ERROR) until after we've
restored the previous locale settings, or at least attempted to.
(I'm sorely tempted to make restore failure be a FATAL error, but
will refrain for the moment.)  Having done that, it's not much more
work to ensure that we clean up strdup'd storage on the way out, too.

This code is substantially the same in all supported branches, so
back-patch all the way.

Michael Paquier and Tom Lane

Discussion: 

8 years agoPrevent multicolumn expansion of "foo.*" in an UPDATE source expression.
Tom Lane [Sun, 20 Nov 2016 19:26:19 +0000 (14:26 -0500)]
Prevent multicolumn expansion of "foo.*" in an UPDATE source expression.

Because we use transformTargetList() for UPDATE as well as SELECT
tlists, the code accidentally tried to expand a "*" reference into
several columns.  This is nonsensical, because the UPDATE syntax
provides exactly one target column to put the value into.  The
immediate result was that transformUpdateTargetList() got confused
and reported "UPDATE target count mismatch --- internal error".
It seems better to treat such a reference as a plain whole-row
variable, as it would be in other contexts.  (This could produce
useful results when the target column is of composite type.)

Fix by tweaking transformTargetList() to perform *-expansion only
conditionally, depending on its exprKind parameter.

Back-patch to 9.3.  The problem exists further back, but a fix would be
much more invasive before that, because transformTargetList() wasn't
told what kind of list it was working on.  Doesn't seem worth the
trouble given the lack of field reports.  (I only noticed it because
I was checking the code while trying to improve the documentation about
how we handle "foo.*".)

Discussion: <4308.1479595330@sss.pgh.pa.us>

8 years agoImprove pg_dump/pg_restore --create --if-exists logic.
Tom Lane [Thu, 17 Nov 2016 19:59:26 +0000 (14:59 -0500)]
Improve pg_dump/pg_restore --create --if-exists logic.

Teach it not to complain if the dropStmt attached to an archive entry
is actually spelled CREATE OR REPLACE VIEW, since that will happen due to
an upcoming bug fix.  Also, if it doesn't recognize a dropStmt, have it
print a WARNING and then emit the dropStmt unmodified.  That seems like a
much saner behavior than Assert'ing or dumping core due to a null-pointer
dereference, which is what would happen before :-(.

Back-patch to 9.4 where this option was introduced.

Discussion: <19092.1479325184@sss.pgh.pa.us>

8 years agoAvoid pin scan for replay of XLOG_BTREE_VACUUM in all cases
Alvaro Herrera [Thu, 17 Nov 2016 16:31:30 +0000 (13:31 -0300)]
Avoid pin scan for replay of XLOG_BTREE_VACUUM in all cases

Replay of XLOG_BTREE_VACUUM during Hot Standby was previously thought to
require complex interlocking that matched the requirements on the
master. This required an O(N) operation that became a significant
problem with large indexes, causing replication delays of seconds or in
some cases minutes while the XLOG_BTREE_VACUUM was replayed.

This commit skips the “pin scan” that was previously required, by
observing in detail when and how it is safe to do so, with full
documentation. The pin scan is skipped only in replay; the VACUUM code
path on master is not touched here.

No tests included. Manual tests using an additional patch to view WAL records
and their timing have shown the change in WAL records and their handling has
successfully reduced replication delay.

This is a back-patch of commits 687f2cd7a0153e4b7d87988fb60284261375
by Simon Riggs, to branches 9.4 and 9.5.  No further backpatch is
possible because this depends on catalog scans being MVCC.  I (Álvaro)
additionally updated a slight problem in the README, which explains why
this touches the 9.6 and master branches.

8 years agoAllow DOS-style line endings in ~/.pgpass files.
Tom Lane [Tue, 15 Nov 2016 21:17:19 +0000 (16:17 -0500)]
Allow DOS-style line endings in ~/.pgpass files.

On Windows, libc will mask \r\n line endings for us, since we read the
password file in text mode.  But that doesn't happen on Unix.  People
who share password files across both systems might have \r\n line endings
in a file they use on Unix, so as a convenience, ignore trailing \r.
Per gripe from Josh Berkus.

In passing, put the existing check for empty line somewhere where it's
actually useful, ie after stripping the newline not before.

Vik Fearing, adjusted a bit by me

Discussion: <0de37763-5843-b2cc-855e-5d0e5df25807@agliodbs.com>

8 years agoAccount for catalog snapshot in PGXACT->xmin updates.
Tom Lane [Tue, 15 Nov 2016 20:55:36 +0000 (15:55 -0500)]
Account for catalog snapshot in PGXACT->xmin updates.

The CatalogSnapshot was not plugged into SnapshotResetXmin()'s accounting
for whether MyPgXact->xmin could be cleared or advanced.  In normal
transactions this was masked by the fact that the transaction snapshot
would be older, but during backend startup and certain utility commands
it was possible to re-use the CatalogSnapshot after MyPgXact->xmin had
been cleared, meaning that recently-deleted rows could be pruned even
though this snapshot could still see them, causing unexpected catalog
lookup failures.  This effect appears to be the explanation for a recent
failure on buildfarm member piculet.

To fix, add the CatalogSnapshot to the RegisteredSnapshots heap whenever
it is valid.

In the previous logic, it was possible for the CatalogSnapshot to remain
valid across waits for client input, but with this change that would mean
it delays advance of global xmin in cases where it did not before.  To
avoid possibly causing new table-bloat problems with clients that sit idle
for long intervals, add code to invalidate the CatalogSnapshot before
waiting for client input.  (When the backend is busy, it's unlikely that
the CatalogSnapshot would be the oldest snap for very long, so we don't
worry about forcing early invalidation of it otherwise.)

In passing, remove the CatalogSnapshotStale flag in favor of using
"CatalogSnapshot != NULL" to represent validity, as we do for the other
special snapshots in snapmgr.c.  And improve some obsolete comments.

No regression test because I don't know a deterministic way to cause this
failure.  But the stress test shown in the original discussion provokes
"cache lookup failed for relation 1255" within a few dozen seconds for me.

Back-patch to 9.4 where MVCC catalog scans were introduced.  (Note: it's
quite easy to produce similar failures with the same test case in branches
before 9.4.  But MVCC catalog scans were supposed to fix that.)

Discussion: <16447.1478818294@sss.pgh.pa.us>

8 years agoFix duplication in ALTER MATERIALIZE VIEW synopsis
Alvaro Herrera [Mon, 14 Nov 2016 14:14:34 +0000 (11:14 -0300)]
Fix duplication in ALTER MATERIALIZE VIEW synopsis

Commit 3c4cf080879b should have removed SET TABLESPACE from the synopsis
of ALTER MATERIALIZE VIEW as a possible "action" when it added a
separate line for it in the main command listing, but failed to.
Repair.

Backpatch to 9.4, like the aforementioned commit.

8 years agoFix typo
Magnus Hagander [Tue, 8 Nov 2016 17:34:59 +0000 (18:34 +0100)]
Fix typo

8 years agoFix handling of symlinked pg_stat_tmp and pg_replslot
Magnus Hagander [Mon, 7 Nov 2016 13:47:30 +0000 (14:47 +0100)]
Fix handling of symlinked pg_stat_tmp and pg_replslot

This was already fixed in HEAD as part of 6ad8ac60 but was not
backpatched.

Also change the way pg_xlog is handled to be the same as the other
directories.

Patch from me with pg_xlog addition from Michael Paquier, test updates
from David Steele.

8 years agoRationalize and document pltcl's handling of magic ".tupno" array element.
Tom Lane [Sun, 6 Nov 2016 19:43:13 +0000 (14:43 -0500)]
Rationalize and document pltcl's handling of magic ".tupno" array element.

For a very long time, pltcl's spi_exec and spi_execp commands have had
a behavior of storing the current row number as an element of output
arrays, but this was never documented.  Fix that.

For an equally long time, pltcl_trigger_handler had a behavior of silently
ignoring ".tupno" as an output column name, evidently so that the result
of spi_exec could be used directly as a trigger result tuple.  Not sure
how useful that really is, but in any case it's bad that it would break
attempts to use ".tupno" as an actual column name.  We can fix it by not
checking for ".tupno" until after we check for a column name match.  This
comports with the effective behavior of spi_exec[p] that ".tupno" is only
magic when you don't have an actual column named that.

In passing, wordsmith the description of returning modified tuples from
a pltcl trigger.

Noted while working on Jim Nasby's patch to support composite results
from pltcl.  The inability to return trigger tuples using ".tupno" as
a column name is a bug, so back-patch to all supported branches.

8 years agoMore zic cleanup.
Tom Lane [Sun, 6 Nov 2016 15:45:58 +0000 (10:45 -0500)]
More zic cleanup.

The workaround the IANA guys chose to get rid of the clang warning
we'd silenced in commit 23ed2ba81 turns out not to satisfy Coverity.
Go back to the previous solution, ie, remove the useless comparison
to SIZE_MAX.  (In principle, there could be machines out there where
it's not useless because ptrdiff_t is wider than size_t.  But the whole
thing is pretty academic anyway, as we could never approach this limit
for any sane estimate of the amount of data that zic will ever be asked
to work with.)

Also, s/lineno/lineno_t/g, because if we accept their decision to start
using "lineno" as a typedef, it is going to have very unpleasant
consequences in our next pgindent run.  Noted that while fooling with
pltcl yesterday.

8 years agoSync our copy of the timezone library with IANA tzcode master.
Tom Lane [Fri, 4 Nov 2016 14:44:16 +0000 (10:44 -0400)]
Sync our copy of the timezone library with IANA tzcode 

This patch absorbs some unreleased fixes for symlink manipulation bugs
introduced in tzcode 2016g.  Ordinarily I'd wait around for a released
version, but in this case it seems like we could do with extra testing,
in particular checking whether it works in EDB's VMware build environment.
This corresponds to commit aec59156abbf8472ba201b6c7ca2592f9c10e077 in
https://github.com/eggert/tz.

Per a report from Sandeep Thakkar, building in an environment where hard
links are not supported in the timezone data installation directory failed,
because upstream code refactoring had broken the case of symlinking from an
existing symlink.  Further experimentation also showed that the symlinks
were sometimes made incorrectly, with too many or too few "../"'s in the
symlink contents.

Back-patch of commit 1f87181e12beb067d21b79493393edcff14c190b.

Report: 
Discussion: http://mm.icann.org/pipermail/tz/2016-November/024431.html

8 years agoFix nasty performance problem in tsquery_rewrite().
Tom Lane [Sun, 30 Oct 2016 21:35:43 +0000 (17:35 -0400)]
Fix nasty performance problem in tsquery_rewrite().

tsquery_rewrite() tries to find matches to subsets of AND/OR conditions;
for example, in the query 'a | b | c' the substitution subquery 'a | c'
should match and lead to replacement of the first and third items.
That's fine, but the matching algorithm apparently takes about O(2^N)
for an N-clause query (I say "apparently" because the code is also both
unintelligible and uncommented).  We could probably do better than that
even without any extra assumptions --- but actually, we know that the
subclauses are sorted, indeed are depending on that elsewhere in this very
same function.  So we can just scan the two lists a single time to detect
matches, as though we were doing a merge join.

Also do a re-flattening call (QTNTernary()) in tsquery_rewrite_query, just
to make sure that the tree fits the expectations of the next search cycle.
I didn't try to devise a test case for this, but I'm pretty sure that the
oversight could have led to failure to match in some cases where a match
would be expected.

Improve comments, and also stick a CHECK_FOR_INTERRUPTS into
dofindsubquery, just in case it's still too slow for somebody.

Per report from Andreas Seltenreich.  Back-patch to all supported branches.

Discussion: <[email protected]>

8 years agoFix bogus tree-flattening logic in QTNTernary().
Tom Lane [Sun, 30 Oct 2016 19:24:40 +0000 (15:24 -0400)]
Fix bogus tree-flattening logic in QTNTernary().

QTNTernary() contains logic to flatten, eg, '(a & b) & c' into 'a & b & c',
which is all well and good, but it tries to do that to NOT nodes as well,
so that '!!a' gets changed to '!a'.  Explicitly restrict the conversion to
be done only on AND and OR nodes, and add a test case illustrating the bug.

In passing, provide some comments for the sadly naked functions in
tsquery_util.c, and simplify some baroque logic in QTNFree(), which
I think may have been leaking some items it intended to free.

Noted while investigating a complaint from Andreas Seltenreich.
Back-patch to all supported versions.

8 years agoIf the stats collector dies during Hot Standby, restart it.
Robert Haas [Thu, 27 Oct 2016 18:27:40 +0000 (14:27 -0400)]
If the stats collector dies during Hot Standby, restart it.

This bug exists as far back as 9.0, when Hot Standby was introduced,
so back-patch to all supported branches.

Report and patch by Takayuki Tsunakawa, reviewed by Michael Paquier
and Kuntal Ghosh.

8 years agoFix possible pg_basebackup failure on standby with "include WAL".
Robert Haas [Thu, 27 Oct 2016 15:19:51 +0000 (11:19 -0400)]
Fix possible pg_basebackup failure on standby with "include WAL".

If a restartpoint flushed no dirty buffers, it could fail to update
the minimum recovery point, leading to a minimum recovery point prior
to the starting REDO location.  perform_base_backup() would interpret
that as meaning that no WAL files at all needed to be included in the
backup, failing an internal sanity check.  To fix, have restartpoints
always update the minimum recovery point to just after the checkpoint
record itself, so that the file (or files) containing the checkpoint
record will always be included in the backup.

Code by Amit Kapila, per a design suggestion by me, with some
additional work on the code comment by me.  Test case by Michael
Paquier.  Report by Kyotaro Horiguchi.

8 years agoFix incorrect trigger-property updating in ALTER CONSTRAINT.
Tom Lane [Wed, 26 Oct 2016 21:05:06 +0000 (17:05 -0400)]
Fix incorrect trigger-property updating in ALTER CONSTRAINT.

The code to change the deferrability properties of a foreign-key constraint
updated all the associated triggers to match; but a moment's examination of
the code that creates those triggers in the first place shows that only
some of them should track the constraint's deferrability properties.  This
leads to odd failures in subsequent exercise of the foreign key, as the
triggers are fired at the wrong times.  Fix that, and add a regression test
comparing the trigger properties produced by ALTER CONSTRAINT with those
you get by creating the constraint as-intended to begin with.

Per report from James Parks.  Back-patch to 9.4 where this ALTER
functionality was introduced.

Report: 

8 years agoFix not-HAVE_SYMLINK code in zic.c.
Tom Lane [Wed, 26 Oct 2016 17:40:41 +0000 (13:40 -0400)]
Fix not-HAVE_SYMLINK code in zic.c.

I broke this in commit f3094920a.  Apparently it's dead code anyway,
at least as far as our buildfarm is concerned (and the upstream IANA
code doesn't worry at all about symlink() not being present).
But as long as the rest of our code is willing to guard against not
having symlink(), this should too.  Noted while investigating a
tangentially-related complaint from Sandeep Thakkar.

Back-patch to keep branches in sync.

8 years agoStamp 9.4.10. REL9_4_10
Tom Lane [Mon, 24 Oct 2016 20:12:53 +0000 (16:12 -0400)]
Stamp 9.4.10.

8 years agoTranslation updates
Peter Eisentraut [Mon, 24 Oct 2016 16:00:00 +0000 (12:00 -0400)]
Translation updates

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

8 years agoRelease notes for 9.6.1, 9.5.5, 9.4.10, 9.3.15, 9.2.19, 9.1.24.
Tom Lane [Mon, 24 Oct 2016 02:13:28 +0000 (22:13 -0400)]
Release notes for 9.6.1, 9.5.5, 9.4.10, 9.3.15, 9.2.19, 9.1.24.

8 years agoAvoid testing tuple visibility without buffer lock in RI_FKey_check().
Tom Lane [Sun, 23 Oct 2016 19:01:24 +0000 (15:01 -0400)]
Avoid testing tuple visibility without buffer lock in RI_FKey_check().

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: <19391.1477244876@sss.pgh.pa.us>

8 years agoImprove documentation about use of Linux huge pages.
Tom Lane [Sat, 22 Oct 2016 18:04:51 +0000 (14:04 -0400)]
Improve documentation about use of Linux huge pages.

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.

8 years agoDoc: wording tweak for PERL, PYTHON, TCLSH configuration variables.
Tom Lane [Fri, 21 Oct 2016 15:01:35 +0000 (11:01 -0400)]
Doc: wording tweak for PERL, PYTHON, TCLSH configuration variables.

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

8 years agoFix EXPLAIN so that it doesn't emit invalid XML in corner cases.
Tom Lane [Thu, 20 Oct 2016 21:17:50 +0000 (17:17 -0400)]
Fix EXPLAIN so that it doesn't emit invalid XML in corner cases.

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: <E0BF6A45-68E8-45E6-918F-741FB332C6BB@winand.at>

8 years agoSync our copy of the timezone library with IANA release tzcode2016h.
Tom Lane [Thu, 20 Oct 2016 19:40:07 +0000 (15:40 -0400)]
Sync our copy of the timezone library with IANA release tzcode2016h.

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.

8 years agoUpdate time zone data files to tzdata release 2016h.
Tom Lane [Thu, 20 Oct 2016 19:20:11 +0000 (15:20 -0400)]
Update time zone data files to tzdata release 2016h.

(Didn't I just do this?  Oh well.)

DST law changes in Palestine.  Historical corrections for Turkey.
Switch to numeric abbreviations for Asia/Colombo.

8 years agoAnother portability fix for tzcode2016g update.
Tom Lane [Thu, 20 Oct 2016 03:32:08 +0000 (23:32 -0400)]
Another portability fix for tzcode2016g update.

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

8 years agoWindows portability fix.
Tom Lane [Wed, 19 Oct 2016 23:28:11 +0000 (19:28 -0400)]
Windows portability fix.

Per buildfarm.

8 years agoSync our copy of the timezone library with IANA release tzcode2016g.
Tom Lane [Wed, 19 Oct 2016 22:55:52 +0000 (18:55 -0400)]
Sync our copy of the timezone library with IANA release tzcode2016g.

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.

8 years agoSuppress "Factory" zone in pg_timezone_names view for tzdata >= 2016g.
Tom Lane [Wed, 19 Oct 2016 22:11:49 +0000 (18:11 -0400)]
Suppress "Factory" zone in pg_timezone_names view for tzdata >= 2016g.

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.

8 years agoUpdate time zone data files to tzdata release 2016g.
Tom Lane [Wed, 19 Oct 2016 21:56:38 +0000 (17:56 -0400)]
Update time zone data files to tzdata release 2016g.

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.

8 years agoFix WAL-logging of FSM and VM truncation.
Heikki Linnakangas [Wed, 19 Oct 2016 11:43:34 +0000 (14:43 +0300)]
Fix WAL-logging of FSM and VM truncation.

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: 

Backpatch to 9.3, where we got data checksums.

8 years agoFix cidin() to handle values above 2^31 platform-independently.
Tom Lane [Tue, 18 Oct 2016 16:24:46 +0000 (12:24 -0400)]
Fix cidin() to handle values above 2^31 platform-independently.

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: <20161018152550[email protected]>

8 years agoFix assorted integer-overflow hazards in varbit.c.
Tom Lane [Fri, 14 Oct 2016 20:28:34 +0000 (16:28 -0400)]
Fix assorted integer-overflow hazards in varbit.c.

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]>

8 years agoFix another bug in merging of inherited CHECK constraints.
Tom Lane [Thu, 13 Oct 2016 21:05:15 +0000 (17:05 -0400)]
Fix another bug in merging of inherited CHECK constraints.

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: <b28ee774-7009-313d-dd55-5bdd81242c41@lab.ntt.co.jp>

8 years agoTry to find out the actual hugepage size when making a MAP_HUGETLB request.
Tom Lane [Thu, 13 Oct 2016 19:06:46 +0000 (15:06 -0400)]
Try to find out the actual hugepage size when making a MAP_HUGETLB request.

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: <31056.1476303954@sss.pgh.pa.us>

8 years agoClean up handling of anonymous mmap'd shared-memory segment.
Tom Lane [Thu, 13 Oct 2016 17:59:56 +0000 (13:59 -0400)]
Clean up handling of anonymous mmap'd shared-memory segment.

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.

8 years agoRevert addition of PGDLLEXPORT in PG_FUNCTION_INFO_V1 macro.
Tom Lane [Wed, 12 Oct 2016 22:01:43 +0000 (18:01 -0400)]
Revert addition of PGDLLEXPORT in PG_FUNCTION_INFO_V1 macro.

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.

8 years agoProvide DLLEXPORT markers for C functions via PG_FUNCTION_INFO_V1 macro.
Tom Lane [Wed, 12 Oct 2016 16:45:50 +0000 (12:45 -0400)]
Provide DLLEXPORT markers for C functions via PG_FUNCTION_INFO_V1 macro.

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>

8 years agoFix copy-pasto in comment.
Heikki Linnakangas [Wed, 12 Oct 2016 09:07:54 +0000 (12:07 +0300)]
Fix copy-pasto in comment.

Amit Langote

8 years agoImprove documentation for CREATE RECURSIVE VIEW.
Tom Lane [Tue, 11 Oct 2016 14:08:45 +0000 (10:08 -0400)]
Improve documentation for CREATE RECURSIVE VIEW.

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]>

8 years agoIn PQsendQueryStart(), avoid leaking any left-over async result.
Tom Lane [Mon, 10 Oct 2016 14:35:58 +0000 (10:35 -0400)]
In PQsendQueryStart(), avoid leaking any left-over async result.

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: 

8 years agoFix two bugs in merging of inherited CHECK constraints.
Tom Lane [Sat, 8 Oct 2016 23:29:27 +0000 (19:29 -0400)]
Fix two bugs in merging of inherited CHECK constraints.

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: 
Discussion: <22108.1475874586@sss.pgh.pa.us>

8 years agoRemove user_relns() SRF from regression tests.
Tom Lane [Sat, 8 Oct 2016 22:43:01 +0000 (18:43 -0400)]
Remove user_relns() SRF from regression tests.

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.

8 years agoMake TAP test suites to work, when @INC does not contain current dir.
Heikki Linnakangas [Fri, 7 Oct 2016 18:48:21 +0000 (21:48 +0300)]
Make TAP test suites to work, when @INC does not contain current dir.

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: <20160908204529[email protected]>

8 years agoClear OpenSSL error queue after failed X509_STORE_load_locations() call.
Heikki Linnakangas [Fri, 7 Oct 2016 09:53:45 +0000 (12:53 +0300)]
Clear OpenSSL error queue after failed X509_STORE_load_locations() call.

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.

8 years agoDon't share SSL_CTX between libpq connections.
Heikki Linnakangas [Fri, 7 Oct 2016 09:20:39 +0000 (12:20 +0300)]
Don't share SSL_CTX between libpq connections.

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: <20160920101051[email protected]>

8 years agoCorrect logical decoding restore behaviour for subtransactions.
Andres Freund [Tue, 4 Oct 2016 05:11:36 +0000 (22:11 -0700)]
Correct logical decoding restore behaviour for subtransactions.

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: <20160909012610[email protected]>
Backport: 9,4-, where logical decoding was added

8 years agoDo ClosePostmasterPorts() earlier in SubPostmasterMain().
Tom Lane [Sat, 1 Oct 2016 21:15:10 +0000 (17:15 -0400)]
Do ClosePostmasterPorts() earlier in SubPostmasterMain().

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: <4157.1475178360@sss.pgh.pa.us>

8 years agoRetry opening new segments in pg_xlogdump --folllow
Magnus Hagander [Fri, 30 Sep 2016 09:19:30 +0000 (11:19 +0200)]
Retry opening new segments in pg_xlogdump --folllow

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.

8 years agoworker_spi: Call pgstat_report_stat.
Robert Haas [Wed, 28 Sep 2016 16:38:33 +0000 (12:38 -0400)]
worker_spi: Call pgstat_report_stat.

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

Julien Rouhaud

8 years agoInclude where needed
Alvaro Herrera [Tue, 27 Sep 2016 04:05:21 +0000 (01:05 -0300)]
Include  where needed

 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.

8 years agoDocument has_type_privilege().
Tom Lane [Mon, 26 Sep 2016 15:50:35 +0000 (11:50 -0400)]
Document has_type_privilege().

Evidently an oversight in commit 729205571.  Back-patch to 9.2 where
privileges for types were introduced.

Report: <20160922173517[email protected]>

8 years agoInstall TAP test infrastructure so it's available for extension testing.
Tom Lane [Fri, 23 Sep 2016 19:50:00 +0000 (15:50 -0400)]
Install TAP test infrastructure so it's available for extension testing.

When configured with --enable-tap-tests, "make install" will now install
the Perl support files for TAP testing where PGXS will find them.
This allows extensions to rely on $(prove_check) even when being built
out-of-tree.  Back-patch to 9.4 where we first started to support TAP
testing, to reduce the number of cases extension makefiles need to
consider.

Craig Ringer

Discussion: 

8 years agoDoc: fix examples of # operators so they actually work.
Tom Lane [Fri, 23 Sep 2016 18:22:07 +0000 (14:22 -0400)]
Doc: fix examples of # operators so they actually work.

These worked as-is until around 7.0, but fail in newer versions because
there are more operators named "#".  Besides it's a bit inconsistent that
only two of the examples on this page lack type names on their constants.

Report: <20160923081530[email protected]>

8 years agoFix incorrect logic for excluding range constructor functions in pg_dump.
Tom Lane [Fri, 23 Sep 2016 17:49:27 +0000 (13:49 -0400)]
Fix incorrect logic for excluding range constructor functions in pg_dump.

Faulty AND/OR nesting in the WHERE clause of getFuncs' SQL query led to
dumping range constructor functions if they are part of an extension
and we're in binary-upgrade mode.  Actually, we don't want to dump them
separately even then, since CREATE TYPE AS RANGE will create the range's
constructor functions regardless.  Per report from Andrew Dunstan.

It looks like this mistake was introduced by me, in commit b985d4877, in
perhaps-overzealous refactoring to reduce code duplication.  I'm suitably
embarrassed.

Report: <34854939-02d7-f591-5677-ce2994104599@dunslane.net>

8 years agoDon't trust CreateFileMapping() to clear the error code on success.
Tom Lane [Fri, 23 Sep 2016 14:09:52 +0000 (10:09 -0400)]
Don't trust CreateFileMapping() to clear the error code on success.

We must test GetLastError() even when CreateFileMapping() returns a
non-null handle.  If that value were left over from some previous system
call, we might be fooled into thinking the segment already existed.
Experimentation on Windows 7 suggests that CreateFileMapping() clears
the error code on success, but it is not documented to do so, so let's
not rely on that happening in all Windows releases.

Amit Kapila

Discussion: <20811.1474390987@sss.pgh.pa.us>

8 years agoAvoid using PostmasterRandom() for DSM control segment ID.
Tom Lane [Fri, 23 Sep 2016 13:54:11 +0000 (09:54 -0400)]
Avoid using PostmasterRandom() for DSM control segment ID.

Commits 470d886c3 et al intended to fix the problem that the postmaster
selected the same "random" DSM control segment ID on every start.  But
using PostmasterRandom() for that destroys the intended property that the
delay between random_start_time and random_stop_time will be unpredictable.
(Said delay is probably already more predictable than we could wish, but
that doesn't mean that reducing it by a couple orders of magnitude is OK.)
Revert the previous patch and add a comment warning against misuse of
PostmasterRandom.  Fix the original problem by calling srandom() early in
PostmasterMain, using a low-security seed that will later be overwritten
by PostmasterRandom.

Discussion: <20789.1474390434@sss.pgh.pa.us>

8 years agoBe sure to rewind the tuplestore read pointer in non-leader CTEScan nodes.
Tom Lane [Thu, 22 Sep 2016 15:34:44 +0000 (11:34 -0400)]
Be sure to rewind the tuplestore read pointer in non-leader CTEScan nodes.

ExecInitCteScan supposed that it didn't have to do anything to the extra
tuplestore read pointer it gets from tuplestore_alloc_read_pointer.
However, it needs this read pointer to be positioned at the start of the
tuplestore, while tuplestore_alloc_read_pointer is actually defined as
cloning the current position of read pointer 0.  In normal situations
that accidentally works because we initialize the whole plan tree at once,
before anything gets read.  But it fails in an EvalPlanQual recheck, as
illustrated in bug #14328 from Dima Pavlov.  To fix, just forcibly rewind
the pointer after tuplestore_alloc_read_pointer.  The cost of doing so is
negligible unless the tuplestore is already in TSS_READFILE state, which
wouldn't happen in normal cases.  We could consider altering tuplestore's
API to make that case cheaper, but that would make for a more invasive
back-patch and it doesn't seem worth it.

This has been broken probably for as long as we've had CTEs, so back-patch
to all supported branches.

Discussion: <32468.1474548308@sss.pgh.pa.us>

8 years agoFix pgbench's calculation of average latency, when -T is not used.
Heikki Linnakangas [Wed, 21 Sep 2016 10:14:48 +0000 (13:14 +0300)]
Fix pgbench's calculation of average latency, when -T is not used.

If the test duration was given in # of transactions (-t or no option),
rather as a duration (-T), the latency average was always printed as 0.
It has been broken ever since the display of latency average was added,
in 9.4.

Fabien Coelho

Discussion: 1607131015370.7486@sto>

8 years agodoc: Fix documentation to match actual make output
Peter Eisentraut [Tue, 20 Sep 2016 16:00:00 +0000 (12:00 -0400)]
doc: Fix documentation to match actual make output

based on patch from Takeshi Ideriha 

8 years agodoc: Correct ALTER USER MAPPING example
Peter Eisentraut [Tue, 20 Sep 2016 16:00:00 +0000 (12:00 -0400)]
doc: Correct ALTER USER MAPPING example

The existing example threw an error.

From: gabrielle 

8 years agoUse PostmasterRandom(), not random(), for DSM control segment ID.
Robert Haas [Tue, 20 Sep 2016 16:24:44 +0000 (12:24 -0400)]
Use PostmasterRandom(), not random(), for DSM control segment ID.

Otherwise, every startup gets the same "random" value, which is
definitely not what was intended.

8 years agoRetry DSM control segment creation if Windows indicates access denied.
Robert Haas [Tue, 20 Sep 2016 16:04:41 +0000 (12:04 -0400)]
Retry DSM control segment creation if Windows indicates access denied.

Otherwise, attempts to run multiple postmasters running on the same
machine may fail, because Windows sometimes returns ERROR_ACCESS_DENIED
rather than ERROR_ALREADY_EXISTS when there is an existing segment.

Hitting this bug is much more likely because of another defect not
fixed by this patch, namely that dsm_postmaster_startup() uses
random() which returns the same value every time.  But that's not
a reason not to fix this.

Kyotaro Horiguchi and Amit Kapila, reviewed by Michael Paquier

Discussion: 

8 years agoFix latency calculation when there are \sleep commands in the script.
Heikki Linnakangas [Mon, 19 Sep 2016 19:55:43 +0000 (22:55 +0300)]
Fix latency calculation when there are \sleep commands in the script.

We can't use txn_scheduled to hold the sleep-until time for \sleep, because
that interferes with calculation of the latency of the transaction as whole.

Backpatch to 9.4, where this bug was introduced.

Fabien COELHO

Discussion: 1608231622170.7102@lancre>

8 years agoMSVC: Include pg_recvlogical in client-only install.
Robert Haas [Mon, 19 Sep 2016 18:21:48 +0000 (14:21 -0400)]
MSVC: Include pg_recvlogical in client-only install.

MauMau, reviewed by Michael Paquier

8 years agoFix ecpg -? option on Windows, add -V alias for --version.
Heikki Linnakangas [Sun, 18 Sep 2016 10:46:32 +0000 (13:46 +0300)]
Fix ecpg -? option on Windows, add -V alias for --version.

This makes the -? and -V options work consistently with other binaries.
--help and --version are now only recognized as the first option, i.e.
"ecpg --foobar --help" no longer prints the help, but that's consistent
with most of our other binaries, too.

Backpatch to all supported versions.

Haribabu Kommi

Discussion: 

8 years agopg_buffercache: Allow huge allocations.
Robert Haas [Thu, 15 Sep 2016 13:22:52 +0000 (09:22 -0400)]
pg_buffercache: Allow huge allocations.

Otherwise, users who have configured shared_buffers >= 256GB won't
be able to use this module.  There probably aren't many of those, but
it doesn't hurt anything to fix it so that it works.

Backpatch to 9.4, where MemoryContextAllocHuge was introduced.  The
same problem exists in older branches, but there's no easy way to
fix it there.

KaiGai Kohei

8 years agoFix copy/pasto in file identification
Simon Riggs [Mon, 12 Sep 2016 08:02:46 +0000 (09:02 +0100)]
Fix copy/pasto in file identification

Daniel Gustafsson

8 years agoImprove unreachability recognition in elog() macro.
Tom Lane [Sat, 10 Sep 2016 21:54:23 +0000 (17:54 -0400)]
Improve unreachability recognition in elog() macro.

Some experimentation with an older version of gcc showed that it is able
to determine whether "if (elevel_ >= ERROR)" is compile-time constant
if elevel_ is declared "const", but otherwise not so much.  We had
accounted for that in ereport() but were too miserly with braces to
make it so in elog().  I don't know how many currently-interesting
compilers have the same quirk, but in case it will save some code
space, let's make sure that elog() is on the same footing as ereport()
for this purpose.

Back-patch to 9.3 where we introduced pg_unreachable() calls into
elog/ereport.

8 years agoFix miserable coding in pg_stat_get_activity().
Tom Lane [Sat, 10 Sep 2016 17:49:04 +0000 (13:49 -0400)]
Fix miserable coding in pg_stat_get_activity().

Commit dd1a3bccc replaced a test on whether a subroutine returned a
null pointer with a test on whether &pointer->backendStatus was null.
This accidentally failed to fail, at least on common compilers, because
backendStatus is the first field in the struct; but it was surely trouble
waiting to happen.  Commit f91feba87 then messed things up further,
changing the logic to

local_beentry = pgstat_fetch_stat_local_beentry(curr_backend);
if (!local_beentry)
continue;
beentry = &local_beentry->backendStatus;
if (!beentry)
{

where the second "if" is now dead code, so that the intended behavior of
printing a row with "" cannot occur.

I suspect this is all moot because pgstat_fetch_stat_local_beentry
will never actually return null in this function's usage, but it's still
very poor coding.  Repair back to 9.4 where the original problem was
introduced.

8 years agoFix locking a tuple updated by an aborted (sub)transaction
Alvaro Herrera [Fri, 9 Sep 2016 18:54:29 +0000 (15:54 -0300)]
Fix locking a tuple updated by an aborted (sub)transaction

When heap_lock_tuple decides to follow the update chain, it tried to
also lock any version of the tuple that was created by an update that
was subsequently rolled back.  This is pointless, since for all intents
and purposes that tuple exists no more; and moreover it causes
misbehavior, as reported independently by Marko Tiikkaja and Marti
Raudsepp: some SELECT FOR UPDATE/SHARE queries may fail to return
the tuples, and assertion-enabled builds crash.

Fix by having heap_lock_updated_tuple test the xmin and return success
immediately if the tuple was created by an aborted transaction.

The condition where tuples become invisible occurs when an updated tuple
chain is followed by heap_lock_updated_tuple, which reports the problem
as HeapTupleSelfUpdated to its caller heap_lock_tuple, which in turn
propagates that code outwards possibly leading the calling code
(ExecLockRows) to believe that the tuple exists no longer.

Backpatch to 9.3.  Only on 9.5 and newer this leads to a visible
failure, because of commit 27846f02c176; before that, heap_lock_tuple
skips the whole dance when the tuple is already locked by the same
transaction, because of the ancient HeapTupleSatisfiesUpdate behavior.
Still, the buggy condition may also exist in more convoluted scenarios
involving concurrent transactions, so it seems safer to fix the bug in
the old branches too.

Discussion:
https://www.postgresql.org/message-id/CABRT9RC81YUf1=jsmWopcKJEro=VoeG2ou6sPwyOUTx_qteRsg@mail.gmail.com
https://www.postgresql.org/message-id/48d3eade-98d3-8b9a-477e-1a8dc32a724d@joh.to

8 years agoFix VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL
Simon Riggs [Fri, 9 Sep 2016 10:44:54 +0000 (11:44 +0100)]
Fix VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL

lazy_truncate_heap() was waiting for
VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL, but in microseconds
not milliseconds as originally intended.

Found by code inspection.

Simon Riggs

8 years agoFix mdtruncate() to close fd.c handle of deleted segments.
Andres Freund [Thu, 8 Sep 2016 23:51:09 +0000 (16:51 -0700)]
Fix mdtruncate() to close fd.c handle of deleted segments.

mdtruncate() forgot to FileClose() a segment's mdfd_vfd, when deleting
it. That lead to a fd.c handle to a truncated file being kept open until
backend exit.

The issue appears to have been introduced way back in 1a5c450f3024ac5,
before that the handle was closed inside FileUnlink().

The impact of this bug is limited - only VACUUM and ON COMMIT TRUNCATE
for temporary tables, truncate files in place (i.e. TRUNCATE itself is
not affected), and the relation has to be bigger than 1GB. The
consequences of a leaked fd.c handle aren't severe either.

Discussion: <20160908220748[email protected]>
Backpatch: all supported releases

8 years agoDoc: small improvements for documentation about VACUUM freezing.
Tom Lane [Tue, 6 Sep 2016 21:50:53 +0000 (17:50 -0400)]
Doc: small improvements for documentation about VACUUM freezing.

Mostly, explain how row xmin's used to be replaced by FrozenTransactionId
and no longer are.  Do a little copy-editing on the side.

Per discussion with Egor Rogov.  Back-patch to 9.4 where the behavioral
change occurred.

Discussion: <575D7955.6060209@postgrespro.ru>

8 years agoAdd regression test coverage for non-default timezone abbreviation sets.
Tom Lane [Mon, 5 Sep 2016 00:02:16 +0000 (20:02 -0400)]
Add regression test coverage for non-default timezone abbreviation sets.

After further reflection about the mess cleaned up in commit 39b691f25,
I decided the main bit of test coverage that was still missing was to
check that the non-default abbreviation-set files we supply are usable.
Add that.

Back-patch to supported branches, just because it seems like a good
idea to keep this all in sync.

8 years agoRemove vestigial references to "zic" in favor of "IANA database".
Tom Lane [Sun, 4 Sep 2016 23:42:08 +0000 (19:42 -0400)]
Remove vestigial references to "zic" in favor of "IANA database".

Commit b2cbced9e instituted a policy of referring to the timezone database
as the "IANA timezone database" in our user-facing documentation.
Propagate that wording into a couple of places that were still using "zic"
to refer to the database, which is definitely not right (zic is the
compilation tool, not the data).

Back-patch, not because this is very important in itself, but because
we routinely cherry-pick updates to the tznames files and I don't want
to risk future merge failures.

8 years agoFix corrupt GIN_SEGMENT_ADDITEMS WAL records on big-endian hardware.
Tom Lane [Sat, 3 Sep 2016 17:28:53 +0000 (13:28 -0400)]
Fix corrupt GIN_SEGMENT_ADDITEMS WAL records on big-endian hardware.

computeLeafRecompressWALData() tried to produce a uint16 WAL log field by
memcpy'ing the first two bytes of an int-sized variable.  That accidentally
works on little-endian hardware, but not at all on big-endian.  Replay then
thinks it's looking at an ADDITEMS action with zero entries, and reads the
first two bytes of the first TID therein as the next segno/action,
typically leading to "unexpected GIN leaf action" errors during replay.
Even if replay failed to crash, the resulting GIN index page would surely
be incorrect.  To fix, just declare the variable as uint16 instead.

Per bug #14295 from Spencer Thomason (much thanks to Spencer for turning
his problem into a self-contained test case).  This likely also explains
a previous report of the same symptom from Bernd Helmle.

Back-patch to 9.4 where the problem was introduced (by commit 14d02f0bb).

Discussion: <20160826072658[email protected]>
Possible-Report: <2DA7350F7296B2A142272901@eje.land.credativ.lan>

8 years agoDon't require dynamic timezone abbreviations to match underlying time zone.
Tom Lane [Fri, 2 Sep 2016 21:29:32 +0000 (17:29 -0400)]
Don't require dynamic timezone abbreviations to match underlying time zone.

Previously, we threw an error if a dynamic timezone abbreviation did not
match any abbreviation recorded in the referenced IANA time zone entry.
That seemed like a good consistency check at the time, but it turns out
that a number of the abbreviations in the IANA database are things that
Olson and crew made up out of whole cloth.  Their current policy is to
remove such names in favor of using simple numeric offsets.  Perhaps
unsurprisingly, a lot of these made-up abbreviations have varied in meaning
over time, which meant that our commit b2cbced9e and later changes made
them into dynamic abbreviations.  So with newer IANA database versions
that don't mention these abbreviations at all, we fail, as reported in bug
#14307 from Neil Anderson.  It's worse than just a few unused-in-the-wild
abbreviations not working, because the pg_timezone_abbrevs view stops
working altogether (since its underlying function tries to compute the
whole view result in one call).

We considered deleting these abbreviations from our abbreviations list, but
the problem with that is that we can't stay ahead of possible future IANA
changes.  Instead, let's leave the abbreviations list alone, and treat any
"orphaned" dynamic abbreviation as just meaning the referenced time zone.
It will behave a bit differently than it used to, in that you can't any
longer override the zone's standard vs. daylight rule by using the "wrong"
abbreviation of a pair, but that's better than failing entirely.  (Also,
this solution can be interpreted as adding a small new feature, which is
that any abbreviation a user wants can be defined as referencing a time
zone name.)

Back-patch to all supported branches, since this problem affects all
of them when using tzdata 2016f or newer.

Report: <20160902031551[email protected]>
Discussion: <6189.1472820913@sss.pgh.pa.us>

8 years agoSuppress GCC 6 warning about self-comparison
Tom Lane [Thu, 1 Sep 2016 16:48:06 +0000 (12:48 -0400)]
Suppress GCC 6 warning about self-comparison

Back-patch commit a2fd62dd53fb606dee69e0f4eb12289c87f5c8b1
into older branches.  Per complaint from Pavel Stehule.

8 years agoFix minor 9.4-only bug in logical decoding.
Tom Lane [Thu, 1 Sep 2016 15:45:16 +0000 (11:45 -0400)]
Fix minor 9.4-only bug in logical decoding.

The 9.4 version of log_heap_update() stores the full length of the
old_key_tuple in xlhdr.t_len, rather than the length less
offsetof(HeapTupleHeaderData, t_bits) as is customary elsewhere.
DecodeUpdate() was expecting the usual definition, which caused it
to copy 23 bytes too much out of the WAL-file buffer.  Most of the
time that's harmless, although Valgrind on buildfarm member skink
has been complaining about it, and there was one actual SIGSEGV
on curculio that might have been caused by this.  The reconstructed
key tuple would also have an over-length t_len, though offhand
I do not see how that could cause any visible problem.

The problem seems to have disappeared in the 9.5 WAL format changes
(commit 2c03216d8), so we only need to fix 9.4.

Discussion: <11035.1469058247@sss.pgh.pa.us>

8 years agoPrevent starting a standalone backend with standby_mode on.
Tom Lane [Wed, 31 Aug 2016 12:52:13 +0000 (08:52 -0400)]
Prevent starting a standalone backend with standby_mode on.

This can't really work because standby_mode expects there to be more
WAL arriving, which there will not ever be because there's no WAL
receiver process to fetch it.  Moreover, if standby_mode is on then
hot standby might also be turned on, causing even more strangeness
because that expects read-only sessions to be executing in parallel.
Bernd Helmle reported a case where btree_xlog_delete_get_latestRemovedXid
got confused, but rather than band-aiding individual problems it seems
best to prevent getting anywhere near this state in the first place.
Back-patch to all supported branches.

In passing, also fix some omissions of errcodes in other ereport's in
readRecoveryCommandFile().

Michael Paquier (errcode hacking by me)

Discussion: <00F0B2CEF6D0CEF8A90119D4@eje.credativ.lan>

8 years agoFix pg_xlogdump so that it handles cross-page XLP_FIRST_IS_CONTRECORD record.
Fujii Masao [Mon, 29 Aug 2016 05:34:58 +0000 (14:34 +0900)]
Fix pg_xlogdump so that it handles cross-page XLP_FIRST_IS_CONTRECORD record.

Previously pg_xlogdump failed to dump the contents of the WAL file
if the file starts with the continuation WAL record which spans
more than one pages. Since pg_xlogdump assumed that the continuation
record always fits on a page, it could not find the valid WAL record to
start reading from in that case.

This patch changes pg_xlogdump so that it can handle a continuation
WAL record which crosses a page boundary and find the valid record
to start reading from.

Back-patch to 9.3 where pg_xlogdump was introduced.

Author: Pavan Deolasee
Reviewed-By: Michael Paquier and Craig Ringer
Discussion: CABOikdPsPByMiG6J01DKq6om2+BNkxHTPkOyqHM2a4oYwGKsqQ@mail.gmail.com

8 years agoFix instability in parallel regression tests.
Tom Lane [Thu, 25 Aug 2016 13:57:09 +0000 (09:57 -0400)]
Fix instability in parallel regression tests.

Commit f0c7b789a added a test case in case.sql that creates and then drops
both an '=' operator and the type it's for.  Given the right timing, that
can cause a "cache lookup failed for type" failure in concurrent sessions,
which see the '=' operator as a potential match for '=' in a query, but
then the type is gone by the time they inquire into its properties.
It might be nice to make that behavior more robust someday, but as a
back-patchable solution, adjust the new test case so that the operator
is never visible to other sessions.  Like the previous commit, back-patch
to all supported branches.

Discussion: <5983.1471371667@sss.pgh.pa.us>

8 years agoFix small query-lifespan memory leak in bulk updates.
Tom Lane [Thu, 25 Aug 2016 02:20:01 +0000 (22:20 -0400)]
Fix small query-lifespan memory leak in bulk updates.

When there is an identifiable REPLICA IDENTITY index on the target table,
heap_update leaks the id_attrs bitmapset.  That's not many bytes, but it
adds up over enough rows, since the code typically runs in a query-lifespan
context.  Bug introduced in commit e55704d8b, which did a rather poor job
of cloning the existing use-pattern for RelationGetIndexAttrBitmap().

Per bug #14293 from Zhou Digoal.  Back-patch to 9.4 where the bug was
introduced.

Report: <20160824114320[email protected]>

8 years agoFix improper repetition of previous results from a hashed aggregate.
Tom Lane [Wed, 24 Aug 2016 18:37:51 +0000 (14:37 -0400)]
Fix improper repetition of previous results from a hashed aggregate.

ExecReScanAgg's check for whether it could re-use a previously calculated
hashtable neglected the possibility that the Agg node might reference
PARAM_EXEC Params that are not referenced by its input plan node.  That's
okay if the Params are in upper tlist or qual expressions; but if one
appears in aggregate input expressions, then the hashtable contents need
to be recomputed when the Param's value changes.

To avoid unnecessary performance degradation in the case of a Param that
isn't within an aggregate input, add logic to the planner to determine
which Params are within aggregate inputs.  This requires a new field in
struct Agg, but fortunately we never write plans to disk, so this isn't
an initdb-forcing change.

Per report from Jeevan Chalke.  This has been broken since forever,
so back-patch to all supported branches.

Andrew Gierth, with minor adjustments by me

Report: 

8 years agoreorderbuffer: preserve errno while reporting error
Alvaro Herrera [Fri, 19 Aug 2016 17:38:55 +0000 (14:38 -0300)]
reorderbuffer: preserve errno while reporting error

Clobbering errno during cleanup after an error is an oft-repeated, easy
to make mistake.  Deal with it here as everywhere else, by saving it
aside and restoring after cleanup, before ereport'ing.

In passing, add a missing errcode declaration in another ereport() call
in the same file, which I noticed while skimming the file looking for
similar problems.

Backpatch to 9.4, where this code was introduced.

8 years agoUpdate Windows timezone mapping from Windows 7 and 10
Magnus Hagander [Thu, 18 Aug 2016 10:32:42 +0000 (12:32 +0200)]
Update Windows timezone mapping from Windows 7 and 10

This adds a couple of new timezones that are present in the newer
versions of Windows. It also updates comments to reference UTC rather
than GMT, as this change has been made in Windows.

Michael Paquier

8 years agoProperly re-initialize replication slot shared memory upon creation.
Andres Freund [Wed, 17 Aug 2016 20:15:04 +0000 (13:15 -0700)]
Properly re-initialize replication slot shared memory upon creation.

Slot creation did not clear all fields upon creation. After start the
memory is zeroed, but when a physical replication slot was created in
the shared memory of a previously existing logical slot, catalog_xmin
would not be cleared. That in turn would prevent vacuum from doing its
duties.

To fix initialize all the fields. To make similar future bugs less
likely, zero all of ReplicationSlotPersistentData, and re-order the
rest of the initialization to be in struct member order.

Analysis: Andrew Gierth
Reported-By: [email protected]
Author: Michael Paquier
Discussion: <20160705173502[email protected]>
Backpatch: 9.4, where replication slots were introduced

8 years agoFix -e option in contrib/intarray/bench/bench.pl.
Tom Lane [Wed, 17 Aug 2016 19:51:10 +0000 (15:51 -0400)]
Fix -e option in contrib/intarray/bench/bench.pl.

As implemented, -e ran an EXPLAIN but then discarded the output, which
certainly seems pointless.  Make it print to stdout instead.  It's been
like that forever, so back-patch to all supported branches.

Daniel Gustafsson, reviewed by Andreas Scherbaum

Patch: <B97BDCB7-A3B3-4734-90B5-EDD586941629@yesql.se>

8 years agoFix assorted places in psql to print version numbers >= 10 in new style.
Tom Lane [Tue, 16 Aug 2016 19:58:31 +0000 (15:58 -0400)]
Fix assorted places in psql to print version numbers >= 10 in new style.

This is somewhat cosmetic, since as long as you know what you are looking
at, "10.0" is a serviceable substitute for "10".  But there is a potential
for confusion between version numbers with minor numbers and those without
--- we don't want people asking "why is psql saying 10.0 when my server is
10.2".  Therefore, back-patch as far as practical, which turns out to be
9.3.  I could have redone the patch to use fprintf(stderr) in place of
psql_error(), but it seems more work than is warranted for branches that
will be EOL or nearly so by the time v10 comes out.

Although only psql seems to contain any code that needs this, I chose
to put the support function into fe_utils, since it seems likely we'll
need it in other client programs in future.  (In 9.3-9.5, use dumputils.c,
the predecessor of fe_utils/string_utils.c.)

In HEAD, also fix the backend code that whines about loadable-library
version mismatch.  I don't see much need to back-patch that.