postgresql
9 years agoFix typo in setrefs.c
Stephen Frost [Thu, 10 Sep 2015 13:22:19 +0000 (09:22 -0400)]
Fix typo in setrefs.c

We're adding OIDs, not TIDs, to invalItems.

Pointed out by Etsuro Fujita.

Back-patch to all supported branches.

9 years agoRevert ed47666 and part of ef57b98
Stephen Frost [Thu, 10 Sep 2015 12:58:35 +0000 (08:58 -0400)]
Revert ed47666 and part of ef57b98

This reverts ed47666, which ended up adding a second tempoary
installation for all 'make check' runs, and reverts the part of
ef57b98 that changed the TAP 'prove_check' section, which appears to
have been unintentional to begin with on this branch.

Analysis and patch by Noah.

9 years agoFix minor bug in regexp makesearch() function.
Tom Lane [Thu, 10 Sep 2015 00:14:58 +0000 (20:14 -0400)]
Fix minor bug in regexp makesearch() function.

The list-wrangling here was done wrong, allowing the same state to get
put into the list twice.  The following loop then would clone it twice.
The second clone would wind up with no inarcs, so that there was no
observable misbehavior AFAICT, but a useless state in the finished NFA
isn't an especially good thing.

9 years agoRemove files signaling a standby promotion request at postmaster startup
Fujii Masao [Wed, 9 Sep 2015 13:51:44 +0000 (22:51 +0900)]
Remove files signaling a standby promotion request at postmaster startup

This commit makes postmaster forcibly remove the files signaling
a standby promotion request. Otherwise, the existence of those files
can trigger a promotion too early, whether a user wants that or not.

This removal of files is usually unnecessary because they can exist
only during a few moments during a standby promotion. However
there is a race condition: if pg_ctl promote is executed and creates
the files during a promotion, the files can stay around even after
the server is brought up to new master. Then, if new standby starts
by using the backup taken from that master, the files can exist
at the server startup and should be removed in order to avoid
an unexpected promotion.

Back-patch to 9.1 where promote signal file was introduced.

Problem reported by Feike Steenbergen.
Original patch by Michael Paquier, modified by me.

Discussion: 20150528100705.4686.91426@wrigleys.postgresql.org

9 years agoAdd temp-check, with_temp_install definition - 9.4
Stephen Frost [Tue, 8 Sep 2015 21:38:30 +0000 (17:38 -0400)]
Add temp-check, with_temp_install definition - 9.4

Commit fa4a4df93c8c28d5684cacb1677fbd13f58bb9f2 attempted to backpatch
to 9.4 a change to improve the logging of TAP tests.  Unfortunately, due
to how 9.4 and 9.5 had diverged, the backpatch to 9.4 depended on a few
things which didn't exist in 9.4 (but did in 9.5), specifically, the
'temp-check' production and the 'with_temp_install' definition (which
also required 'abs_top_builddir').

Add these definitions into REL9_4_STABLE to allow the TAP tests to run
correctly under 'make check'.

9 years agoLock all relations referred to in updatable views
Stephen Frost [Tue, 8 Sep 2015 21:02:56 +0000 (17:02 -0400)]
Lock all relations referred to in updatable views

Even views considered "simple" enough to be automatically updatable may
have mulitple relations involved (eg: in a where clause).  We need to
make sure and lock those relations when rewriting the query.

Back-patch to 9.3 where updatable views were added.

Pointed out by Andres, patch thanks to Dean Rasheed.

9 years agoAdd gin_fuzzy_search_limit to postgresql.conf.sample.
Fujii Masao [Tue, 8 Sep 2015 17:25:50 +0000 (02:25 +0900)]
Add gin_fuzzy_search_limit to postgresql.conf.sample.

This was forgotten in 8a3631f (commit that originally added the parameter)
and 0ca9907 (commit that added the documentation later that year).

Back-patch to all supported versions.

9 years agoFix error message wording in previous sslinfo commit
Alvaro Herrera [Tue, 8 Sep 2015 14:10:20 +0000 (11:10 -0300)]
Fix error message wording in previous sslinfo commit

9 years agoAdd more sanity checks in contrib/sslinfo
Alvaro Herrera [Mon, 7 Sep 2015 22:18:29 +0000 (19:18 -0300)]
Add more sanity checks in contrib/sslinfo

We were missing a few return checks on OpenSSL calls.  Should be pretty
harmless, since we haven't seen any user reports about problems, and
this is not a high-traffic module anyway; still, a bug is a bug, so
backpatch this all the way back to 9.0.

Author: Michael Paquier, while reviewing another sslinfo patch

9 years agoChange type of DOW/DOY to UNITS
Greg Stark [Mon, 7 Sep 2015 12:35:09 +0000 (13:35 +0100)]
Change type of DOW/DOY to UNITS

9 years agoMake GIN's cleanup pending list process interruptable
Teodor Sigaev [Mon, 7 Sep 2015 14:17:42 +0000 (17:17 +0300)]
Make GIN's cleanup pending list process interruptable

Cleanup process could be called by ordinary insert/update and could take a lot
of time. Add vacuum_delay_point() to make this process interruptable. Under
vacuum this call will also throttle a vacuum process to decrease system load,
called from insert/update it will not throttle, and that reduces a latency.

Backpatch for all supported branches.

Jeff Janes <jeff.janes@gmail.com>

9 years agoUpdate site address of Snowball project
Teodor Sigaev [Mon, 7 Sep 2015 12:21:44 +0000 (15:21 +0300)]
Update site address of Snowball project

9 years agoMove DTK_ISODOW DTK_DOW and DTK_DOY to be type UNITS rather than
Greg Stark [Sun, 6 Sep 2015 01:04:37 +0000 (02:04 +0100)]
Move DTK_ISODOW DTK_DOW and DTK_DOY to be type UNITS rather than
RESERV. RESERV is meant for tokens like "now" and having them in that
category throws errors like these when used as an input date:

stark=# SELECT 'doy'::timestamptz;
ERROR:  unexpected dtype 33 while parsing timestamptz "doy"
LINE 1: SELECT 'doy'::timestamptz;
               ^
stark=# SELECT 'dow'::timestamptz;
ERROR:  unexpected dtype 32 while parsing timestamptz "dow"
LINE 1: SELECT 'dow'::timestamptz;
               ^

Found by LLVM's Libfuzzer

9 years agoFix CreateTableSpace() so it will compile without HAVE_SYMLINK.
Tom Lane [Sat, 5 Sep 2015 20:15:38 +0000 (16:15 -0400)]
Fix CreateTableSpace() so it will compile without HAVE_SYMLINK.

This has been broken since 9.3 (commit 82b1b213cad3a69c to be exact),
which suggests that nobody is any longer using a Windows build system that
doesn't provide a symlink emulation.  Still, it's wrong on its own terms,
so repair.

YUriy Zhuravlev

9 years agoFix misc typos.
Heikki Linnakangas [Sat, 5 Sep 2015 08:35:49 +0000 (11:35 +0300)]
Fix misc typos.

Oskari Saarenmaa. Backpatch to stable branches where applicable.

9 years agoFix subtransaction cleanup after an outer-subtransaction portal fails.
Tom Lane [Fri, 4 Sep 2015 17:36:50 +0000 (13:36 -0400)]
Fix subtransaction cleanup after an outer-subtransaction portal fails.

Formerly, we treated only portals created in the current subtransaction as
having failed during subtransaction abort.  However, if the error occurred
while running a portal created in an outer subtransaction (ie, a cursor
declared before the last savepoint), that has to be considered broken too.

To allow reliable detection of which ones those are, add a bookkeeping
field to struct Portal that tracks the innermost subtransaction in which
each portal has actually been executed.  (Without this, we'd end up
failing portals containing functions that had called the subtransaction,
thereby breaking plpgsql exception blocks completely.)

In addition, when we fail an outer-subtransaction Portal, transfer its
resources into the subtransaction's resource owner, so that they're
released early in cleanup of the subxact.  This fixes a problem reported by
Jim Nasby in which a function executed in an outer-subtransaction cursor
could cause an Assert failure or crash by referencing a relation created
within the inner subtransaction.

The proximate cause of the Assert failure is that AtEOSubXact_RelationCache
assumed it could blow away a relcache entry without first checking that the
entry had zero refcount.  That was a bad idea on its own terms, so add such
a check there, and to the similar coding in AtEOXact_RelationCache.  This
provides an independent safety measure in case there are still ways to
provoke the situation despite the Portal-level changes.

This has been broken since subtransactions were invented, so back-patch
to all supported branches.

Tom Lane and Michael Paquier

9 years agoDocument that max_worker_processes must be high enough in standby.
Fujii Masao [Thu, 3 Sep 2015 13:30:16 +0000 (22:30 +0900)]
Document that max_worker_processes must be high enough in standby.

The setting values of some parameters including max_worker_processes
must be equal to or higher than the values on the master. However,
previously max_worker_processes was not listed as such parameter
in the document. So this commit adds it to that list.

Back-patch to 9.4 where max_worker_processes was added.

9 years agopsql: print longtable as a possible \pset option
Bruce Momjian [Mon, 31 Aug 2015 16:24:16 +0000 (12:24 -0400)]
psql:  print longtable as a possible \pset option

