postgresql
8 years agoLog the creation of an init fork unconditionally.
Robert Haas [Thu, 8 Dec 2016 19:09:09 +0000 (14:09 -0500)]
Log the creation of an init fork unconditionally.

Previously, it was thought that this only needed to be done for the
benefit of possible standbys, so wal_level = minimal skipped it.
But that's not safe, because during crash recovery we might replay
XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record which recursively
removes the directory that contains the new init fork.  So log it
always.

The user-visible effect of this bug is that if you create a database
or tablespace, then create an unlogged table, then crash without
checkpointing, then restart, accessing the table will fail, because
the it won't have been properly reset.  This commit fixes that.

Michael Paquier, per a report from Konstantin Knizhnik.  Wording of
the comments per a suggestion from me.

8 years agoRestore psql's SIGPIPE setting if popen() fails.
Tom Lane [Wed, 7 Dec 2016 17:39:24 +0000 (12:39 -0500)]
Restore psql's SIGPIPE setting if popen() fails.

Ancient oversight in PageOutput(): if popen() fails, we'd better reset
the SIGPIPE handler before returning stdout, because ClosePager() won't.
Noticed while fixing the empty-PAGER issue.

8 years agoHandle empty or all-blank PAGER setting more sanely in psql.
Tom Lane [Wed, 7 Dec 2016 17:19:57 +0000 (12:19 -0500)]
Handle empty or all-blank PAGER setting more sanely in psql.

If the PAGER environment variable is set but contains an empty string,
psql would pass it to "sh" which would silently exit, causing whatever
query output we were printing to vanish entirely.  This is quite
mystifying; it took a long time for us to figure out that this was the
cause of Joseph Brenner's trouble report.  Rather than allowing that
to happen, we should treat this as another way to specify "no pager".
(We could alternatively treat it as selecting the default pager, but
it seems more likely that the former is what the user meant to achieve
by setting PAGER this way.)

Nonempty, but all-white-space, PAGER values have the same behavior, and
it's pretty easy to test for that, so let's handle that case the same way.

Most other cases of faulty PAGER values will result in the shell printing
some kind of complaint to stderr, which should be enough to diagnose the
problem, so we don't need to work harder than this.  (Note that there's
been an intentional decision not to be very chatty about apparent failure
returns from the pager process, since that may happen if, eg, the user
quits the pager with control-C or some such.  I'd just as soon not start
splitting hairs about which exit codes might merit making our own report.)

libpq's old PQprint() function was already on board with ignoring empty
PAGER values, but for consistency, make it ignore all-white-space values
as well.

It's been like this a long time, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAFfgvXWLOE2novHzYjmQK8-J6TmHz42G8f3X0SORM44+stUGmw@mail.gmail.com

8 years agoMake pgwin32_putenv() visit debug CRTs.
Noah Misch [Sat, 3 Dec 2016 20:46:36 +0000 (15:46 -0500)]
Make pgwin32_putenv() visit debug CRTs.

This has no effect in the most conventional case, where no relevant DLL
uses a debug build.  For an example where it does matter, given a debug
build of MIT Kerberos, the krb_server_keyfile parameter usually had no
effect.  Since nobody wants a Heisenbug, back-patch to 9.2 (all
supported versions).

Christian Ullrich, reviewed by Michael Paquier.

8 years agoRemove wrong CloseHandle() call.
Noah Misch [Sat, 3 Dec 2016 20:46:35 +0000 (15:46 -0500)]
Remove wrong CloseHandle() call.

In accordance with its own documentation, invoke CloseHandle() only when
directed in the documentation for the function that furnished the
handle.  GetModuleHandle() does not so direct.  We have been issuing
this call only in the rare event that a CRT DLL contains no "_putenv"
symbol, so lack of bug reports is uninformative.  Back-patch to 9.2 (all
supported versions).

Christian Ullrich, reviewed by Michael Paquier.

8 years agoRefine win32env.c cosmetics.
Noah Misch [Sat, 3 Dec 2016 20:46:35 +0000 (15:46 -0500)]
Refine win32env.c cosmetics.

Replace use of plain 0 as a null pointer constant.  In comments, update
terminology and lessen redundancy.  Back-patch to 9.2 (all supported
versions) for the convenience of back-patching the next two commits.

Christian Ullrich and Noah Misch, reviewed (in earlier versions) by
Michael Paquier.

