35421 Commits

Author SHA1 Message Date
Heikki Linnakangas
06651648a6 Don't allow logging in with empty password.
Some authentication methods allowed it, others did not. In the client-side,
libpq does not even try to authenticate with an empty password, which makes
using empty passwords hazardous: an administrator might think that an
account with an empty password cannot be used to log in, because psql
doesn't allow it, and not realize that a different client would in fact
allow it. To clear that confusion and to be be consistent, disallow empty
passwords in all authentication methods.

All the authentication methods that used plaintext authentication over the
wire, except for BSD authentication, already checked that the password
received from the user was not empty. To avoid forgetting it in the future
again, move the check to the recv_password_packet function. That only
forbids using an empty password with plaintext authentication, however.
MD5 and SCRAM need a different fix:

* In stable branches, check that the MD5 hash stored for the user does not
not correspond to an empty string. This adds some overhead to MD5
authentication, because the server needs to compute an extra MD5 hash, but
it is not noticeable in practice.

* In HEAD, modify CREATE and ALTER ROLE to clear the password if an empty
string, or a password hash that corresponds to an empty string, is
specified. The user-visible behavior is the same as in the stable branches,
the user cannot log in, but it seems better to stop the empty password from
entering the system in the first place. Secondly, it is fairly expensive to
check that a SCRAM hash doesn't correspond to an empty string, because
computing a SCRAM hash is much more expensive than an MD5 hash by design,
so better avoid doing that on every authentication.

We could clear the password on CREATE/ALTER ROLE also in stable branches,
but we would still need to check at authentication time, because even if we
prevent empty passwords from being stored in pg_authid, there might be
existing ones there already.

Reported by Jeroen van der Ham, Ben de Graaff and Jelte Fennema.

Security: CVE-2017-7546
2017-08-07 17:04:17 +03:00
Tom Lane
41d2d46e96 Release notes for 9.6.4, 9.5.8, 9.4.13, 9.3.18, 9.2.22. 2017-08-06 17:56:49 -04:00
Tom Lane
c180d2eb76 Disallow SSL session tickets.
We don't actually support session tickets, since we do not create an SSL
session identifier.  But it seems that OpenSSL will issue a session ticket
on-demand anyway, which will then fail when used.  This results in
reconnection failures when using ticket-aware client-side SSL libraries
(such as the Npgsql .NET driver), as reported by Shay Rojansky.

To fix, just tell OpenSSL not to issue tickets.  At some point in the
far future, we might consider enabling tickets instead.  But the security
implications of that aren't entirely clear; and besides it would have
little benefit except for very short-lived database connections, which is
Something We're Bad At anyhow.  It would take a lot of other work to get
to a point where that would really be an exciting thing to do.

While at it, also tell OpenSSL not to use a session cache.  This doesn't
really do anything, since a backend would never populate the cache anyway,
but it might gain some micro-efficiencies and/or reduce security
exposures.

Patch by me, per discussion with Heikki Linnakangas and Shay Rojansky.
Back-patch to all supported versions.

Discussion: https://postgr.es/m/CADT4RqBU8N-csyZuzaook-c795dt22Zcwg1aHWB6tfVdAkodZA@mail.gmail.com
2017-08-04 11:07:11 -04:00
Peter Eisentraut
22eb38caa1 Add missing ALTER USER variants
ALTER USER ... SET did not support all the syntax variants of ALTER ROLE
...  SET.

Reported-by: Pavel Golub <pavel@microolap.com>
2017-08-03 21:42:46 -04:00
Tom Lane
2666726cd2 Add pgtcl back to the list of externally-maintained client interfaces.
FlightAware is still maintaining this, and indeed is seemingly being
more active with it than the pgtclng fork is.  List both, for the
time being anyway.

In the back branches, also back-port commit e20f679f6 and other
recent updates to the client-interfaces list.  I think these are
probably of current interest to users of back branches.  I did
not touch the list of externally maintained PLs in the back
branches, though.  Those are much more likely to be server version
sensitive, and I don't know which of these PLs work all the way back.

Discussion: https://postgr.es/m/20170730162612.1449.58796@wrigleys.postgresql.org
2017-08-02 16:55:03 -04:00
Tom Lane
1188b9b2c3 Silence warning from modern perl about unescaped braces
Back-patch commit 76a1c97bf21c301f61bb41345e0cdd0d365b2086 into
the older branches (9.5 and before).  This is needed because perl
5.26 and later treats the case as an error not just a warning.

Original patch by Andrew Dunstan, need for back-patch noted by
Ashutosh Sharma