For some reason this message was not updated when the longtable option
was added.

Backpatch through 9.3

9 years agoFix sepgsql regression tests.
Joe Conway [Sun, 30 Aug 2015 18:09:31 +0000 (11:09 -0700)]
Fix sepgsql regression tests.

The regression tests for sepgsql were broken by changes in the
base distro as-shipped policies. Specifically, definition of
unconfined_t in the system default policy was changed to bypass
multi-category rules, which the regression test depended on.
Fix that by defining a custom privileged domain
(sepgsql_regtest_superuser_t) and using it instead of system's
unconfined_t domain. The new sepgsql_regtest_superuser_t domain
performs almost like the current unconfined_t, but restricted by
multi-category policy as the traditional unconfined_t was.

The custom policy module is a self defined domain, and so should not
be affected by related future system policy changes. However, it still
uses the unconfined_u:unconfined_r pair for selinux-user and role.
Those definitions have not been changed for several years and seem
less risky to rely on than the unconfined_t domain. Additionally, if
we define custom user/role, they would need to be manually defined
at the operating system level, adding more complexity to an already
non-standard and complex regression test.

Back-patch to 9.3. The regression tests will need more work before
working correctly on 9.2. Starting with 9.2, sepgsql has had dependencies
on libselinux versions that are only available on newer distros with
the changed set of policies (e.g. RHEL 7.x). On 9.1 sepgsql works
fine with the older distros with original policy set (e.g. RHEL 6.x),
and on which the existing regression tests work fine. We might want
eventually change 9.1 sepgsql regression tests to be more independent
from the underlying OS policies, however more work will be needed to
make that happen and it is not clear that it is worth the effort.

Kohei KaiGai with review by Adam Brightwell and me, commentary by
Stephen, Alvaro, Tom, Robert, and others.

9 years agoUse "mb" not the nonexistent "rmb" for pg_read_barrier() on Alpha.
Tom Lane [Sat, 29 Aug 2015 20:34:30 +0000 (16:34 -0400)]
Use "mb" not the nonexistent "rmb" for pg_read_barrier() on Alpha.

It's only necessary to fix this in 9.4; later versions don't have this
code (because we ripped out Alpha support entirely), while earlier ones
aren't actually using pg_read_barrier() anywhere.

Per rather belated report from Christoph Berg.

9 years agoFix s_lock.h PPC assembly code to be compatible with native AIX assembler.
Tom Lane [Sat, 29 Aug 2015 20:09:25 +0000 (16:09 -0400)]
Fix s_lock.h PPC assembly code to be compatible with native AIX assembler.