8 years agoDoc: improve description of trim() and related functions.
Tom Lane [Wed, 30 Nov 2016 18:34:14 +0000 (13:34 -0500)]
Doc: improve description of trim() and related functions.

Per bug #14441 from Mark Pether, the documentation could be misread,
mainly because some of the examples failed to show what happens with
a multicharacter "characters to trim" string.  Also, while the text
description in most of these entries was fairly clear that the
"characters" argument is a set of characters not a substring to match,
some of them used variant wording that was a bit less clear.
trim() itself suffered from both deficiencies and was thus pretty
misinterpretable.

Also fix failure to explain which of LEADING/TRAILING/BOTH is the
default.

Discussion: https://postgr.es/m/20161130011710.6539.53657@wrigleys.postgresql.org

8 years agoClarify pg_dump -b documentation
Stephen Frost [Tue, 29 Nov 2016 15:35:16 +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.GA13284@tamriel.snowman.net

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.6529.33039@wrigleys.postgresql.org>
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.6530.31200@wrigleys.postgresql.org>

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.post@n3.nabble.com>

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 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: <CAB7nPqRMbGqa_mesopcn4MPyTs34eqtVEK7ELYxvvV=oqS00YA@mail.gmail.com>

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 agoFix typo
Magnus Hagander [Tue, 8 Nov 2016 17:34:59 +0000 (18:34 +0100)]
Fix typo

8 years agoRationalize and document pltcl's handling of magic ".tupno" array element.
Tom Lane [Sun, 6 Nov 2016 19:43:14 +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 master.

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: <CANFyU94_p6mqRQc2i26PFp5QAOQGB++AjGX=FO8LDpXw0GSTjw@mail.gmail.com>
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: <8760oasf2y.fsf@credativ.de>

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 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.2.19. REL9_2_19
Tom Lane [Mon, 24 Oct 2016 20:17:41 +0000 (16:17 -0400)]
Stamp 9.2.19.

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: 4536e21f450b2265a262f9f9acf912ae5416a1ee

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:29 +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 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 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.1413.6439@wrigleys.postgresql.org>

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: <87d1j2zqtz.fsf@credativ.de>

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 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 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: <CAD-Qf1eLUtBOTPXyFQGW-4eEsop31tVVdZPu4kL9pbQ6tJPO8g@mail.gmail.com>

8 years agoFix two bugs in merging of inherited CHECK constraints.
Tom Lane [Sat, 8 Oct 2016 23:29:28 +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: <CADbMkNPT-Jz5PRSQ4RbUASYAjocV_KHUWapR%2Bg8fNvhUAyRpxA%40mail.gmail.com>
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 agoClear OpenSSL error queue after failed X509_STORE_load_locations() call.
Heikki Linnakangas [Fri, 7 Oct 2016 09:53:49 +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.1355.79453@wrigleys.postgresql.org>

8 years agoInclude <sys/select.h> where needed
Alvaro Herrera [Tue, 27 Sep 2016 04:05:21 +0000 (01:05 -0300)]
Include <sys/select.h> where needed

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

Per bug #14334 by Sean Farrell.

8 years agoDocument has_type_privilege().
Tom Lane [Mon, 26 Sep 2016 15:50:36 +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.8214.88959@wrigleys.postgresql.org>

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.1517.75670@wrigleys.postgresql.org>

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 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 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 <iderihatakeshi@gmail.com>

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 <gorthx@gmail.com>

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: <CAJrrPGfnRXvmCzxq6Dy=stAWebfNHxiL+Y_z7uqksZUCkW_waQ@mail.gmail.com>

8 years agoFix VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL
Simon Riggs [Fri, 9 Sep 2016 10:45:40 +0000 (11:45 +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.oqh37ukwqqncbl3n@alap3.anarazel.de>
Backpatch: all supported releases

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 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.15674.67337@wrigleys.postgresql.org>
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 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 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 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: <CAM2+6=VY8ykfLT5Q8vb9B6EbeBk-NGuLbT6seaQ+Fq4zXvrDcA@mail.gmail.com>

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 agoFix -e option in contrib/intarray/bench/bench.pl.
Tom Lane [Wed, 17 Aug 2016 19:51:11 +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 agoRemove bogus dependencies on NUMERIC_MAX_PRECISION.
Tom Lane [Sun, 14 Aug 2016 19:06:02 +0000 (15:06 -0400)]
Remove bogus dependencies on NUMERIC_MAX_PRECISION.

NUMERIC_MAX_PRECISION is a purely arbitrary constraint on the precision
and scale you can write in a numeric typmod.  It might once have had
something to do with the allowed range of a typmod-less numeric value,
but at least since 9.1 we've allowed, and documented that we allowed,
any value that would physically fit in the numeric storage format;
which is something over 100000 decimal digits, not 1000.

Hence, get rid of numeric_in()'s use of NUMERIC_MAX_PRECISION as a limit
on the allowed range of the exponent in scientific-format input.  That was
especially silly in view of the fact that you can enter larger numbers as
long as you don't use 'e' to do it.  Just constrain the value enough to
avoid localized overflow, and let make_result be the final arbiter of what
is too large.  Likewise adjust ecpg's equivalent of this code.

Also get rid of numeric_recv()'s use of NUMERIC_MAX_PRECISION to limit the
number of base-NBASE digits it would accept.  That created a dump/restore
hazard for binary COPY without doing anything useful; the wire-format
limit on number of digits (65535) is about as tight as we would want.

In HEAD, also get rid of pg_size_bytes()'s unnecessary intimacy with what
the numeric range limit is.  That code doesn't exist in the back branches.

Per gripe from Aravind Kumar.  Back-patch to all supported branches,
since they all contain the documentation claim about allowed range of
NUMERIC (cf commit cabf5d84b).

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

8 years agoFix regression test parallel-make hazard.
Tom Lane [Sat, 13 Aug 2016 00:51:59 +0000 (20:51 -0400)]
Fix regression test parallel-make hazard.

Back-patch 9.4-era commit 384f933046dc9e9a2b416f5f7b3be30b93587c63 into
the previous branches.  Although that was only advertised as repairing a
problem with missed header-file dependencies, it turns out to also be
important for parallel make safety.  The previous coding allowed two
independent make jobs to get launched concurrently in contrib/spi.
Normally this would be OK, because they are building independent targets;
but if --enable-depend is in use, it's unsafe, because one make run might
try to read a .deps file that the other one is in process of rewriting.
This is evidently the cause of buildfarm member francolin's recent failure
in the 9.2 branch.  I believe this patch will result in only one subsidiary
make run, making it safe(r).

Report: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=francolin&dt=2016-08-12%2017%3A12%3A52

8 years agoFix inappropriate printing of never-measured times in EXPLAIN.
Tom Lane [Fri, 12 Aug 2016 16:13:04 +0000 (12:13 -0400)]
Fix inappropriate printing of never-measured times in EXPLAIN.

EXPLAIN (ANALYZE, TIMING OFF) would print an elapsed time of zero for
a trigger function, because no measurement has been taken but it printed
the field anyway.  This isn't what EXPLAIN does elsewhere, so suppress it.

In the same vein, EXPLAIN (ANALYZE, BUFFERS) with non-text output format
would print buffer I/O timing numbers even when no measurement has been
taken because track_io_timing is off.  That seems not per policy, either,
so change it.

Back-patch to 9.2 where these features were introduced.

Maksim Milyutin

Discussion: <081c0540-ecaa-bd29-3fd2-6358f3b359a9@postgrespro.ru>

8 years agoDoc: write some for adminpack.
Tom Lane [Thu, 11 Aug 2016 01:39:50 +0000 (21:39 -0400)]
Doc: write some for adminpack.

Previous contents of adminpack.sgml were rather far short of project norms.
Not to mention being outright wrong about the signature of pg_file_read().

8 years agoFix typo
Peter Eisentraut [Tue, 9 Aug 2016 23:07:24 +0000 (19:07 -0400)]
Fix typo

8 years agoDoc: clarify description of CREATE/ALTER FUNCTION ... SET FROM CURRENT.
Tom Lane [Tue, 9 Aug 2016 17:39:24 +0000 (13:39 -0400)]
Doc: clarify description of CREATE/ALTER FUNCTION ... SET FROM CURRENT.

Per discussion with David Johnston.

8 years agoStamp 9.2.18. REL9_2_18
Tom Lane [Mon, 8 Aug 2016 20:33:27 +0000 (16:33 -0400)]
Stamp 9.2.18.

8 years agoLast-minute updates for release notes.
Tom Lane [Mon, 8 Aug 2016 15:56:10 +0000 (11:56 -0400)]
Last-minute updates for release notes.

Security: CVE-2016-5423, CVE-2016-5424

8 years agoFix several one-byte buffer over-reads in to_number
Peter Eisentraut [Mon, 8 Aug 2016 15:12:59 +0000 (11:12 -0400)]
Fix several one-byte buffer over-reads in to_number

Several places in NUM_numpart_from_char(), which is called from the SQL
function to_number(text, text), could accidentally read one byte past
the end of the input buffer (which comes from the input text datum and
is not null-terminated).

1. One leading space character would be skipped, but there was no check
   that the input was at least one byte long.  This does not happen in
   practice, but for defensiveness, add a check anyway.

2. Commit 4a3a1e2cf apparently accidentally doubled that code that skips
   one space character (so that two spaces might be skipped), but there
   was no overflow check before skipping the second byte.  Fix by
   removing that duplicate code.

3. A logic error would allow a one-byte over-read when looking for a
   trailing sign (S) placeholder.

In each case, the extra byte cannot be read out directly, but looking at
it might cause a crash.

The third item was discovered by Piotr Stefaniak, the first two were
found and analyzed by Tom Lane and Peter Eisentraut.

8 years agoTranslation updates
Peter Eisentraut [Mon, 8 Aug 2016 14:48:56 +0000 (10:48 -0400)]
Translation updates

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

8 years agoFix two errors with nested CASE/WHEN constructs.
Tom Lane [Mon, 8 Aug 2016 14:33:47 +0000 (10:33 -0400)]
Fix two errors with nested CASE/WHEN constructs.

ExecEvalCase() tried to save a cycle or two by passing
&econtext->caseValue_isNull as the isNull argument to its sub-evaluation of
the CASE value expression.  If that subexpression itself contained a CASE,
then *isNull was an alias for econtext->caseValue_isNull within the
recursive call of ExecEvalCase(), leading to confusion about whether the
inner call's caseValue was null or not.  In the worst case this could lead
to a core dump due to dereferencing a null pointer.  Fix by not assigning
to the global variable until control comes back from the subexpression.
Also, avoid using the passed-in isNull pointer transiently for evaluation
of WHEN expressions.  (Either one of these changes would have been
sufficient to fix the known misbehavior, but it's clear now that each of
these choices was in itself dangerous coding practice and best avoided.
There do not seem to be any similar hazards elsewhere in execQual.c.)

Also, it was possible for inlining of a SQL function that implements the
equality operator used for a CASE comparison to result in one CASE
expression's CaseTestExpr node being inserted inside another CASE
expression.  This would certainly result in wrong answers since the
improperly nested CaseTestExpr would be caused to return the inner CASE's
comparison value not the outer's.  If the CASE values were of different
data types, a crash might result; moreover such situations could be abused
to allow disclosure of portions of server memory.  To fix, teach
inline_function to check for "bare" CaseTestExpr nodes in the arguments of
a function to be inlined, and avoid inlining if there are any.

Heikki Linnakangas, Michael Paquier, Tom Lane

Report: https://github.com/greenplum-db/gpdb/pull/327
Report: <4DDCEEB8.50602@enterprisedb.com>
Security: CVE-2016-5423

8 years agoObstruct shell, SQL, and conninfo injection via database and role names.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Obstruct shell, SQL, and conninfo injection via database and role names.

Due to simplistic quoting and confusion of database names with conninfo
strings, roles with the CREATEDB or CREATEROLE option could escalate to
superuser privileges when a superuser next ran certain maintenance
commands.  The new coding rule for PQconnectdbParams() calls, documented
at conninfo_array_parse(), is to pass expand_dbname=true and wrap
literal database names in a trivial connection string.  Escape
zero-length values in appendConnStrVal().  Back-patch to 9.1 (all
supported versions).

Nathan Bossart, Michael Paquier, and Noah Misch.  Reviewed by Peter
Eisentraut.  Reported by Nathan Bossart.

Security: CVE-2016-5424

8 years agoPromote pg_dumpall shell/connstr quoting functions to src/fe_utils.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Promote pg_dumpall shell/connstr quoting functions to src/fe_utils.

Rename these newly-extern functions with terms more typical of their new
neighbors.  No functional changes; a subsequent commit will use them in
more places.  Back-patch to 9.1 (all supported versions).  Back branches
lack src/fe_utils, so instead rename the functions in place; the
subsequent commit will copy them into the other programs using them.

Security: CVE-2016-5424

8 years agoBack-patch "Only quote libpq connection string values that need quoting."
Noah Misch [Mon, 8 Aug 2016 14:07:53 +0000 (10:07 -0400)]
Back-patch "Only quote libpq connection string values that need quoting."

Back-patch commit 2953cd6d17210935098c803c52c6df5b12a725b9 and certain
runPgDump() bits of 3dee636e0404885d07885d41c0d70e50c784f324 to 9.2 and
9.1.  This synchronizes their doConnStrQuoting() implementations with
later releases.  Subsequent security patches will modify that function.

Security: CVE-2016-5424

8 years agoFix Windows shell argument quoting.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Fix Windows shell argument quoting.

The incorrect quoting may have permitted arbitrary command execution.
At a minimum, it gave broader control over the command line to actors
supposed to have control over a single argument.  Back-patch to 9.1 (all
supported versions).

Security: CVE-2016-5424

8 years agoReject, in pg_dumpall, names containing CR or LF.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Reject, in pg_dumpall, names containing CR or LF.

These characters prematurely terminate Windows shell command processing,
causing the shell to execute a prefix of the intended command.  The
chief alternative to rejecting these characters was to bypass the
Windows shell with CreateProcess(), but the ability to use such names
has little value.  Back-patch to 9.1 (all supported versions).

This change formally revokes support for these characters in database
names and roles names.  Don't document this; the error message is
self-explanatory, and too few users would benefit.  A future major
release may forbid creation of databases and roles so named.  For now,
check only at known weak points in pg_dumpall.  Future commits will,
without notice, reject affected names from other frontend programs.

Also extend the restriction to pg_dumpall --dbname=CONNSTR arguments and
--file arguments.  Unlike the effects on role name arguments and
database names, this does not reflect a broad policy change.  A
migration to CreateProcess() could lift these two restrictions.

Reviewed by Peter Eisentraut.

Security: CVE-2016-5424

8 years agoField conninfo strings throughout src/bin/scripts.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Field conninfo strings throughout src/bin/scripts.

These programs nominally accepted conninfo strings, but they would
proceed to use the original dbname parameter as though it were an
unadorned database name.  This caused "reindexdb dbname=foo" to issue an
SQL command that always failed, and other programs printed a conninfo
string in error messages that purported to print a database name.  Fix
both problems by using PQdb() to retrieve actual database names.
Continue to print the full conninfo string when reporting a connection
failure.  It is informative there, and if the database name is the sole
problem, the server-side error message will include the name.  Beyond
those user-visible fixes, this allows a subsequent commit to synthesize
and use conninfo strings without that implementation detail leaking into
messages.  As a side effect, the "vacuuming database" message now
appears after, not before, the connection attempt.  Back-patch to 9.1
(all supported versions).

Reviewed by Michael Paquier and Peter Eisentraut.

Security: CVE-2016-5424

8 years agoIntroduce a psql "\connect -reuse-previous=on|off" option.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Introduce a psql "\connect -reuse-previous=on|off" option.

The decision to reuse values of parameters from a previous connection
has been based on whether the new target is a conninfo string.  Add this
means of overriding that default.  This feature arose as one component
of a fix for security vulnerabilities in pg_dump, pg_dumpall, and
pg_upgrade, so back-patch to 9.1 (all supported versions).  In 9.3 and
later, comment paragraphs that required update had already-incorrect
claims about behavior when no connection is open; fix those problems.

Security: CVE-2016-5424

8 years agoSort out paired double quotes in \connect, \password and \crosstabview.
Noah Misch [Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)]
Sort out paired double quotes in \connect, \password and \crosstabview.

In arguments, these meta-commands wrongly treated each pair as closing
the double quoted string.  Make the behavior match the documentation.
This is a compatibility break, but I more expect to find software with
untested reliance on the documented behavior than software reliant on
today's behavior.  Back-patch to 9.1 (all supported versions).

Reviewed by Tom Lane and Peter Eisentraut.

Security: CVE-2016-5424

8 years agoRelease notes for 9.5.4, 9.4.9, 9.3.14, 9.2.18, 9.1.23.
Tom Lane [Mon, 8 Aug 2016 01:31:02 +0000 (21:31 -0400)]
Release notes for 9.5.4, 9.4.9, 9.3.14, 9.2.18, 9.1.23.

8 years agoFix misestimation of n_distinct for a nearly-unique column with many nulls.
Tom Lane [Sun, 7 Aug 2016 22:52:02 +0000 (18:52 -0400)]
Fix misestimation of n_distinct for a nearly-unique column with many nulls.

If ANALYZE found no repeated non-null entries in its sample, it set the
column's stadistinct value to -1.0, intending to indicate that the entries
are all distinct.  But what this value actually means is that the number
of distinct values is 100% of the table's rowcount, and thus it was
overestimating the number of distinct values by however many nulls there
are.  This could lead to very poor selectivity estimates, as for example
in a recent report from Andreas Joseph Krogh.  We should discount the
stadistinct value by whatever we've estimated the nulls fraction to be.
(That is what will happen if we choose to use a negative stadistinct for
a column that does have repeated entries, so this code path was just
inconsistent.)

In addition to fixing the stadistinct entries stored by several different
ANALYZE code paths, adjust the logic where get_variable_numdistinct()
forces an "all distinct" estimate on the basis of finding a relevant unique
index.  Unique indexes don't reject nulls, so there's no reason to assume
that the null fraction doesn't apply.

Back-patch to all supported branches.  Back-patching is a bit of a judgment
call, but this problem seems to affect only a few users (else we'd have
identified it long ago), and it's bad enough when it does happen that
destabilizing plan choices in a worse direction seems unlikely.

Patch by me, with documentation wording suggested by Dean Rasheed

Report: <VisenaEmail.26.df42f82acae38a58.156463942b8@tc7-visena>
Discussion: <16143.1470350371@sss.pgh.pa.us>

8 years agoTeach libpq to decode server version correctly from future servers.
Tom Lane [Fri, 5 Aug 2016 22:58:12 +0000 (18:58 -0400)]
Teach libpq to decode server version correctly from future servers.

Beginning with the next development cycle, PG servers will report two-part
not three-part version numbers.  Fix libpq so that it will compute the
correct numeric representation of such server versions for reporting by
PQserverVersion().  It's desirable to get this into the field and
back-patched ASAP, so that older clients are more likely to understand the
new server version numbering by the time any such servers are in the wild.

(The results with an old client would probably not be catastrophic anyway
for a released server; for example "10.1" would be interpreted as 100100
which would be wrong in detail but would not likely cause an old client to
misbehave badly.  But "10devel" or "10beta1" would result in sversion==0
which at best would result in disabling all use of modern features.)

Extracted from a patch by Peter Eisentraut; comments added by me

Patch: <802ec140-635d-ad86-5fdf-d3af0e260c22@2ndquadrant.com>

8 years agoUpdate time zone data files to tzdata release 2016f.
Tom Lane [Fri, 5 Aug 2016 16:58:17 +0000 (12:58 -0400)]
Update time zone data files to tzdata release 2016f.

DST law changes in Kemerovo and Novosibirsk.  Historical corrections for
Azerbaijan, Belarus, and Morocco.  Asia/Novokuznetsk and Asia/Novosibirsk
now use numeric time zone abbreviations instead of invented ones.  Zones
for Antarctic bases and other locations that have been uninhabited for
portions of the time span known to the tzdata database now report "-00"
rather than "zzz" as the zone abbreviation for those time spans.

Also, I decided to remove some of the timezone/data/ files that we don't
use.  At one time that subdirectory was a complete copy of what IANA
distributes in the tzdata tarballs, but that hasn't been true for a long
time.  There seems no good reason to keep shipping those specific files
but not others; they're just bloating our tarballs.

8 years agodoc: Remove documentation of nonexistent information schema columns
Peter Eisentraut [Wed, 3 Aug 2016 17:45:55 +0000 (13:45 -0400)]
doc: Remove documentation of nonexistent information schema columns

These were probably copied in by accident.

From: Clément Prévost <prevostclement@gmail.com>

8 years agodoc: OS collation changes can break indexes
Bruce Momjian [Tue, 2 Aug 2016 21:13:10 +0000 (17:13 -0400)]
doc:  OS collation changes can break indexes

Discussion: 20160702155517.GD18610@momjian.us

Reviewed-by: Christoph Berg
Backpatch-through: 9.1

8 years agoFix pg_dump's handling of public schema with both -c and -C options.
Tom Lane [Tue, 2 Aug 2016 16:48:51 +0000 (12:48 -0400)]
Fix pg_dump's handling of public schema with both -c and -C options.

Since -c plus -C requests dropping and recreating the target database
as a whole, not dropping individual objects in it, we should assume that
the public schema already exists and need not be created.  The previous
coding considered only the state of the -c option, so it would emit
"CREATE SCHEMA public" anyway, leading to an unexpected error in restore.

Back-patch to 9.2.  Older versions did not accept -c with -C so the
issue doesn't arise there.  (The logic being patched here dates to 8.0,
cf commit 2193121fa, so it's not really wrong that it didn't consider
the case at the time.)

Note that versions before 9.6 will still attempt to emit REVOKE/GRANT
on the public schema; but that happens without -c/-C too, and doesn't
seem to be the focus of this complaint.  I considered extending this
stanza to also skip the public schema's ACL, but that would be a
misfeature, as it'd break cases where users intentionally changed that
ACL.  The real fix for this aspect is Stephen Frost's work to not dump
built-in ACLs, and that's not going to get back-ported.

Per bugs #13804 and #14271.  Solution found by David Johnston and later
rediscovered by me.

Report: <20151207163520.2628.95990@wrigleys.postgresql.org>
Report: <20160801021955.1430.47434@wrigleys.postgresql.org>

8 years agoFixed array checking code for "unsigned long long" datatypes in libecpg.
Michael Meskes [Mon, 1 Aug 2016 04:36:27 +0000 (06:36 +0200)]
Fixed array checking code for "unsigned long long" datatypes in libecpg.

8 years agoFix pg_basebackup so that it accepts 0 as a valid compression level.
Fujii Masao [Mon, 1 Aug 2016 08:36:14 +0000 (17:36 +0900)]
Fix pg_basebackup so that it accepts 0 as a valid compression level.

The help message for pg_basebackup specifies that the numbers 0 through 9
are accepted as valid values of -Z option. But, previously -Z 0 was rejected
as an invalid compression level.

Per discussion, it's better to make pg_basebackup treat 0 as valid
compression level meaning no compression, like pg_dump.

Back-patch to all supported versions.

Reported-By: Jeff Janes
Reviewed-By: Amit Kapila
Discussion: CAMkU=1x+GwjSayc57v6w87ij6iRGFWt=hVfM0B64b1_bPVKRqg@mail.gmail.com

8 years agoDoc: remove claim that hash index creation depends on effective_cache_size.
Tom Lane [Sun, 31 Jul 2016 22:32:34 +0000 (18:32 -0400)]
Doc: remove claim that hash index creation depends on effective_cache_size.

This text was added by commit ff213239c, and not long thereafter obsoleted
by commit 4adc2f72a (which made the test depend on NBuffers instead); but
nobody noticed the need for an update.  Commit 9563d5b5e adds some further
dependency on maintenance_work_mem, but the existing verbiage seems to
cover that with about as much precision as we really want here.  Let's
just take it all out rather than leaving ourselves open to more errors of
omission in future.  (That solution makes this change back-patchable, too.)

Noted by Peter Geoghegan.

Discussion: <CAM3SWZRVANbj9GA9j40fAwheQCZQtSwqTN1GBTVwRrRbmSf7cg@mail.gmail.com>

8 years agodoc: apply hypen fix that was not backpatched
Bruce Momjian [Sat, 30 Jul 2016 18:52:17 +0000 (14:52 -0400)]
doc:  apply hypen fix that was not backpatched

Head patch was 42ec6c2da699e8e0b1774988fa97297a2cdf716c.

Reported-by: Alexander Law
Discussion: 5785FBE7.7060508@gmail.com

Backpatch-through: 9.1

8 years agoGuard against empty buffer in gets_fromFile()'s check for a newline.
Tom Lane [Thu, 28 Jul 2016 22:57:24 +0000 (18:57 -0400)]
Guard against empty buffer in gets_fromFile()'s check for a newline.

Per the fgets() specification, it cannot return without reading some data
unless it reports EOF or error.  So the code here assumed that the data
buffer would necessarily be nonempty when we go to check for a newline
having been read.  However, Agostino Sarubbo noticed that this could fail
to be true if the first byte of the data is a NUL (\0).  The fgets() API
doesn't really work for embedded NULs, which is something I don't feel
any great need for us to worry about since we generally don't allow NULs
in SQL strings anyway.  But we should not access off the end of our own
buffer if the case occurs.  Normally this would just be a harmless read,
but if you were unlucky the byte before the buffer would contain '\n'
and we'd overwrite it with '\0', and if you were really unlucky that
might be valuable data and psql would crash.

Agostino reported this to pgsql-security, but after discussion we concluded
that it isn't worth treating as a security bug; if you can control the
input to psql you can do far more interesting things than just maybe-crash
it.  Nonetheless, it is a bug, so back-patch to all supported versions.

8 years agoFix assorted fallout from IS [NOT] NULL patch.
Tom Lane [Thu, 28 Jul 2016 20:09:15 +0000 (16:09 -0400)]
Fix assorted fallout from IS [NOT] NULL patch.

Commits 4452000f3 et al established semantics for NullTest.argisrow that
are a bit different from its initial conception: rather than being merely
a cache of whether we've determined the input to have composite type,
the flag now has the further meaning that we should apply field-by-field
testing as per the standard's definition of IS [NOT] NULL.  If argisrow
is false and yet the input has composite type, the construct instead has
the semantics of IS [NOT] DISTINCT FROM NULL.  Update the comments in
primnodes.h to clarify this, and fix ruleutils.c and deparse.c to print
such cases correctly.  In the case of ruleutils.c, this merely results in
cosmetic changes in EXPLAIN output, since the case can't currently arise
in stored rules.  However, it represents a live bug for deparse.c, which
would formerly have sent a remote query that had semantics different
from the local behavior.  (From the user's standpoint, this means that
testing a remote nested-composite column for null-ness could have had
unexpected recursive behavior much like that fixed in 4452000f3.)

In a related but somewhat independent fix, make plancat.c set argisrow
to false in all NullTest expressions constructed to represent "attnotnull"
constructs.  Since attnotnull is actually enforced as a simple null-value
check, this is a more accurate representation of the semantics; we were
previously overpromising what it meant for composite columns, which might
possibly lead to incorrect planner optimizations.  (It seems that what the
SQL spec expects a NOT NULL constraint to mean is an IS NOT NULL test, so
arguably we are violating the spec and should fix attnotnull to do the
other thing.  If we ever do, this part should get reverted.)

Back-patch, same as the previous commit.

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

8 years agoImprove documentation about CREATE TABLE ... LIKE.
Tom Lane [Thu, 28 Jul 2016 17:26:59 +0000 (13:26 -0400)]
Improve documentation about CREATE TABLE ... LIKE.

The docs failed to explain that LIKE INCLUDING INDEXES would not preserve
the names of indexes and associated constraints.  Also, it wasn't mentioned
that EXCLUDE constraints would be copied by this option.  The latter
oversight seems enough of a documentation bug to justify back-patching.

In passing, do some minor copy-editing in the same area, and add an entry
for LIKE under "Compatibility", since it's not exactly a faithful
implementation of the standard's feature.

Discussion: <20160728151154.AABE64016B@smtp.hushmail.com>

8 years agoRegister atexit hook only once in pg_upgrade.
Tom Lane [Thu, 28 Jul 2016 15:39:11 +0000 (11:39 -0400)]
Register atexit hook only once in pg_upgrade.

start_postmaster() registered stop_postmaster_atexit as an atexit(3)
callback each time through, although the obvious intention was to do
so only once per program run.  The extra registrations were harmless,
so long as we didn't exceed ATEXIT_MAX, but still it's a bug.

Artur Zakirov, with bikeshedding by Kyotaro Horiguchi and me

Discussion: <d279e817-02b5-caa6-215f-cfb05dce109a@postgrespro.ru>

8 years agoFix incorrect description of udt_privileges view in documentation.
Fujii Masao [Thu, 28 Jul 2016 13:34:42 +0000 (22:34 +0900)]
Fix incorrect description of udt_privileges view in documentation.

The description of udt_privileges view contained an incorrect copy-pasted word.

Back-patch to 9.2 where udt_privileges view was added.

Author: Alexander Law