40908 Commits

Author SHA1 Message Date
Noah Misch
5c0fee0be2 In the pg_upgrade test suite, don't write to src/test/regress.
When this suite runs installcheck, redirect file creations from
src/test/regress to src/bin/pg_upgrade/tmp_check/regress.  This closes a
race condition in "make -j check-world".  If the pg_upgrade suite wrote
to a given src/test/regress/results file in parallel with the regular
src/test/regress invocation writing it, a test failed spuriously.  Even
without parallelism, in "make -k check-world", the suite finishing
second overwrote the other's regression.diffs.  This revealed test
"largeobject" assuming @abs_builddir@ is getcwd(), so fix that, too.

Buildfarm client REL_10, released fifty-four days ago, supports saving
regression.diffs from its new location.  When an older client reports a
pg_upgradeCheck failure, it will no longer include regression.diffs.
Back-patch to 9.5, where pg_upgrade moved to src/bin.

Reviewed (in earlier versions) by Andrew Dunstan.

Discussion: https://postgr.es/m/20181224034411.GA3224776@rfd.leadboat.com
2019-05-28 13:00:26 -07:00
Noah Misch
f3798089d8 In the pg_upgrade test suite, remove and recreate "tmp_check".
This allows "vcregress upgradecheck" to pass twice in immediate
succession, and it's more like how $(prove_check) works.  Back-patch to
9.5, where pg_upgrade moved to src/bin.

Discussion: https://postgr.es/m/20190520012436.GA1480421@rfd.leadboat.com
2019-05-28 12:58:34 -07:00
Andres Freund
7ac46b1ae9 pg_upgrade: Make test.sh's installcheck use to-be-upgraded version's bindir.
On master (after 700538) the old version's installed psql was used -
even when the old version might not actually be installed / might be
installed into a temporary directory. As commonly the case when just
executing make check for pg_upgrade, as $oldbindir is just the current
version's $bindir.

In the back branches, with --install specified, psql from the new
version's temporary installation was used, without --install (e.g for
NO_TEMP_INSTALL, cf 47b3c26642), the new version's installed psql was
used (which might or might not exist).

Author: Andres Freund
Discussion: https://postgr.es/m/20190522175150.c26f4jkqytahajdg@alap3.anarazel.de
2019-05-23 14:50:09 -07:00
Michael Paquier
cd703f5e17 Fix some grammar in documentation of spgist and pgbench
Discussion: https://postgr.es/m/92961161-9b49-e42f-0a72-d5d47e0ed4de@postgrespro.ru
Author: Liudmila Mantrova
Reviewed-by: Jonathan Katz, Tom Lane, Michael Paquier
Backpatch-through: 9.4
2019-05-20 09:48:50 +09:00
Noah Misch
2604751c73 Revert "In the pg_upgrade test suite, don't write to src/test/regress."
This reverts commit bd1592e8570282b1650af6b8eede0016496daecd.  It had
multiple defects.

Discussion: https://postgr.es/m/12717.1558304356@sss.pgh.pa.us
2019-05-19 15:24:46 -07:00
Noah Misch
e00b5f090f In the pg_upgrade test suite, don't write to src/test/regress.
When this suite runs installcheck, redirect file creations from
src/test/regress to src/bin/pg_upgrade/tmp_check/regress.  This closes a
race condition in "make -j check-world".  If the pg_upgrade suite wrote
to a given src/test/regress/results file in parallel with the regular
src/test/regress invocation writing it, a test failed spuriously.  Even
without parallelism, in "make -k check-world", the suite finishing
second overwrote the other's regression.diffs.  This revealed test
"largeobject" assuming @abs_builddir@ is getcwd(), so fix that, too.

Buildfarm client REL_10, released forty-five days ago, supports saving
regression.diffs from its new location.  When an older client reports a
pg_upgradeCheck failure, it will no longer include regression.diffs.
Back-patch to 9.5, where pg_upgrade moved to src/bin.

Reviewed by Andrew Dunstan.

Discussion: https://postgr.es/m/20181224034411.GA3224776@rfd.leadboat.com
2019-05-19 14:37:33 -07:00
Peter Geoghegan
dd01bbf00c Doc: Refer to line pointers as item identifiers.
An upcoming HEAD-only patch will standardize the terminology around
ItemIdData variables/line pointers, ending the practice of referring to
them as "item pointers".  Make the "Database Page Layout" docs
consistent with the new policy.  The term "item identifier" is already
used in the same section, so stick with that.

Discussion: https://postgr.es/m/CAH2-Wz=c=MZQjUzde3o9+2PLAPuHTpVZPPdYxN=E4ndQ2--8ew@mail.gmail.com
Backpatch: All supported branches.
2019-05-13 15:38:59 -07:00
Tom Lane
0ec3d2ab25 Fix misuse of an integer as a bool.
pgtls_read_pending is declared to return bool, but what the underlying
SSL_pending function returns is a count of available bytes.