On recent AIX it's necessary to configure gcc to use the native assembler
(because the GNU assembler hasn't been updated to handle AIX 6+).  This
caused PG builds to fail with assembler syntax errors, because we'd try
to compile s_lock.h's gcc asm fragment for PPC, and that assembly code
relied on GNU-style local labels.  We can't substitute normal labels
because it would fail in any file containing more than one inlined use of
tas().  Fortunately, that code is stable enough, and the PPC ISA is simple
enough, that it doesn't seem like too much of a maintenance burden to just
hand-code the branch offsets, removing the need for any labels.

Note that the AIX assembler only accepts "$" for the location counter
pseudo-symbol.  The usual GNU convention is "."; but it appears that all
versions of gas for PPC also accept "$", so in theory this patch will not
break any other PPC platforms.

This has been reported by a few people, but Steve Underwood gets the credit
for being the first to pursue the problem far enough to understand why it
was failing.  Thanks also to Noah Misch for additional testing.

9 years agodblink docs: fix typo to use "connname" (3 n's), not "conname"
Bruce Momjian [Thu, 27 Aug 2015 17:43:10 +0000 (13:43 -0400)]
dblink docs:  fix typo to use "connname" (3 n's), not "conname"

This makes the parameter names match the documented prototype names.

Report by Erwin Brandstetter

Backpatch through 9.0

9 years agoDocs: be explicit about datatype matching for lead/lag functions.
Tom Lane [Tue, 25 Aug 2015 23:11:17 +0000 (19:11 -0400)]
Docs: be explicit about datatype matching for lead/lag functions.

The default argument, if given, has to be of exactly the same datatype
as the first argument; but this was not stated in so many words, and
the error message you get about it might not lead your thought in the
right direction.  Per bug #13587 from Robert McGehee.

A quick scan says that these are the only two built-in functions with two
anyelement arguments and no other polymorphic arguments.  There are plenty
of cases of, eg, anyarray and anyelement, but those seem less likely to
confuse.  For instance this doesn't seem terribly hard to figure out:
"function array_remove(integer[], numeric) does not exist".  So I've
contented myself with fixing these two cases.

9 years agoAvoid O(N^2) behavior when enlarging SPI tuple table in spi_printtup().
Tom Lane [Sat, 22 Aug 2015 00:32:11 +0000 (20:32 -0400)]
Avoid O(N^2) behavior when enlarging SPI tuple table in spi_printtup().

For no obvious reason, spi_printtup() was coded to enlarge the tuple
pointer table by just 256 slots at a time, rather than doubling the size at
each reallocation, as is our usual habit.  For very large SPI results, this
makes for O(N^2) time spent in repalloc(), which of course soon comes to
dominate the runtime.  Use the standard doubling approach instead.

This is a longstanding performance bug, so back-patch to all active
branches.

Neil Conway

9 years agoFix plpython crash when returning string representation of a RECORD result.
Tom Lane [Fri, 21 Aug 2015 16:21:37 +0000 (12:21 -0400)]
Fix plpython crash when returning string representation of a RECORD result.

PLyString_ToComposite() blithely overwrote proc->result.out.d, even though
for a composite result type the other union variant proc->result.out.r is
the one that should be valid.  This could result in a crash if out.r had
in fact been filled in (proc->result.is_rowtype == 1) and then somebody
later attempted to use that data; as per bug #13579 from Paweł Michalak.

Just to add insult to injury, it didn't work for RECORD results anyway,
because record_in() would refuse the case.

Fix by doing the I/O function lookup in a local PLyTypeInfo variable,
as we were doing already in PLyObject_ToComposite().  This is not a great
technique because any fn_extra data allocated by the input function will
be leaked permanently (thanks to using TopMemoryContext as fn_mcxt).
But that's a pre-existing issue that is much less serious than a crash,
so leave it to be fixed separately.

This bug would be a potential security issue, except that plpython is
only available to superusers and the crash requires coding the function
in a way that didn't work before today's patches.

Add regression test cases covering all the supported methods of converting
composite results.

Back-patch to 9.1 where the faulty coding was introduced.

9 years agoAllow record_in() and record_recv() to work for transient record types.
Tom Lane [Fri, 21 Aug 2015 15:19:33 +0000 (11:19 -0400)]
Allow record_in() and record_recv() to work for transient record types.

If we have the typmod that identifies a registered record type, there's no
reason that record_in() should refuse to perform input conversion for it.
Now, in direct SQL usage, record_in() will always be passed typmod = -1
with type OID RECORDOID, because no typmodin exists for type RECORD, so the
case can't arise.  However, some InputFunctionCall users such as PLs may be
able to supply the right typmod, so we should allow this to support them.

Note: the previous coding and comment here predate commit 59c016aa9f490b53.
There has been no case since 8.1 in which the passed type OID wouldn't be
valid; and if it weren't, this error message wouldn't be apropos anyway.
Better to let lookup_rowtype_tupdesc complain about it.

Back-patch to 9.1, as this is necessary for my upcoming plpython fix.
I'm committing it separately just to make it a bit more visible in the
commit history.

9 years agoFix a few bogus statement type names in plpgsql error messages.
Tom Lane [Tue, 18 Aug 2015 23:22:38 +0000 (19:22 -0400)]
Fix a few bogus statement type names in plpgsql error messages.

plpgsql's error location context messages ("PL/pgSQL function fn-name line
line-no at stmt-type") would misreport a CONTINUE statement as being an
EXIT, and misreport a MOVE statement as being a FETCH.  These are clear
bugs that have been there a long time, so back-patch to all supported
branches.

In addition, in 9.5 and HEAD, change the description of EXECUTE from
"EXECUTE statement" to just plain EXECUTE; there seems no good reason why
this statement type should be described differently from others that have
a well-defined head keyword.  And distinguish GET STACKED DIAGNOSTICS from
plain GET DIAGNOSTICS.  These are a bit more of a judgment call, and also
affect existing regression-test outputs, so I did not back-patch into
stable branches.

Pavel Stehule and Tom Lane

9 years agoAdd docs about postgres_fdw's setting of search_path and other GUCs.
Tom Lane [Sat, 15 Aug 2015 18:31:04 +0000 (14:31 -0400)]
Add docs about postgres_fdw's setting of search_path and other GUCs.

This behavior wasn't documented, but it should be because it's user-visible
in triggers and other functions executed on the remote server.
Per question from Adam Fuchs.

Back-patch to 9.3 where postgres_fdw was added.

9 years agoImprove documentation about MVCC-unsafe utility commands.
Tom Lane [Sat, 15 Aug 2015 17:30:16 +0000 (13:30 -0400)]
Improve documentation about MVCC-unsafe utility commands.

The table-rewriting forms of ALTER TABLE are MVCC-unsafe, in much the same
way as TRUNCATE, because they replace all rows of the table with newly-made
rows with a new xmin.  (Ideally, concurrent transactions with old snapshots
would continue to see the old table contents, but the data is not there
anymore --- and if it were there, it would be inconsistent with the table's
updated rowtype, so there would be serious implementation problems to fix.)
This was nowhere documented though, and the problem was only documented for
TRUNCATE in a note in the TRUNCATE reference page.  Create a new "Caveats"
section in the MVCC chapter that can be home to this and other limitations
on serializable consistency.

In passing, fix a mistaken statement that VACUUM and CLUSTER would reclaim
space occupied by a dropped column.  They don't reconstruct existing tuples
so they couldn't do that.

Back-patch to all supported branches.

9 years agoDon't use 'bool' as a struct member name in help_config.c.
Andres Freund [Wed, 12 Aug 2015 14:02:20 +0000 (16:02 +0200)]
Don't use 'bool' as a struct member name in help_config.c.

Doing so doesn't work if bool is a macro rather than a typedef.

Although c.h spends some effort to support configurations where bool is
a preexisting macro, help_config.c has existed this way since
2003 (b700a6), and there have not been any reports of
problems. Backpatch anyway since this is as riskless as it gets.

Discussion: 20150812084351.GD8470@awork2.anarazel.de
Backpatch: 9.0-master

9 years agoUse the correct type for TableInfo->relreplident.
Andres Freund [Wed, 12 Aug 2015 13:52:10 +0000 (15:52 +0200)]
Use the correct type for TableInfo->relreplident.

Mistakenly relreplident was stored as a bool. That works today as c.h
typedefs bool to a char, but isn't very future proof.

Discussion: 20150812084351.GD8470@awork2.anarazel.de
Backpatch: 9.4 where replica identity was introduced.

9 years agoEncoding PG_UHC is code page 949.
Noah Misch [Sat, 15 Aug 2015 00:23:13 +0000 (20:23 -0400)]
Encoding PG_UHC is code page 949.

This fixes presentation of non-ASCII messages to the Windows event log
and console in rare cases involving Korean locale.  Processes like the
postmaster and checkpointer, but not processes attached to databases,
were affected.  Back-patch to 9.4, where MessageEncoding was introduced.
The problem exists in all supported versions, but this change has no
effect in the absence of the code recognizing PG_UHC MessageEncoding.

Noticed while investigating bug #13427 from Dmitri Bourlatchkov.

9 years agoRestore old pgwin32_message_to_UTF16() behavior outside transactions.
Noah Misch [Sat, 15 Aug 2015 00:23:09 +0000 (20:23 -0400)]
Restore old pgwin32_message_to_UTF16() behavior outside transactions.

Commit 49c817eab78c6f0ce8c3bf46766b73d6cf3190b7 replaced with a hard
error the dubious pg_do_encoding_conversion() behavior when outside a
transaction.  Reintroduce the historic soft failure locally within
pgwin32_message_to_UTF16().  This fixes errors when writing messages in
less-common encodings to the Windows event log or console.  Back-patch
to 9.4, where the aforementioned commit first appeared.

Per bug #13427 from Dmitri Bourlatchkov.

9 years agoImprove regression test case to avoid depending on system catalog stats.
Tom Lane [Thu, 13 Aug 2015 17:25:01 +0000 (13:25 -0400)]
Improve regression test case to avoid depending on system catalog stats.

In commit 95f4e59c32866716 I added a regression test case that examined
the plan of a query on system catalogs.  That isn't a terribly great idea
because the catalogs tend to change from version to version, or even
within a version if someone makes an unrelated regression-test change that
populates the catalogs a bit differently.  Usually I try to make planner
test cases rely on test tables that have not changed since Berkeley days,
but I got sloppy in this case because the submitted crasher example queried
the catalogs and I didn't spend enough time on rewriting it.  But it was a
problem waiting to happen, as I was rudely reminded when I tried to port
that patch into Salesforce's Postgres variant :-(.  So spend a little more
effort and rewrite the query to not use any system catalogs.  I verified
that this version still provokes the Assert if 95f4e59c32866716's code fix
is reverted.

I also removed the EXPLAIN output from the test, as it turns out that the
assertion occurs while considering a plan that isn't the one ultimately
selected anyway; so there's no value in risking any cross-platform
variation in that printout.

Back-patch to 9.2, like the previous patch.

9 years agoFix declaration of isarray variable.
Michael Meskes [Thu, 13 Aug 2015 11:22:29 +0000 (13:22 +0200)]
Fix declaration of isarray variable.

Found and fixed by Andres Freund.

9 years agoUndo mistaken tightening in join_is_legal().
Tom Lane [Thu, 13 Aug 2015 01:18:45 +0000 (21:18 -0400)]
Undo mistaken tightening in join_is_legal().

One of the changes I made in commit 8703059c6b55c427 turns out not to have
been such a good idea: we still need the exception in join_is_legal() that
allows a join if both inputs already overlap the RHS of the special join
we're checking.  Otherwise we can miss valid plans, and might indeed fail
to find a plan at all, as in recent report from Andreas Seltenreich.

That code was added way back in commit c17117649b9ae23d, but I failed to
include a regression test case then; my bad.  Put it back with a better
explanation, and a test this time.  The logic does end up a bit different
than before though: I now believe it's appropriate to make this check
first, thereby allowing such a case whether or not we'd consider the
previous SJ(s) to commute with this one.  (Presumably, we already decided
they did; but it was confusing to have this consideration in the middle
of the code that was handling the other case.)

Back-patch to all active branches, like the previous patch.

9 years agoThis routine was calling ecpg_alloc to allocate to memory but did not
Michael Meskes [Thu, 5 Feb 2015 14:12:34 +0000 (15:12 +0100)]
This routine was calling ecpg_alloc to allocate to memory but did not
actually check the returned pointer allocated, potentially NULL which
could be the result of a malloc call.

Issue noted by Coverity, fixed by Michael Paquier <michael@otacoo.com>

9 years agoFix some possible low-memory failures in regexp compilation.
Tom Lane [Wed, 12 Aug 2015 04:48:11 +0000 (00:48 -0400)]
Fix some possible low-memory failures in regexp compilation.

newnfa() failed to set the regex error state when malloc() fails.
Several places in regcomp.c failed to check for an error after calling
subre().  Each of these mistakes could lead to null-pointer-dereference
crashes in memory-starved backends.

Report and patch by Andreas Seltenreich.  Back-patch to all branches.

9 years agoMinor cleanups in slot related code.
Andres Freund [Tue, 11 Aug 2015 10:32:49 +0000 (12:32 +0200)]
Minor cleanups in slot related code.

Fix a bunch of typos, and remove two superflous includes.

Author: Gurjeet Singh
Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
Backpatch: 9.4

9 years agoFix privilege dumping from servers too old to have that type of privilege.
Tom Lane [Tue, 11 Aug 2015 00:10:16 +0000 (20:10 -0400)]
Fix privilege dumping from servers too old to have that type of privilege.

pg_dump produced fairly silly GRANT/REVOKE commands when dumping types from
pre-9.2 servers, and when dumping functions or procedural languages from
pre-7.3 servers.  Those server versions lack the typacl, proacl, and/or
lanacl columns respectively, and pg_dump substituted default values that
were in fact incorrect.  We ended up revoking all the owner's own
privileges for the object while granting all privileges to PUBLIC.
Of course the owner would then have those privileges again via PUBLIC, so
long as she did not try to revoke PUBLIC's privileges; which may explain
the lack of field reports.  Nonetheless this is pretty silly behavior.

The stakes were raised by my recent patch to make pg_dump dump shell types,
because 9.2 and up pg_dump would proceed to emit bogus GRANT/REVOKE
commands for a shell type if dumping from a pre-9.2 server; and the server
will not accept GRANT/REVOKE commands for a shell type.  (Perhaps it
should, but that's a topic for another day.)  So the resulting dump script
wouldn't load without errors.

The right thing to do is to act as though these objects have default
privileges (null ACL entries), which causes pg_dump to print no
GRANT/REVOKE commands at all for them.  That fixes the silly results
and also dodges the problem with shell types.

In passing, modify getProcLangs() to be less creatively different about
how to handle missing columns when dumping from older server versions.
Every other data-acquisition function in pg_dump does that by substituting
appropriate default values in the version-specific SQL commands, and I see
no reason why this one should march to its own drummer.  Its use of
"SELECT *" was likewise not conformant with anyplace else, not to mention
it's not considered good SQL style for production queries.

Back-patch to all supported versions.  Although 9.0 and 9.1 pg_dump don't
have the issue with typacl, they are more likely than newer versions to be
used to dump from ancient servers, so we ought to fix the proacl/lanacl
issues all the way back.

9 years agoAccept alternate spellings of __sparcv7 and __sparcv8.
Tom Lane [Mon, 10 Aug 2015 21:34:51 +0000 (17:34 -0400)]
Accept alternate spellings of __sparcv7 and __sparcv8.

Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__.
Per report from Waldemar Brodkorb.

9 years agoFurther mucking with PlaceHolderVar-related restrictions on join order.
Tom Lane [Mon, 10 Aug 2015 21:18:17 +0000 (17:18 -0400)]
Further mucking with PlaceHolderVar-related restrictions on join order.

Commit 85e5e222b1dd02f135a8c3bf387d0d6d88e669bd turns out not to have taken
care of all cases of the partially-evaluatable-PlaceHolderVar problem found
by Andreas Seltenreich's fuzz testing.  I had set it up to check for risky
PHVs only in the event that we were making a star-schema-based exception to
the param_source_rels join ordering heuristic.  However, it turns out that
the problem can occur even in joins that satisfy the param_source_rels
heuristic, in which case allow_star_schema_join() isn't consulted.
Refactor so that we check for risky PHVs whenever the proposed join has
any remaining parameterization.

Back-patch to 9.2, like the previous patch (except for the regression test
case, which only works back to 9.3 because it uses LATERAL).

Note that this discovery implies that problems of this sort could've
occurred in 9.2 and up even before the star-schema patch; though I've not
tried to prove that experimentally.

9 years agoFix copy & paste mistake in pg_get_replication_slots().
Andres Freund [Mon, 10 Aug 2015 11:28:19 +0000 (13:28 +0200)]
Fix copy & paste mistake in pg_get_replication_slots().

XLogRecPtr was compared with InvalidTransactionId instead of
InvalidXLogRecPtr. As both are defined to the same value this doesn't
cause any actual problems, but it's still wrong.

Backpatch: 9.4-master, bug was introduced in 9.4

9 years agoFix typo in LDAP example
Magnus Hagander [Sun, 9 Aug 2015 12:49:47 +0000 (14:49 +0200)]
Fix typo in LDAP example

Reported by William Meitzen

9 years agoFurther adjustments to PlaceHolderVar removal.
Tom Lane [Fri, 7 Aug 2015 18:13:39 +0000 (14:13 -0400)]
Further adjustments to PlaceHolderVar removal.

A new test case from Andreas Seltenreich showed that we were still a bit
confused about removing PlaceHolderVars during join removal.  Specifically,
remove_rel_from_query would remove a PHV that was used only underneath
the removable join, even if the place where it's used was the join partner
relation and not the join clause being deleted.  This would lead to a
"too late to create a new PlaceHolderInfo" error later on.  We can defend
against that by checking ph_eval_at to see if the PHV could possibly be
getting used at some partner rel.

Also improve some nearby LATERAL-related logic.  I decided that the check
on ph_lateral needed to take precedence over the check on ph_needed, in
case there's a lateral reference underneath the join being considered.
(That may be impossible, but I'm not convinced of it, and it's easy enough
to defend against the case.)  Also, I realized that remove_rel_from_query's
logic for updating LateralJoinInfos is dead code, because we don't build
those at all until after join removal.

Back-patch to 9.3.  Previous versions didn't have the LATERAL issues, of
course, and they also didn't attempt to remove PlaceHolderInfos during join
removal.  (I'm starting to wonder if changing that was really such a great
idea.)

9 years agoFix attach-related race condition in shm_mq_send_bytes.
Robert Haas [Fri, 7 Aug 2015 14:04:07 +0000 (09:04 -0500)]
Fix attach-related race condition in shm_mq_send_bytes.

Spotted by Antonin Houska.

9 years agoFix old oversight in join removal logic.
Tom Lane [Fri, 7 Aug 2015 02:14:07 +0000 (22:14 -0400)]
Fix old oversight in join removal logic.

Commit 9e7e29c75ad441450f9b8287bd51c13521641e3b introduced an Assert that
join removal didn't reduce the eval_at set of any PlaceHolderVar to empty.
At first glance it looks like join_is_removable ensures that's true --- but
actually, the loop in join_is_removable skips PlaceHolderVars that are not
referenced above the join due to be removed.  So, if we don't want any
empty eval_at sets, the right thing to do is to delete any now-unreferenced
PlaceHolderVars from the data structure entirely.

Per fuzz testing by Andreas Seltenreich.  Back-patch to 9.3 where the
aforesaid Assert was added.

9 years agoFix eclass_useful_for_merging to give valid results for appendrel children.
Tom Lane [Fri, 7 Aug 2015 00:14:37 +0000 (20:14 -0400)]
Fix eclass_useful_for_merging to give valid results for appendrel children.

Formerly, this function would always return "true" for an appendrel child
relation, because it would think that the appendrel parent was a potential
join target for the child.  In principle that should only lead to some
inefficiency in planning, but fuzz testing by Andreas Seltenreich disclosed
that it could lead to "could not find pathkey item to sort" planner errors
in odd corner cases.  Specifically, we would think that all columns of a
child table's multicolumn index were interesting pathkeys, causing us to
generate a MergeAppend path that sorts by all the columns.  However, if any
of those columns weren't actually used above the level of the appendrel,
they would not get added to that rel's targetlist, which would result in
being unable to resolve the MergeAppend's sort keys against its targetlist
during createplan.c.

Backpatch to 9.3.  In older versions, columns of an appendrel get added
to its targetlist even if they're not mentioned above the scan level,
so that the failure doesn't occur.  It might be worth back-patching this
fix to older versions anyway, but I'll refrain for the moment.

9 years agoFurther fixes for degenerate outer join clauses.
Tom Lane [Thu, 6 Aug 2015 19:35:27 +0000 (15:35 -0400)]
Further fixes for degenerate outer join clauses.

Further testing revealed that commit f69b4b9495269cc4 was still a few
bricks shy of a load: minor tweaking of the previous test cases resulted
in the same wrong-outer-join-order problem coming back.  After study
I concluded that my previous changes in make_outerjoininfo() were just
accidentally masking the problem, and should be reverted in favor of
forcing syntactic join order whenever an upper outer join's predicate
doesn't mention a lower outer join's LHS.  This still allows the
chained-outer-joins style that is the normally optimizable case.

I also tightened things up some more in join_is_legal().  It seems to me
on review that what's really happening in the exception case where we
ignore a mismatched special join is that we're allowing the proposed join
to associate into the RHS of the outer join we're comparing it to.  As
such, we should *always* insist that the proposed join be a left join,
which eliminates a bunch of rather dubious argumentation.  The case where
we weren't enforcing that was the one that was already known buggy anyway
(it had a violatable Assert before the aforesaid commit) so it hardly
deserves a lot of deference.

Back-patch to all active branches, like the previous patch.  The added
regression test case failed in all branches back to 9.1, and I think it's
only an unrelated change in costing calculations that kept 9.0 from
choosing a broken plan.

9 years agoFix incorrect calculation in shm_mq_receive.
Robert Haas [Thu, 6 Aug 2015 17:25:45 +0000 (13:25 -0400)]
Fix incorrect calculation in shm_mq_receive.

If some, but not all, of the length word has already been read, and the
next attempt to read sees exactly the number of bytes needed to complete
the length word, or fewer, then we'll incorrectly read less than all of
the available data.

Antonin Houska

9 years agoFix `make installcheck` for serializable transactions.
Kevin Grittner [Thu, 6 Aug 2015 15:35:19 +0000 (10:35 -0500)]
Fix `make installcheck` for serializable transactions.

Commit e5550d5fec66aa74caad1f79b79826ec64898688 added some new
tests for ALTER TABLE which involved table scans.  When
default_transaction_isolation = 'serializable' these acquire
relation-level SIReadLocks.  The test results didn't cope with
that.  Add SIReadLock as the minimum lock level for purposes of
these tests.

This could also be fixed by excluding this type of lock from the
my_locks view, but it would be a bug for SIReadLock to show up for
a relation which was not otherwise locked, so do it this way to
allow that sort of condition to cause a regression test failure.

There is some question whether we could avoid taking SIReadLocks
during these operations, but confirming the safety of that and
figuring out how to avoid the locks is not trivial, and would be
a separate patch.

Backpatch to 9.4 where the new tests were added.

9 years agoMake real sure we don't reassociate joins into or out of SEMI/ANTI joins.
Tom Lane [Wed, 5 Aug 2015 18:39:07 +0000 (14:39 -0400)]
Make real sure we don't reassociate joins into or out of SEMI/ANTI joins.

Per the discussion in optimizer/README, it's unsafe to reassociate anything
into or out of the RHS of a SEMI or ANTI join.  An example from Piotr
Stefaniak showed that join_is_legal() wasn't sufficiently enforcing this
rule, so lock it down a little harder.

I couldn't find a reasonably simple example of the optimizer trying to
do this, so no new regression test.  (Piotr's example involved the random
search in GEQO accidentally trying an invalid case and triggering a sanity
check way downstream in clause selectivity estimation, which did not seem
like a sequence of events that would be useful to memorialize in a
regression test as-is.)

Back-patch to all active branches.

9 years agoDocs: add an explicit example about controlling overall greediness of REs.
Tom Lane [Wed, 5 Aug 2015 01:09:12 +0000 (21:09 -0400)]
Docs: add an explicit example about controlling overall greediness of REs.

Per discussion of bug #13538.

9 years agoFix pg_dump to dump shell types.
Tom Lane [Tue, 4 Aug 2015 23:34:12 +0000 (19:34 -0400)]
Fix pg_dump to dump shell types.

Per discussion, it really ought to do this.  The original choice to
exclude shell types was probably made in the dark ages before we made
it harder to accidentally create shell types; but that was in 7.3.

Also, cause the standard regression tests to leave a shell type behind,
for convenience in testing the case in pg_dump and pg_upgrade.

Back-patch to all supported branches.

9 years agoFix bogus "out of memory" reports in tuplestore.c.
Tom Lane [Tue, 4 Aug 2015 22:18:46 +0000 (18:18 -0400)]
Fix bogus "out of memory" reports in tuplestore.c.

The tuplesort/tuplestore memory management logic assumed that the chunk
allocation overhead for its memtuples array could not increase when
increasing the array size.  This is and always was true for tuplesort,
but we (I, I think) blindly copied that logic into tuplestore.c without
noticing that the assumption failed to hold for the much smaller array
elements used by tuplestore.  Given rather small work_mem, this could
result in an improper complaint about "unexpected out-of-memory situation",
as reported by Brent DeSpain in bug #13530.

The easiest way to fix this is just to increase tuplestore's initial
array size so that the assumption holds.  Rather than relying on magic
constants, though, let's export a #define from aset.c that represents
the safe allocation threshold, and make tuplestore's calculation depend
on that.

Do the same in tuplesort.c to keep the logic looking parallel, even though
tuplesort.c isn't actually at risk at present.  This will keep us from
breaking it if we ever muck with the allocation parameters in aset.c.

Back-patch to all supported versions.  The error message doesn't occur
pre-9.3, not so much because the problem can't happen as because the
pre-9.3 tuplestore code neglected to check for it.  (The chance of
trouble is a great deal larger as of 9.3, though, due to changes in the
array-size-increasing strategy.)  However, allowing LACKMEM() to become
true unexpectedly could still result in less-than-desirable behavior,
so let's patch it all the way back.

9 years agoFix a PlaceHolderVar-related oversight in star-schema planning patch.
Tom Lane [Tue, 4 Aug 2015 18:55:32 +0000 (14:55 -0400)]
Fix a PlaceHolderVar-related oversight in star-schema planning patch.

In commit b514a7460d9127ddda6598307272c701cbb133b7, I changed the planner
so that it would allow nestloop paths to remain partially parameterized,
ie the inner relation might need parameters from both the current outer
relation and some upper-level outer relation.  That's fine so long as we're
talking about distinct parameters; but the patch also allowed creation of
nestloop paths for cases where the inner relation's parameter was a
PlaceHolderVar whose eval_at set included the current outer relation and
some upper-level one.  That does *not* work.

In principle we could allow such a PlaceHolderVar to be evaluated at the
lower join node using values passed down from the upper relation along with
values from the join's own outer relation.  However, nodeNestloop.c only
supports simple Vars not arbitrary expressions as nestloop parameters.
createplan.c is also a few bricks shy of being able to handle such cases;
it misplaces the PlaceHolderVar parameters in the plan tree, which is why
the visible symptoms of this bug are "plan should not reference subplan's
variable" and "failed to assign all NestLoopParams to plan nodes" planner
errors.

Adding the necessary complexity to make this work doesn't seem like it
would be repaid in significantly better plans, because in cases where such
a PHV exists, there is probably a corresponding join order constraint that
would allow a good plan to be found without using the star-schema exception.
Furthermore, adding complexity to nodeNestloop.c would create a run-time
penalty even for plans where this whole consideration is irrelevant.
So let's just reject such paths instead.

Per fuzz testing by Andreas Seltenreich; the added regression test is based
on his example query.  Back-patch to 9.2, like the previous patch.

9 years agoCap wal_buffers to avoid a server crash when it's set very large.
Robert Haas [Tue, 4 Aug 2015 16:58:54 +0000 (12:58 -0400)]
Cap wal_buffers to avoid a server crash when it's set very large.

It must be possible to multiply wal_buffers by XLOG_BLCKSZ without
overflowing int, or calculations in StartupXLOG will go badly wrong
and crash the server.  Avoid that by imposing a maximum value on
wal_buffers.  This will be just under 2GB, assuming the usual value
for XLOG_BLCKSZ.

Josh Berkus, per an analysis by Andrew Gierth.

9 years agocontrib/isn now needs a .gitignore file.
Tom Lane [Mon, 3 Aug 2015 03:57:32 +0000 (23:57 -0400)]
contrib/isn now needs a .gitignore file.

Oversight in commit cb3384a0cb4cf900622b77865f60e31259923079.
Back-patch to 9.1, like that commit.

9 years agoFix output of ISBN-13 numbers beginning with 979.
Heikki Linnakangas [Sun, 2 Aug 2015 19:12:33 +0000 (22:12 +0300)]
Fix output of ISBN-13 numbers beginning with 979.

An EAN beginning with 979 (but not 9790 - those are ISMN's) are accepted
as ISBN numbers, but they cannot be represented in the old, 10-digit ISBN
format. They must be output in the new 13-digit ISBN-13 format. We printed
out an incorrect value for those.

Also add a regression test, to test this and some other basic functionality
of the module.

Patch by Fabien Coelho. This fixes bug #13442, reported by B.Z. Backpatch
to 9.1, where we started to recognize ISBN-13 numbers.

9 years agoFix incorrect order of lock file removal and failure to close() sockets.
Tom Lane [Sun, 2 Aug 2015 18:54:44 +0000 (14:54 -0400)]
Fix incorrect order of lock file removal and failure to close() sockets.

Commit c9b0cbe98bd783e24a8c4d8d8ac472a494b81292 accidentally broke the
order of operations during postmaster shutdown: it resulted in removing
the per-socket lockfiles after, not before, postmaster.pid.  This creates
a race-condition hazard for a new postmaster that's started immediately
after observing that postmaster.pid has disappeared; if it sees the
socket lockfile still present, it will quite properly refuse to start.
This error appears to be the explanation for at least some of the
intermittent buildfarm failures we've seen in the pg_upgrade test.

Another problem, which has been there all along, is that the postmaster
has never bothered to close() its listen sockets, but has just allowed them
to close at process death.  This creates a different race condition for an
incoming postmaster: it might be unable to bind to the desired listen
address because the old postmaster is still incumbent.  This might explain
some odd failures we've seen in the past, too.  (Note: this is not related
to the fact that individual backends don't close their client communication
sockets.  That behavior is intentional and is not changed by this patch.)

Fix by adding an on_proc_exit function that closes the postmaster's ports
explicitly, and (in 9.3 and up) reshuffling the responsibility for where
to unlink the Unix socket files.  Lock file unlinking can stay where it
is, but teach it to unlink the lock files in reverse order of creation.

9 years agoFix race condition that lead to WALInsertLock deadlock with commit_delay.
Heikki Linnakangas [Sun, 2 Aug 2015 17:08:10 +0000 (20:08 +0300)]
Fix race condition that lead to WALInsertLock deadlock with commit_delay.

If a call to WaitForXLogInsertionsToFinish() returned a value in the middle
of a page, and another backend then started to insert a record to the same
page, and then you called WaitXLogInsertionsToFinish() again, the second
call might return a smaller value than the first call. The problem was in
GetXLogBuffer(), which always updated the insertingAt value to the
beginning of the requested page, not the actual requested location. Because
of that, the second call might return a xlog pointer to the beginning of
the page, while the first one returned a later position on the same page.
XLogFlush() performs two calls to WaitXLogInsertionsToFinish() in
succession, and holds WALWriteLock on the second call, which can deadlock
if the second call to WaitXLogInsertionsToFinish() blocks.

Reported by Spiros Ioannou. Backpatch to 9.4, where the more scalable
WALInsertLock mechanism, and this bug, was introduced.

9 years agoFix some planner issues with degenerate outer join clauses.
Tom Lane [Sun, 2 Aug 2015 00:57:41 +0000 (20:57 -0400)]
Fix some planner issues with degenerate outer join clauses.

An outer join clause that didn't actually reference the RHS (perhaps only
after constant-folding) could confuse the join order enforcement logic,
leading to wrong query results.  Also, nested occurrences of such things
could trigger an Assertion that on reflection seems incorrect.

Per fuzz testing by Andreas Seltenreich.  The practical use of such cases
seems thin enough that it's not too surprising we've not heard field
reports about it.

This has been broken for a long time, so back-patch to all active branches.

9 years agoFix an oversight in checking whether a join with LATERAL refs is legal.
Tom Lane [Fri, 31 Jul 2015 23:26:33 +0000 (19:26 -0400)]
Fix an oversight in checking whether a join with LATERAL refs is legal.

In many cases, we can implement a semijoin as a plain innerjoin by first
passing the righthand-side relation through a unique-ification step.
However, one of the cases where this does NOT work is where the RHS has
a LATERAL reference to the LHS; that makes the RHS dependent on the LHS
so that unique-ification is meaningless.  joinpath.c understood this,
and so would not generate any join paths of this kind ... but join_is_legal
neglected to check for the case, so it would think that we could do it.
The upshot would be a "could not devise a query plan for the given query"
failure once we had failed to generate any join paths at all for the bogus
join pair.

Back-patch to 9.3 where LATERAL was added.

9 years agoConsolidate makefile code for setting top_srcdir, srcdir and VPATH.
Noah Misch [Fri, 31 Jul 2015 00:48:41 +0000 (20:48 -0400)]
Consolidate makefile code for setting top_srcdir, srcdir and VPATH.

Responsibility was formerly split between Makefile.global and pgxs.mk.
As a result of commit b58233c71b93a32fcab7219585cafc25a27eb769, in the
PGXS case, these variables were unset while parsing Makefile.global and
callees.  Inclusion of Makefile.custom did not work from PGXS, and the
subtle difference seemed like a recipe for future bugs.  Back-patch to
9.4, where that commit first appeared.

9 years agoAvoid some zero-divide hazards in the planner.
Tom Lane [Thu, 30 Jul 2015 16:11:23 +0000 (12:11 -0400)]
Avoid some zero-divide hazards in the planner.

Although I think on all modern machines floating division by zero
results in Infinity not SIGFPE, we still don't want infinities
running around in the planner's costing estimates; too much risk
of that leading to insane behavior.

grouping_planner() failed to consider the possibility that final_rel
might be known dummy and hence have zero rowcount.  (I wonder if it
would be better to set a rows estimate of 1 for dummy relations?
But at least in the back branches, changing this convention seems
like a bad idea, so I'll leave that for another day.)

Make certain that get_variable_numdistinct() produces a nonzero result.
The case that can be shown to be broken is with stadistinct < 0.0 and
small ntuples; we did not prevent the result from rounding to zero.
For good luck I applied clamp_row_est() to all the nonconstant return
values.

In ExecChooseHashTableSize(), Assert that we compute positive nbuckets
and nbatch.  I know of no reason to think this isn't the case, but it
seems like a good safety check.

Per reports from Piotr Stefaniak.  Back-patch to all active branches.

9 years agoBlacklist xlc 32-bit inlining.
Noah Misch [Thu, 30 Jul 2015 02:49:48 +0000 (22:49 -0400)]
Blacklist xlc 32-bit inlining.

Per a suggestion from Tom Lane.  Back-patch to 9.0 (all supported
versions).  While only 9.4 and up have code known to elicit this
compiler bug, we were disabling inlining by accident until commit
43d89a23d59c487bc9258fad7a6187864cb8c0c0.

9 years agoUpdate our documentation concerning where to create data directories.
Tom Lane [Tue, 28 Jul 2015 22:42:59 +0000 (18:42 -0400)]
Update our documentation concerning where to create data directories.

Although initdb has long discouraged use of a filesystem mount-point
directory as a PG data directory, this point was covered nowhere in the
user-facing documentation.  Also, with the popularity of pg_upgrade,
we really need to recommend that the PG user own not only the data
directory but its parent directory too.  (Without a writable parent
directory, operations such as "mv data data.old" fail immediately.
pg_upgrade itself doesn't do that, but wrapper scripts for it often do.)

Hence, adjust the "Creating a Database Cluster" section to address
these points.  I also took the liberty of wordsmithing the discussion
of NFS a bit.

These considerations aren't by any means new, so back-patch to all
supported branches.

9 years agoReduce chatter from signaling of autovacuum workers.
Tom Lane [Tue, 28 Jul 2015 21:34:00 +0000 (17:34 -0400)]
Reduce chatter from signaling of autovacuum workers.

Don't print a WARNING if we get ESRCH from a kill() that's attempting
to cancel an autovacuum worker.  It's possible (and has been seen in the
buildfarm) that the worker is already gone by the time we are able to
execute the kill, in which case the failure is harmless.  About the only
plausible reason for reporting such cases would be to help debug corrupted
lock table contents, but this is hardly likely to be the most important
symptom if that happens.  Moreover issuing a WARNING might scare users
more than is warranted.

Also, since sending a signal to an autovacuum worker is now entirely a
routine thing, and the worker will log the query cancel on its end anyway,
reduce the message saying we're doing that from LOG to DEBUG1 level.

Very minor cosmetic cleanup as well.

Since the main practical reason for doing this is to avoid unnecessary
buildfarm failures, back-patch to all active branches.

9 years agoDisable ssl renegotiation by default.
Andres Freund [Tue, 28 Jul 2015 19:39:40 +0000 (21:39 +0200)]
Disable ssl renegotiation by default.

While postgres' use of SSL renegotiation is a good idea in theory, it
turned out to not work well in practice. The specification and openssl's
implementation of it have lead to several security issues. Postgres' use
of renegotiation also had its share of bugs.

Additionally OpenSSL has a bunch of bugs around renegotiation, reported
and open for years, that regularly lead to connections breaking with
obscure error messages. We tried increasingly complex workarounds to get
around these bugs, but we didn't find anything complete.

Since these connection breakages often lead to hard to debug problems,
e.g. spuriously failing base backups and significant latency spikes when
synchronous replication is used, we have decided to change the default
setting for ssl renegotiation to 0 (disabled) in the released
backbranches and remove it entirely in 9.5 and master..

Author: Michael Paquier, with changes by me
Discussion: 20150624144148.GQ4797@alap3.anarazel.de
Backpatch: 9.0-9.4; 9.5 and master get a different patch

9 years agoMake tap tests store postmaster logs and handle vpaths correctly
Andrew Dunstan [Tue, 28 Jul 2015 20:04:54 +0000 (16:04 -0400)]
Make tap tests store postmaster logs and handle vpaths correctly

Given this it is possible that the buildfarm animals running these tests
will be able to capture adequate logging to allow diagnosis of failures.

9 years agoRemove an unsafe Assert, and explain join_clause_is_movable_into() better.
Tom Lane [Tue, 28 Jul 2015 17:20:40 +0000 (13:20 -0400)]
Remove an unsafe Assert, and explain join_clause_is_movable_into() better.

join_clause_is_movable_into() is approximate, in the sense that it might
sometimes return "false" when actually it would be valid to push the given
join clause down to the specified level.  This is okay ... but there was
an Assert in get_joinrel_parampathinfo() that's only safe if the answers
are always exact.  Comment out the Assert, and add a bunch of commentary
to clarify what's going on.

Per fuzz testing by Andreas Seltenreich.  The added regression test is
a pretty silly query, but it's based on his crasher example.

Back-patch to 9.2 where the faulty logic was introduced.

9 years agoImprove logging of TAP tests.
Andrew Dunstan [Tue, 28 Jul 2015 16:22:21 +0000 (12:22 -0400)]
Improve logging of TAP tests.

Create a log file for each test run. Stdout and stderr of the test script,
as well as any subprocesses run as part of the test, are redirected to
the log file. This makes it a lot easier to debug test failures. Also print
the test output (ok 12 - ... messages) to the log file, and the command
line of any external programs executed with the system_or_bail and run_log
functions. This makes it a lot easier to debug failing tests.

Modify some of the pg_ctl and other command invocations to not use 'silent'
or 'quiet' options, and don't redirect output to /dev/null, so that you get
all the information in the log instead.

In the passing, construct some command lines in a way that works if $tempdir
contains quote-characters. I haven't systematically gone through all of
them or tested that, so I don't know if this is enough to make that work.

pg_rewind tests had a custom mechanism for creating a similar log file. Use
the new generic facility instead.

Michael Paquier and Heikki Linnakangas.

This os a backpatch of Heikki's commit
1ea06203b82b98b5098808667f6ba652181ef5b2 modified by me to suit 9.4

9 years agoDon't assume that PageIsEmpty() returns true on an all-zeros page.
Heikki Linnakangas [Mon, 27 Jul 2015 15:54:09 +0000 (18:54 +0300)]
Don't assume that PageIsEmpty() returns true on an all-zeros page.

It does currently, and I don't see us changing that any time soon, but we
don't make that assumption anywhere else.

Per Tom Lane's suggestion. Backpatch to 9.2, like the previous patch that
added this assumption.

9 years agoReuse all-zero pages in GIN.
Heikki Linnakangas [Mon, 27 Jul 2015 09:30:26 +0000 (12:30 +0300)]
Reuse all-zero pages in GIN.

In GIN, an all-zeros page would be leaked forever, and never reused. Just
add them to the FSM in vacuum, and they will be reinitialized when grabbed
from the FSM. On master and 9.5, attempting to access the page's opaque
struct also caused an assertion failure, although that was otherwise
harmless.

Reported by Jeff Janes. Backpatch to all supported versions.

9 years agoFix handling of all-zero pages in SP-GiST vacuum.
Heikki Linnakangas [Mon, 27 Jul 2015 09:28:21 +0000 (12:28 +0300)]
Fix handling of all-zero pages in SP-GiST vacuum.

SP-GiST initialized an all-zeros page at vacuum, but that was not
WAL-logged, which is not safe. You might get a torn page write, when it gets
flushed to disk, and end-up with a half-initialized index page. To fix,
leave it in the all-zeros state, and add it to the FSM. It will be
initialized when reused. Also don't set the page-deleted flag when recycling
an empty page. That was also not WAL-logged, and a torn write of that would
cause the page to have an invalid checksum.

Backpatch to 9.2, where SP-GiST indexes were added.

9 years agoMake entirely-dummy appendrels get marked as such in set_append_rel_size.
Tom Lane [Sun, 26 Jul 2015 20:19:08 +0000 (16:19 -0400)]
Make entirely-dummy appendrels get marked as such in set_append_rel_size.

The planner generally expects that the estimated rowcount of any relation
is at least one row, *unless* it has been proven empty by constraint
exclusion or similar mechanisms, which is marked by installing a dummy path
as the rel's cheapest path (cf. IS_DUMMY_REL).  When I split up
allpaths.c's processing of base rels into separate set_base_rel_sizes and
set_base_rel_pathlists steps, the intention was that dummy rels would get
marked as such during the "set size" step; this is what justifies an Assert
in indxpath.c's get_loop_count that other relations should either be dummy
or have positive rowcount.  Unfortunately I didn't get that quite right
for append relations: if all the child rels have been proven empty then
set_append_rel_size would come up with a rowcount of zero, which is
correct, but it didn't then do set_dummy_rel_pathlist.  (We would have
ended up with the right state after set_append_rel_pathlist, but that's
too late, if we generate indexpaths for some other rel first.)

In addition to fixing the actual bug, I installed an Assert enforcing this
convention in set_rel_size; that then allows simplification of a couple
of now-redundant tests for zero rowcount in set_append_rel_size.

Also, to cover the possibility that third-party FDWs have been careless
about not returning a zero rowcount estimate, apply clamp_row_est to
whatever an FDW comes up with as the rows estimate.

Per report from Andreas Seltenreich.  Back-patch to 9.2.  Earlier branches
did not have the separation between set_base_rel_sizes and
set_base_rel_pathlists steps, so there was no intermediate state where an
appendrel would have had inconsistent rowcount and pathlist.  It's possible
that adding the Assert to set_rel_size would be a good idea in older
branches too; but since they're not under development any more, it's likely
not worth the trouble.

9 years agoRestore use of zlib default compression in pg_dump directory mode.
Andrew Dunstan [Sat, 25 Jul 2015 21:14:36 +0000 (17:14 -0400)]
Restore use of zlib default compression in pg_dump directory mode.

This was broken by commit 0e7e355f27302b62af3e1add93853ccd45678443 and
friends, which ignored the fact that gzopen() will treat "-1" in the
mode argument as an invalid character, which it ignores, and a flag for
compression level 1. Now, when this value is encountered no compression
level flag is passed  to gzopen, leaving it to use the zlib default.

Also, enforce the documented allowed range for pg_dump's -Z option,
namely 0 .. 9, and remove some consequently dead code from
pg_backup_tar.c.

Problem reported by Marc Mamin.

Backpatch to 9.1, like the patch that introduced the bug.

9 years agoFix off-by-one error in calculating subtrans/multixact truncation point.
Heikki Linnakangas [Wed, 22 Jul 2015 22:30:09 +0000 (01:30 +0300)]
Fix off-by-one error in calculating subtrans/multixact truncation point.

If there were no subtransactions (or multixacts) active, we would calculate
the oldestxid == next xid. That's correct, but if next XID happens to be
on the next pg_subtrans (pg_multixact) page, the page does not exist yet,
and SimpleLruTruncate will produce an "apparent wraparound" warning. The
warning is harmless in this case, but looks very alarming to users.

Backpatch to all supported versions. Patch and analysis by Thomas Munro.

9 years agoFix add_rte_to_flat_rtable() for recent feature additions.
Tom Lane [Wed, 22 Jul 2015 00:03:58 +0000 (20:03 -0400)]
Fix add_rte_to_flat_rtable() for recent feature additions.

The TABLESAMPLE and row security patches each overlooked this function,
though their errors of omission were opposite: RLS failed to zero out the
securityQuals field, leading to wasteful copying of useless expression
trees in finished plans, while TABLESAMPLE neglected to add a comment
saying that it intentionally *isn't* deleting the tablesample subtree.
There probably should be a similar comment about ctename, too.

Back-patch as appropriate.

9 years agoFix (some of) pltcl memory usage
Alvaro Herrera [Mon, 20 Jul 2015 12:18:08 +0000 (14:18 +0200)]
Fix (some of) pltcl memory usage

As reported by Bill Parker, PL/Tcl did not validate some malloc() calls
against NULL return.  Fix by using palloc() in a new long-lived memory
context instead.  This allows us to simplify error handling too, by
simply deleting the memory context instead of doing retail frees.

There's still a lot that could be done to improve PL/Tcl's memory
handling ...

This is pretty ancient, so backpatch all the way back.

Author: Michael Paquier and Álvaro Herrera
Discussion: https://www.postgresql.org/message-id/CAFrbyQwyLDYXfBOhPfoBGqnvuZO_Y90YgqFM11T2jvnxjLFmqw@mail.gmail.com

9 years agoMake WaitLatchOrSocket's timeout detection more robust.
Tom Lane [Sat, 18 Jul 2015 15:47:13 +0000 (11:47 -0400)]
Make WaitLatchOrSocket's timeout detection more robust.

In the previous coding, timeout would be noticed and reported only when
poll() or socket() returned zero (or the equivalent behavior on Windows).
Ordinarily that should work well enough, but it seems conceivable that we
could get into a state where poll() always returns a nonzero value --- for
example, if it is noticing a condition on one of the file descriptors that
we do not think is reason to exit the loop.  If that happened, we'd be in a
busy-wait loop that would fail to terminate even when the timeout expires.

We can make this more robust at essentially no cost, by deciding to exit
of our own accord if we compute a zero or negative time-remaining-to-wait.
Previously the code noted this but just clamped the time-remaining to zero,
expecting that we'd detect timeout on the next loop iteration.

Back-patch to 9.2.  While 9.1 had a version of WaitLatchOrSocket, it was
primitive compared to later versions, and did not guarantee reliable
detection of timeouts anyway.  (Essentially, this is a refinement of
commit 3e7fdcffd6f77187, which was back-patched only as far as 9.2.)

9 years agoAIX: Test the -qlonglong option before use.
Noah Misch [Fri, 17 Jul 2015 07:01:14 +0000 (03:01 -0400)]
AIX: Test the -qlonglong option before use.

xlc provides "long long" unconditionally at C99-compatible language
levels, and this option provokes a warning.  The warning interferes with
"configure" tests that fail in response to any warning.  Notably, before
commit 85a2a8903f7e9151793308d0638621003aded5ae, it interfered with the
test for -qnoansialias.  Back-patch to 9.0 (all supported versions).

9 years agoFix a low-probability crash in our qsort implementation.
Tom Lane [Fri, 17 Jul 2015 02:57:46 +0000 (22:57 -0400)]
Fix a low-probability crash in our qsort implementation.

It's standard for quicksort implementations, after having partitioned the
input into two subgroups, to recurse to process the smaller partition and
then handle the larger partition by iterating.  This method guarantees
that no more than log2(N) levels of recursion can be needed.  However,
Bentley and McIlroy argued that checking to see which partition is smaller
isn't worth the cycles, and so their code doesn't do that but just always
recurses on the left partition.  In most cases that's fine; but with
worst-case input we might need O(N) levels of recursion, and that means
that qsort could be driven to stack overflow.  Such an overflow seems to
be the only explanation for today's report from Yiqing Jin of a SIGSEGV
in med3_tuple while creating an index of a couple billion entries with a
very large maintenance_work_mem setting.  Therefore, let's spend the few
additional cycles and lines of code needed to choose the smaller partition
for recursion.

Also, fix up the qsort code so that it properly uses size_t not int for
some intermediate values representing numbers of items.  This would only
be a live risk when sorting more than INT_MAX bytes (in qsort/qsort_arg)
or tuples (in qsort_tuple), which I believe would never happen with any
caller in the current core code --- but perhaps it could happen with
call sites in third-party modules?  In any case, this is trouble waiting
to happen, and the corrected code is probably if anything shorter and
faster than before, since it removes sign-extension steps that had to
happen when converting between int and size_t.

In passing, move a couple of CHECK_FOR_INTERRUPTS() calls so that it's
not necessary to preserve the value of "r" across them, and prettify
the output of gen_qsort_tuple.pl a little.

Back-patch to all supported branches.  The odds of hitting this issue
are probably higher in 9.4 and up than before, due to the new ability
to allocate sort workspaces exceeding 1GB, but there's no good reason
to believe that it's impossible to crash older branches this way.

9 years agoFix spelling error
Magnus Hagander [Thu, 16 Jul 2015 07:31:58 +0000 (10:31 +0300)]
Fix spelling error

David Rowley

9 years agoAIX: Link the postgres executable with -Wl,-brtllib.
Noah Misch [Thu, 16 Jul 2015 01:00:26 +0000 (21:00 -0400)]
AIX: Link the postgres executable with -Wl,-brtllib.

This allows PostgreSQL modules and their dependencies to have undefined
symbols, resolved at runtime.  Perl module shared objects rely on that
in Perl 5.8.0 and later.  This fixes the crash when PL/PerlU loads such
modules, as the hstore_plperl test suite does.  Module authors can link
using -Wl,-G to permit undefined symbols; by default, linking will fail
as it has.  Back-patch to 9.0 (all supported versions).

9 years agoFix assorted memory leaks.
Tom Lane [Sun, 12 Jul 2015 20:25:51 +0000 (16:25 -0400)]
Fix assorted memory leaks.

Per Coverity (not that any of these are so non-obvious that they should not
have been caught before commit).  The extent of leakage is probably minor
to unnoticeable, but a leak is a leak.  Back-patch as necessary.

Michael Paquier

9 years agoImprove documentation about array concat operator vs. underlying functions.
Tom Lane [Thu, 9 Jul 2015 22:50:31 +0000 (18:50 -0400)]
Improve documentation about array concat operator vs. underlying functions.

The documentation implied that there was seldom any reason to use the
array_append, array_prepend, and array_cat functions directly.  But that's
not really true, because they can help make it clear which case is meant,
which the || operator can't do since it's overloaded to represent all three
cases.  Add some discussion and examples illustrating the potentially
confusing behavior that can ensue if the parser misinterprets what was
meant.

Per a complaint from Michael Herold.  Back-patch to 9.2, which is where ||
started to behave this way.

9 years agoFix postmaster's handling of a startup-process crash.
Tom Lane [Thu, 9 Jul 2015 17:22:23 +0000 (13:22 -0400)]
Fix postmaster's handling of a startup-process crash.

Ordinarily, a failure (unexpected exit status) of the startup subprocess
should be considered fatal, so the postmaster should just close up shop
and quit.  However, if we sent the startup process a SIGQUIT or SIGKILL
signal, the failure is hardly "unexpected", and we should attempt restart;
this is necessary for recovery from ordinary backend crashes in hot-standby
scenarios.  I attempted to implement the latter rule with a two-line patch
in commit 442231d7f71764b8c628044e7ce2225f9aa43b67, but it now emerges that
that patch was a few bricks shy of a load: it failed to distinguish the
case of a signaled startup process from the case where the new startup
process crashes before reaching database consistency.  That resulted in
infinitely respawning a new startup process only to have it crash again.

To handle this properly, we really must track whether we have sent the
*current* startup process a kill signal.  Rather than add yet another
ad-hoc boolean to the postmaster's state, I chose to unify this with the
existing RecoveryError flag into an enum tracking the startup process's
state.  That seems more consistent with the postmaster's general state
machine design.

Back-patch to 9.0, like the previous patch.

9 years agoFix another broken link in documentation.
Heikki Linnakangas [Thu, 9 Jul 2015 13:00:14 +0000 (16:00 +0300)]
Fix another broken link in documentation.

Tom fixed another one of these in commit 7f32dbcd, but there was another
almost identical one in libpq docs. Per his comment:

HP's web server has apparently become case-sensitive sometime recently.
Per bug #13479 from Daniel Abraham.  Corrected link identified by Alvaro.

9 years agoReplace use of "diff -q".
Noah Misch [Thu, 9 Jul 2015 00:44:21 +0000 (20:44 -0400)]
Replace use of "diff -q".

POSIX does not specify the -q option, and many implementations do not
offer it.  Don't bother changing the MSVC build system, because having
non-GNU diff on Windows is vanishingly unlikely.  Back-patch to 9.2,
where this invocation was introduced.

9 years agoFix null pointer dereference in "\c" psql command.
Noah Misch [Thu, 9 Jul 2015 00:44:21 +0000 (20:44 -0400)]
Fix null pointer dereference in "\c" psql command.

The psql crash happened when no current connection existed.  (The second
new check is optional given today's undocumented NULL argument handling
in PQhost() etc.)  Back-patch to 9.0 (all supported versions).

9 years agoFix portability issue in pg_upgrade test script: avoid $PWD.
Tom Lane [Tue, 7 Jul 2015 16:49:18 +0000 (12:49 -0400)]
Fix portability issue in pg_upgrade test script: avoid $PWD.

SUSv2-era shells don't set the PWD variable, though anything more modern
does.  In the buildfarm environment this could lead to test.sh executing
with PWD pointing to $HOME or another high-level directory, so that there
were conflicts between concurrent executions of the test in different
branch subdirectories.  This appears to be the explanation for recent
intermittent failures on buildfarm members binturong and dingo (and might
well have something to do with the buildfarm script's failure to capture
log files from pg_upgrade tests, too).

To fix, just use `pwd` in place of $PWD.  AFAICS test.sh is the only place
in our source tree that depended on $PWD.  Back-patch to all versions
containing this script.

Per buildfarm.  Thanks to Oskari Saarenmaa for diagnosing the problem.

9 years agoImprove handling of out-of-memory in libpq.
Heikki Linnakangas [Tue, 7 Jul 2015 15:37:45 +0000 (18:37 +0300)]
Improve handling of out-of-memory in libpq.

If an allocation fails in the main message handling loop, pqParseInput3
or pqParseInput2, it should not be treated as "not enough data available
yet". Otherwise libpq will wait indefinitely for more data to arrive from
the server, and gets stuck forever.

This isn't a complete fix - getParamDescriptions and getCopyStart still
have the same issue, but it's a step in the right direction.

Michael Paquier and me. Backpatch to all supported versions.

9 years agoTurn install.bat into a pure one line wrapper fort he perl script.
Heikki Linnakangas [Tue, 7 Jul 2015 13:31:52 +0000 (16:31 +0300)]
Turn install.bat into a pure one line wrapper fort he perl script.

Build.bat and vcregress.bat got similar treatment years ago. I'm not sure
why install.bat wasn't treated at the same time, but it seems like a good
idea anyway.

The immediate problem with the old install.bat was that it had quoting
issues, and wouldn't work if the target directory's name contained spaces.
This fixes that problem.

I committed this to master yesterday, this is a backpatch of the same for
all supported versions.

9 years agoFix logical decoding bug leading to inefficient reopening of files.
Andres Freund [Tue, 7 Jul 2015 11:12:59 +0000 (13:12 +0200)]
Fix logical decoding bug leading to inefficient reopening of files.

When spilling transaction data to disk a simple typo caused the output
file to be closed and reopened for every serialized change. That happens
to not have a huge impact on linux, which is why it probably wasn't
noticed so far, but on windows that appears to trigger actual disk
writes after every change. Not fun.

The bug fortunately does not have any impact besides speed. A change
could end up being in the wrong segment (last instead of next), but
since we read all files to the end, that's just ugly, not really
problematic. It's not a problem to upgrade, since transaction spill
files do not persist across restarts.

Bug: #13484
Reported-By: Olivier Gosseaume
Discussion: 20150703090217.1190.63940@wrigleys.postgresql.org

Backpatch to 9.4, where logical decoding was added.

9 years agoFix pg_recvlogical not to fsync output when it's a tty or pipe.
Andres Freund [Tue, 7 Jul 2015 10:47:44 +0000 (12:47 +0200)]
Fix pg_recvlogical not to fsync output when it's a tty or pipe.

The previous coding tried to handle possible failures when fsyncing a
tty or pipe fd by accepting EINVAL - but apparently some
platforms (windows, OSX) don't reliably return that. So instead check
whether the output fd refers to a pipe or a tty when opening it.

Reported-By: Olivier Gosseaume, Marko Tiikkaja
Discussion: 559AF98B.3050901@joh.to

Backpatch to 9.4, where pg_recvlogical was added.

9 years agoRemove incorrect warning from pg_archivecleanup document.
Fujii Masao [Mon, 6 Jul 2015 11:58:58 +0000 (20:58 +0900)]
Remove incorrect warning from pg_archivecleanup document.

The .backup file name can be passed to pg_archivecleanup even if
it includes the extension which is specified in -x option.
However, previously the document incorrectly warned a user
not to do that.

Back-patch to 9.2 where pg_archivecleanup's -x option and
the warning were added.

9 years agoFix some typos in regression test comments.
Tom Lane [Sun, 5 Jul 2015 17:14:38 +0000 (13:14 -0400)]
Fix some typos in regression test comments.

Back-patch to avoid unnecessary cross-branch differences.

CharSyam

9 years agoMake numeric form of PG version number readily available in Makefiles.
Tom Lane [Sun, 5 Jul 2015 16:01:01 +0000 (12:01 -0400)]
Make numeric form of PG version number readily available in Makefiles.

Expose PG_VERSION_NUM (e.g., "90600") as a Make variable; but for
consistency with the other Make variables holding similar info,
call the variable just VERSION_NUM not PG_VERSION_NUM.

There was some discussion of making this value available as a pg_config
value as well.  However, that would entail substantially more work than
this two-line patch.  Given that there was not exactly universal consensus
that we need this at all, let's just do a minimal amount of work for now.

Back-patch of commit a5d489ccb7e613c7ca3be6141092b8c1d2c13fa7, so that this
variable is actually useful for its intended purpose sometime before 2020.

Michael Paquier, reviewed by Pavel Stehule

9 years agoPL/Perl: Add alternative expected file for Perl 5.22
Peter Eisentraut [Sun, 21 Jun 2015 14:37:24 +0000 (10:37 -0400)]
PL/Perl: Add alternative expected file for Perl 5.22