Discussion: https://postgr.es/m/CAE9k0PkfNcmj9pA7Yx4qQ=K=3aY4TuiRhp7KYpayDWm9MYsnjg@mail.gmail.com
2017-08-02 15:07:21 -04:00
Tom Lane
c522b52d3c Doc: specify that the minimum supported version of Perl is 5.8.3.
Previously the docs just said "5.8 or later".  Experimentation shows
that while you can build on Unix from a git checkout with 5.8.0,
compiling recent PL/Perl requires at least 5.8.1, and you won't be
able to run the TAP tests with less than 5.8.3 because that's when
they added "prove".  (I do not have any information on just what the
MSVC build scripts require.)

Since all these versions are quite ancient, let's not split hairs
in the docs, but just say that 5.8.3 is the minimum requirement.

Discussion: https://postgr.es/m/16894.1501392088@sss.pgh.pa.us
2017-07-31 13:42:48 -04:00
Tom Lane
456c7dff2a PL/Perl portability fix: absorb relevant -D switches from Perl.
Back-patch of commit 3c163a7fc76debbbdad1bdd3c43721cffe72f4db,
which see for more info.

Also throw in commit b4cc35fbb709bd6fcae8998f041fd731c9acbf42,
so Coverity doesn't whine about the back branches.

Ashutosh Sharma, some adjustments by me

Discussion: https://postgr.es/m/CANFyU97OVQ3+Mzfmt3MhuUm5NwPU=-FtbNH5Eb7nZL9ua8=rcA@mail.gmail.com
2017-07-31 12:38:35 -04:00
Tom Lane
b4b958251f PL/Perl portability fix: avoid including XSUB.h in plperl.c.
Back-patch of commit bebe174bb4462ef079a1d7eeafb82ff969f160a4,
which see for more info.

Patch by me, with some help from Ashutosh Sharma

Discussion: https://postgr.es/m/CANFyU97OVQ3+Mzfmt3MhuUm5NwPU=-FtbNH5Eb7nZL9ua8=rcA@mail.gmail.com
2017-07-31 12:10:36 -04:00
Tatsuo Ishii
7b38063603 Add missing comment in postgresql.conf.
current_source requires to restart server to reflect the new
value. Per Yugo Nagata and Masahiko Sawada.

Back patched to 9.2 and beyond.
2017-07-31 11:28:49 +09:00
Tom Lane
70f90a103c Fix psql tab completion for CREATE USER MAPPING.
After typing CREATE USER M..., it would not fill in MAPPING FOR,
even though that was clearly intended behavior.

Jeff Janes

Discussion: https://postgr.es/m/CAMkU=1wo2iQ6jWnN=egqOb5NxEPn0PpANEtKHr3uPooQ+nYPtw@mail.gmail.com
2017-07-27 14:13:15 -04:00
Tom Lane
51b76f9741 Clean up SQL emitted by psql/describe.c.
Fix assorted places that had not bothered with the convention of
prefixing catalog and function names with "pg_catalog.".  That
could possibly result in query failure when running with a nondefault
search_path.  Also fix two places that weren't quoting OID literals.
I think the latter hasn't mattered much since about 7.3, but it's still
a bad idea to be doing it in 99 places and not in 2 others.

Also remove a useless EXISTS sub-select that someone had stuck into
describeOneTableDetails' queries for child tables.  We just got the OID
out of pg_class, so I hardly see how checking that it exists in pg_class
was doing anything helpful.

In passing, try to improve the emitted formatting of a couple of
these queries, though I didn't work really hard on that.  And merge
unnecessarily duplicative coding in some other places.

Much of this was new in HEAD, but some was quite old; back-patch
as appropriate.
2017-07-26 19:35:35 -04:00
Tom Lane
05a562d9ce Fix race condition in predicate-lock init code in EXEC_BACKEND builds.
Trading a little too heavily on letting the code path be the same whether
we were creating shared data structures or only attaching to them,
InitPredicateLocks() inserted the "scratch" PredicateLockTargetHash entry
unconditionally.  This is just wrong if we're in a postmaster child,
which would only reach this code in EXEC_BACKEND builds.  Most of the
time, the hash_search(HASH_ENTER) call would simply report that the
entry already existed, causing no visible effect since the code did not
bother to check for that possibility.  However, if this happened while
some other backend had transiently removed the "scratch" entry, then
that other backend's eventual RestoreScratchTarget would suffer an
assert failure; this appears to be the explanation for a recent failure
on buildfarm member culicidae.  In non-assert builds, there would be
no visible consequences there either.  But nonetheless this is a pretty
bad bug for EXEC_BACKEND builds, for two reasons:

1. Each new backend would perform the hash_search(HASH_ENTER) call
without holding any lock that would prevent concurrent access to the
PredicateLockTargetHash hash table.  This creates a low but certainly
nonzero risk of corruption of that hash table.