This is actually somewhat harmless if we're using C99 bools, but in
the back branches it's a live bug: if the available-bytes count happened
to be a multiple of 256, it would get converted to a zero char value.
On machines where char is signed, counts of 128 and up could misbehave
as well.  The net effect is that when using SSL, libpq might block
waiting for data even though some has already been received.

Broken by careless refactoring in commit 4e86f1b16, so back-patch
to 9.5 where that came in.

Per bug #15802 from David Binderman.

Discussion: https://postgr.es/m/15802-f0911a97f0346526@postgresql.org
2019-05-13 10:53:19 -04:00
Tom Lane
d6de526368 Fix misoptimization of "{1,1}" quantifiers in regular expressions.
A bounded quantifier with m = n = 1 might be thought a no-op.  But
according to our documentation (which traces back to Henry Spencer's
original man page) it still imposes greediness, or non-greediness in the
case of the non-greedy variant "{1,1}?", on whatever it's attached to.

This turns out not to work though, because parseqatom() optimizes away
the m = n = 1 case without regard for whether it's supposed to change
the greediness of the argument RE.

We can fix this by just not applying the optimization when the greediness
needs to change; the subsequent general cases handle it fine.

The three cases in which we can still apply the optimization are
(a) no quantifier, or quantifier does not impose a preference;
(b) atom has no greediness property, implying it cannot match a
variable amount of text anyway; or
(c) quantifier's greediness is same as atom's.
Note that in most cases where one of these applies, we'd have exited
earlier in the "not a messy case" fast path.  I think it's now only
possible to get to the optimization when the atom involves capturing
parentheses or a non-top-level backref.

Back-patch to all supported branches.  I'd ordinarily be hesitant to
put a subtle behavioral change into back branches, but in this case
it's very hard to see a reason why somebody would write "{1,1}?" unless
they're trying to get the documented change-of-greediness behavior.

Discussion: https://postgr.es/m/5bb27a41-350d-37bf-901e-9d26f5592dd0@charter.net
2019-05-12 18:53:43 -04:00
Noah Misch
9dc65bcf9d Fail pgwin32_message_to_UTF16() for SQL_ASCII messages.
The function had been interpreting SQL_ASCII messages as UTF8, throwing
an error when they were invalid UTF8.  The new behavior is consistent
with pg_do_encoding_conversion().  This affects LOG_DESTINATION_STDERR
and LOG_DESTINATION_EVENTLOG, which will send untranslated bytes to
write() and ReportEventA().  On buildfarm member bowerbird, enabling
log_connections caused an error whenever the role name was not valid
UTF8.  Back-patch to 9.4 (all supported versions).

Discussion: https://postgr.es/m/20190512015615.GD1124997@rfd.leadboat.com
2019-05-12 10:34:22 -07:00
Tom Lane
899f943ca4 Rearrange pgstat_bestart() to avoid failures within its critical section.
We long ago decided to design the shared PgBackendStatus data structure to
minimize the cost of writing status updates, which means that writers just
have to increment the st_changecount field twice.  That isn't hooked into
any sort of resource management mechanism, which means that if something
were to throw error between the two increments, the st_changecount field
would be left odd indefinitely.  That would cause readers to lock up.
Now, since it's also a bad idea to leave the field odd for longer than
absolutely necessary (because readers will spin while we have it set),
the expectation was that we'd treat these segments like spinlock critical
sections, with only short, more or less straight-line, code in them.

That was fine as originally designed, but commit 9029f4b37 broke it
by inserting a significant amount of non-straight-line code into
pgstat_bestart(), code that is very capable of throwing errors, not to
mention taking a significant amount of time during which readers will spin.
We have a report from Neeraj Kumar of readers actually locking up, which
I suspect was due to an encoding conversion error in X509_NAME_to_cstring,
though conceivably it was just a garden-variety OOM failure.

Subsequent commits have loaded even more dubious code into pgstat_bestart's
critical section (and commit fc70a4b0d deserves some kind of booby prize
for managing to miss the critical section entirely, although the negative
consequences seem minimal given that the PgBackendStatus entry should be
seen by readers as inactive at that point).

The right way to fix this mess seems to be to compute all these values
into a local copy of the process' PgBackendStatus struct, and then just
copy the data back within the critical section proper.  This plan can't
be implemented completely cleanly because of the struct's heavy reliance
on out-of-line strings, which we must initialize separately within the
critical section.  But still, the critical section is far smaller and
safer than it was before.

In hopes of forestalling future errors of the same ilk, rename the
macros for st_changecount management to make it more apparent that
the writer-side macros create a critical section.  And to prevent
the worst consequences if we nonetheless manage to mess it up anyway,
adjust those macros so that they really are a critical section, ie
they now bump CritSectionCount.  That doesn't add much overhead, and
it guarantees that if we do somehow throw an error while the counter
is odd, it will lead to PANIC and a database restart to reset shared
memory.

