]> granicus.if.org Git - postgresql/log
postgresql
9 years agoFix rare startup failure induced by MVCC-catalog-scans patch.
Tom Lane [Fri, 3 Apr 2015 04:07:29 +0000 (00:07 -0400)]
Fix rare startup failure induced by MVCC-catalog-scans patch.

While a new backend nominally participates in sinval signaling starting
from the SharedInvalBackendInit call near the top of InitPostgres, it
cannot recognize sinval messages for unshared catalogs of its database
until it has set up MyDatabaseId.  This is not problematic for the catcache
or relcache, which by definition won't have loaded any data from or about
such catalogs before that point.  However, commit 568d4138c646cd7c
introduced a mechanism for re-using MVCC snapshots for catalog scans, and
made invalidation of those depend on recognizing relevant sinval messages.
So it's possible to establish a catalog snapshot to read pg_authid and
pg_database, then before we set MyDatabaseId, receive sinval messages that
should result in invalidating that snapshot --- but do not, because we
don't realize they are for our database.  This mechanism explains the
intermittent buildfarm failures we've seen since commit 31eae6028eca4365.
That commit was not itself at fault, but it introduced a new regression
test that does reconnections concurrently with the "vacuum full pg_am"
command in vacuum.sql.  This allowed the pre-existing error to be exposed,
given just the right timing, because we'd fail to update our information
about how to access pg_am.  In principle any VACUUM FULL on a system
catalog could have created a similar hazard for concurrent incoming
connections.  Perhaps there are more subtle failure cases as well.

To fix, force invalidation of the catalog snapshot as soon as we've
set MyDatabaseId.

Back-patch to 9.4 where the error was introduced.

9 years agoAfter a crash, don't restart workers with BGW_NEVER_RESTART.
Robert Haas [Thu, 2 Apr 2015 18:38:06 +0000 (14:38 -0400)]
After a crash, don't restart workers with BGW_NEVER_RESTART.

Amit Khandekar

9 years agoCorrect comment to use RS_EPHEMERAL
Simon Riggs [Thu, 2 Apr 2015 11:49:48 +0000 (07:49 -0400)]
Correct comment to use RS_EPHEMERAL

9 years agopsql: fix \connect with URIs and conninfo strings
Alvaro Herrera [Wed, 1 Apr 2015 23:00:07 +0000 (20:00 -0300)]
psql: fix \connect with URIs and conninfo strings

psql was already accepting conninfo strings as the first parameter in
\connect, but the way it worked wasn't sane; some of the other
parameters would get the previous connection's values, causing it to
connect to a completely unexpected server or, more likely, not finding
any server at all because of completely wrong combinations of
parameters.

Fix by explicitely checking for a conninfo-looking parameter in the
dbname position; if one is found, use its complete specification rather
than mix with the other arguments.  Also, change tab-completion to not
try to complete conninfo/URI-looking "dbnames" and document that
conninfos are accepted as first argument.

There was a weak consensus to backpatch this, because while the behavior
of using the dbname as a conninfo is nowhere documented for \connect, it
is reasonable to expect that it works because it does work in many other
contexts.  Therefore this is backpatched all the way back to 9.0.

To implement this, routines previously private to libpq have been
duplicated so that psql can decide what looks like a conninfo/URI
string.  In back branches, just duplicate the same code all the way back
to 9.2, where URIs where introduced; 9.0 and 9.1 have a simpler version.
In master, the routines are moved to src/common and renamed.