2. In the event that the race condition occurred, by reinserting the
scratch entry too soon, we were defeating the entire purpose of the
scratch entry, namely to guarantee that transaction commit could move
hash table entries around with no risk of out-of-memory failure.
The odds of an actual OOM failure are quite low, but not zero, and if
it did happen it would again result in corruption of the hash table.

The user-visible symptoms of such corruption are a little hard to predict,
but would presumably amount to misbehavior of SERIALIZABLE transactions
that'd require a crash or postmaster restart to fix.

To fix, just skip the hash insertion if IsUnderPostmaster.  I also
inserted a bunch of assertions that the expected things happen
depending on whether IsUnderPostmaster is true.  That might be overkill,
since most comparable code in other functions isn't quite that paranoid,
but once burnt twice shy.

In passing, also move a couple of lines to places where they seemed
to make more sense.

Diagnosis of problem by Thomas Munro, patch by me.  Back-patch to
all supported branches.

Discussion: https://postgr.es/m/10593.1500670709@sss.pgh.pa.us
2017-07-24 16:45:47 -04:00
Tom Lane
d9874fde89 Ensure that pg_get_ruledef()'s output matches pg_get_viewdef()'s.
Various cases involving renaming of view columns are handled by having
make_viewdef pass down the view's current relation tupledesc to
get_query_def, which then takes care to use the column names from the
tupledesc for the output column names of the SELECT.  For some reason
though, we'd missed teaching make_ruledef to do similarly when it is
printing an ON SELECT rule, even though this is exactly the same case.
The results from pg_get_ruledef would then be different and arguably wrong.
In particular, this breaks pre-v10 versions of pg_dump, which in some
situations would define views by means of emitting a CREATE RULE ... ON
SELECT command.  Third-party tools might not be happy either.

In passing, clean up some crufty code in make_viewdef; we'd apparently
modernized the equivalent code in make_ruledef somewhere along the way,
and missed this copy.

Per report from Gilles Darold.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/ec05659a-40ff-4510-fc45-ca9d965d0838@dalibo.com
2017-07-24 15:16:31 -04:00
Noah Misch
4885e5c88b MSVC: Accept tcl86.lib in addition to tcl86t.lib.
ActiveTcl8.6.4.1.299124-win32-x86_64-threaded.exe ships just tcl86.lib.
Back-patch to 9.2, like the commit recognizing tcl86t.lib.
2017-07-23 23:53:55 -07:00
Tom Lane
80af818dd6 Doc: clarify description of degenerate NATURAL joins.
Claiming that NATURAL JOIN is equivalent to CROSS JOIN when there are
no common column names is only strictly correct if it's an inner join;
you can't say e.g. CROSS LEFT JOIN.  Better to explain it as meaning
JOIN ON TRUE, instead.  Per a suggestion from David Johnston.

Discussion: https://postgr.es/m/CAKFQuwb+mYszQhDS9f_dqRrk1=Pe-S6D=XMkAXcDf4ykKPmgKQ@mail.gmail.com
2017-07-20 12:41:50 -04:00
Tom Lane
861f4fb9fe Doc: add missing note about permissions needed to change log_lock_waits.
log_lock_waits is PGC_SUSET, but config.sgml lacked the standard
boilerplate sentence noting that.

Report: https://postgr.es/m/20170719100838.19352.16320@wrigleys.postgresql.org
2017-07-19 12:59:15 -04:00
Tom Lane
2d08d036ea Doc: explain dollar quoting in the intro part of the pl/pgsql chapter.
We're throwing people into the guts of the syntax with not much context;
let's back up one step and point out that this goes inside a literal in
a CREATE FUNCTION command.  Per suggestion from Kurt Kartaltepe.

Discussion: https://postgr.es/m/CACawnnyWAmH+au8nfZhLiFfWKjXy4d0kY+eZWfcxPRnjVfaa_Q@mail.gmail.com
2017-07-17 16:43:45 -04:00
Tom Lane
da43d624d0 Merge large_object.sql test into largeobject.source.
It seems pretty confusing to have tests named both largeobject and
large_object.  The latter is of very recent vintage (commit ff992c074),
so get rid of it in favor of merging into the former.

Also, enable the LO comment test that was added by commit 70ad7ed4e,
since the later commit added the then-missing pg_upgrade functionality.
The large_object.sql test case is almost completely redundant with that,
but not quite: it seems like creating a user-defined LO with an OID in
the system range might be an interesting case for pg_upgrade, so let's
keep it.

Like the earlier patch, back-patch to all supported branches.

Discussion: https://postgr.es/m/18665.1500306372@sss.pgh.pa.us
2017-07-17 15:28:17 -04:00
Heikki Linnakangas
4b994a96c7 Fix pg_basebackup output to stdout on Windows.
When writing a backup to stdout with pg_basebackup on Windows, put stdout
to binary mode. Any CR bytes in the output will otherwise be output
incorrectly as CR+LF.