Back-patch to 9.5 where the problem was introduced.

In HEAD, also fix an oversight in commit b0b39f72b: it failed to teach
pgstat_read_current_status to copy st_gssstatus data from shared memory to
local memory.  Hence, subsequent use of that data within the transaction
would potentially see changing data that it shouldn't see.

Discussion: https://postgr.es/m/CAPR3Wj5Z17=+eeyrn_ZDG3NQGYgMEOY6JV6Y-WRRhGgwc16U3Q@mail.gmail.com
2019-05-11 21:27:13 -04:00
Michael Paquier
e17fba8d8f Fix error reporting in reindexdb
When failing to reindex a table or an index, reindexdb would generate an
extra error message related to a database failure, which is misleading.

Backpatch all the way down, as this has been introduced by 85e9a5a0.

Discussion: https://postgr.es/m/CAOBaU_Yo61RwNO3cW6WVYWwH7EYMPuexhKqufb2nFGOdunbcHw@mail.gmail.com
Author: Julien Rouhaud
Reviewed-by: Daniel Gustafsson, Álvaro Herrera, Tom Lane, Michael
Paquier
Backpatch-through: 9.4
2019-05-11 13:01:24 +09:00
Tom Lane
91a05390c3 Cope with EINVAL and EIDRM shmat() failures in PGSharedMemoryAttach.
There's a very old race condition in our code to see whether a pre-existing
shared memory segment is still in use by a conflicting postmaster: it's
possible for the other postmaster to remove the segment in between our
shmctl() and shmat() calls.  It's a narrow window, and there's no risk
unless both postmasters are using the same port number, but that's possible
during parallelized "make check" tests.  (Note that while the TAP tests
take some pains to choose a randomized port number, pg_regress doesn't.)
If it does happen, we treated that as an unexpected case and errored out.

To fix, allow EINVAL to be treated as segment-not-present, and the same
for EIDRM on Linux.  AFAICS, the considerations here are basically
identical to the checks for acceptable shmctl() failures, so I documented
and coded it that way.

While at it, adjust PGSharedMemoryAttach's API to remove its undocumented
dependency on UsedShmemSegAddr in favor of passing the attach address
explicitly.  This makes it easier to be sure we're using a null shmaddr
when probing for segment conflicts (thus avoiding questions about what
EINVAL means).  I don't think there was a bug there, but it required
fragile assumptions about the state of UsedShmemSegAddr during
PGSharedMemoryIsInUse.

Commit c09850992 may have made this failure more probable by applying
the conflicting-segment tests more often.  Hence, back-patch to all
supported branches, as that was.

Discussion: https://postgr.es/m/22224.1557340366@sss.pgh.pa.us
2019-05-10 14:56:41 -04:00
Michael Paquier
a475b1e77e Fix error status of vacuumdb when multiple jobs are used
When running a batch of VACUUM or ANALYZE commands on a given database,
there were cases where it is possible to have vacuumdb not report an
error where it actually should, leading to incorrect status results.

Author: Julien Rouhaud
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/CAOBaU_ZuTwz7CtqLYJ1Ouuh272bTQPLN8b1bAPk0bCBm4PDMTQ@mail.gmail.com
Backpatch-through: 9.5
2019-05-09 10:30:08 +09:00
Fujii Masao
2be2fee248 Fix documentation for the privileges required for replication functions.
Previously it's documented that use of replication functions is
restricted to superusers. This is true for the functions which
use replication origin, but not for pg_logicl_emit_message() and
functions which use replication slot. For example, not only
superusers but also users with REPLICATION privilege is allowed
to use the functions for replication slot. This commit fixes
the documentation for the privileges required for those replication
functions.

Back-patch to 9.4 (all supported versions).

Author: Matsumura Ryo
Discussion: https://postgr.es/m/03040DFF97E6E54E88D3BFEE5F5480F74ABA6E16@G01JPEXMBYT04
2019-05-09 01:43:41 +09:00
Heikki Linnakangas
f40fb143ed Remove leftover reference to old "flat file" mechanism in a comment.
The flat file mechanism was removed in PostgreSQL 9.0.
2019-05-08 09:34:17 +03:00
Michael Paquier
6de005121c Remove some code related to 7.3 and older servers from tools of src/bin/
This code was broken as of 582edc3, and is most likely not used anymore.
Note that pg_dump supports servers down to 8.0, and psql has code to
support servers down to 7.4.

Author: Julien Rouhaud
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAOBaU_Y5y=zo3+2gf+2NJC1pvMYPcbRXoQaPXx=U7+C8Qh4CzQ@mail.gmail.com
2019-05-07 14:20:19 +09:00
Tom Lane
b221b78f83 Stamp 9.5.17. REL9_5_17 2019-05-06 16:52:44 -04:00
Tom Lane
1760514e73 Last-minute updates for release notes.
Security: CVE-2019-10129, CVE-2019-10130
2019-05-06 12:45:59 -04:00
Peter Eisentraut
1fa14a1a24 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 499248e4e6cd0dea44450fb13352e7a03fccb00e
2019-05-06 14:56:15 +02:00
Dean Rasheed
01256815a6 Use checkAsUser for selectivity estimator checks, if it's set.
In examine_variable() and examine_simple_variable(), when checking the
user's table and column privileges to determine whether to grant
access to the pg_statistic data, use checkAsUser for the privilege
checks, if it's set. This will be the case if we're accessing the
table via a view, to indicate that we should perform privilege checks
as the view owner rather than the current user.

This change makes this planner check consistent with the check in the
executor, so the planner will be able to make use of statistics if the
table is accessible via the view. This fixes a performance regression
introduced by commit e2d4ef8de8, which affects queries against
non-security barrier views in the case where the user doesn't have
privileges on the underlying table, but the view owner does.

Note that it continues to provide the same safeguards controlling
access to pg_statistic for direct table access (in which case
checkAsUser won't be set) and for security barrier views, because of
the nearby checks on rte->security_barrier and rte->securityQuals.

Back-patch to all supported branches because e2d4ef8de8 was.

Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
2019-05-06 12:01:44 +01:00
Dean Rasheed
93c36145ad Fix security checks for selectivity estimation functions with RLS.
In commit e2d4ef8de8, security checks were added to prevent
user-supplied operators from running over data from pg_statistic
unless the user has table or column privileges on the table, or the
operator is leakproof. For a table with RLS, however, checking for
table or column privileges is insufficient, since that does not
guarantee that the user has permission to view all of the column's
data.

Fix this by also checking for securityQuals on the RTE, and insisting
that the operator be leakproof if there are any. Thus the
leakproofness check will only be skipped if there are no securityQuals
and the user has table or column privileges on the table -- i.e., only
if we know that the user has access to all the data in the column.

Back-patch to 9.5 where RLS was added.

Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.

Security: CVE-2019-10130
2019-05-06 11:46:33 +01:00
Andres Freund
8439589bc7 Remove reindex_catalog test from test schedules.
As the test currently causes occasional deadlocks (due to the schema
cleanup from previous sessions potentially still running), and the
patch from f912d7dec2 has gotten a fair bit of buildfarm coverage,
remove the test from the test schedules. There's a set of minor
releases coming up.

Leave the tests in place, so it can manually be run using EXTRA_TESTS.

For now also leave it in master, as there's no imminent release, and
there's plenty (re-)index related work in 12. But we'll have to
disable it before long there too, unless somebody comes up with simple
enough fixes for the deadlock (I'm about to post a vague idea to the
list).

Discussion: https://postgr.es/m/4622.1556982247@sss.pgh.pa.us
Backpatch: 9.4-11 (no master!)
2019-05-05 23:35:01 -07:00
Tom Lane
0b41329a3a Release notes for 11.3, 10.8, 9.6.13, 9.5.17, 9.4.22. 2019-05-05 14:57:16 -04:00
Tom Lane
d18ef69050 Fix reindexing of pg_class indexes some more.
Commits 3dbb317d3 et al failed under CLOBBER_CACHE_ALWAYS testing.
Investigation showed that to reindex pg_class_oid_index, we must
suppress accesses to the index (via SetReindexProcessing) before we call
RelationSetNewRelfilenode, or at least before we do CommandCounterIncrement
therein; otherwise, relcache reloads happening within the CCI may try to
fetch pg_class rows using the index's new relfilenode value, which is as
yet an empty file.

Of course, the point of 3dbb317d3 was that that ordering didn't work
either, because then RelationSetNewRelfilenode's own update of the index's
pg_class row cannot access the index, should it need to.

There are various ways we might have got around that, but Andres Freund
came up with a brilliant solution: for a mapped index, we can really just
skip the pg_class update altogether.  The only fields it was actually
changing were relpages etc, but it was just setting them to zeroes which
is useless make-work.  (Correct new values will be installed at the end
of index build.)  All pg_class indexes are mapped and probably always will
be, so this eliminates the problem by removing work rather than adding it,
always a pleasant outcome.  Having taught RelationSetNewRelfilenode to do
it that way, we can revert the code reordering in reindex_index.  (But
I left the moved setup code where it was; there seems no reason why it
has to run without use of the old index.  If you're trying to fix a
busted pg_class index, you'll have had to disable system index use
altogether to get this far.)

Moreover, this means we don't need RelationSetIndexList at all, because
reindex_relation's hacking to make "REINDEX TABLE pg_class" work is
likewise now unnecessary.  We'll leave that code in place in the back
branches, but a follow-on patch will remove it in HEAD.

In passing, do some minor cleanup for commit 5c1560606 (in HEAD only),
notably removing a duplicate newrnode assignment.

Patch by me, using a core idea due to Andres Freund.  Back-patch to all
supported branches, as 3dbb317d3 was.

Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us
2019-05-02 19:11:28 -04:00
Andres Freund
3630c6bfad Run catalog reindexing test from 3dbb317d32 serially, to avoid deadlocks.
The tests turn out to cause deadlocks in some circumstances. Fairly
reproducibly so with -DRELCACHE_FORCE_RELEASE
-DCATCACHE_FORCE_RELEASE.  Some of the deadlocks may be hard to fix
without disproportionate measures, but others probably should be fixed
- but not in 12.

We discussed removing the new tests until we can fix the issues
underlying the deadlocks, but results from buildfarm animal
markhor (which runs with CLOBBER_CACHE_ALWAYS) indicates that there
might be a more severe, as of yet undiagnosed, issue (including on
stable branches) with reindexing catalogs. The failure is:
ERROR: could not read block 0 in file "base/16384/28025": read only 0 of 8192 bytes
Therefore it seems advisable to keep the tests.

It's not certain that running the tests in isolation removes the risk
of deadlocks. It's possible that additional locks are needed to
protect against a concurrent auto-analyze or such.

Per discussion with Tom Lane.

Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us
Backpatch: 9.4-, like 3dbb317d3
2019-04-30 17:45:32 -07:00
Andres Freund
5409e359f8 Fix unused variable compiler warning in !debug builds.
Introduced in 3dbb317d3.  Fix by using the new local variable in more
places.

Reported-By: Bruce Momjian (off-list)
Backpatch: 9.4-, like 3dbb317d3
2019-04-30 17:45:32 -07:00
Andres Freund
670a492c5f Fix potential assertion failure when reindexing a pg_class index.
When reindexing individual indexes on pg_class it was possible to
either trigger an assertion failure:
TRAP: FailedAssertion("!(!ReindexIsProcessingIndex(((index)->rd_id)))

That's because reindex_index() called SetReindexProcessing() - which
enables an asserts ensuring no index insertions happen into the index
- before calling RelationSetNewRelfilenode(). That not correct for
indexes on pg_class, because RelationSetNewRelfilenode() updates the
relevant pg_class row, which needs to update the indexes.

The are two reasons this wasn't noticed earlier. Firstly the bug
doesn't trigger when reindexing all of pg_class, as reindex_relation
has code "hiding" all yet-to-be-reindexed indexes. Secondly, the bug
only triggers when the the update to pg_class doesn't turn out to be a
HOT update - otherwise there's no index insertion to trigger the
bug. Most of the time there's enough space, making this bug hard to
trigger.

To fix, move RelationSetNewRelfilenode() to before the
SetReindexProcessing() (and, together with some other code, to outside
of the PG_TRY()).

To make sure the error checking intended by SetReindexProcessing() is
more robust, modify CatalogIndexInsert() to check
ReindexIsProcessingIndex() even when the update is a HOT update.

Also add a few regression tests for REINDEXing of system catalogs.

The last two improvements would have prevented some of the issues
fixed in 5c1560606dc4c from being introduced in the first place.

Reported-By: Michael Paquier
Diagnosed-By: Tom Lane and Andres Freund
Author: Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/20190418011430.GA19133@paquier.xyz
Backpatch: 9.4-, the bug is present in all branches
2019-04-29 19:43:32 -07:00
Joe Conway
e729d0b8c7 Correct the URL pointing to PL/R
As pointed out by documentation comment, the URL for PL/R
needs to be updated to the correct current repository. Back-patch
to all supported branches.
2019-04-27 09:28:06 -04:00
Tom Lane
b3904abb4d Portability fix for zic.c.
Missed an inttypes.h dependency in previous patch.  Per buildfarm.
2019-04-26 21:20:39 -04:00
Tom Lane
22ba955367 Sync our copy of the timezone library with IANA release tzcode2019a.
This corrects a small bug in zic that caused it to output an incorrect
year-2440 transition in the Africa/Casablanca zone.

More interestingly, zic has grown a "-r" option that limits the range of
zone transitions that it will put into the output files.  That might be
useful to people who don't like the weird GMT offsets that tzdb likes
to use for very old dates.  It appears that for dates before the cutoff
time specified with -r, zic will use the zone's standard-time offset
as of the cutoff time.  So for example one might do

	make install ZIC_OPTIONS='-r @-1893456000'

to cause all dates before 1910-01-01 to be treated as though 1910
standard time prevailed indefinitely far back.  (Don't blame me for
the unfriendly way of specifying the cutoff time --- it's seconds
since or before the Unix epoch.  You can use extract(epoch ...)
to calculate it.)

As usual, back-patch to all supported branches.
2019-04-26 19:47:00 -04:00
Tom Lane
c1c76431c6 Update time zone data files to tzdata release 2019a.
DST law changes in Palestine and Metlakatla.
Historical corrections for Israel.

Etc/UCT is now a backward-compatibility link to Etc/UTC, instead
of being a separate zone that generates the abbreviation "UCT",
which nowadays is typically a typo.  Postgres will still accept
"UCT" as an input zone name, but it won't output it.
2019-04-26 17:56:57 -04:00
Tom Lane
c7e38be4df Fix some minor postmaster-state-machine issues.
In sigusr1_handler, don't ignore PMSIGNAL_ADVANCE_STATE_MACHINE based
on pmState.  The restriction is unnecessary (PostmasterStateMachine
should work in any state), not future-proof (since it makes too many
assumptions about why the signal might be sent), and broken even today
because a race condition can make it necessary to respond to the signal
in PM_WAIT_READONLY state.  The race condition seems unlikely, but
if it did happen, a hot-standby postmaster could fail to shut down
after receiving a smart-shutdown request.

In MaybeStartWalReceiver, don't clear the WalReceiverRequested flag
if the fork attempt fails.  Leaving it set allows us to try
again in future iterations of the postmaster idle loop.  (The startup
process would eventually send a fresh request signal, but this change
may allow us to retry the fork sooner.)

Remove an obsolete comment and unnecessary test in
PostmasterStateMachine's handling of PM_SHUTDOWN_2 state.  It's not
possible to have a live walreceiver in that state, and AFAICT has not
been possible since commit 5e85315ea.  This isn't a live bug, but the
false comment is quite confusing to readers.

In passing, rearrange sigusr1_handler's CheckPromoteSignal tests so that
we don't uselessly perform stat() calls that we're going to ignore the
results of.

Add some comments clarifying the behavior of MaybeStartWalReceiver;
I very nearly rearranged it in a way that'd reintroduce the race
condition fixed in e5d494d78.  Mea culpa for not commenting that
properly at the time.

Back-patch to all supported branches.  The PMSIGNAL_ADVANCE_STATE_MACHINE
change is the only one of even minor significance, but we might as well
keep this code in sync across branches.

Discussion: https://postgr.es/m/9001.1556046681@sss.pgh.pa.us
2019-04-24 14:15:45 -04:00
Michael Paquier
20dbc84bd0 Fix detection of passwords hashed with MD5
This commit fixes an issue related to the way password verifiers hashed
with MD5 are detected, leading to possibly detect that plain passwords
are legit MD5 hashes.  A MD5-hashed entry was checked based on if its
header uses "md5" and if the string length matches what is expected.
Unfortunately the code never checked if the hash only used hexadecimal
characters after the three-character prefix.

Fix 9.6 down to 9.4, where this code is present.  This area of the code
has changed in 10 and upwards with the introduction of SCRAM, which led
to a different fix committed as of ccae190.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Jonathan Katz
Discussion: https://postgr.es/m/016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org
Backpatch-through: 9.4
2019-04-24 09:05:30 +09:00
Tom Lane
74589026f7 Repair assorted issues in locale data extraction.
cache_locale_time (extraction of LC_TIME-related info) had never been
taught the lessons we previously learned about extraction of info related
to LC_MONETARY and LC_NUMERIC.  Specifically, commit 95a777c61 taught
PGLC_localeconv() that data coming out of localeconv() was in an encoding
determined by the relevant locale, but we didn't realize that there's a
similar issue with strftime().  And commit a4930e7ca hardened
PGLC_localeconv() against errors occurring partway through, but failed
to do likewise for cache_locale_time().  So, rearrange the latter
function to perform encoding conversion and not risk failure while
it's got the locales set to temporary values.

This time around I also changed PGLC_localeconv() to treat it as FATAL
if it can't restore the previous settings of the locale values.  There
is no reason (except possibly OOM) for that to fail, and proceeding with
the wrong locale values seems like a seriously bad idea --- especially
on Windows where we have to also temporarily change LC_CTYPE.  Also,
protect against the possibility that we can't identify the codeset
reported for LC_MONETARY or LC_NUMERIC; rather than just failing,
try to validate the data without conversion.

The user-visible symptom this fixes is that if LC_TIME is set to a locale
name that implies an encoding different from the database encoding,
non-ASCII localized day and month names would be retrieved in the wrong
encoding, leading to either unexpected encoding-conversion error reports
or wrong output from to_char().  The other possible failure modes are
unlikely enough that we've not seen reports of them, AFAIK.

The encoding conversion problems do not manifest on Windows, since
we'd already created special-case code to handle that issue there.

Per report from Juan José Santamaría Flecha.  Back-patch to all
supported versions.

Juan José Santamaría Flecha and Tom Lane

Discussion: https://postgr.es/m/CAC+AXB22So5aZm2vZe+MChYXec7gWfr-n-SK-iO091R0P_1Tew@mail.gmail.com
2019-04-23 18:51:31 -04:00
Bruce Momjian
475f07aa2e postgresql.conf.sample: add proper defaults for include actions
Previously, include actions include_dir, include_if_exists, and include
listed commented-out values which were not the defaults, which is
inconsistent with other entries.  Instead, replace them with '', which
is the default value.

Reported-by: Emanuel Araújo

Discussion: https://postgr.es/m/CAMuTAkYMx6Q27wpELDR3_v9aG443y7ZjeXu15_+1nGUjhMWOJA@mail.gmail.com

Backpatch-through: 9.4
2019-04-17 18:12:10 -04:00
Bruce Momjian
dfdab98cde docs: clarify pg_upgrade's recovery behavior
The previous paragraph trying to explain --check, --link, and no --link
modes and the various points of failure was too complex.  Instead, use
bullet lists and sublists.

Reported-by: Daniel Gustafsson

Discussion: https://postgr.es/m/qtqiv7hI87s_Xvz5ZXHCaH-1-_AZGpIDJowzlRjF3-AbCr3RhSNydM_JCuJ8DE4WZozrtxhIWmyYTbv0syKyfGB6cYMQitp9yN-NZMm-oAo=@yesql.se

Backpatch-through: 9.4
2019-04-17 18:01:01 -04:00
Noah Misch
c565de6431 Consistently test for in-use shared memory.
postmaster startup scrutinizes any shared memory segment recorded in
postmaster.pid, exiting if that segment matches the current data
directory and has an attached process.  When the postmaster.pid file was
missing, a starting postmaster used weaker checks.  Change to use the
same checks in both scenarios.  This increases the chance of a startup
failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
postmaster.pid` && rm postmaster.pid && pg_ctl -w start".  A postmaster
will no longer stop if shmat() of an old segment fails with EACCES.  A
postmaster will no longer recycle segments pertaining to other data
directories.  That's good for production, but it's bad for integration
tests that crash a postmaster and immediately delete its data directory.
Such a test now leaks a segment indefinitely.  No "make check-world"
test does that.  win32_shmem.c already avoided all these problems.  In
9.6 and later, enhance PostgresNode to facilitate testing.  Back-patch
to 9.4 (all supported versions).

Reviewed (in earlier versions) by Daniel Gustafsson and Kyotaro HORIGUCHI.

Discussion: https://postgr.es/m/20190408064141.GA2016666@rfd.leadboat.com
2019-04-12 22:41:42 -07:00
Michael Meskes
67f6e645e7 Fix off-by-one check that can lead to a memory overflow in ecpg.
Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>
2019-04-11 21:06:10 +02:00
Bruce Momjian
d3cf886e31 doc: adjust libpq wording to be neither/nor
Reported-by: postgresql@cohi.at

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

Backpatch-through: 9.4
2019-04-11 13:25:34 -04:00
Tom Lane
b9b7fe8ca4 Fix backwards test in operator_precedence_warning logic.
Warnings about unary minus might have been wrong.  It's a bit
surprising that nobody noticed yet ... probably the precedence-warning
feature hasn't really been used much in the field.

Rikard Falkeborn

Discussion: https://postgr.es/m/CADRDgG6fzA8A2oeygUw4=o7ywo4kvz26NxCSgpq22nMD73Bx4Q@mail.gmail.com
2019-04-10 19:03:29 -04:00
Noah Misch
292e2000e1 Define WIN32_STACK_RLIMIT throughout win32 and cygwin builds.
The MSVC build system already did this, and commit
617dc6d299c957e2784320382b3277ede01d9c63 used it in a second file.
Back-patch to 9.4, like that commit.

Discussion: https://postgr.es/m/CAA8=A7_1SWc3+3Z=-utQrQFOtrj_DeohRVt7diA2tZozxsyUOQ@mail.gmail.com
2019-04-09 08:25:43 -07:00
Noah Misch
7a56778185 Avoid "could not reattach" by providing space for concurrent allocation.
We've long had reports of intermittent "could not reattach to shared
memory" errors on Windows.  Buildfarm member dory fails that way when
PGSharedMemoryReAttach() execution overlaps with creation of a thread
for the process's "default thread pool".  Fix that by providing a second
region to receive asynchronous allocations that would otherwise intrude
into UsedShmemSegAddr.  In pgwin32_ReserveSharedMemoryRegion(), stop
trying to free reservations landing at incorrect addresses; the caller's
next step has been to terminate the affected process.  Back-patch to 9.4
(all supported versions).

Reviewed by Tom Lane.  He also did much of the prerequisite research;
see commit bcbf2346d69f6006f126044864dd9383d50d87b4.

Discussion: https://postgr.es/m/20190402135442.GA1173872@rfd.leadboat.com
2019-04-08 21:39:04 -07:00
Tom Lane
2f78974e29 Fix improper interaction of FULL JOINs with lateral references.
join_is_legal() needs to reject forming certain outer joins in cases
where that would lead the planner down a blind alley.  However, it
mistakenly supposed that the way to handle full joins was to treat them
as applying the same constraints as for left joins, only to both sides.
That doesn't work, as shown in bug #15741 from Anthony Skorski: given
a lateral reference out of a join that's fully enclosed by a full join,
the code would fail to believe that any join ordering is legal, resulting
in errors like "failed to build any N-way joins".

However, we don't really need to consider full joins at all for this
purpose, because we effectively force them to be evaluated in syntactic
order, and that order is always legal for lateral references.  Hence,
get rid of this broken logic for full joins and just ignore them instead.

This seems to have been an oversight in commit 7e19db0c0.
Back-patch to all supported branches, as that was.

Discussion: https://postgr.es/m/15741-276f1f464b3f40eb@postgresql.org
2019-04-08 16:09:07 -04:00
Noah Misch
9ec582b64d Revert "Consistently test for in-use shared memory."
This reverts commits 2f932f71d9f2963bbd201129d7b971c8f5f077fd,
16ee6eaf80a40007a138b60bb5661660058d0422 and
6f0e190056fe441f7cf788ff19b62b13c94f68f3.  The buildfarm has revealed
several bugs.  Back-patch like the original commits.

Discussion: https://postgr.es/m/20190404145319.GA1720877@rfd.leadboat.com
2019-04-05 00:00:55 -07:00
Michael Paquier
1f63f69c43 Fix some documentation in pg_rewind
A confusion which comes a lot from users is that it is necessary to
issue a checkpoint on a freshly-promoted standby so as its control file
has up-to-date timeline information which is used by pg_rewind to
validate the operation.  Let's document that properly.  This is
back-patched down to 9.5 where pg_rewind has been introduced.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/CABUevEz5bpvbwVsYCaSMV80CBZ5-82nkMzbb+Bu=h1m=rLdn=g@mail.gmail.com
Backpatch-through: 9.5
2019-04-05 10:38:47 +09:00
Noah Misch
e9559e7add Silence -Wimplicit-fallthrough in sysv_shmem.c.
Commit 2f932f71d9f2963bbd201129d7b971c8f5f077fd added code that elicits
a warning on buildfarm member flaviventris.  Back-patch to 9.4, like
that commit.

Reported by Andres Freund.

Discussion: https://postgr.es/m/20190404020057.galelv7by75ekqrh@alap3.anarazel.de
2019-04-03 23:23:39 -07:00
Noah Misch
267d83c7d7 Handle USE_MODULE_DB for all tests able to use an installed postmaster.
When $(MODULES) and $(MODULE_big) are empty, derive the database name
from the first element of $(REGRESS) instead of using a constant string.
When deriving the database name from $(MODULES), use its first element
instead of the entire list; the earlier approach would fail if any
multi-module directory had $(REGRESS) tests.  Treat isolation suites and
src/pl correspondingly.  Under USE_MODULE_DB=1, installcheck-world and
check-world no longer reuse any database name in a given postmaster.
Buildfarm members axolotl, mandrill and frogfish saw spurious "is being
accessed by other users" failures that would not have happened without
database name reuse.  (The CountOtherDBBackends() 5s deadline expired
during DROP DATABASE; a backend for an earlier test suite had used the
same database name and had not yet exited.)  Back-patch to 9.4 (all
supported versions), except bits pertaining to isolation suites.

Concept reviewed by Andrew Dunstan, Andres Freund and Tom Lane.

Discussion: https://postgr.es/m/20190401135213.GE891537@rfd.leadboat.com
2019-04-03 17:10:39 -07:00
Noah Misch
e7f89a5dec Consistently test for in-use shared memory.
postmaster startup scrutinizes any shared memory segment recorded in
postmaster.pid, exiting if that segment matches the current data
directory and has an attached process.  When the postmaster.pid file was
missing, a starting postmaster used weaker checks.  Change to use the
same checks in both scenarios.  This increases the chance of a startup
failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
postmaster.pid` && rm postmaster.pid && pg_ctl -w start".  A postmaster
will no longer recycle segments pertaining to other data directories.
That's good for production, but it's bad for integration tests that
crash a postmaster and immediately delete its data directory.  Such a
test now leaks a segment indefinitely.  No "make check-world" test does
that.  win32_shmem.c already avoided all these problems.  In 9.6 and
later, enhance PostgresNode to facilitate testing.  Back-patch to 9.4
(all supported versions).

Reviewed by Daniel Gustafsson and Kyotaro HORIGUCHI.

Discussion: https://postgr.es/m/20130911033341.GD225735@tornado.leadboat.com
2019-04-03 17:03:50 -07:00
Dean Rasheed
2e606d0ad7 Perform RLS subquery checks as the right user when going via a view.
When accessing a table with RLS via a view, the RLS checks are
performed as the view owner. However, the code neglected to propagate
that to any subqueries in the RLS checks. Fix that by calling
setRuleCheckAsUser() for all RLS policy quals and withCheckOption
checks for RTEs with RLS.

Back-patch to 9.5 where RLS was added.

Per bug #15708 from daurnimator.

Discussion: https://postgr.es/m/15708-d65cab2ce9b1717a@postgresql.org
2019-04-02 08:22:48 +01:00