Author: David Fetter, Andrew Dunstan.  Some editorialization by me
(probably earning a Gierth's "Sloppy" badge in the process.)
Reviewers: Andrew Gierth, Erik Rijkers, Pavel Stěhule, Stephen Frost,
Robert Haas, Andrew Dunstan.

9 years agoFix incorrect markup in documentation of window frame clauses.
Tom Lane [Wed, 1 Apr 2015 00:02:40 +0000 (20:02 -0400)]
Fix incorrect markup in documentation of window frame clauses.

You're required to write either RANGE or ROWS to start a frame clause,
but the documentation incorrectly implied this is optional.  Noted by
David Johnston.

9 years agoRemove spurious semicolons.
Heikki Linnakangas [Tue, 31 Mar 2015 12:12:27 +0000 (15:12 +0300)]
Remove spurious semicolons.

Petr Jelinek

9 years agoRun pg_upgrade and pg_resetxlog with restricted token on Windows
Andrew Dunstan [Mon, 30 Mar 2015 21:16:57 +0000 (17:16 -0400)]
Run pg_upgrade and pg_resetxlog with restricted token on Windows

As with initdb these programs need to run with a restricted token, and
if they don't pg_upgrade will fail when run as a user with Adminstrator
privileges.

Backpatch to all live branches. On the development branch the code is
reorganized so that the restricted token code is now in a single
location. On the stable bramches a less invasive change is made by
simply copying the relevant code to pg_upgrade.c and pg_resetxlog.c.

Patches and bug report from Muhammad Asif Naeem, reviewed by Michael
Paquier, slightly edited by me.

9 years agoFix bogus concurrent use of _hash_getnewbuf() in bucket split code.
Tom Lane [Mon, 30 Mar 2015 20:40:05 +0000 (16:40 -0400)]
Fix bogus concurrent use of _hash_getnewbuf() in bucket split code.

_hash_splitbucket() obtained the base page of the new bucket by calling
_hash_getnewbuf(), but it held no exclusive lock that would prevent some
other process from calling _hash_getnewbuf() at the same time.  This is
contrary to _hash_getnewbuf()'s API spec and could in fact cause failures.
In practice, we must only call that function while holding write lock on
the hash index's metapage.

An additional problem was that we'd already modified the metapage's bucket
mapping data, meaning that failure to extend the index would leave us with
a corrupt index.

Fix both issues by moving the _hash_getnewbuf() call to just before we
modify the metapage in _hash_expandtable().

Unfortunately there's still a large problem here, which is that we could
also incur ENOSPC while trying to get an overflow page for the new bucket.
That would leave the index corrupt in a more subtle way, namely that some
index tuples that should be in the new bucket might still be in the old
one.  Fixing that seems substantially more difficult; even preallocating as
many pages as we could possibly need wouldn't entirely guarantee that the
bucket split would complete successfully.  So for today let's just deal
with the base case.

Per report from Antonin Houska.  Back-patch to all active branches.

9 years agoFix rare core dump in BackendIdGetTransactionIds().
Tom Lane [Mon, 30 Mar 2015 17:05:27 +0000 (13:05 -0400)]
Fix rare core dump in BackendIdGetTransactionIds().

BackendIdGetTransactionIds() neglected the possibility that the PROC
pointer in a ProcState array entry is null.  In current usage, this could
only crash if the other backend had exited since pgstat_read_current_status
saw it as active, which is a pretty narrow window.  But it's reachable in
the field, per bug #12918 from Vladimir Borodin.

Back-patch to 9.4 where the faulty code was introduced.

9 years agoAdd vacuum_delay_point call in compute_index_stats's per-sample-row loop.
Tom Lane [Sun, 29 Mar 2015 19:04:09 +0000 (15:04 -0400)]
Add vacuum_delay_point call in compute_index_stats's per-sample-row loop.

Slow functions in index expressions might cause this loop to take long
enough to make it worth being cancellable.  Probably it would be enough
to call CHECK_FOR_INTERRUPTS here, but for consistency with other
per-sample-row loops in this file, let's use vacuum_delay_point.

Report and patch by Jeff Janes.  Back-patch to all supported branches.

9 years agoMake SyncRepWakeQueue to a static function
Tatsuo Ishii [Thu, 26 Mar 2015 01:38:11 +0000 (10:38 +0900)]
Make SyncRepWakeQueue to a static function

It is only used in src/backend/replication/syncrep.c.

Back-patch to all supported branches except 9.1 which declares the
function as static.

9 years agoFix ExecOpenScanRelation to take a lock on a ROW_MARK_COPY relation.
Tom Lane [Tue, 24 Mar 2015 19:53:06 +0000 (15:53 -0400)]
Fix ExecOpenScanRelation to take a lock on a ROW_MARK_COPY relation.

ExecOpenScanRelation assumed that any relation listed in the ExecRowMark
list has been locked by InitPlan; but this is not true if the rel's
markType is ROW_MARK_COPY, which is possible if it's a foreign table.

In most (possibly all) cases, failure to acquire a lock here isn't really
problematic because the parser, planner, or plancache would have taken the
appropriate lock already.  In principle though it might leave us vulnerable
to working with a relation that we hold no lock on, and in any case if the
executor isn't depending on previously-taken locks otherwise then it should
not do so for ROW_MARK_COPY relations.

Noted by Etsuro Fujita.  Back-patch to all active versions, since the
inconsistency has been there a long time.  (It's almost certainly
irrelevant in 9.0, since that predates foreign tables, but the code's
still wrong on its own terms.)

9 years agoDon't delay replication for less than recovery_min_apply_delay's resolution.
Andres Freund [Mon, 23 Mar 2015 15:40:10 +0000 (16:40 +0100)]
Don't delay replication for less than recovery_min_apply_delay's resolution.

Recovery delays are implemented by waiting on a latch, and latches take
milliseconds as a parameter. The required amount of waiting was computed
using microsecond resolution though and the wait loop's abort condition
was checking the delay in microseconds as well.  This could lead to
short spurts of busy looping when the overall wait time was below a
millisecond, but above 0 microseconds.

Instead just formulate the wait loop's abort condition in millisecond
granularity as well. Given that that's recovery_min_apply_delay
resolution, it seems harmless to not wait for less than a millisecond.

Backpatch to 9.4 where recovery_min_apply_delay was introduced.

Discussion: 20150323141819.GH26995@alap3.anarazel.de

9 years agoFix status reporting for terminated bgworkers that were never started.
Robert Haas [Thu, 19 Mar 2015 14:56:34 +0000 (10:56 -0400)]
Fix status reporting for terminated bgworkers that were never started.

Previously, GetBackgroundWorkerPid() would return BGWH_NOT_YET_STARTED
if the slot used for the worker registration had not been reused by
unrelated activity, and BGWH_STOPPED if it had.  Either way, a process
that had requested notification when the state of one of its
background workers changed did not receive such notifications.  Fix
things so that GetBackgroundWorkerPid() always returns BGWH_STOPPED in
this situation, so that we do not erroneously give waiters the
impression that the worker will eventually be started; and send
notifications just as we would if the process terminated after having
been started, so that it's possible to wait for the postmaster to
process a worker termination request without polling.

Discovered by Amit Kapila during testing of parallel sequential scan.
Analysis and fix by me.  Back-patch to 9.4; there may not be anyone
relying on this interface yet, but if anyone is, the new behavior is a
clear improvement.

9 years agoReplace insertion sort in contrib/intarray with qsort().
Tom Lane [Mon, 16 Mar 2015 03:22:03 +0000 (23:22 -0400)]
Replace insertion sort in contrib/intarray with qsort().

It's all very well to claim that a simplistic sort is fast in easy
cases, but O(N^2) in the worst case is not good ... especially if the
worst case is as easy to hit as "descending order input".  Replace that
bit with our standard qsort.

Per bug #12866 from Maksym Boguk.  Back-patch to all active branches.

9 years agosrc/port/dirmod.c needs to be built on Cygwin too.
Tom Lane [Sun, 15 Mar 2015 18:14:24 +0000 (14:14 -0400)]
src/port/dirmod.c needs to be built on Cygwin too.

Oversight in my commit 91f4a5a976500517e492320e389342d7436cf9d4.
Per buildfarm member brolga.

9 years agoBuild src/port/dirmod.c only on Windows.
Tom Lane [Sat, 14 Mar 2015 18:08:45 +0000 (14:08 -0400)]
Build src/port/dirmod.c only on Windows.

Since commit ba7c5975adea74c6f17bdb0e0427ad85962092a2, port/dirmod.c
has contained only Windows-specific functions.  Most platforms don't
seem to mind uselessly building an empty file, but OS X for one issues
warnings.  Hence, treat dirmod.c as a Windows-specific file selected
by configure rather than one that's always built.  We can revert this
change if dirmod.c ever gains any non-Windows functionality again.

Back-patch to 9.4 where the mentioned commit appeared.

9 years agoRemove workaround for ancient incompatibility between readline and libedit.
Tom Lane [Sat, 14 Mar 2015 17:43:00 +0000 (13:43 -0400)]
Remove workaround for ancient incompatibility between readline and libedit.

GNU readline defines the return value of write_history() as "zero if OK,
else an errno code".  libedit's version of that function used to have a
different definition (to wit, "-1 if error, else the number of lines
written to the file").  We tried to work around that by checking whether
errno had become nonzero, but this method has never been kosher according
to the published API of either library.  It's reportedly completely broken
in recent Ubuntu releases: psql bleats about "No such file or directory"
when saving ~/.psql_history, even though the write worked fine.

However, libedit has been following the readline definition since somewhere
around 2006, so it seems all right to finally break compatibility with
ancient libedit releases and trust that the return value is what readline
specifies.  (I'm not sure when the various Linux distributions incorporated
this fix, but I did find that OS X has been shipping fixed versions since
10.5/Leopard.)

If anyone is still using such an ancient libedit, they will find that psql
complains it can't write ~/.psql_history at exit, even when the file was
written correctly.  This is no worse than the behavior we're fixing for
current releases.

Back-patch to all supported branches.

9 years agoFix integer overflow in debug message of walreceiver
Tatsuo Ishii [Fri, 13 Mar 2015 23:16:50 +0000 (08:16 +0900)]
Fix integer overflow in debug message of walreceiver

The message tries to tell the replication apply delay which fails if
the first WAL record is not applied yet. Fix is, instead of telling
overflowed minus numeric, showing "N/A" which indicates that the delay
data is not yet available. Problem reported by me and patch by
Fabrízio de Royes Mello.

Back patched to 9.4, 9.3 and 9.2 stable branches (9.1 and 9.0 do not
have the debug message).

9 years agoEnsure tableoid reads correctly in EvalPlanQual-manufactured tuples.
Tom Lane [Thu, 12 Mar 2015 17:38:49 +0000 (13:38 -0400)]
Ensure tableoid reads correctly in EvalPlanQual-manufactured tuples.

The ROW_MARK_COPY path in EvalPlanQualFetchRowMarks() was just setting
tableoid to InvalidOid, I think on the assumption that the referenced
RTE must be a subquery or other case without a meaningful OID.  However,
foreign tables also use this code path, and they do have meaningful
table OIDs; so failure to set the tuple field can lead to user-visible
misbehavior.  Fix that by fetching the appropriate OID from the range
table.

There's still an issue about whether CTID can ever have a meaningful
value in this case; at least with postgres_fdw foreign tables, it does.
But that is a different problem that seems to require a significantly
different patch --- it's debatable whether postgres_fdw really wants to
use this code path at all.

Simplified version of a patch by Etsuro Fujita, who also noted the
problem to begin with.  The issue can be demonstrated in all versions
having FDWs, so back-patch to 9.1.

9 years agoFix memory leaks in GIN index vacuum.
Heikki Linnakangas [Thu, 12 Mar 2015 14:29:58 +0000 (15:29 +0100)]
Fix memory leaks in GIN index vacuum.

Per bug #12850 by Walter Nordmann. Backpatch to 9.4 where the leak was
introduced.

9 years agoCast to (void *) rather than (int *) when passing int64's to PQfn().
Tom Lane [Sun, 8 Mar 2015 17:58:28 +0000 (13:58 -0400)]
Cast to (void *) rather than (int *) when passing int64's to PQfn().

This is a possibly-vain effort to silence a Coverity warning about
bogus endianness dependency.  The code's fine, because it takes care
of endianness issues for itself, but Coverity sees an int64 being
passed to an int* argument and not unreasonably suspects something's
wrong.  I'm not sure if putting the void* cast in the way will shut it
up; but it can't hurt and seems better from a documentation standpoint
anyway, since the pointer is not used as an int* in this code path.

Just for a bit of additional safety, verify that the result length
is 8 bytes as expected.

Back-patch to 9.3 where the code in question was added.

9 years agoFix documentation for libpq's PQfn().
Tom Lane [Sun, 8 Mar 2015 17:35:28 +0000 (13:35 -0400)]
Fix documentation for libpq's PQfn().

The SGML docs claimed that 1-byte integers could be sent or received with
the "isint" options, but no such behavior has ever been implemented in
pqGetInt() or pqPutInt().  The in-code documentation header for PQfn() was
even less in tune with reality, and the code itself used parameter names
matching neither the SGML docs nor its libpq-fe.h declaration.  Do a bit
of additional wordsmithing on the SGML docs while at it.

Since the business about 1-byte integers is a clear documentation bug,
back-patch to all supported branches.

9 years agoRethink function argument sorting in pg_dump.
Tom Lane [Fri, 6 Mar 2015 18:27:46 +0000 (13:27 -0500)]
Rethink function argument sorting in pg_dump.

Commit 7b583b20b1c95acb621c71251150beef958bb603 created an unnecessary
dump failure hazard by applying pg_get_function_identity_arguments()
to every function in the database, even those that won't get dumped.
This could result in snapshot-related problems if concurrent sessions are,
for example, creating and dropping temporary functions, as noted by Marko
Tiikkaja in bug #12832.  While this is by no means pg_dump's only such
issue with concurrent DDL, it's unfortunate that we added a new failure
mode for cases that used to work, and even more so that the failure was
created for basically cosmetic reasons (ie, to sort overloaded functions
more deterministically).

To fix, revert that patch and instead sort function arguments using
information that pg_dump has available anyway, namely the names of the
argument types.  This will produce a slightly different sort ordering for
overloaded functions than the previous coding; but applying strcmp
directly to the output of pg_get_function_identity_arguments really was
a bit odd anyway.  The sorting will still be name-based and hence
independent of possibly-installation-specific OID assignments.  A small
additional benefit is that sorting now works regardless of server version.

Back-patch to 9.3, where the previous commit appeared.

9 years agoFix contrib/file_fdw's expected file
Alvaro Herrera [Fri, 6 Mar 2015 14:47:09 +0000 (11:47 -0300)]
Fix contrib/file_fdw's expected file

I forgot to update it on yesterday's cf34e373fcf.

9 years agoFix user mapping object description
Alvaro Herrera [Thu, 5 Mar 2015 21:03:16 +0000 (18:03 -0300)]
Fix user mapping object description

We were using "user mapping for user XYZ" as description for user mappings, but
that's ambiguous because users can have mappings on multiple foreign
servers; therefore change it to "for user XYZ on server UVW" instead.
Object identities for user mappings are also updated in the same way, in
branches 9.3 and above.

The incomplete description string was introduced together with the whole
SQL/MED infrastructure by commit cae565e503 of 8.4 era, so backpatch all
the way back.

9 years agoAdd comment for "is_internal" parameter
Alvaro Herrera [Tue, 3 Mar 2015 17:03:33 +0000 (14:03 -0300)]
Add comment for "is_internal" parameter

This was missed in my commit f4c4335 of 9.3 vintage, so backpatch to
that.

9 years agoFix pg_dump handling of extension config tables
Stephen Frost [Mon, 2 Mar 2015 19:12:28 +0000 (14:12 -0500)]
Fix pg_dump handling of extension config tables

Since 9.1, we've provided extensions with a way to denote
"configuration" tables- tables created by an extension which the user
may modify.  By marking these as "configuration" tables, the extension
is asking for the data in these tables to be pg_dump'd (tables which
are not marked in this way are assumed to be entirely handled during
CREATE EXTENSION and are not included at all in a pg_dump).

Unfortunately, pg_dump neglected to consider foreign key relationships
between extension configuration tables and therefore could end up
trying to reload the data in an order which would cause FK violations.

This patch teaches pg_dump about these dependencies, so that the data
dumped out is done so in the best order possible.  Note that there's no
way to handle circular dependencies, but those have yet to be seen in
the wild.

The release notes for this should include a caution to users that
existing pg_dump-based backups may be invalid due to this issue.  The
data is all there, but restoring from it will require extracting the
data for the configuration tables and then loading them in the correct
order by hand.

Discussed initially back in bug #6738, more recently brought up by
Gilles Darold, who provided an initial patch which was further reworked
by Michael Paquier.  Further modifications and documentation updates
by me.

Back-patch to 9.1 where we added the concept of extension configuration
tables.

9 years agoFix targetRelation initializiation in prepsecurity
Stephen Frost [Sun, 1 Mar 2015 20:27:06 +0000 (15:27 -0500)]
Fix targetRelation initializiation in prepsecurity

In 6f9bd50eabb0a4960e94c83dac8855771c9f340d, we modified
expand_security_quals() to tell expand_security_qual() about when the
current RTE was the targetRelation.  Unfortunately, that commit
initialized the targetRelation variable used outside of the loop over
the RTEs instead of at the start of it.

This patch moves the variable and the initialization of it into the
loop, where it should have been to begin with.

Pointed out by Dean Rasheed.

Back-patch to 9.4 as the original commit was.

9 years agoUnlink static libraries before rebuilding them.
Noah Misch [Sun, 1 Mar 2015 18:05:23 +0000 (13:05 -0500)]
Unlink static libraries before rebuilding them.

When the library already exists in the build directory, "ar" preserves
members not named on its command line.  This mattered when, for example,
a "configure" rerun dropped a file from $(LIBOBJS).  libpgport carried
the obsolete member until "make clean".  Back-patch to 9.0 (all
supported versions).

9 years agoFix planning of star-schema-style queries.
Tom Lane [Sat, 28 Feb 2015 17:43:04 +0000 (12:43 -0500)]
Fix planning of star-schema-style queries.

Part of the intent of the parameterized-path mechanism was to handle
star-schema queries efficiently, but some overly-restrictive search
limiting logic added in commit e2fa76d80ba571d4de8992de6386536867250474
prevented such cases from working as desired.  Fix that and add a
regression test about it.  Per gripe from Marc Cousin.

This is arguably a bug rather than a new feature, so back-patch to 9.2
where parameterized paths were introduced.

9 years agoSuppress uninitialized-variable warning from less-bright compilers.
Tom Lane [Fri, 27 Feb 2015 23:19:22 +0000 (18:19 -0500)]
Suppress uninitialized-variable warning from less-bright compilers.

The type variable must get set on first iteration of the while loop,
but there are reasonably modern gcc versions that don't realize that.
Initialize it with a dummy value.  This undoes a removal of initialization
in commit 654809e770ce270c0bb9de726c5df1ab193d60f0.

9 years agoFix a couple of trivial issues in jsonb.c
Alvaro Herrera [Fri, 27 Feb 2015 21:54:49 +0000 (18:54 -0300)]
Fix a couple of trivial issues in jsonb.c

Typo "aggreagate" appeared three times, and the return value of function
JsonbIteratorNext() was being assigned to an int variable in a bunch of
places.

9 years agoRender infinite date/timestamps as 'infinity' for json/jsonb
Andrew Dunstan [Thu, 26 Feb 2015 17:34:43 +0000 (12:34 -0500)]
Render infinite date/timestamps as 'infinity' for json/jsonb

Commit ab14a73a6c raised an error in these cases and later the
behaviour was copied to jsonb. This is what the XML code, which we
then adopted, does, as the XSD types don't accept infinite values.
However, json dates and timestamps are just strings as far as json is
concerned, so there is no reason not to render these values as
'infinity'.

The json portion of this is backpatched to 9.4 where the behaviour was
introduced. The jsonb portion only affects the development branch.

Per gripe on pgsql-general.

9 years agoReconsider when to wait for WAL flushes/syncrep during commit.
Andres Freund [Thu, 26 Feb 2015 11:50:07 +0000 (12:50 +0100)]
Reconsider when to wait for WAL flushes/syncrep during commit.

Up to now RecordTransactionCommit() waited for WAL to be flushed (if
synchronous_commit != off) and to be synchronously replicated (if
enabled), even if a transaction did not have a xid assigned. The primary
reason for that is that sequence's nextval() did not assign a xid, but
are worthwhile to wait for on commit.

This can be problematic because sometimes read only transactions do
write WAL, e.g. HOT page prune records. That then could lead to read only
transactions having to wait during commit. Not something people expect
in a read only transaction.

This lead to such strange symptoms as backends being seemingly stuck
during connection establishment when all synchronous replicas are
down. Especially annoying when said stuck connection is the standby
trying to reconnect to allow syncrep again...

This behavior also is involved in a rather complicated <= 9.4 bug where
the transaction started by catchup interrupt processing waited for
syncrep using latches, but didn't get the wakeup because it was already
running inside the same overloaded signal handler. Fix the issue here
doesn't properly solve that issue, merely papers over the problems. In
9.5 catchup interrupts aren't processed out of signal handlers anymore.

To fix all this, make nextval() acquire a top level xid, and only wait for
transaction commit if a transaction both acquired a xid and emitted WAL
records.  If only a xid has been assigned we don't uselessly want to
wait just because of writes to temporary/unlogged tables; if only WAL
has been written we don't want to wait just because of HOT prunes.

The xid assignment in nextval() is unlikely to cause overhead in
real-world workloads. For one it only happens SEQ_LOG_VALS/32 values
anyway, for another only usage of nextval() without using the result in
an insert or similar is affected.

Discussion: 20150223165359.GF30784@awork2.anarazel.de,
    369698E947874884A77849D8FE3680C2@maumau,
    5CF4ABBA67674088B3941894E22A0D25@maumau

Per complaint from maumau and Thom Brown

Backpatch all the way back; 9.0 doesn't have syncrep, but it seems
better to be consistent behavior across all maintained branches.

9 years agoFree SQLSTATE and SQLERRM no earlier than other PL/pgSQL variables.
Noah Misch [Thu, 26 Feb 2015 04:48:28 +0000 (23:48 -0500)]
Free SQLSTATE and SQLERRM no earlier than other PL/pgSQL variables.

"RETURN SQLERRM" prompted plpgsql_exec_function() to read from freed
memory.  Back-patch to 9.0 (all supported versions).  Little code ran
between the premature free and the read, so non-assert builds are
unlikely to witness user-visible consequences.

9 years agoAdd locking clause for SB views for update/delete
Stephen Frost [Thu, 26 Feb 2015 02:36:40 +0000 (21:36 -0500)]
Add locking clause for SB views for update/delete

In expand_security_qual(), we were handling locking correctly when a
PlanRowMark existed, but not when we were working with the target
relation (which doesn't have any PlanRowMarks, but the subquery created
for the security barrier quals still needs to lock the rows under it).

Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't
properly issuing a SELECT ... FOR UPDATE to the remote side under a
DELETE.

Back-patch to 9.4 where updatable security barrier views were
introduced.

Per discussion with Etsuro and Dean Rasheed.

9 years agoFix dumping of views that are just VALUES(...) but have column aliases.
Tom Lane [Wed, 25 Feb 2015 17:01:12 +0000 (12:01 -0500)]
Fix dumping of views that are just VALUES(...) but have column aliases.

The "simple" path for printing VALUES clauses doesn't work if we need
to attach nondefault column aliases, because there's noplace to do that
in the minimal VALUES() syntax.  So modify get_simple_values_rte() to
detect nondefault aliases and treat that as a non-simple case.  This
further exposes that the "non-simple" path never actually worked;
it didn't produce valid syntax.  Fix that too.  Per bug #12789 from
Curtis McEnroe, and analysis by Andrew Gierth.

Back-patch to all supported branches.  Before 9.3, this also requires
back-patching the part of commit 092d7ded29f36b0539046b23b81b9f0bf2d637f1
that created get_simple_values_rte() to begin with; inserting the extra
test into the old factorization of that logic would've been too messy.

9 years agoGuard against spurious signals in LockBufferForCleanup.
Andres Freund [Mon, 23 Feb 2015 15:11:11 +0000 (16:11 +0100)]
Guard against spurious signals in LockBufferForCleanup.

When LockBufferForCleanup() has to wait for getting a cleanup lock on a
buffer it does so by setting a flag in the buffer header and then wait
for other backends to signal it using ProcWaitForSignal().
Unfortunately LockBufferForCleanup() missed that ProcWaitForSignal() can
return for other reasons than the signal it is hoping for. If such a
spurious signal arrives the wait flags on the buffer header will still
be set. That then triggers "ERROR: multiple backends attempting to wait
for pincount 1".

The fix is simple, unset the flag if still set when retrying. That
implies an additional spinlock acquisition/release, but that's unlikely
to matter given the cost of waiting for a cleanup lock.  Alternatively
it'd have been possible to move responsibility for maintaining the
relevant flag to the waiter all together, but that might have had
negative consequences due to possible floods of signals. Besides being
more invasive.

This looks to be a very longstanding bug. The relevant code in
LockBufferForCleanup() hasn't changed materially since its introduction
and ProcWaitForSignal() was documented to return for unrelated reasons
since 8.2.  The master only patch series removing ImmediateInterruptOK
made it much easier to hit though, as ProcSendSignal/ProcWaitForSignal
now uses a latch shared with other tasks.

Per discussion with Kevin Grittner, Tom Lane and me.

Backpatch to all supported branches.

Discussion: 11553.1423805224@sss.pgh.pa.us

9 years agoFix potential deadlock with libpq non-blocking mode.
Heikki Linnakangas [Mon, 23 Feb 2015 11:32:34 +0000 (13:32 +0200)]
Fix potential deadlock with libpq non-blocking mode.

If libpq output buffer is full, pqSendSome() function tries to drain any
incoming data. This avoids deadlock, if the server e.g. sends a lot of
NOTICE messages, and blocks until we read them. However, pqSendSome() only
did that in blocking mode. In non-blocking mode, the deadlock could still
happen.

To fix, take a two-pronged approach:

1. Change the documentation to instruct that when PQflush() returns 1, you
should wait for both read- and write-ready, and call PQconsumeInput() if it
becomes read-ready. That fixes the deadlock, but applications are not going
to change overnight.

2. In pqSendSome(), drain the input buffer before returning 1. This
alleviates the problem for applications that only wait for write-ready. In
particular, a slow but steady stream of NOTICE messages during COPY FROM
STDIN will no longer cause a deadlock. The risk remains that the server
attempts to send a large burst of data and fills its output buffer, and at
the same time the client also sends enough data to fill its output buffer.
The application will deadlock if it goes to sleep, waiting for the socket
to become write-ready, before the server's data arrives. In practice,
NOTICE messages and such that the server might be sending are usually
short, so it's highly unlikely that the server would fill its output buffer
so quickly.

Backpatch to all supported versions.

9 years agoFix misparsing of empty value in conninfo_uri_parse_params().
Tom Lane [Sat, 21 Feb 2015 17:59:25 +0000 (12:59 -0500)]
Fix misparsing of empty value in conninfo_uri_parse_params().

After finding an "=" character, the pointer was advanced twice when it
should only advance once.  This is harmless as long as the value after "="
has at least one character; but if it doesn't, we'd miss the terminator
character and include too much in the value.

In principle this could lead to reading off the end of memory.  It does not
seem worth treating as a security issue though, because it would happen on
client side, and besides client logic that's taking conninfo strings from
untrusted sources has much worse security problems than this.

Report and patch received off-list from Thomas Fanghaenel.
Back-patch to 9.2 where the faulty code was introduced.

9 years agoFix object identities for pg_conversion objects
Alvaro Herrera [Wed, 18 Feb 2015 17:28:12 +0000 (14:28 -0300)]
Fix object identities for pg_conversion objects

We were neglecting to schema-qualify them.

Backpatch to 9.3, where object identities were introduced as a concept
by commit f8348ea32ec8.

9 years agoFix failure to honor -Z compression level option in pg_dump -Fd.
Tom Lane [Wed, 18 Feb 2015 16:43:00 +0000 (11:43 -0500)]
Fix failure to honor -Z compression level option in pg_dump -Fd.

cfopen() and cfopen_write() failed to pass the compression level through
to zlib, so that you always got the default compression level if you got
any at all.

In passing, also fix these and related functions so that the correct errno
is reliably returned on failure; the original coding supposes that free()
cannot change errno, which is untrue on at least some platforms.

Per bug #12779 from Christoph Berg.  Back-patch to 9.1 where the faulty
code was introduced.

Michael Paquier

9 years agoRemove code to match IPv4 pg_hba.conf entries to IPv4-in-IPv6 addresses.
Tom Lane [Tue, 17 Feb 2015 17:49:18 +0000 (12:49 -0500)]
Remove code to match IPv4 pg_hba.conf entries to IPv4-in-IPv6 addresses.

In investigating yesterday's crash report from Hugo Osvaldo Barrera, I only
looked back as far as commit f3aec2c7f51904e7 where the breakage occurred
(which is why I thought the IPv4-in-IPv6 business was undocumented).  But
actually the logic dates back to commit 3c9bb8886df7d56a and was simply
broken by erroneous refactoring in the later commit.  A bit of archives
excavation shows that we added the whole business in response to a report
that some 2003-era Linux kernels would report IPv4 connections as having
IPv4-in-IPv6 addresses.  The fact that we've had no complaints since 9.0
seems to be sufficient confirmation that no modern kernels do that, so
let's just rip it all out rather than trying to fix it.

Do this in the back branches too, thus essentially deciding that our
effective behavior since 9.0 is correct.  If there are any platforms on
which the kernel reports IPv4-in-IPv6 addresses as such, yesterday's fix
would have made for a subtle and potentially security-sensitive change in
the effective meaning of IPv4 pg_hba.conf entries, which does not seem like
a good thing to do in minor releases.  So let's let the post-9.0 behavior
stand, and change the documentation to match it.

In passing, I failed to resist the temptation to wordsmith the description
of pg_hba.conf IPv4 and IPv6 address entries a bit.  A lot of this text
hasn't been touched since we were IPv4-only.

9 years agoImprove pg_check_dir code and comments.
Robert Haas [Tue, 17 Feb 2015 15:19:30 +0000 (10:19 -0500)]
Improve pg_check_dir code and comments.

Avoid losing errno if readdir() fails and closedir() works.  Consistently
return 4 rather than 3 if both a lost+found directory and other files are
found, rather than returning one value or the other depending on the
order of the directory listing.  Update comments to match the actual
behavior.

These oversights date to commits 6f03927fce038096f53ca67eeab9adb24938f8a6
and 17f15239325a88581bb4f9cf91d38005f1f52d69.

Marco Nenciarini

9 years agoFix misuse of memcpy() in check_ip().
Tom Lane [Mon, 16 Feb 2015 21:17:48 +0000 (16:17 -0500)]
Fix misuse of memcpy() in check_ip().

The previous coding copied garbage into a local variable, pretty much
ensuring that the intended test of an IPv6 connection address against a
promoted IPv4 address from pg_hba.conf would never match.  The lack of
field complaints likely indicates that nobody realized this was supposed
to work, which is unsurprising considering that no user-facing docs suggest
it should work.

In principle this could have led to a SIGSEGV due to reading off the end of
memory, but since the source address would have pointed to somewhere in the
function's stack frame, that's quite unlikely.  What led to discovery of
the bug is Hugo Osvaldo Barrera's report of a crash after an OS upgrade,
which is probably because he is now running a system in which memcpy raises
abort() upon detecting overlapping source and destination areas.  (You'd
have to additionally suppose some things about the stack frame layout to
arrive at this conclusion, but it seems plausible.)

This has been broken since the code was added, in commit f3aec2c7f51904e7,
so back-patch to all supported branches.

9 years agoFix null-pointer-deref crash while doing COPY IN with check constraints.
Tom Lane [Mon, 16 Feb 2015 04:26:45 +0000 (23:26 -0500)]
Fix null-pointer-deref crash while doing COPY IN with check constraints.

In commit bf7ca15875988a88e97302e012d7c4808bef3ea9 I introduced an
assumption that an RTE referenced by a whole-row Var must have a valid eref
field.  This is false for RTEs constructed by DoCopy, and there are other
places taking similar shortcuts.  Perhaps we should make all those places
go through addRangeTableEntryForRelation or its siblings instead of having
ad-hoc logic, but the most reliable fix seems to be to make the new code in
ExecEvalWholeRowVar cope if there's no eref.  We can reasonably assume that
there's no need to insert column aliases if no aliases were provided.

Add a regression test case covering this, and also verifying that a sane
column name is in fact available in this situation.

Although the known case only crashes in 9.4 and HEAD, it seems prudent to
back-patch the code change to 9.2, since all the ingredients for a similar
failure exist in the variant patch applied to 9.3 and 9.2.

Per report from Jean-Pierre Pelletier.

9 years agopg_regress: Write processed input/*.source into output dir
Peter Eisentraut [Sun, 15 Feb 2015 02:33:41 +0000 (21:33 -0500)]
pg_regress: Write processed input/*.source into output dir

Before, it was writing the processed files into the input directory,
which is incorrect in a vpath build.

9 years agoFix broken #ifdef for __sparcv8
Heikki Linnakangas [Fri, 13 Feb 2015 21:51:23 +0000 (23:51 +0200)]
Fix broken #ifdef for __sparcv8

Rob Rowan. Backpatch to all supported versions, like the patch that added
the broken #ifdef.

9 years agopg_upgrade: quote directory names in delete_old_cluster script
Bruce Momjian [Thu, 12 Feb 2015 03:06:04 +0000 (22:06 -0500)]
pg_upgrade:  quote directory names in delete_old_cluster script

This allows the delete script to properly function when special
characters appear in directory paths, e.g. spaces.

Backpatch through 9.0

9 years agopg_upgrade: preserve freeze info for postgres/template1 dbs
Bruce Momjian [Thu, 12 Feb 2015 02:02:07 +0000 (21:02 -0500)]
pg_upgrade:  preserve freeze info for postgres/template1 dbs

pg_database.datfrozenxid and pg_database.datminmxid were not preserved
for the 'postgres' and 'template1' databases.  This could cause missing
clog file errors on access to user tables and indexes after upgrades in
these databases.

Backpatch through 9.0

9 years agoFix typo in logicaldecoding.sgml.
Andres Freund [Thu, 12 Feb 2015 00:16:32 +0000 (01:16 +0100)]
Fix typo in logicaldecoding.sgml.

Author: Tatsuo Ishii

Backpatch to 9.4, where logicaldecoding was introduced.

9 years agoFix missing PQclear() in libpqrcv_endstreaming().
Tom Lane [Thu, 12 Feb 2015 00:20:49 +0000 (19:20 -0500)]
Fix missing PQclear() in libpqrcv_endstreaming().

This omission leaked one PGresult per WAL streaming cycle, which possibly
would never be enough to notice in the real world, but it's still a leak.

Per Coverity.  Back-patch to 9.3 where the error was introduced.

9 years agoFix minor memory leak in ident_inet().
Tom Lane [Thu, 12 Feb 2015 00:09:54 +0000 (19:09 -0500)]
Fix minor memory leak in ident_inet().

We'd leak the ident_serv data structure if the second pg_getaddrinfo_all
(the one for the local address) failed.  This is not of great consequence
because a failure return here just leads directly to backend exit(), but
if this function is going to try to clean up after itself at all, it should
not have such holes in the logic.  Try to fix it in a future-proof way by
having all the failure exits go through the same cleanup path, rather than
"optimizing" some of them.

Per Coverity.  Back-patch to 9.2, which is as far back as this patch
applies cleanly.

9 years agoFix more memory leaks in failure path in buildACLCommands.
Tom Lane [Wed, 11 Feb 2015 23:35:23 +0000 (18:35 -0500)]
Fix more memory leaks in failure path in buildACLCommands.

We already had one go at this issue in commit d73b7f973db5ec7e, but we
failed to notice that buildACLCommands also leaked several PQExpBuffers
along with a simply malloc'd string.  This time let's try to make the
fix a bit more future-proof by eliminating the separate exit path.

It's still not exactly critical because pg_dump will curl up and die on
failure; but since the amount of the potential leak is now several KB,
it seems worth back-patching as far as 9.2 where the previous fix landed.

Per Coverity, which evidently is smarter than clang's static analyzer.

9 years agoFixed array handling in ecpg.
Michael Meskes [Wed, 11 Feb 2015 09:45:46 +0000 (10:45 +0100)]
Fixed array handling in ecpg.

When ecpg was rewritten to the new protocol version not all variable types
were corrected. This patch rewrites the code for these types to fix that. It
also fixes the documentation to correctly tell the status of array handling.

9 years agoFix pg_dump's heuristic for deciding which casts to dump.
Tom Lane [Wed, 11 Feb 2015 03:38:17 +0000 (22:38 -0500)]
Fix pg_dump's heuristic for deciding which casts to dump.

Back in 2003 we had a discussion about how to decide which casts to dump.
At the time pg_dump really only considered an object's containing schema
to decide what to dump (ie, dump whatever's not in pg_catalog), and so
we chose a complicated idea involving whether the underlying types were to
be dumped (cf commit a6790ce85752b67ad994f55fdf1a450262ccc32e).  But users
are allowed to create casts between built-in types, and we failed to dump
such casts.  Let's get rid of that heuristic, which has accreted even more
ugliness since then, in favor of just looking at the cast's OID to decide
if it's a built-in cast or not.

In passing, also fix some really ancient code that supposed that it had to
manufacture a dependency for the cast on its cast function; that's only
true when dumping from a pre-7.3 server.  This just resulted in some wasted
cycles and duplicate dependency-list entries with newer servers, but we
might as well improve it.

Per gripes from a number of people, most recently Greg Sabino Mullane.
Back-patch to all supported branches.

9 years agoFix GEQO to not assume its join order heuristic always works.
Tom Lane [Wed, 11 Feb 2015 01:37:22 +0000 (20:37 -0500)]
Fix GEQO to not assume its join order heuristic always works.

Back in commit 400e2c934457bef4bc3cc9a3e49b6289bd761bc0 I rewrote GEQO's
gimme_tree function to improve its heuristic for modifying the given tour
into a legal join order.  In what can only be called a fit of hubris,
I supposed that this new heuristic would *always* find a legal join order,
and ripped out the old logic that allowed gimme_tree to sometimes fail.

The folly of this is exposed by bug #12760, in which the "greedy" clumping
behavior of merge_clump() can lead it into a dead end which could only be
recovered from by un-clumping.  We have no code for that and wouldn't know
exactly what to do with it if we did.  Rather than try to improve the
heuristic rules still further, let's just recognize that it *is* a
heuristic and probably must always have failure cases.  So, put back the
code removed in the previous commit to allow for failure (but comment it
a bit better this time).

It's possible that this code was actually fully correct at the time and
has only been broken by the introduction of LATERAL.  But having seen this
example I no longer have much faith in that proposition, so back-patch to
all supported branches.

9 years agoMinor cleanup/code review for "indirect toast" stuff.
Tom Lane [Mon, 9 Feb 2015 17:30:55 +0000 (12:30 -0500)]
Minor cleanup/code review for "indirect toast" stuff.

Fix some issues I noticed while fooling with an extension to allow an
additional kind of toast pointer.  Much of this is just comment
improvement, but there are a couple of actual bugs, which might or might
not be reachable today depending on what can happen during logical
decoding.  An example is that toast_flatten_tuple() failed to cover the
possibility of an indirection pointer in its input.  Back-patch to 9.4
just in case that is reachable now.

In HEAD, also correct some really minor issues with recent compression
reorganization, such as dangerously underparenthesized macros.

9 years agoReport WAL flush, not insert, position in replication IDENTIFY_SYSTEM
Heikki Linnakangas [Fri, 6 Feb 2015 09:18:14 +0000 (11:18 +0200)]
Report WAL flush, not insert, position in replication IDENTIFY_SYSTEM

When beginning streaming replication, the client usually issues the
IDENTIFY_SYSTEM command, which used to return the current WAL insert
position. That's not suitable for the intended purpose of that field,
however. pg_receivexlog uses it to start replication from the reported
point, but if it hasn't been flushed to disk yet, it will fail. Change
IDENTIFY_SYSTEM to report the flush position instead.

Backpatch to 9.1 and above. 9.0 doesn't report any WAL position.

9 years agoFix reference-after-free when waiting for another xact due to constraint.
Heikki Linnakangas [Wed, 4 Feb 2015 14:00:34 +0000 (16:00 +0200)]
Fix reference-after-free when waiting for another xact due to constraint.

If an insertion or update had to wait for another transaction to finish,
because there was another insertion with conflicting key in progress,
we would pass a just-free'd item pointer to XactLockTableWait().

All calls to XactLockTableWait() and MultiXactIdWait() had similar issues.
Some passed a pointer to a buffer in the buffer cache, after already
releasing the lock. The call in EvalPlanQualFetch had already released the
pin too. All but the call in execUtils.c would merely lead to reporting a
bogus ctid, however (or an assertion failure, if enabled).

All the callers that passed HeapTuple->t_data->t_ctid were slightly bogus
anyway: if the tuple was updated (again) in the same transaction, its ctid
field would point to the next tuple in the chain, not the tuple itself.

Backpatch to 9.4, where the 'ctid' argument to XactLockTableWait was added
(in commit f88d4cfc)

9 years agoAdd missing float.h include to snprintf.c.
Andres Freund [Wed, 4 Feb 2015 12:27:31 +0000 (13:27 +0100)]
Add missing float.h include to snprintf.c.

On windows _isnan() (which isnan() is redirected to in port/win32.h)
is declared in float.h, not math.h.

Per buildfarm animal currawong.

Backpatch to all supported branches.

9 years agodoc: Fix markup
Fujii Masao [Wed, 4 Feb 2015 10:00:09 +0000 (19:00 +0900)]
doc: Fix markup

Ian Barwick

9 years agoFix breakage in GEODEBUG debug code.
Tom Lane [Tue, 3 Feb 2015 20:20:48 +0000 (15:20 -0500)]
Fix breakage in GEODEBUG debug code.

LINE doesn't have an "m" field (anymore anyway).  Also fix unportable
assumption that %x can print the result of pointer subtraction.

In passing, improve single_decode() in minor ways:
* Remove unnecessary leading-whitespace skip (strtod does that already).
* Make GEODEBUG message more intelligible.
* Remove entirely-useless test to see if strtod returned a silly pointer.
* Don't bother computing trailing-whitespace skip unless caller wants
  an ending pointer.

This has been broken since 261c7d4b653bc3e44c31fd456d94f292caa50d8f.
Although it's only debug code, might as well fix the 9.4 branch too.

9 years agoStamp 9.4.1. REL9_4_1
Tom Lane [Mon, 2 Feb 2015 20:42:55 +0000 (15:42 -0500)]
Stamp 9.4.1.

9 years agoLast-minute updates for release notes.
Tom Lane [Mon, 2 Feb 2015 16:24:02 +0000 (11:24 -0500)]
Last-minute updates for release notes.

Add entries for security issues.

Security: CVE-2015-0241 through CVE-2015-0244

9 years agoBe more careful to not lose sync in the FE/BE protocol.
Heikki Linnakangas [Mon, 2 Feb 2015 15:08:56 +0000 (17:08 +0200)]
Be more careful to not lose sync in the FE/BE protocol.

If any error occurred while we were in the middle of reading a protocol
message from the client, we could lose sync, and incorrectly try to
interpret a part of another message as a new protocol message. That will
usually lead to an "invalid frontend message" error that terminates the
connection. However, this is a security issue because an attacker might
be able to deliberately cause an error, inject a Query message in what's
supposed to be just user data, and have the server execute it.

We were quite careful to not have CHECK_FOR_INTERRUPTS() calls or other
operations that could ereport(ERROR) in the middle of processing a message,
but a query cancel interrupt or statement timeout could nevertheless cause
it to happen. Also, the V2 fastpath and COPY handling were not so careful.
It's very difficult to recover in the V2 COPY protocol, so we will just
terminate the connection on error. In practice, that's what happened
previously anyway, as we lost protocol sync.

To fix, add a new variable in pqcomm.c, PqCommReadingMsg, that is set
whenever we're in the middle of reading a message. When it's set, we cannot
safely ERROR out and continue running, because we might've read only part
of a message. PqCommReadingMsg acts somewhat similarly to critical sections
in that if an error occurs while it's set, the error handler will force the
connection to be terminated, as if the error was FATAL. It's not
implemented by promoting ERROR to FATAL in elog.c, like ERROR is promoted
to PANIC in critical sections, because we want to be able to use
PG_TRY/CATCH to recover and regain protocol sync. pq_getmessage() takes
advantage of that to prevent an OOM error from terminating the connection.

To prevent unnecessary connection terminations, add a holdoff mechanism
similar to HOLD/RESUME_INTERRUPTS() that can be used hold off query cancel
interrupts, but still allow die interrupts. The rules on which interrupts
are processed when are now a bit more complicated, so refactor
ProcessInterrupts() and the calls to it in signal handlers so that the
signal handlers always call it if ImmediateInterruptOK is set, and
ProcessInterrupts() can decide to not do anything if the other conditions
are not met.

Reported by Emil Lenngren. Patch reviewed by Noah Misch and Andres Freund.
Backpatch to all supported versions.

Security: CVE-2015-0244

9 years agoPrevent Valgrind Memcheck errors around px_acquire_system_randomness().
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Prevent Valgrind Memcheck errors around px_acquire_system_randomness().

This function uses uninitialized stack and heap buffers as supplementary
entropy sources.  Mark them so Memcheck will not complain.  Back-patch
to 9.4, where Valgrind Memcheck cooperation first appeared.

Marko Tiikkaja

9 years agoCherry-pick security-relevant fixes from upstream imath library.
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Cherry-pick security-relevant fixes from upstream imath library.

This covers alterations to buffer sizing and zeroing made between imath
1.3 and imath 1.20.  Valgrind Memcheck identified the buffer overruns
and reliance on uninitialized data; their exploit potential is unknown.
Builds specifying --with-openssl are unaffected, because they use the
OpenSSL BIGNUM facility instead of imath.  Back-patch to 9.0 (all
supported versions).

Security: CVE-2015-0243

9 years agoFix buffer overrun after incomplete read in pullf_read_max().
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Fix buffer overrun after incomplete read in pullf_read_max().

Most callers pass a stack buffer.  The ensuing stack smash can crash the
server, and we have not ruled out the viability of attacks that lead to
privilege escalation.  Back-patch to 9.0 (all supported versions).

Marko Tiikkaja

Security: CVE-2015-0243

9 years agoport/snprintf(): fix overflow and do padding
Bruce Momjian [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
port/snprintf():  fix overflow and do padding

Prevent port/snprintf() from overflowing its local fixed-size
buffer and pad to the desired number of digits with zeros, even
if the precision is beyond the ability of the native sprintf().
port/snprintf() is only used on systems that lack a native
snprintf().

Reported by Bruce Momjian. Patch by Tom Lane. Backpatch to all
supported versions.

Security: CVE-2015-0242

9 years agoto_char(): prevent writing beyond the allocated buffer
Bruce Momjian [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
to_char():  prevent writing beyond the allocated buffer

Previously very long localized month and weekday strings could
overflow the allocated buffers, causing a server crash.

Reported and patch reviewed by Noah Misch.  Backpatch to all
supported versions.

Security: CVE-2015-0241

9 years agoto_char(): prevent accesses beyond the allocated buffer
Bruce Momjian [Mon, 2 Feb 2015 15:00:44 +0000 (10:00 -0500)]
to_char():  prevent accesses beyond the allocated buffer

Previously very long field masks for floats could access memory
beyond the existing buffer allocated to hold the result.

Reported by Andres Freund and Peter Geoghegan. Backpatch to all
supported versions.

Security: CVE-2015-0241

9 years agoDoc: fix syntax description for psql's \setenv.
Tom Lane [Mon, 2 Feb 2015 05:18:54 +0000 (00:18 -0500)]
Doc: fix syntax description for psql's \setenv.

The variable name isn't optional --- looks like a copy-and-paste-o from
the \set command, where it is.

Dilip Kumar

9 years agoTranslation updates
Peter Eisentraut [Mon, 2 Feb 2015 04:18:42 +0000 (23:18 -0500)]
Translation updates

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

9 years agodoc: Improve claim about location of pg_service.conf
Peter Eisentraut [Mon, 2 Feb 2015 03:36:44 +0000 (22:36 -0500)]
doc: Improve claim about location of pg_service.conf

The previous wording claimed that the file was always in /etc, but of
course this varies with the installation layout.  Write instead that it
can be found via `pg_config --sysconfdir`.  Even though this is still
somewhat incorrect because it doesn't account of moved installations, it
at least conveys that the location depends on the installation.

9 years agoRelease notes for 9.4.1, 9.3.6, 9.2.10, 9.1.15, 9.0.19.
Tom Lane [Sun, 1 Feb 2015 21:53:17 +0000 (16:53 -0500)]
Release notes for 9.4.1, 9.3.6, 9.2.10, 9.1.15, 9.0.19.

9 years agoFix documentation of psql's ECHO all mode.
Tom Lane [Sat, 31 Jan 2015 23:35:17 +0000 (18:35 -0500)]
Fix documentation of psql's ECHO all mode.

"ECHO all" is ignored for interactive input, and has been for a very long
time, though possibly not for as long as the documentation has claimed the
opposite.  Fix that, and also note that empty lines aren't echoed, which
while dubious is another longstanding behavior (it's embedded in our
regression test files for one thing).  Per bug #12721 from Hans Ginzel.

In HEAD, also improve the code comments in this area, and suppress an
unnecessary fflush(stdout) when we're not echoing.  That would likely
be safe to back-patch, but I'll not risk it mere hours before a release
wrap.

9 years agoUpdate time zone data files to tzdata release 2015a.
Tom Lane [Sat, 31 Jan 2015 03:45:44 +0000 (22:45 -0500)]
Update time zone data files to tzdata release 2015a.

DST law changes in Chile and Mexico (state of Quintana Roo).
Historical changes for Iceland.

9 years agoFix jsonb Unicode escape processing, and in consequence disallow \u0000.
Tom Lane [Fri, 30 Jan 2015 19:44:49 +0000 (14:44 -0500)]
Fix jsonb Unicode escape processing, and in consequence disallow \u0000.

We've been trying to support \u0000 in JSON values since commit
78ed8e03c67d7333, and have introduced increasingly worse hacks to try to
make it work, such as commit 0ad1a816320a2b53.  However, it fundamentally
can't work in the way envisioned, because the stored representation looks
the same as for \\u0000 which is not the same thing at all.  It's also
entirely bogus to output \u0000 when de-escaped output is called for.

The right way to do this would be to store an actual 0x00 byte, and then
throw error only if asked to produce de-escaped textual output.  However,
getting to that point seems likely to take considerable work and may well
never be practical in the 9.4.x series.

To preserve our options for better behavior while getting rid of the nasty
side-effects of 0ad1a816320a2b53, revert that commit in toto and instead
throw error if \u0000 is used in a context where it needs to be de-escaped.
(These are the same contexts where non-ASCII Unicode escapes throw error
if the database encoding isn't UTF8, so this behavior is by no means
without precedent.)

In passing, make both the \u0000 case and the non-ASCII Unicode case report
ERRCODE_UNTRANSLATABLE_CHARACTER / "unsupported Unicode escape sequence"
rather than claiming there's something wrong with the input syntax.

Back-patch to 9.4, where we have to do something because 0ad1a816320a2b53
broke things for many cases having nothing to do with \u0000.  9.3 also has
bogus behavior, but only for that specific escape value, so given the lack
of field complaints it seems better to leave 9.3 alone.

9 years agoFix Coverity warning about contrib/pgcrypto's mdc_finish().
Tom Lane [Fri, 30 Jan 2015 18:04:59 +0000 (13:04 -0500)]
Fix Coverity warning about contrib/pgcrypto's mdc_finish().

Coverity points out that mdc_finish returns a pointer to a local buffer
(which of course is gone as soon as the function returns), leaving open
a risk of misbehaviors possibly as bad as a stack overwrite.

In reality, the only possible call site is in process_data_packets()
which does not examine the returned pointer at all.  So there's no
live bug, but nonetheless the code is confusing and risky.  Refactor
to avoid the issue by letting process_data_packets() call mdc_finish()
directly instead of going through the pullf_read() API.

Although this is only cosmetic, it seems good to back-patch so that
the logic in pgp-decrypt.c stays in sync across all branches.

Marko Kreen

9 years agoFix assorted oversights in range selectivity estimation.
Tom Lane [Fri, 30 Jan 2015 17:30:41 +0000 (12:30 -0500)]
Fix assorted oversights in range selectivity estimation.

calc_rangesel() failed outright when comparing range variables to empty
constant ranges with < or >=, as a result of missing cases in a switch.
It also produced a bogus estimate for > comparison to an empty range.

On top of that, the >= and > cases were mislabeled throughout.  For
nonempty constant ranges, they managed to produce the right answers
anyway as a result of counterbalancing typos.

Also, default_range_selectivity() omitted cases for elem <@ range,
range &< range, and range &> range, so that rather dubious defaults
were applied for these operators.

In passing, rearrange the code in rangesel() so that the elem <@ range
case is handled in a less opaque fashion.

Report and patch by Emre Hasegeli, some additional work by me

9 years agoFix query-duration memory leak with GIN rescans.
Heikki Linnakangas [Fri, 30 Jan 2015 16:58:23 +0000 (17:58 +0100)]
Fix query-duration memory leak with GIN rescans.

The requiredEntries / additionalEntries arrays were not freed in
freeScanKeys() like other per-key stuff.

It's not obvious, but startScanKey() was only ever called after the keys
have been initialized with ginNewScanKey(). That's why it doesn't need to
worry about freeing existing arrays. The ginIsNewKey() test in gingetbitmap
was never true, because ginrescan free's the existing keys, and it's not OK
to call gingetbitmap twice in a row without calling ginrescan in between.
To make that clear, remove the unnecessary ginIsNewKey(). And just to be
extra sure that nothing funny happens if there is an existing key after all,
call freeScanKeys() to free it if it exists. This makes the code more
straightforward.

(I'm seeing other similar leaks in testing a query that rescans an GIN index
scan, but that's a different issue. This just fixes the obvious leak with
those two arrays.)

Backpatch to 9.4, where GIN fast scan was added.

9 years agoAllow pg_dump to use jobs and serializable transactions together.
Kevin Grittner [Fri, 30 Jan 2015 14:57:53 +0000 (08:57 -0600)]
Allow pg_dump to use jobs and serializable transactions together.

Since 9.3, when the --jobs option was introduced, using it together
with the --serializable-deferrable option generated multiple
errors.  We can get correct behavior by allowing the connection
which acquires the snapshot to use SERIALIZABLE, READ ONLY,
DEFERRABLE and pass that to the workers running the other
connections using REPEATABLE READ, READ ONLY.  This is a bit of a
kluge since the SERIALIZABLE behavior is achieved by running some
of the participating connections at a different isolation level,
but it is a simple and safe change, suitable for back-patching.

This will be followed by a proposal for a more invasive fix with
some slight behavioral changes on just the master branch, based on
suggestions from Andres Freund, but the kluge will be applied to
master until something is agreed along those lines.

Back-patched to 9.3, where the --jobs option was added.

Based on report from Alexander Korotkov

9 years agoFix BuildIndexValueDescription for expressions
Stephen Frost [Fri, 30 Jan 2015 02:59:43 +0000 (21:59 -0500)]
Fix BuildIndexValueDescription for expressions

In 804b6b6db4dcfc590a468e7be390738f9f7755fb we modified
BuildIndexValueDescription to pay attention to which columns are visible
to the user, but unfortunatley that commit neglected to consider indexes
which are built on expressions.

Handle error-reporting of violations of constraint indexes based on
expressions by not returning any detail when the user does not have
table-level SELECT rights.

Backpatch to 9.0, as the prior commit was.

Pointed out by Tom.

9 years agoHandle unexpected query results, especially NULLs, safely in connectby().
Tom Lane [Fri, 30 Jan 2015 01:18:37 +0000 (20:18 -0500)]
Handle unexpected query results, especially NULLs, safely in connectby().

connectby() didn't adequately check that the constructed SQL query returns
what it's expected to; in fact, since commit 08c33c426bfebb32 it wasn't
checking that at all.  This could result in a null-pointer-dereference
crash if the constructed query returns only one column instead of the
expected two.  Less excitingly, it could also result in surprising data
conversion failures if the constructed query returned values that were
not I/O-conversion-compatible with the types specified by the query
calling connectby().

In all branches, insist that the query return at least two columns;
this seems like a minimal sanity check that can't break any reasonable
use-cases.

In HEAD, insist that the constructed query return the types specified by
the outer query, including checking for typmod incompatibility, which the
code never did even before it got broken.  This is to hide the fact that
the implementation does a conversion to text and back; someday we might
want to improve that.

In back branches, leave that alone, since adding a type check in a minor
release is more likely to break things than make people happy.  Type
inconsistencies will continue to work so long as the actual type and
declared type are I/O representation compatible, and otherwise will fail
the same way they used to.

Also, in all branches, be on guard for NULL results from the constructed
query, which formerly would cause null-pointer dereference crashes.
We now print the row with the NULL but don't recurse down from it.

In passing, get rid of the rather pointless idea that
build_tuplestore_recursively() should return the same tuplestore that's
passed to it.

Michael Paquier, adjusted somewhat by me

9 years agoProperly terminate the array returned by GetLockConflicts().
Andres Freund [Thu, 29 Jan 2015 16:49:03 +0000 (17:49 +0100)]
Properly terminate the array returned by GetLockConflicts().

GetLockConflicts() has for a long time not properly terminated the
returned array. During normal processing the returned array is zero
initialized which, while not pretty, is sufficient to be recognized as
a invalid virtual transaction id. But the HotStandby case is more than
aesthetically broken: The allocated (and reused) array is neither
zeroed upon allocation, nor reinitialized, nor terminated.

Not having a terminating element means that the end of the array will
not be recognized and that recovery conflict handling will thus read
ahead into adjacent memory. Only terminating when hitting memory
content that looks like a invalid virtual transaction id.  Luckily
this seems so far not have caused significant problems, besides making
recovery conflict more expensive.

Discussion: 20150127142713.GD29457@awork2.anarazel.de

Backpatch into all supported branches.

9 years agoFix bug where GIN scan keys were not initialized with gin_fuzzy_search_limit.
Heikki Linnakangas [Thu, 29 Jan 2015 17:35:55 +0000 (19:35 +0200)]
Fix bug where GIN scan keys were not initialized with gin_fuzzy_search_limit.

When gin_fuzzy_search_limit was used, we could jump out of startScan()
without calling startScanKey(). That was harmless in 9.3 and below, because
startScanKey()() didn't do anything interesting, but in 9.4 it initializes
information needed for skipping entries (aka GIN fast scans), and you
readily get a segfault if it's not done. Nevertheless, it was clearly wrong
all along, so backpatch all the way to 9.1 where the early return was
introduced.

(AFAICS startScanKey() did nothing useful in 9.3 and below, because the
fields it initialized were already initialized in ginFillScanKey(), but I
don't dare to change that in a minor release. ginFillScanKey() is always
called in gingetbitmap() even though there's a check there to see if the
scan keys have already been initialized, because they never are; ginrescan()
free's them.)

In the passing, remove unnecessary if-check from the second inner loop in
startScan(). We already check in the first loop that the condition is true
for all entries.

Reported by Olaf Gawenda, bug #12694, Backpatch to 9.1 and above, although
AFAICS it causes a live bug only in 9.4.

9 years agoClean up range-table building in copy.c
Stephen Frost [Wed, 28 Jan 2015 22:42:51 +0000 (17:42 -0500)]
Clean up range-table building in copy.c

Commit 804b6b6db4dcfc590a468e7be390738f9f7755fb added the build of a
range table in copy.c to initialize the EState es_range_table since it
can be needed in error paths.  Unfortunately, that commit didn't
appreciate that some code paths might end up not initializing the rte
which is used to build the range table.

Fix that and clean up a couple others things along the way- build it
only once and don't explicitly set it on the !is_from path as it
doesn't make any sense there (cstate is palloc0'd, so this isn't an
issue from an initializing standpoint either).

The prior commit went back to 9.0, but this only goes back to 9.1 as
prior to that the range table build happens immediately after building
the RTE and therefore doesn't suffer from this issue.

Pointed out by Robert.

9 years agoFix column-privilege leak in error-message paths
Stephen Frost [Mon, 12 Jan 2015 22:04:11 +0000 (17:04 -0500)]
Fix column-privilege leak in error-message paths

While building error messages to return to the user,
BuildIndexValueDescription, ExecBuildSlotValueDescription and
ri_ReportViolation would happily include the entire key or entire row in
the result returned to the user, even if the user didn't have access to
view all of the columns being included.

Instead, include only those columns which the user is providing or which
the user has select rights on.  If the user does not have any rights
to view the table or any of the columns involved then no detail is
provided and a NULL value is returned from BuildIndexValueDescription
and ExecBuildSlotValueDescription.  Note that, for key cases, the user
must have access to all of the columns for the key to be shown; a
partial key will not be returned.

Back-patch all the way, as column-level privileges are now in all
supported versions.

This has been assigned CVE-2014-8161, but since the issue and the patch
have already been publicized on pgsql-hackers, there's no point in trying
to hide this commit.

9 years agoFix NUMERIC field access macros to treat NaNs consistently.
Tom Lane [Tue, 27 Jan 2015 17:06:36 +0000 (12:06 -0500)]
Fix NUMERIC field access macros to treat NaNs consistently.

Commit 145343534c153d1e6c3cff1fa1855787684d9a38 arranged to store numeric
NaN values as short-header numerics, but the field access macros did not
get the memo: they thought only "SHORT" numerics have short headers.

Most of the time this makes no difference because we don't access the
weight or dscale of a NaN; but numeric_send does that.  As pointed out
by Andrew Gierth, this led to fetching uninitialized bytes.

AFAICS this could not have any worse consequences than that; in particular,
an unaligned stored numeric would have been detoasted by PG_GETARG_NUMERIC,
so that there's no risk of a fetch off the end of memory.  Still, the code
is wrong on its own terms, and it's not hard to foresee future changes that
might expose us to real risks.  So back-patch to all affected branches.

9 years agoFix volatile-safety issue in dblink's materializeQueryResult().
Tom Lane [Mon, 26 Jan 2015 20:17:36 +0000 (15:17 -0500)]
Fix volatile-safety issue in dblink's materializeQueryResult().

Some fields of the sinfo struct are modified within PG_TRY and then
referenced within PG_CATCH, so as with recent patch to async.c, "volatile"
is necessary for strict POSIX compliance; and that propagates to a couple
of subroutines as well as materializeQueryResult() itself.  I think the
risk of actual issues here is probably higher than in async.c, because
storeQueryResult() is likely to get inlined into materializeQueryResult(),
leaving the compiler free to conclude that its stores into sinfo fields are
dead code.

9 years agoFix volatile-safety issue in pltcl_SPI_execute_plan().
Tom Lane [Mon, 26 Jan 2015 17:18:25 +0000 (12:18 -0500)]
Fix volatile-safety issue in pltcl_SPI_execute_plan().

The "callargs" variable is modified within PG_TRY and then referenced
within PG_CATCH, which is exactly the coding pattern we've now found
to be unsafe.  Marking "callargs" volatile would be problematic because
it is passed by reference to some Tcl functions, so fix the problem
by not modifying it within PG_TRY.  We can just postpone the free()
till we exit the PG_TRY construct, as is already done elsewhere in this
same file.

Also, fix failure to free(callargs) when exiting on too-many-arguments
error.  This is only a minor memory leak, but a leak nonetheless.

In passing, remove some unnecessary "volatile" markings in the same
function.  Those doubtless are there because gcc 2.95.3 whinged about
them, but we now know that its algorithm for complaining is many bricks
shy of a load.

This is certainly a live bug with compilers that optimize similarly
to current gcc, so back-patch to all active branches.

9 years agoFix volatile-safety issue in asyncQueueReadAllNotifications().
Tom Lane [Mon, 26 Jan 2015 16:57:36 +0000 (11:57 -0500)]
Fix volatile-safety issue in asyncQueueReadAllNotifications().

The "pos" variable is modified within PG_TRY and then referenced
within PG_CATCH, so for strict POSIX conformance it must be marked
volatile.  Superficially the code looked safe because pos's address
was taken, which was sufficient to force it into memory ... but it's
not sufficient to ensure that the compiler applies updates exactly
where the program text says to.  The volatility marking has to extend
into a couple of subroutines too, but I think that's probably a good
thing because the risk of out-of-order updates is mostly in those
subroutines not asyncQueueReadAllNotifications() itself.  In principle
the compiler could have re-ordered operations such that an error could
be thrown while "pos" had an incorrect value.

It's unclear how real the risk is here, but for safety back-patch
to all active branches.

9 years agoFurther cleanup of ReorderBufferCommit().
Tom Lane [Mon, 26 Jan 2015 03:49:59 +0000 (22:49 -0500)]
Further cleanup of ReorderBufferCommit().

On closer inspection, we can remove the "volatile" qualifier on
"using_subtxn" so long as we initialize that before the PG_TRY block,
which there's no particularly good reason not to do.
Also, push the "change" variable inside the PG_TRY so as to remove
all question of whether it needs "volatile", and remove useless
early initializations of "snapshow_now" and "using_subtxn".

9 years agoClean up assorted issues in ALTER SYSTEM coding.
Tom Lane [Mon, 26 Jan 2015 01:19:07 +0000 (20:19 -0500)]
Clean up assorted issues in ALTER SYSTEM coding.

Fix unsafe use of a non-volatile variable in PG_TRY/PG_CATCH in
AlterSystemSetConfigFile().  While at it, clean up a bundle of other
infelicities and outright bugs, including corner-case-incorrect linked list
manipulation, a poorly designed and worse documented parse-and-validate
function (which even included some randomly chosen hard-wired substitutes
for the specified elevel in one code path ... wtf?), direct use of open()
instead of fd.c's facilities, inadequate checking of write()'s return
value, and generally poorly written commentary.

9 years agoFix unsafe coding in ReorderBufferCommit().
Tom Lane [Sat, 24 Jan 2015 18:25:22 +0000 (13:25 -0500)]
Fix unsafe coding in ReorderBufferCommit().

"iterstate" must be marked volatile since it's changed inside the PG_TRY
block and then used in the PG_CATCH stanza.  Noted by Mark Wilding of
Salesforce.  (We really need to see if we can't get the C compiler to warn
about this.)

Also, reset iterstate to NULL after the mainline ReorderBufferIterTXNFinish
call, to ensure the PG_CATCH block doesn't try to do that a second time.

9 years agoReplace a bunch more uses of strncpy() with safer coding.
Tom Lane [Sat, 24 Jan 2015 18:05:45 +0000 (13:05 -0500)]
Replace a bunch more uses of strncpy() with safer coding.

strncpy() has a well-deserved reputation for being unsafe, so make an
effort to get rid of nearly all occurrences in HEAD.

A large fraction of the remaining uses were passing length less than or
equal to the known strlen() of the source, in which case no null-padding
can occur and the behavior is equivalent to memcpy(), though doubtless
slower and certainly harder to reason about.  So just use memcpy() in
these cases.

In other cases, use either StrNCpy() or strlcpy() as appropriate (depending
on whether padding to the full length of the destination buffer seems
useful).

I left a few strncpy() calls alone in the src/timezone/ code, to keep it
in sync with upstream (the IANA tzcode distribution).  There are also a
few such calls in ecpg that could possibly do with more analysis.

AFAICT, none of these changes are more than cosmetic, except for the four
occurrences in fe-secure-openssl.c, which are in fact buggy: an overlength
source leads to a non-null-terminated destination buffer and ensuing
misbehavior.  These don't seem like security issues, first because no stack
clobber is possible and second because if your values of sslcert etc are
coming from untrusted sources then you've got problems way worse than this.
Still, it's undesirable to have unpredictable behavior for overlength
inputs, so back-patch those four changes to all active branches.

9 years agoIn pg_regress, remove the temporary installation upon successful exit.
Tom Lane [Tue, 20 Jan 2015 04:44:22 +0000 (23:44 -0500)]
In pg_regress, remove the temporary installation upon successful exit.

This results in a very substantial reduction in disk space usage during
"make check-world", since that sequence involves creation of numerous
temporary installations.  It should also help a bit in the buildfarm, even
though the buildfarm script doesn't create as many temp installations,
because the current script misses deleting some of them; and anyway it
seems better to do this once in one place rather than expecting that
script to get it right every time.

In 9.4 and HEAD, also undo the unwise choice in commit b1aebbb6a86e96d7
to report strerror(errno) after a rmtree() failure.  rmtree has already
reported that, possibly for multiple failures with distinct errnos; and
what's more, by the time it returns there is no good reason to assume
that errno still reflects the last reportable error.  So reporting errno
here is at best redundant and at worst badly misleading.

Back-patch to all supported branches, so that future revisions of the
buildfarm script can rely on this behavior.

9 years agoAdjust "pgstat wait timeout" message to be a translatable LOG message.
Tom Lane [Tue, 20 Jan 2015 04:01:36 +0000 (23:01 -0500)]
Adjust "pgstat wait timeout" message to be a translatable LOG message.

Per discussion, change the log level of this message to be LOG not WARNING.
The main point of this change is to avoid causing buildfarm run failures
when the stats collector is exceptionally slow to respond, which it not
infrequently is on some of the smaller/slower buildfarm members.

This change does lose notice to an interactive user when his stats query
is looking at out-of-date stats, but the majority opinion (not necessarily
that of yours truly) is that WARNING messages would probably not get
noticed anyway on heavily loaded production systems.  A LOG message at
least ensures that the problem is recorded somewhere where bulk auditing
for the issue is possible.

Also, instead of an untranslated "pgstat wait timeout" message, provide
a translatable and hopefully more understandable message "using stale
statistics instead of current ones because stats collector is not
responding".  The original text was written hastily under the assumption
that it would never really happen in practice, which we now know to be
unduly optimistic.

Back-patch to all active branches, since we've seen the buildfarm issue
in all branches.