In the passing, standardize on using "_setmode" instead of "setmode", for
the sake of consistency. They both do the same thing, but according to
MSDN documentation, setmode is deprecated.

Fixes bug #14634, reported by Henry Boehlert. Patch by Haribabu Kommi.
Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/20170428082818.24366.13134@wrigleys.postgresql.org
2017-07-14 16:03:27 +03:00
Tom Lane
bccfb17766 Fix dumping of FUNCTION RTEs that contain non-function-call expressions.
The grammar will only accept something syntactically similar to a function
call in a function-in-FROM expression.  However, there are various ways
to input something that ruleutils.c won't deparse that way, potentially
leading to a view or rule that fails dump/reload.  Fix by inserting a
dummy CAST around anything that isn't going to deparse as a function
(which is one of the ways to get something like that in there in the
first place).

In HEAD, also make use of the infrastructure added by this to avoid
emitting unnecessary parentheses in CREATE INDEX deparsing.  I did
not change that in back branches, thinking that people might find it
to be unexpected/unnecessary behavioral change.

In HEAD, also fix incorrect logic for when to add extra parens to
partition key expressions.  Somebody apparently thought they could
get away with simpler logic than pg_get_indexdef_worker has, but
they were wrong --- a counterexample is PARTITION BY LIST ((a[1])).
Ignoring the prettyprint flag for partition expressions isn't exactly
a nice solution anyway.

This has been broken all along, so back-patch to all supported branches.

Discussion: https://postgr.es/m/10477.1499970459@sss.pgh.pa.us
2017-07-13 19:24:44 -04:00
Heikki Linnakangas
40c49bd719 Fix race between GetNewTransactionId and GetOldestActiveTransactionId.
The race condition goes like this:

1. GetNewTransactionId advances nextXid e.g. from 100 to 101
2. GetOldestActiveTransactionId reads the new nextXid, 101
3. GetOldestActiveTransactionId loops through the proc array. There are no
   active XIDs there, so it returns 101 as the oldest active XID.
4. GetNewTransactionid stores XID 100 to MyPgXact->xid

So, GetOldestActiveTransactionId returned XID 101, even though 100 only
just started and is surely still running.

This would be hard to hit in practice, and even harder to spot any ill
effect if it happens. GetOldestActiveTransactionId is only used when
creating a checkpoint in a master server, and the race condition can only
happen on an online checkpoint, as there are no backends running during a
shutdown checkpoint. The oldestActiveXid value of an online checkpoint is
only used when starting up a hot standby server, to determine the starting
point where pg_subtrans is initialized from. For the race condition to
happen, there must be no other XIDs in the proc array that would hold back
the oldest-active XID value, which means that the missed XID must be a top
transaction's XID. However, pg_subtrans is not used for top XIDs, so I
believe an off-by-one error is in fact inconsequential. Nevertheless, let's
fix it, as it's clearly wrong and the fix is simple.

This has been wrong ever since hot standby was introduced, so backport to
all supported versions.

Discussion: https://www.postgresql.org/message-id/e7258662-82b6-7a45-56d4-99b337a32bf7@iki.fi
2017-07-13 15:48:20 +03:00
Tom Lane
75670ec373 Fix ruleutils.c for domain-over-array cases, too.
Further investigation shows that ruleutils isn't quite up to speed either
for cases where we have a domain-over-array: it needs to be prepared to
look past a CoerceToDomain at the top level of field and element
assignments, else it decompiles them incorrectly.  Potentially this would
result in failure to dump/reload a rule, if it looked like the one in the
new test case.  (I also added a test for EXPLAIN; that output isn't broken,
but clearly we need more test coverage here.)

Like commit b1cb32fb6, this bug is reachable in cases we already support,
so back-patch all the way.
2017-07-12 18:00:04 -04:00
Heikki Linnakangas
40ba61b447 Reduce memory usage of tsvector type analyze function.
compute_tsvector_stats() detoasted and kept in memory every tsvector value
in the sample, but that can be a lot of memory. The original bug report
described a case using over 10 gigabytes, with statistics target of 10000
(the maximum).

To fix, allocate a separate copy of just the lexemes that we keep around,
and free the detoasted tsvector values as we go. This adds some palloc/pfree
overhead, when you have a lot of distinct lexemes in the sample, but it's
better than running out of memory.

Fixes bug #14654 reported by James C. Reviewed by Tom Lane. Backport to
all supported versions.

Discussion: https://www.postgresql.org/message-id/20170514200602.1451.46797@wrigleys.postgresql.org
2017-07-12 22:04:15 +03:00
Heikki Linnakangas
c65ed13ce7 Fix variable and type name in comment.
Kyotaro Horiguchi

Discussion: https://www.postgresql.org/message-id/20170711.163441.241981736.horiguchi.kyotaro@lab.ntt.co.jp
2017-07-12 17:09:19 +03:00
Heikki Linnakangas
f4d71bdd13 Remove unnecessary braces, to match the surrounding style.
Mostly in the new subscription-related commands. Backport the few that
were also present in older versions.

Thomas Munro

Discussion: https://www.postgresql.org/message-id/CAEepm=3CyW1QmXcXJXmqiJXtXzFDc8SvSfnxkEGD3Bkv2SrkeQ@mail.gmail.com
2017-07-12 12:31:22 +03:00
Tom Lane
55204850a9 Fix multiple assignments to a column of a domain type.
We allow INSERT and UPDATE commands to assign to the same column more than
once, as long as the assignments are to subfields or elements rather than
the whole column.  However, this failed when the target column was a domain
over array rather than plain array.  Fix by teaching process_matched_tle()
to look through CoerceToDomain nodes, and add relevant test cases.

Also add a group of test cases exercising domains over array of composite.
It's doubtless accidental that CREATE DOMAIN allows this case while not
allowing straight domain over composite; but it does, so we'd better make
sure we don't break it.  (I could not find any documentation mentioning
either side of that, so no doc changes.)

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

Discussion: https://postgr.es/m/4206.1499798337@sss.pgh.pa.us
2017-07-11 16:48:59 -04:00
Tom Lane
a4a590ee79 On Windows, retry process creation if we fail to reserve shared memory.
We've heard occasional reports of backend launch failing because
pgwin32_ReserveSharedMemoryRegion() fails, indicating that something
has already used that address space in the child process.  It's not
very clear what, given that we disable ASLR in Windows builds, but
suspicion falls on antivirus products.  It'd be better if we didn't
have to disable ASLR, anyway.  So let's try to ameliorate the problem
by retrying the process launch after such a failure, up to 100 times.

Patch by me, based on previous work by Amit Kapila and others.
This is a longstanding issue, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAA4eK1+R6hSx6t_yvwtx+NRzneVp+MRqXAdGJZChcau8Uij-8g@mail.gmail.com
2017-07-10 11:00:09 -04:00
Tom Lane
b33faac521 Doc: clarify wording about tool requirements in sourcerepo.sgml.
Original wording had confusingly vague antecedent for "they", so replace
that with a more repetitive but clearer formulation.  In passing, make the
link to the installation requirements section more specific.  Per gripe
from Martin Mai, though this is not the fix he initially proposed.

Discussion: https://postgr.es/m/CAN_NWRu-cWuNaiXUjV3m4H-riWURuPW=j21bSaLADs6rjjzXgQ@mail.gmail.com
2017-07-10 00:08:59 -04:00
Heikki Linnakangas
fb1110085b Treat clean shutdown of an SSL connection same as the non-SSL case.
If the client closes an SSL connection, treat it the same as EOF on a
non-SSL connection. In particular, don't write a message in the log about
that.

Michael Paquier.

Discussion: https://www.postgresql.org/message-id/CAB7nPqSfyVV42Q2acFo%3DvrvF2gxoZAMJLAPq3S3KkjhZAYi7aw@mail.gmail.com
2017-07-03 14:53:12 +03:00
Tom Lane
5e74471320 Second try at fixing tcp_keepalives_idle option on Solaris.
Buildfarm evidence shows that TCP_KEEPALIVE_THRESHOLD doesn't exist
after all on Solaris < 11.  This means we need to take positive action to
prevent the TCP_KEEPALIVE code path from being taken on that platform.
I've chosen to limit it with "&& defined(__darwin__)", since it's unclear
that anyone else would follow Apple's precedent of spelling the symbol
that way.

Also, follow a suggestion from Michael Paquier of eliminating code
duplication by defining a couple of intermediate symbols for the
socket option.

In passing, make some effort to reduce the number of translatable messages
by replacing "setsockopt(foo) failed" with "setsockopt(%s) failed", etc,
throughout the affected files.  And update relevant documentation so
that it doesn't claim to provide an exhaustive list of the possible
socket option names.

Like the previous commit (f0256c774), back-patch to all supported branches.

Discussion: https://postgr.es/m/20170627163757.25161.528@wrigleys.postgresql.org
2017-06-28 12:30:16 -04:00
Tom Lane
6ccab9242b Support tcp_keepalives_idle option on Solaris.
Turns out that the socket option for this is named TCP_KEEPALIVE_THRESHOLD,
at least according to the tcp(7P) man page for Solaris 11.  (But since that
text refers to "SunOS", it's likely pretty ancient.)  It appears that the
symbol TCP_KEEPALIVE does get defined on that platform, but it doesn't
seem to represent a valid protocol-level socket option.  This leads to
bleats in the postmaster log, and no tcp_keepalives_idle functionality.

Per bug #14720 from Andrey Lizenko, as well as an earlier report from
Dhiraj Chawla that nobody had followed up on.  The issue's been there
since we added the TCP_KEEPALIVE code path in commit 5acd417c8, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/20170627163757.25161.528@wrigleys.postgresql.org
2017-06-27 18:47:57 -04:00
Tom Lane
e96adaacdc Don't lose walreceiver start requests due to race condition in postmaster.
When a walreceiver dies, the startup process will notice that and send
a PMSIGNAL_START_WALRECEIVER signal to the postmaster, asking for a new
walreceiver to be launched.  There's a race condition, which at least
in HEAD is very easy to hit, whereby the postmaster might see that
signal before it processes the SIGCHLD from the walreceiver process.
In that situation, sigusr1_handler() just dropped the start request
on the floor, reasoning that it must be redundant.  Eventually, after
10 seconds (WALRCV_STARTUP_TIMEOUT), the startup process would make a
fresh request --- but that's a long time if the connection could have
been re-established almost immediately.

Fix it by setting a state flag inside the postmaster that we won't
clear until we do launch a walreceiver.  In cases where that results
in an extra walreceiver launch, it's up to the walreceiver to realize
it's unwanted and go away --- but we have, and need, that logic anyway
for the opposite race case.

I came across this through investigating unexpected delays in the
src/test/recovery TAP tests: it manifests there in test cases where
a master server is stopped and restarted while leaving streaming
slaves active.

This logic has been broken all along, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/21344.1498494720@sss.pgh.pa.us
2017-06-26 17:31:56 -04:00
Tom Lane
439b6363d8 Ignore old stats file timestamps when starting the stats collector.
The stats collector disregards inquiry messages that bear a cutoff_time
before when it last wrote the relevant stats file.  That's fine, but at
startup when it reads the "permanent" stats files, it absorbed their
timestamps as if they were the times at which the corresponding temporary
stats files had been written.  In reality, of course, there's no data
out there at all.  This led to disregarding inquiry messages soon after
startup if the postmaster had been shut down and restarted within less
than PGSTAT_STAT_INTERVAL; which is a pretty common scenario, both for
testing and in the field.  Requesting backends would hang for 10 seconds
and then report failure to read statistics, unless they got bailed out
by some other backend coming along and making a newer request within
that interval.

I came across this through investigating unexpected delays in the
src/test/recovery TAP tests: it manifests there because the autovacuum
launcher hangs for 10 seconds when it can't get statistics at startup,
thus preventing a second shutdown from occurring promptly.  We might
want to do some things in the autovac code to make it less prone to
getting stuck that way, but this change is a good bug fix regardless.

In passing, also fix pgstat_read_statsfiles() to ensure that it
re-zeroes its global stats variables if they are corrupted by a
short read from the stats file.  (Other reads in that function
go into temp variables, so that the issue doesn't arise.)

This has been broken since we created the separation between permanent
and temporary stats files in 8.4, so back-patch to all supported branches.

Discussion: https://postgr.es/m/16860.1498442626@sss.pgh.pa.us
2017-06-26 16:17:06 -04:00
Andres Freund
81bf7b5b1e Fix possibility of creating a "phantom" segment after promotion.
When promoting a standby just after a XLOG_SWITCH record was replayed,
and next segment(s) are already are locally available (via walsender,
restore_command + trigger/recovery target), that segment could
accidentally be recycled onto the past of the new timeline.  Later
checkpointer would create a .ready file for it, assuming there was an
error during creation, and it would get archived.  That causes trouble
if another standby is later brought up from a basebackup from before
the timeline creation, because it would try to read the
segment, because XLogFileReadAnyTLI just tries all possible timelines,
which doesn't have valid contents.  Thus replay would fail.

The problem, if already occurred, can be fixed by removing the segment
and/or having restore_command filter it out.

The reason for the creation of such "phantom" segments was, that after
an XLOG_SWITCH record the EndOfLog variable points to the beginning of
the next segment, and RemoveXlogFile() used XLByteToPrevSeg().
Normally RemoveXlogFile() doing so is harmless, because the last
segment will still exist preventing InstallXLogFileSegment() from
causing harm, but just after promotion there's no previous segment on
the new timeline.

Fix that by using XLByteToSeg() instead of XLByteToPrevSeg().

Author: Andres Freund
Reported-By: Greg Burek
Discussion: https://postgr.es/m/20170619073026.zcwpe6mydsaz5ygd@alap3.anarazel.de
Backpatch: 9.2-, bug older than all supported versions
2017-06-21 14:13:58 -07:00
Bruce Momjian
65beccae51 pg_upgrade: start/stop new server after pg_resetwal
When commit 0f33a719fdbb5d8c43839ea0d2c90cd03e2af2d2 removed the
instructions to start/stop the new cluster before running rsync, it was
now possible for pg_resetwal/pg_resetxlog to leave the final WAL record
at wal_level=minimum, preventing upgraded standby servers from
reconnecting.

This patch fixes that by having pg_upgrade unconditionally start/stop
the new cluster after pg_resetwal/pg_resetxlog has run.

Backpatch through 9.2 since, though the instructions were added in PG
9.5, they worked all the way back to 9.2.

Discussion: https://postgr.es/m/20170620171844.GC24975@momjian.us

Backpatch-through: 9.2
2017-06-20 13:20:02 -04:00
Tom Lane
c10cbf77ae On Windows, make pg_dump use binary mode for compressed plain text output.
The combination of -Z -Fp and output to stdout resulted in corrupted
output data, because we left stdout in text mode, resulting in newline
conversion being done on the compressed stream.  Switch stdout to binary
mode for this case, at the same place where we do it for non-text output
formats.

Report and patch by Kuntal Ghosh, tested by Ashutosh Sharma and Neha
Sharma.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAGz5QCJPvbBjXAmJuGx1B_41yVCetAJhp7rtaDf7XQGWuB1GSw@mail.gmail.com
2017-06-19 11:03:21 -04:00
Heikki Linnakangas
ac93a78b0e Fix dependency, when changing a function's argument/return type.
When a new base type is created using the old-style procedure of first
creating the input/output functions with "opaque" in place of the base
type, the "opaque" argument/return type is changed to the final base type,
on CREATE TYPE. However, we did not create a pg_depend record when doing
that, so the functions were left not depending on the type.

Fixes bug #14706, reported by Karen Huddleston.

Discussion: https://www.postgresql.org/message-id/20170614232259.1424.82774@wrigleys.postgresql.org
2017-06-16 11:46:26 +03:00
Tom Lane
e6fee616d4 Fix low-probability leaks of PGresult objects in the backend.
We had three occurrences of essentially the same coding pattern
wherein we tried to retrieve a query result from a libpq connection
without blocking.  In the case where PQconsumeInput failed (typically
indicating a lost connection), all three loops simply gave up and
returned, forgetting to clear any previously-collected PGresult
object.  Since those are malloc'd not palloc'd, the oversight results
in a process-lifespan memory leak.

One instance, in libpqwalreceiver, is of little significance because
the walreceiver process would just quit anyway if its connection fails.
But we might as well fix it.

The other two instances, in postgres_fdw, are somewhat more worrisome
because at least in principle the scenario could be repeated, allowing
the amount of memory leaked to build up to something worth worrying
about.  Moreover, in these cases the loops contain CHECK_FOR_INTERRUPTS
calls, as well as other calls that could potentially elog(ERROR),
providing another way to exit without having cleared the PGresult.
Here we need to add PG_TRY logic similar to what exists in quite a
few other places in postgres_fdw.

Coverity noted the libpqwalreceiver bug; I found the other two cases
by checking all calls of PQconsumeInput.

Back-patch to all supported versions as appropriate (9.2 lacks
postgres_fdw, so this is really quite unexciting for that branch).

Discussion: https://postgr.es/m/22620.1497486981@sss.pgh.pa.us
2017-06-15 15:03:39 -04:00
Tatsuo Ishii
1fe10463d9 Fix document bug regarding read only transactions.
It was explained that read only transactions (not in standby) allow to
update sequences. This had been wrong since the commit:
05d8a561ff85db1545f5768fe8d8dc9d99ad2ef7

Discussion: https://www.postgresql.org/message-id/20170614.110826.425627939780392324.t-ishii%40sraoss.co.jp
2017-06-15 10:09:00 +09:00
Michael Meskes
ccd717a2e8 Fix docs to not claim ECPG's SET CONNECTION is not thread-aware.
Changed by: Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
2017-06-07 16:16:04 +02:00
Andres Freund
133b1920c5 Unify SIGHUP handling between normal and walsender backends.
Because walsender and normal backends share the same main loop it's
problematic to have two different flag variables, set in signal
handlers, indicating a pending configuration reload.  Only certain
walsender commands reach code paths checking for the
variable (START_[LOGICAL_]REPLICATION, CREATE_REPLICATION_SLOT
... LOGICAL, notably not base backups).

This is a bug present since the introduction of walsender, but has
gotten worse in releases since then which allow walsender to do more.

A later patch, not slated for v10, will similarly unify SIGHUP
handling in other types of processes as well.

Author: Petr Jelinek, Andres Freund
Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/20170423235941.qosiuoyqprq4nu7v@alap3.anarazel.de
Backpatch: 9.2-, bug is present since 9.0
2017-06-05 19:18:16 -07:00
Andrew Dunstan
f964a7c5ab Fix thinko in previous openssl change 2017-06-05 20:41:01 -04:00
Andrew Dunstan
545dc4d216 Find openssl lib files in right directory for MSVC
Some openssl builds put their lib files in a VC subdirectory, others do
not. Cater for both cases.

Backpatch to all live branches.

From an offline discussion with Leonardo Cecchi.
2017-06-05 14:29:00 -04:00
Tom Lane
a378b9bc28 Always use -fPIC, not -fpic, when building shared libraries with gcc.
On some platforms, -fpic fails for sufficiently large shared libraries.
We've mostly not hit that boundary yet, but there are some extensions
such as Citus and pglogical where it's becoming a problem.  A bit of
research suggests that the penalty for -fPIC is small, in the
single-digit-percentage range --- and there's none at all on popular
platforms such as x86_64.  So let's just default to -fPIC everywhere
and provide one less thing for extension developers to worry about.

Per complaint from Christoph Berg.  Back-patch to all supported branches.
(I did not bother to touch the recently-removed Makefiles for sco and
unixware in the back branches, though.  We'd have no way to test that
it doesn't break anything on those platforms.)

Discussion: https://postgr.es/m/20170529155850.qojdfrwkkqnjb3ap@msg.df7cb.de
2017-06-01 13:32:56 -04:00
Tom Lane
364100e0bf Try to ensure that stats collector's receive buffer size is at least 100KB.
Back-patch of commit 8b0b6303e991079726e83d17401405e94da11564.

Discussion: https://postgr.es/m/22173.1494788088@sss.pgh.pa.us
2017-05-29 20:27:45 -04:00
Tom Lane
6b48f06da8 Prevent running pg_resetwal/pg_resetxlog against wrong-version data dirs.
pg_resetwal (formerly pg_resetxlog) doesn't insist on finding a matching
version number in pg_control, and that seems like an important thing to
preserve since recovering from corrupt pg_control is a prime reason to
need to run it.  However, that means you can try to run it against a
data directory of a different major version, which is at best useless
and at worst disastrous.  So as to provide some protection against that
type of pilot error, inspect PG_VERSION at startup and refuse to do
anything if it doesn't match.  PG_VERSION is read-only after initdb,
so it's unlikely to get corrupted, and even if it were corrupted it would
be easy to fix by hand.

This hazard has been there all along, so back-patch to all supported
branches.

Michael Paquier, with some kibitzing by me

Discussion: https://postgr.es/m/f4b8eb91-b934-8a0d-b3cc-68f06e2279d1@enterprisedb.com
2017-05-29 17:08:16 -04:00
Tom Lane
a653606709 Allow NumericOnly to be "+ FCONST".
The NumericOnly grammar production accepted ICONST, + ICONST, - ICONST,
FCONST, and - FCONST, but for some reason not + FCONST.  This led to
strange inconsistencies like

regression=# set random_page_cost = +4;
SET
regression=# set random_page_cost = 4000000000;
SET
regression=# set random_page_cost = +4000000000;
ERROR:  syntax error at or near "4000000000"

(because 4000000000 is too large to be an ICONST).  While there's
no actual functional reason to need to write a "+", if we allow
it for integers it seems like we should allow it for numerics too.

It's been like that forever, so back-patch to all supported branches.

Discussion: https://postgr.es/m/30908.1496006184@sss.pgh.pa.us
2017-05-29 15:19:07 -04:00
Tom Lane
da91656866 Move autogenerated array types out of the way during ALTER ... RENAME.
Commit 9aa3c782c added code to allow CREATE TABLE/CREATE TYPE to not fail
when the desired type name conflicts with an autogenerated array type, by
dint of renaming the array type out of the way.  But I (tgl) overlooked
that the same case arises in ALTER TABLE/TYPE RENAME.  Fix that too.
Back-patch to all supported branches.

Report and patch by Vik Fearing, modified a bit by me

Discussion: https://postgr.es/m/0f4ade49-4f0b-a9a3-c120-7589f01d1eb8@2ndquadrant.com
2017-05-26 15:16:59 -04:00
Tom Lane
f62e1eff52 Fix pg_dump to not emit invalid SQL for an empty operator class.
If an operator class has no operators or functions, and doesn't need
a STORAGE clause, we emitted "CREATE OPERATOR CLASS ... AS ;" which
is syntactically invalid.  Fix by forcing a STORAGE clause to be
emitted anyway in this case.

(At some point we might consider changing the grammar to allow CREATE
OPERATOR CLASS without an opclass_item_list.  But probably we'd want to
omit the AS in that case, so that wouldn't fix this pg_dump issue anyway.)

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

Daniel Gustafsson, tweaked by me to avoid a dangling-pointer bug

Discussion: https://postgr.es/m/D9E5FC64-7A37-4F3D-B946-7E4FB468F88A@yesql.se
2017-05-26 12:51:06 -04:00