postgresql.git
7 years agoFix typo
Magnus Hagander [Thu, 19 Oct 2017 11:59:19 +0000 (13:59 +0200)]
Fix typo

David Rowley

7 years agoFix typo in release notes
Magnus Hagander [Thu, 19 Oct 2017 11:56:21 +0000 (13:56 +0200)]
Fix typo in release notes

Spotted by Piotr Stefaniak

7 years agoMake release notes aware that --xlog-method was renamed
Alvaro Herrera [Wed, 18 Oct 2017 11:21:43 +0000 (13:21 +0200)]
Make release notes aware that --xlog-method was renamed

Author: David G. Johnston
Discussion: https:/postgr.es/m/CAKFQuwaCsb-OKOjQXGeN0R7byxiRWvr7OtyKDbJoYgiF2vBG4Q@mail.gmail.com

7 years agoFix incorrect handling of CTEs and ENRs as DML target relations.
Tom Lane [Mon, 16 Oct 2017 21:56:43 +0000 (17:56 -0400)]
Fix incorrect handling of CTEs and ENRs as DML target relations.

setTargetTable threw an error if the proposed target RangeVar's relname
matched any visible CTE or ENR.  This breaks backwards compatibility in
the CTE case, since pre-v10 we never looked for a CTE here at all, so that
CTE names did not mask regular tables.  It does seem like a good idea to
throw an error for the ENR case, though, thus causing ENRs to mask tables
for this purpose; ENRs are new in v10 so we're not breaking existing code,
and we may someday want to allow them to be the targets of DML.

To fix that, replace use of getRTEForSpecialRelationTypes, which was
overkill anyway, with use of scanNameSpaceForENR.

A second problem was that the check neglected to verify null schemaname,
so that a CTE or ENR could incorrectly be thought to match a qualified
RangeVar.  That happened because getRTEForSpecialRelationTypes relied
on its caller to have checked for null schemaname.  Even though the one
remaining caller got it right, this is obviously bug-prone, so move
the check inside getRTEForSpecialRelationTypes.

Also, revert commit 18ce3a4ab's extremely poorly thought out decision to
add a NULL return case to parserOpenTable --- without either documenting
that or adjusting any of the callers to check for it.  The current bug
seems to have arisen in part due to working around that bad idea.

In passing, remove the one-line shim functions transformCTEReference and
transformENRReference --- they don't seem to be adding any clarity or
functionality.

Per report from Hugo Mercier (via Julien Rouhaud).  Back-patch to v10
where the bug was introduced.

Thomas Munro, with minor editing by me

Discussion: https://postgr.es/m/CAOBaU_YdPVH+PTtiKSSLOiiW3mVDYsnNUekK+XPbHXiP=wrFLA@mail.gmail.com

7 years agoRepair breakage of aggregate FILTER option.
Tom Lane [Mon, 16 Oct 2017 19:24:36 +0000 (15:24 -0400)]
Repair breakage of aggregate FILTER option.

An aggregate's input expression(s) are not supposed to be evaluated
at all for a row where its FILTER test fails ... but commit 8ed3f11bb
overlooked that requirement.  Reshuffle so that aggregates having a
filter clause evaluate their arguments separately from those without.
This still gets the benefit of doing only one ExecProject in the
common case of multiple Aggrefs, none of which have filters.

While at it, arrange for filter clauses to be included in the common
ExecProject evaluation, thus perhaps buying a little bit even when
there are filters.

Back-patch to v10 where the bug was introduced.

Discussion: https://postgr.es/m/30065.1508161354@sss.pgh.pa.us

7 years agoRestore nodeAgg.c's ability to check for improperly-nested aggregates.
Tom Lane [Sun, 15 Oct 2017 23:19:19 +0000 (19:19 -0400)]
Restore nodeAgg.c's ability to check for improperly-nested aggregates.

While poking around in the aggregate logic, I noticed that commit
8ed3f11bb broke the logic in nodeAgg.c that purports to detect nested
aggregates, by moving initialization of regular aggregate argument
expressions out of the code segment that checks for that.

You could argue that this check is unnecessary, but it's not much code
so I'm inclined to keep it as a backstop against parser and planner
bugs.  However, there's certainly zero value in checking only some of
the subexpressions.

We can make the check complete again, and as a bonus make it a good
deal more bulletproof against future mistakes of the same ilk, by
moving it out to the outermost level of ExecInitAgg.  This means we
need to check only once per Agg node not once per aggregate, which
also seems like a good thing --- if the check does find something
wrong, it's not urgent that we report it before the plan node
initialization finishes.

Since this requires remembering the original length of the aggs list,
I deleted a long-obsolete stanza that changed numaggs from 0 to 1.
That's so old it predates our decision that palloc(0) is a valid
operation, in (digs...) 2004, see commit 24a1e20f1.

In passing improve a few comments.

Back-patch to v10, just in case.

7 years agoFix possible crash with Parallel Bitmap Heap Scan.
Robert Haas [Fri, 13 Oct 2017 18:53:28 +0000 (14:53 -0400)]
Fix possible crash with Parallel Bitmap Heap Scan.

If a Parallel Bitmap Heap scan's chain of leftmost descendents
includes a BitmapOr whose first child is a BitmapAnd, the prior coding
would mistakenly create a non-shared TIDBitmap and then try to perform
shared iteration.

Report by Tomas Vondra.  Patch by Dilip Kumar.

Discussion: http://postgr.es/m/50e89684-8ad9-dead-8767-c9545bafd3b6@2ndquadrant.com

7 years agoFix AggGetAggref() so it won't lie to aggregate final functions.
Tom Lane [Thu, 12 Oct 2017 19:20:04 +0000 (15:20 -0400)]
Fix AggGetAggref() so it won't lie to aggregate final functions.

If we merge the transition calculations for two different aggregates,
it's reasonable to assume that the transition function should not care
which of those Aggref structs it gets from AggGetAggref().  It is not
reasonable to make the same assumption about an aggregate final function,
however.  Commit 804163bc2 broke this, as it will pass whichever Aggref
was first associated with the transition state in both cases.

This doesn't create an observable bug so far as the core system is
concerned, because the only existing uses of AggGetAggref() are in
ordered-set aggregates that happen to not pay attention to anything
but the input properties of the Aggref; and besides that, we disabled
sharing of transition calculations for OSAs yesterday.  Nonetheless,
if some third-party code were using AggGetAggref() in a normal aggregate,
they would be entitled to call this a bug.  Hence, back-patch the fix
to 9.6 where the problem was introduced.

In passing, improve some of the comments about transition state sharing.

Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com

7 years agoDoc: fix typo in release notes.
Tom Lane [Thu, 12 Oct 2017 15:35:54 +0000 (11:35 -0400)]
Doc: fix typo in release notes.

Ioseph Kim

Discussion: https://postgr.es/m/e7a79f91-8244-5bcb-afcc-96c817e86f4e@postgresql.kr

7 years agoInfer functional dependency past RelabelType
Alvaro Herrera [Thu, 12 Oct 2017 15:23:47 +0000 (17:23 +0200)]
Infer functional dependency past RelabelType

Vars hidden within a RelabelType would not be detected as compatible
with some functional dependency.  Repair by properly ignoring the
RelabelType.

Author: David Rowley
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAKJS1f-y-UEy=rsBXynBOgiW1fKMr_LVoYSGL9QOc36mLEC-ww@mail.gmail.com

7 years agoFix logical replication to fire BEFORE ROW DELETE triggers.
Robert Haas [Thu, 12 Oct 2017 14:09:26 +0000 (10:09 -0400)]
Fix logical replication to fire BEFORE ROW DELETE triggers.

Before, that would fail to happen unless a BEFORE ROW UPDATE trigger
was also present.

Noted by me while reviewing a patch from Masahiko Sawada, who also
wrote this patch.  Reviewed by Petr Jelinek.

Discussion: http://postgr.es/m/CA+TgmobAZvCxduG8y_mQKBK7nz-vhbdLvjM354KEFozpuzMN5A@mail.gmail.com

7 years agoPrevent sharing transition states between ordered-set aggregates.
Tom Lane [Thu, 12 Oct 2017 02:18:01 +0000 (22:18 -0400)]
Prevent sharing transition states between ordered-set aggregates.

This ought to work, but the built-in OSAs are not capable of coping,
because their final-functions destructively modify their transition
state (specifically, the contained tuplesort object).  That was fine
when those functions were written, but commit 804163bc2 moved the
goalposts without telling orderedsetaggs.c.

We should fix the built-in OSAs to support this, but it will take
a little work, especially if we don't want to sacrifice performance
in the normal non-shared-state case.  Given that it took a year after
9.6 release for anyone to notice this bug, we should not prioritize
sharable-state over nonsharable-state performance.  And a proper fix
is likely to be more complicated than we'd want to back-patch, too.

Therefore, let's just put in this stop-gap patch to prevent nodeAgg.c
from choosing to use shared state for OSAs.  We can revert it in HEAD
when we get a better fix.

Report from Lukas Eder, diagnosis by me, patch by David Rowley.
Back-patch to 9.6 where the problem was introduced.

Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com

7 years agoPrevent idle in transaction session timeout from sometimes being ignored.
Andres Freund [Wed, 11 Oct 2017 19:03:26 +0000 (12:03 -0700)]
Prevent idle in transaction session timeout from sometimes being ignored.

The previous coding in ProcessInterrupts() could lead to
idle_in_transaction_session_timeout being ignored, when
statement_timeout occurred earlier.

The problem was that ProcessInterrupts() would return before
processing the transaction timeout if QueryCancelPending was set while
QueryCancelHoldoffCount != 0 - which is the case when reading new
commands from the client. Ergo when the idle transaction timeout would
hit.

Fix that by removing the early return. Alternatively the transaction
timeout code could have been moved up, but that early return seems
like an issue that could hit other cases too.

Author: Lukas Fittl
Bug: #14821
Discussion:
    https://www.postgresql.org/message-id/20170921010956.17345.61461%40wrigleys.postgresql.org
    https://www.postgresql.org/message-id/CAP53PkxQnv3OWJpyNPGJYT62uY=n1=2CF_Lpc6gVOFnc0-gazw@mail.gmail.com
Backpatch: 9.6-, where idle_in_transaction_session_timeout was introduced.

7 years agoDoc: fix missing explanation of default object privileges.
Tom Lane [Wed, 11 Oct 2017 20:56:23 +0000 (16:56 -0400)]
Doc: fix missing explanation of default object privileges.

The GRANT reference page, which lists the default privileges for new
objects, failed to mention that USAGE is granted by default for data
types and domains.  As a lesser sin, it also did not specify anything
about the initial privileges for sequences, FDWs, foreign servers,
or large objects.  Fix that, and add a comment to acldefault() in the
probably vain hope of getting people to maintain this list in future.

Noted by Laurenz Albe, though I editorialized on the wording a bit.
Back-patch to all supported branches, since they all have this behavior.

Discussion: https://postgr.es/m/1507620895.4152.1.camel@cybertec.at

7 years agoFix mistakes in comments.
Robert Haas [Wed, 11 Oct 2017 19:55:10 +0000 (15:55 -0400)]
Fix mistakes in comments.

Masahiko Sawada

Discussion: http://postgr.es/m/CAD21AoBsfYsMHD6_SL9iN3n_Foaa+oPbL5jG55DxU1ChaujqwQ@mail.gmail.com

7 years agoFix low-probability loss of NOTIFY messages due to XID wraparound.
Tom Lane [Wed, 11 Oct 2017 18:28:33 +0000 (14:28 -0400)]
Fix low-probability loss of NOTIFY messages due to XID wraparound.

Up to now async.c has used TransactionIdIsInProgress() to detect whether
a notify message's source transaction is still running.  However, that
function has a quick-exit path that reports that XIDs before RecentXmin
are no longer running.  If a listening backend is doing nothing but
listening, and not running any queries, there is nothing that will advance
its value of RecentXmin.  Once 2 billion transactions elapse, the
RecentXmin check causes active transactions to be reported as not running.
If they aren't committed yet according to CLOG, async.c decides they
aborted and discards their messages.  The timing for that is a bit tight
but it can happen when multiple backends are sending notifies concurrently.
The net symptom therefore is that a sufficiently-long-surviving
listen-only backend starts to miss some fraction of NOTIFY traffic,
but only under heavy load.

The only function that updates RecentXmin is GetSnapshotData().
A brute-force fix would therefore be to take a snapshot before
processing incoming notify messages.  But that would add cycles,
as well as contention for the ProcArrayLock.  We can be smarter:
having taken the snapshot, let's use that to check for running
XIDs, and not call TransactionIdIsInProgress() at all.  In this
way we reduce the number of ProcArrayLock acquisitions from one
per message to one per notify interrupt; that's the same under
light load but should be a benefit under heavy load.  Light testing
says that this change is a wash performance-wise for normal loads.

I looked around for other callers of TransactionIdIsInProgress()
that might be at similar risk, and didn't find any; all of them
are inside transactions that presumably have already taken a
snapshot.

Problem report and diagnosis by Marko Tiikkaja, patch by me.
Back-patch to all supported branches, since it's been like this
since 9.0.

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

7 years agoAdd missing clean step to src/test/modules/brin/Makefile.
Tom Lane [Tue, 10 Oct 2017 16:51:09 +0000 (12:51 -0400)]
Add missing clean step to src/test/modules/brin/Makefile.

I noticed the tmp_check subdirectory wasn't getting cleaned up
after a check-world run.  Apparently pgxs.mk will only do this
for you if you've defined REGRESS.  The only other src/test/modules
Makefile that does not set that is snapshot_too_old, and it
does it like this.

7 years agoIncrease distance between flush requests during bulk file copies.
Tom Lane [Sun, 8 Oct 2017 19:25:26 +0000 (15:25 -0400)]
Increase distance between flush requests during bulk file copies.

copy_file() reads and writes data 64KB at a time (with default BLCKSZ),
and historically has issued a pg_flush_data request after each write.
This turns out to interact really badly with macOS's new APFS file
system: a large file copy takes over 100X longer than it ought to on
APFS, as reported by Brent Dearth.  While that's arguably a macOS bug,
it's not clear whether Apple will do anything about it in the near
future, and in any case experimentation suggests that issuing flushes
a bit less often can be helpful on other platforms too.

Hence, rearrange the logic in copy_file() so that flush requests are
issued once per N writes rather than every time through the loop.
I set the FLUSH_DISTANCE to 32MB on macOS (any less than that still
results in a noticeable speed degradation on APFS), but 1MB elsewhere.
In limited testing on Linux and FreeBSD, this seems slightly faster
than the previous code, and certainly no worse.  It helps noticeably
on macOS even with the older HFS filesystem.

A simpler change would have been to just increase the size of the
copy buffer without changing the loop logic, but that seems likely
to trash the processor cache without really helping much.

Back-patch to 9.6 where we introduced msync() as an implementation
option for pg_flush_data().  The problem seems specific to APFS's
mmap/msync support, so I don't think we need to go further back.

Discussion: https://postgr.es/m/CADkxhTNv-j2jw2g8H57deMeAbfRgYBoLmVuXkC=YCFBXRuCOww@mail.gmail.com

7 years agoFix crash when logical decoding is invoked from a PL function.
Tom Lane [Fri, 6 Oct 2017 23:18:58 +0000 (19:18 -0400)]
Fix crash when logical decoding is invoked from a PL function.

The logical decoding functions do BeginInternalSubTransaction and
RollbackAndReleaseCurrentSubTransaction to clean up after themselves.
It turns out that AtEOSubXact_SPI has an unrecognized assumption that
we always need to cancel the active SPI operation in the SPI context
that surrounds the subtransaction (if there is one).  That's true
when the RollbackAndReleaseCurrentSubTransaction call is coming from
the SPI-using function itself, but not when it's happening inside
some unrelated function invoked by a SPI query.  In practice the
affected callers are the various PLs.

To fix, record the current subtransaction ID when we begin a SPI
operation, and clean up only if that ID is the subtransaction being
canceled.

Also, remove AtEOSubXact_SPI's assertion that it must have cleaned
up the surrounding SPI context's active tuptable.  That's proven
wrong by the same test case.

Also clarify (or, if you prefer, reinterpret) the calling conventions
for _SPI_begin_call and _SPI_end_call.  The memory context cleanup
in the latter means that these have always had the flavor of a matched
resource-management pair, but they weren't documented that way before.

Per report from Ben Chobot.

Back-patch to 9.4 where logical decoding came in.  In principle,
the SPI changes should go all the way back, since the problem dates
back to commit 7ec1c5a86.  But given the lack of field complaints
it seems few people are using internal subtransactions in this way.
So I don't feel a need to take any risks in 9.2/9.3.

Discussion: https://postgr.es/m/73FBA179-C68C-4540-9473-71E865408B15@silentmedia.com

7 years agoFix intra-query memory leakage in nodeProjectSet.c.
Tom Lane [Fri, 6 Oct 2017 18:28:42 +0000 (14:28 -0400)]
Fix intra-query memory leakage in nodeProjectSet.c.

Both ExecMakeFunctionResultSet() and evaluation of simple expressions
need to be done in the per-tuple memory context, not per-query, else
we leak data until end of query.  This is a consideration that was
missed while refactoring code in the ProjectSet patch (note that in
pre-v10, ExecMakeFunctionResult is called in the per-tuple context).

Per bug #14843 from Ben M.  Diagnosed independently by Andres and myself.

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

7 years agoFix access-off-end-of-array in clog.c.
Tom Lane [Fri, 6 Oct 2017 16:20:13 +0000 (12:20 -0400)]
Fix access-off-end-of-array in clog.c.

Sloppy loop coding in set_status_by_pages() resulted in fetching one array
element more than it should from the subxids[] array.  The odds of this
resulting in SIGSEGV are pretty small, but we've certainly seen that happen
with similar mistakes elsewhere.  While at it, we can get rid of an extra
TransactionIdToPage() calculation per loop.

Per report from David Binderman.  Back-patch to all supported branches,
since this code is quite old.

Discussion: https://postgr.es/m/HE1PR0802MB2331CBA919CBFFF0C465EB429C710@HE1PR0802MB2331.eurprd08.prod.outlook.com

7 years agoFix traversal of half-frozen update chains
Alvaro Herrera [Fri, 6 Oct 2017 15:14:42 +0000 (17:14 +0200)]
Fix traversal of half-frozen update chains

When some tuple versions in an update chain are frozen due to them being
older than freeze_min_age, the xmax/xmin trail can become broken.  This
breaks HOT (and probably other things).  A subsequent VACUUM can break
things in more serious ways, such as leaving orphan heap-only tuples
whose root HOT redirect items were removed.  This can be seen because
index creation (or REINDEX) complain like
  ERROR:  XX000: failed to find parent tuple for heap-only tuple at (0,7) in table "t"

Because of relfrozenxid contraints, we cannot avoid the freezing of the
early tuples, so we must cope with the results: whenever we see an Xmin
of FrozenTransactionId, consider it a match for whatever the previous
Xmax value was.

This problem seems to have appeared in 9.3 with multixact changes,
though strictly speaking it seems unrelated.

Since 9.4 we have commit 37484ad2a "Change the way we mark tuples as
frozen", so the fix is simple: just compare the raw Xmin (still stored
in the tuple header, since freezing merely set an infomask bit) to the
Xmax.  But in 9.3 we rewrite the Xmin value to FrozenTransactionId, so
the original value is lost and we have nothing to compare the Xmax with.
To cope with that case we need to compare the Xmin with FrozenXid,
assume it's a match, and hope for the best.  Sadly, since you can
pg_upgrade a 9.3 instance containing half-frozen pages to newer
releases, we need to keep the old check in newer versions too, which
seems a bit brittle; I hope we can somehow get rid of that.

I didn't optimize the new function for performance.  The new coding is
probably a bit slower than before, since there is a function call rather
than a straight comparison, but I'd rather have it work correctly than
be fast but wrong.

This is a followup after 20b655224249 fixed a few related problems.
Apparently, in 9.6 and up there are more ways to get into trouble, but
in 9.3 - 9.5 I cannot reproduce a problem anymore with this patch, so
there must be a separate bug.

Reported-by: Peter Geoghegan
Diagnosed-by: Peter Geoghegan, Michael Paquier, Daniel Wood,
Yi Wen Wong, Álvaro
Discussion: https://postgr.es/m/CAH2-Wznm4rCrhFAiwKPWTpEw2bXDtgROZK7jWWGucXeH3D1fmA@mail.gmail.com

7 years agoFix more user-visible elog() calls.
Robert Haas [Thu, 5 Oct 2017 11:58:02 +0000 (07:58 -0400)]
Fix more user-visible elog() calls.

Michael Paquier discovered that this could be triggered via SQL;
give a nicer message instead.

Patch by Michael Paquier, reviewed by Masahiko Sawada.

Discussion: http://postgr.es/m/CAB7nPqQtPg+LKKtzdKN26judHcvPZ0s1gNigzOT4j8CYuuuBYg@mail.gmail.com

7 years agoFix race condition with unprotected use of a latch pointer variable.
Tom Lane [Tue, 3 Oct 2017 18:00:57 +0000 (14:00 -0400)]
Fix race condition with unprotected use of a latch pointer variable.

Commit 597a87ccc introduced a latch pointer variable to replace use
of a long-lived shared latch in the shared WalRcvData structure.
This was not well thought out, because there are now hazards of the
pointer variable changing while it's being inspected by another
process.  This could obviously lead to a core dump in code like

if (WalRcv->latch)
SetLatch(WalRcv->latch);

and there's a more remote risk of a torn read, if we have any
platforms where reading/writing a pointer is not atomic.

An actual problem would occur only if the walreceiver process
exits (gracefully) while the startup process is trying to
signal it, but that seems well within the realm of possibility.

To fix, treat the pointer variable (not the referenced latch)
as being protected by the WalRcv->mutex spinlock.  There
remains a race condition that we could apply SetLatch to a
process latch that no longer belongs to the walreceiver, but
I believe that's harmless: at worst it'd cause an extra wakeup
of the next process to use that PGPROC structure.

Back-patch to v10 where the faulty code was added.

Discussion: https://postgr.es/m/22735.1507048202@sss.pgh.pa.us

7 years agoFix coding rules violations in walreceiver.c
Alvaro Herrera [Tue, 3 Oct 2017 12:58:25 +0000 (14:58 +0200)]
Fix coding rules violations in walreceiver.c

1. Since commit b1a9bad9e744 we had pstrdup() inside a
spinlock-protected critical section; reported by Andreas Seltenreich.
Turn those into strlcpy() to stack-allocated variables instead.
Backpatch to 9.6.

2. Since commit 9ed551e0a4fd we had a pfree() uselessly inside a
spinlock-protected critical section.  Tom Lane noticed in code review.
Move down.  Backpatch to 9.6.

3. Since commit 64233902d22b we had GetCurrentTimestamp() (a kernel
call) inside a spinlock-protected critical section.  Tom Lane noticed in
code review.  Move it up.  Backpatch to 9.2.

4. Since commit 1bb2558046cc we did elog(PANIC) while holding spinlock.
Tom Lane noticed in code review.  Release spinlock before dying.
Backpatch to 9.2.

Discussion: https://postgr.es/m/87h8vhtgj2.fsf@ansel.ydns.eu

7 years agoStamp 10.0. REL_10_0
Tom Lane [Mon, 2 Oct 2017 21:09:15 +0000 (17:09 -0400)]
Stamp 10.0.

7 years agoTranslation updates
Peter Eisentraut [Mon, 2 Oct 2017 16:03:01 +0000 (12:03 -0400)]
Translation updates

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4eb5acee0bc0ba7b40220367dfc44bb4af188c88

7 years agoExpand collation documentation
Peter Eisentraut [Fri, 22 Sep 2017 17:51:01 +0000 (13:51 -0400)]
Expand collation documentation

Document better how to create custom collations and what locale strings
ICU accepts.  Explain the ICU examples in more detail.  Also update the
text on the CREATE COLLATION reference page a bit to take ICU more into
account.

7 years agoUpdate v10 release notes, and set the official release date.
Tom Lane [Sun, 1 Oct 2017 17:32:26 +0000 (13:32 -0400)]
Update v10 release notes, and set the official release date.

Last(?) round of changes for 10.0.

7 years agoUse a longer connection timeout in pg_isready test.
Tom Lane [Sun, 1 Oct 2017 16:43:47 +0000 (12:43 -0400)]
Use a longer connection timeout in pg_isready test.

Buildfarm members skink and sungazer have both recently failed this
test, with symptoms indicating that the default 3-second timeout
isn't quite enough for those very slow systems.  There's no reason
to be miserly with this timeout, so boost it to 60 seconds.

Back-patch to all versions containing this test.  That may be overkill,
because the failure has only been observed in the v10 branch, but
I don't feel like having to revisit this later.

7 years agoAdd list of acknowledgments to release notes
Peter Eisentraut [Sun, 1 Oct 2017 12:51:20 +0000 (08:51 -0400)]
Add list of acknowledgments to release notes

This contains all individuals mentioned in the commit messages during
PostgreSQL 10 development.

current through babf18579455e85269ad75e1ddb03f34138f77b6

Discussion: https://www.postgresql.org/message-id/flat/54ad0e42-770e-dfe1-123e-bce9361ad452%402ndquadrant.com

7 years agoFix busy-wait in pgbench, with --rate.
Heikki Linnakangas [Sun, 1 Oct 2017 06:29:27 +0000 (09:29 +0300)]
Fix busy-wait in pgbench, with --rate.

If --rate was used to throttle pgbench, it failed to sleep when it had
nothing to do, leading to a busy-wait with 100% CPU usage. This bug was
introduced in the refactoring in v10. Before that, sleep() was called with
a timeout, even when there were no file descriptors to wait for.

Reported by Jeff Janes, patch by Fabien COELHO. Backpatch to v10.

Discussion: https://www.postgresql.org/message-id/CAMkU%3D1x5hoX0pLLKPRnXCy0T8uHoDvXdq%2B7kAM9eoC9_z72ucw%40mail.gmail.com

7 years agoFix inadequate locking during get_rel_oids().
Tom Lane [Fri, 29 Sep 2017 20:26:21 +0000 (16:26 -0400)]
Fix inadequate locking during get_rel_oids().

get_rel_oids used to not take any relation locks at all, but that stopped
being a good idea with commit 3c3bb9933, which inserted a syscache lookup
into the function.  A concurrent DROP TABLE could now produce "cache lookup
failed", which we don't want to have happen in normal operation.  The best
solution seems to be to transiently take a lock on the relation named by
the RangeVar (which also makes the result of RangeVarGetRelid a lot less
spongy).  But we shouldn't hold the lock beyond this function, because we
don't want VACUUM to lock more than one table at a time.  (That would not
be a big problem right now, but it will become one after the pending
feature patch to allow multiple tables to be named in VACUUM.)

In passing, adjust vacuum_rel and analyze_rel to document that we don't
trust the passed RangeVar to be accurate, and allow the RangeVar to
possibly be NULL --- which it is anyway for a whole-database VACUUM,
though we accidentally didn't crash for that case.

The passed RangeVar is in fact inaccurate when dealing with a child
partition, as of v10, and it has been wrong for a whole long time in the
case of vacuum_rel() recursing to a TOAST table.  None of these things
present visible bugs up to now, because the passed RangeVar is in fact
only consulted for autovacuum logging, and in that particular context it's
always accurate because autovacuum doesn't let vacuum.c expand partitions
nor recurse to toast tables.  Still, this seems like trouble waiting to
happen, so let's nail the door at least partly shut.  (Further cleanup
is planned, in HEAD only, as part of the pending feature patch.)

Fix some sadly inaccurate/obsolete comments too.  Back-patch to v10.

Michael Paquier and Tom Lane

Discussion: https://postgr.es/m/25023.1506107590@sss.pgh.pa.us

7 years agopgbench: If we fail to send a command to the server, fail.
Robert Haas [Fri, 29 Sep 2017 17:51:14 +0000 (13:51 -0400)]
pgbench: If we fail to send a command to the server, fail.

This beats the old behavior of busy-waiting hands down.

Oversight in commit 12788ae49e1933f463bc59a6efe46c4a01701b76.

Report by Pavan Deolasee. Patch by Fabien Coelho.  Reviewed by
Pavan Deolasee.

Discussion: http://postgr.es/m/CABOikdPhfXTypckMC1Ux6Ko+hKBWwUBA=EXsvamXYSg8M9J94w@mail.gmail.com

7 years agopsql: Update \d sequence display
Peter Eisentraut [Mon, 25 Sep 2017 15:59:46 +0000 (11:59 -0400)]
psql: Update \d sequence display

For \d sequencename, the psql code just did SELECT * FROM sequencename
to get the information to display, but this does not contain much
interesting information anymore in PostgreSQL 10, because the metadata
has been moved to a separate system catalog.

This patch creates a newly designed sequence display that is not merely
an extension of the general relation/table display as it was previously.

Example:

PostgreSQL 9.6:

=> \d foobar
           Sequence "public.foobar"
    Column     |  Type   |        Value
---------------+---------+---------------------
 sequence_name | name    | foobar
 last_value    | bigint  | 1
 start_value   | bigint  | 1
 increment_by  | bigint  | 1
 max_value     | bigint  | 9223372036854775807
 min_value     | bigint  | 1
 cache_value   | bigint  | 1
 log_cnt       | bigint  | 0
 is_cycled     | boolean | f
 is_called     | boolean | f

PostgreSQL 10 before this change:

=> \d foobar
   Sequence "public.foobar"
   Column   |  Type   | Value
------------+---------+-------
 last_value | bigint  | 1
 log_cnt    | bigint  | 0
 is_called  | boolean | f

New:

=> \d foobar
                           Sequence "public.foobar"
  Type  | Start | Minimum |       Maximum       | Increment | Cycles? | Cache
--------+-------+---------+---------------------+-----------+---------+-------
 bigint |     1 |       1 | 9223372036854775807 |         1 | no      |     1

Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
7 years agoFix freezing of a dead HOT-updated tuple
Alvaro Herrera [Thu, 28 Sep 2017 14:44:01 +0000 (16:44 +0200)]
Fix freezing of a dead HOT-updated tuple

Vacuum calls page-level HOT prune to remove dead HOT tuples before doing
liveness checks (HeapTupleSatisfiesVacuum) on the remaining tuples.  But
concurrent transaction commit/abort may turn DEAD some of the HOT tuples
that survived the prune, before HeapTupleSatisfiesVacuum tests them.
This happens to activate the code that decides to freeze the tuple ...
which resuscitates it, duplicating data.

(This is especially bad if there's any unique constraints, because those
are now internally violated due to the duplicate entries, though you
won't know until you try to REINDEX or dump/restore the table.)

One possible fix would be to simply skip doing anything to the tuple,
and hope that the next HOT prune would remove it.  But there is a
problem: if the tuple is older than freeze horizon, this would leave an
unfrozen XID behind, and if no HOT prune happens to clean it up before
the containing pg_clog segment is truncated away, it'd later cause an
error when the XID is looked up.

Fix the problem by having the tuple freezing routines cope with the
situation: don't freeze the tuple (and keep it dead).  In the cases that
the XID is older than the freeze age, set the HEAP_XMAX_COMMITTED flag
so that there is no need to look up the XID in pg_clog later on.

An isolation test is included, authored by Michael Paquier, loosely
based on Daniel Wood's original reproducer.  It only tests one
particular scenario, though, not all the possible ways for this problem
to surface; it be good to have a more reliable way to test this more
fully, but it'd require more work.
In message https://postgr.es/m/20170911140103.5akxptyrwgpc25bw@alvherre.pgsql
I outlined another test case (more closely matching Dan Wood's) that
exposed a few more ways for the problem to occur.

Backpatch all the way back to 9.3, where this problem was introduced by
multixact juggling.  In branches 9.3 and 9.4, this includes a backpatch
of commit e5ff9fefcd50 (of 9.5 era), since the original is not
correctable without matching the coding pattern in 9.5 up.

Reported-by: Daniel Wood
Diagnosed-by: Daniel Wood
Reviewed-by: Yi Wen Wong, Michaël Paquier
Discussion: https://postgr.es/m/E5711E62-8FDF-4DCA-A888-C200BF6B5742@amazon.com

7 years agoFix behavior when converting a float infinity to numeric.
Tom Lane [Wed, 27 Sep 2017 21:05:53 +0000 (17:05 -0400)]
Fix behavior when converting a float infinity to numeric.

float8_numeric() and float4_numeric() failed to consider the possibility
that the input is an IEEE infinity.  The results depended on the
platform-specific behavior of sprintf(): on most platforms you'd get
something like

ERROR:  invalid input syntax for type numeric: "inf"

but at least on Windows it's possible for the conversion to succeed and
deliver a finite value (typically 1), due to a nonstandard output format
from sprintf and lack of syntax error checking in these functions.

Since our numeric type lacks the concept of infinity, a suitable conversion
is impossible; the best thing to do is throw an explicit error before
letting sprintf do its thing.

While at it, let's use snprintf not sprintf.  Overrunning the buffer
should be impossible if sprintf does what it's supposed to, but this
is cheap insurance against a stack smash if it doesn't.

Problem reported by Taiki Kondo.  Patch by me based on fix suggestion
from KaiGai Kohei.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/12A9442FBAE80D4E8953883E0B84E088C8C7A2@BPXM01GP.gisp.nec.co.jp

7 years agoRevert to 9.6 treatment of ALTER TYPE enumtype ADD VALUE.
Tom Lane [Wed, 27 Sep 2017 20:14:37 +0000 (16:14 -0400)]
Revert to 9.6 treatment of ALTER TYPE enumtype ADD VALUE.

This reverts commit 15bc038f9, along with the followon commits 1635e80d3
and 984c92074 that tried to clean up the problems exposed by bug #14825.
The result was incomplete because it failed to address parallel-query
requirements.  With 10.0 release so close upon us, now does not seem like
the time to be adding more code to fix that.  I hope we can un-revert this
code and add the missing parallel query support during the v11 cycle.

Back-patch to v10.

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

7 years agoImprove the CREATE POLICY documentation.
Dean Rasheed [Wed, 27 Sep 2017 16:13:37 +0000 (17:13 +0100)]
Improve the CREATE POLICY documentation.

Provide a correct description of how multiple policies are combined,
clarify when SELECT permissions are required, mention SELECT FOR
UPDATE/SHARE, and do some other more minor tidying up.

Reviewed by Stephen Frost

Discussion: https://postgr.es/m/CAEZATCVrxyYbOFU8XbGHicz%2BmXPYzw%3DhfNL2XTphDt-53TomQQ%40mail.gmail.com

Back-patch to 9.5.

7 years agoDon't recommend "DROP SCHEMA information_schema CASCADE".
Noah Misch [Wed, 27 Sep 2017 05:39:44 +0000 (22:39 -0700)]
Don't recommend "DROP SCHEMA information_schema CASCADE".

It drops objects outside information_schema that depend on objects
inside information_schema.  For example, it will drop a user-defined
view if the view query refers to information_schema.

Discussion: https://postgr.es/m/20170831025345.GE3963697@rfd.leadboat.com

7 years agoImprove wording of error message added in commit 714805010.
Tom Lane [Tue, 26 Sep 2017 19:25:56 +0000 (15:25 -0400)]
Improve wording of error message added in commit 714805010.

Per suggestions from Peter Eisentraut and David Johnston.
Back-patch, like the previous commit.

Discussion: https://postgr.es/m/E1dv9jI-0006oT-Fn@gemulon.postgresql.org

7 years agoFix failure-to-read-man-page in commit 899bd785c.
Tom Lane [Tue, 26 Sep 2017 17:42:53 +0000 (13:42 -0400)]
Fix failure-to-read-man-page in commit 899bd785c.

posix_fallocate() is not quite a drop-in replacement for fallocate(),
because it is defined to return the error code as its function result,
not in "errno".  I (tgl) missed this because RHEL6's version seems
to set errno as well.  That is not the case on more modern Linuxen,
though, as per buildfarm results.

Aside from fixing the return-convention confusion, remove the test
for ENOSYS; we expect that glibc will mask that for posix_fallocate,
though it does not for fallocate.  Keep the test for EINTR, because
POSIX specifies that as a possible result, and buildfarm results
suggest that it can happen in practice.

Back-patch to 9.4, like the previous commit.

Thomas Munro

Discussion: https://postgr.es/m/1002664500.12301802.1471008223422.JavaMail.yahoo@mail.yahoo.com

7 years agoRemove heuristic same-transaction test from check_safe_enum_use().
Tom Lane [Tue, 26 Sep 2017 17:12:13 +0000 (13:12 -0400)]
Remove heuristic same-transaction test from check_safe_enum_use().

The blacklist mechanism added by the preceding commit directly fixes
most of the practical cases that the same-transaction test was meant
to cover.  What remains is use-cases like

begin;
create type e as enum('x');
alter type e add value 'y';
-- use 'y' somehow
commit;

However, because the same-transaction test is heuristic, it fails on
small variants of that, such as renaming the type or changing its
owner.  Rather than try to explain the behavior to users, let's
remove it and just have a rule that the newly added value can't be
used before being committed, full stop.  Perhaps later it will be
worth the implementation effort and overhead to have a more accurate
test for type-was-created-in-this-transaction.  We'll wait for some
field experience with v10 before deciding to do that.

Back-patch to v10.

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

7 years agoUse a blacklist to distinguish original from add-on enum values.
Tom Lane [Tue, 26 Sep 2017 17:12:03 +0000 (13:12 -0400)]
Use a blacklist to distinguish original from add-on enum values.

Commit 15bc038f9 allowed ALTER TYPE ADD VALUE to be executed inside
transaction blocks, by disallowing the use of the added value later
in the same transaction, except under limited circumstances.  However,
the test for "limited circumstances" was heuristic and could reject
references to enum values that were created during CREATE TYPE AS ENUM,
not just later.  This breaks the use-case of restoring pg_dump scripts
in a single transaction, as reported in bug #14825 from Balazs Szilfai.

We can improve this by keeping a "blacklist" table of enum value OIDs
created by ALTER TYPE ADD VALUE during the current transaction.  Any
visible-but-uncommitted value whose OID is not in the blacklist must
have been created by CREATE TYPE AS ENUM, and can be used safely
because it could not have a lifespan shorter than its parent enum type.

This change also removes the restriction that a renamed enum value
can't be used before being committed (unless it was on the blacklist).

Andrew Dunstan, with cosmetic improvements by me.
Back-patch to v10.

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

7 years agoHandle heap rewrites better in logical replication
Peter Eisentraut [Tue, 26 Sep 2017 14:03:56 +0000 (10:03 -0400)]
Handle heap rewrites better in logical replication

A FOR ALL TABLES publication naturally considers all base tables to be a
candidate for replication.  This includes transient heaps that are
created during a table rewrite during DDL.  This causes failures on the
subscriber side because it will not have a table like pg_temp_16386 to
receive data (and if it did, it would be the wrong table).

The prevent this problem, we filter out any tables that match this
naming pattern and match an actual table from FOR ALL TABLES
publications.  This is only a heuristic, meaning that user tables that
match that naming could accidentally be omitted.  A more robust solution
might require an explicit marking of such tables in pg_class somehow.

Reported-by: yxq <yxq@o2.pl>
Bug: #14785
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Petr Jelinek <petr.jelinek@2ndquadrant.com>
7 years agoAvoid SIGBUS on Linux when a DSM memory request overruns tmpfs.
Tom Lane [Mon, 25 Sep 2017 20:09:19 +0000 (16:09 -0400)]
Avoid SIGBUS on Linux when a DSM memory request overruns tmpfs.

On Linux, shared memory segments created with shm_open() are backed by
swap files created in tmpfs.  If the swap file needs to be extended,
but there's no tmpfs space left, you get a very unfriendly SIGBUS trap.
To avoid this, force allocation of the full request size when we create
the segment.  This adds a few cycles, but none that we wouldn't expend
later anyway, assuming the request isn't hugely bigger than the actual
need.

Make this code #ifdef __linux__, because (a) there's not currently a
reason to think the same problem exists on other platforms, and (b)
applying posix_fallocate() to an FD created by shm_open() isn't very
portable anyway.

Back-patch to 9.4 where the DSM code came in.

Thomas Munro, per a bug report from Amul Sul

Discussion: https://postgr.es/m/1002664500.12301802.1471008223422.JavaMail.yahoo@mail.yahoo.com

7 years agoSupport building with Visual Studio 2017
Andrew Dunstan [Mon, 25 Sep 2017 12:03:05 +0000 (08:03 -0400)]
Support building with Visual Studio 2017

Haribabu Kommi, reviewed by Takeshi Ideriha and Christian Ullrich

Backpatch to 9.6

7 years agoAllow ICU to use SortSupport on Windows with UTF-8
Peter Eisentraut [Sun, 24 Sep 2017 04:56:31 +0000 (00:56 -0400)]
Allow ICU to use SortSupport on Windows with UTF-8

There is no reason to ever prevent the use of SortSupport on Windows
when ICU locales are used.  We previously avoided SortSupport on Windows
with UTF-8 server encoding and a non C-locale due to restrictions in
Windows' libc functionality.

This is now considered to be a restriction in one platform's libc
collation provider, and not a more general platform restriction.

Reported-by: Peter Geoghegan <pg@bowt.ie>
7 years agodoc: Expand user documentation on SCRAM
Peter Eisentraut [Sun, 24 Sep 2017 04:29:59 +0000 (00:29 -0400)]
doc: Expand user documentation on SCRAM

Explain more about how the different password authentication methods and
the password_encryption settings relate to each other, give some
upgrading advice, and set a better link from the release notes.

Reviewed-by: Jeff Janes <jeff.janes@gmail.com>
7 years agoFix pg_basebackup test to original intent
Peter Eisentraut [Sun, 24 Sep 2017 02:59:26 +0000 (22:59 -0400)]
Fix pg_basebackup test to original intent

One test case was meant to check that pg_basebackup does not succeed
when a slot is specified with -S but WAL streaming is not selected,
which used to require specifying -X stream.  Since -X stream is the
default in PostgreSQL 10, this test case no longer covers that meaning,
but the pg_basebackup invocation happened to fail anyway for the
unrelated reason that the specified replication slot does not exist.  To
fix, move the test case to later in the file where the slot does exist,
and add -X none to the invocation so that it covers the originally meant
behavior.

extracted from a patch by Michael Banck <michael.banck@credativ.de>

7 years agoFix saving and restoring umask
Peter Eisentraut [Fri, 22 Sep 2017 20:50:59 +0000 (16:50 -0400)]
Fix saving and restoring umask

In two cases, we set a different umask for some piece of code and
restore it afterwards.  But if the contained code errors out, the umask
is not restored.  So add TRY/CATCH blocks to fix that.

7 years agoTest BRIN autosummarization
Alvaro Herrera [Sat, 23 Sep 2017 12:05:57 +0000 (14:05 +0200)]
Test BRIN autosummarization

There was no coverage for this code.

Reported-by: Nikolay Shaplov, Tom Lane
Discussion: https://postgr.es/m/2700647.XEouBYNZic@x200m
https://postgr.es/m/13849.1506114543@sss.pgh.pa.us

7 years agodoc: Document commands that cannot be run in a transaction block
Peter Eisentraut [Fri, 22 Sep 2017 19:01:13 +0000 (15:01 -0400)]
doc: Document commands that cannot be run in a transaction block

Mainly covering the new CREATE SUBSCRIPTION and DROP SUBSCRIPTION, but
ALTER DATABASE SET TABLESPACE was also missing.

7 years agoFor wal_consistency_checking, mask page checksum as well as page LSN.
Robert Haas [Fri, 22 Sep 2017 18:28:22 +0000 (14:28 -0400)]
For wal_consistency_checking, mask page checksum as well as page LSN.

If the LSN is different, the checksum will be different, too.

Ashwin Agrawal, reviewed by Michael Paquier and Kuntal Ghosh

Discussion: http://postgr.es/m/CALfoeis5iqrAU-+JAN+ZzXkpPr7+-0OAGv7QUHwFn=-wDy4o4Q@mail.gmail.com

7 years agoFix build with !USE_WIDE_UPPER_LOWER
Peter Eisentraut [Thu, 21 Sep 2017 18:42:10 +0000 (14:42 -0400)]
Fix build with !USE_WIDE_UPPER_LOWER

The placement of the ifdef blocks in formatting.c was pretty bogus, so
the code failed to compile if USE_WIDE_UPPER_LOWER was not defined.

Reported-by: Peter Geoghegan <pg@bowt.ie>
Reported-by: Noah Misch <noah@leadboat.com>
7 years agoDocument further existing locks as wait events
Alvaro Herrera [Fri, 22 Sep 2017 11:35:54 +0000 (13:35 +0200)]
Document further existing locks as wait events

Reported-by: Jeremy Schneider
Author: Michael Paquier
Discussion: https://postgr.es/m/CA+fnDAZaPCwfY8Lp-pfLnUGFAXRu1VfLyRgdup-L-kwcBj8MqQ@mail.gmail.com

7 years agoSync our copy of the timezone library with IANA tzcode master.
Tom Lane [Fri, 22 Sep 2017 04:04:21 +0000 (00:04 -0400)]
Sync our copy of the timezone library with IANA tzcode 

This patch absorbs a few unreleased fixes in the IANA code.
It corresponds to commit 2d8b944c1cec0808ac4f7a9ee1a463c28f9cd00a
in https://github.com/eggert/tz.  Non-cosmetic changes include:

TZDEFRULESTRING is updated to match current US DST practice,
rather than what it was over ten years ago.  This only matters
for interpretation of POSIX-style zone names (e.g., "EST5EDT"),
and only if the timezone database doesn't include either an exact
match for the zone name or a "posixrules" entry.  The latter
should not be true in any current Postgres installation, but
this could possibly matter when using --with-system-tzdata.

Get rid of a nonportable use of "++var" on a bool var.
This is part of a larger fix that eliminates some vestigial
support for consecutive leap seconds, and adds checks to
the "zic" compiler that the data files do not specify that.

Remove a couple of ancient compatibility hacks.  The IANA
crew think these are obsolete, and I tend to agree.  But
perhaps our buildfarm will think different.

Back-patch to all supported branches, in line with our policy
that all branches should be using current IANA code.  Before v10,
this includes application of current pgindent rules, to avoid
whitespace problems in future back-patches.

Discussion: https://postgr.es/m/E1dsWhf-0000pT-F9@gemulon.postgresql.org

7 years agoGive a better error for duplicate entries in VACUUM/ANALYZE column list.
Tom Lane [Thu, 21 Sep 2017 22:13:11 +0000 (18:13 -0400)]
Give a better error for duplicate entries in VACUUM/ANALYZE column list.

Previously, the code didn't think about this case and would just try to
analyze such a column twice.  That would fail at the point of inserting
the second version of the pg_statistic row, with obscure error messsages
like "duplicate key value violates unique constraint" or "tuple already
updated by self", depending on context and PG version.  We could allow
the case by ignoring duplicate column specifications, but it seems better
to reject it explicitly.

The bogus error messages seem like arguably a bug, so back-patch to
all supported versions.

Nathan Bossart, per a report from Michael Paquier, and whacked
around a bit by me.

Discussion: https://postgr.es/m/E061A8E3-5E3D-494D-94F0-E8A9B312BBFC@amazon.com

7 years agoImprove dubious memory management in pg_newlocale_from_collation().
Tom Lane [Wed, 20 Sep 2017 17:52:36 +0000 (13:52 -0400)]
Improve dubious memory management in pg_newlocale_from_collation().

pg_newlocale_from_collation() used malloc() and strdup() directly,
which is generally not per backend coding style, and it didn't bother
to check for failure results, but would just SIGSEGV instead.  Also,
if one of the numerous error checks in the middle of the function
failed, the already-allocated memory would be leaked permanently.
Admittedly, it's not a lot of memory, but it could build up if this
function were called repeatedly for a bad collation.

The first two problems are easily cured by palloc'ing in TopMemoryContext
instead of calling libc directly.  We can fairly easily dodge the leakage
problem for the struct pg_locale_struct by filling in a temporary variable
and allocating permanent storage only once we reach the bottom of the
function.  It's harder to get rid of the potential leakage for ICU's copy
of the collcollate string, but at least that's only allocated after most
of the error checks; so live with that aspect.

Back-patch to v10 where this code came in, with one or another of the
ICU patches.

7 years agoFix instability in subscription regression test.
Tom Lane [Wed, 20 Sep 2017 15:28:34 +0000 (11:28 -0400)]
Fix instability in subscription regression test.

005_encoding.pl neglected to wait for the subscriber's initial
synchronization to happen.  While we have not seen this fail in
the buildfarm, it's pretty easy to demonstrate there's an issue
by hacking logicalrep_worker_launch() to fail most of the time.

Michael Paquier

Discussion: https://postgr.es/m/27032.1505749806@sss.pgh.pa.us

7 years agoFix erroneous documentation about noise word GROUP.
Tom Lane [Wed, 20 Sep 2017 15:10:42 +0000 (11:10 -0400)]
Fix erroneous documentation about noise word GROUP.

GRANT, REVOKE, and some allied commands allow the noise word GROUP
before a role name (cf. grantee production in gram.y).  This option
does not exist elsewhere, but it had nonetheless snuck into the
documentation for ALTER ROLE, ALTER USER, and CREATE SCHEMA.

Seems to be a copy-and-pasteo in commit 31eae6028, which did expand the
syntax choices here, but not in that way.  Back-patch to 9.5 where that
came in.

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

7 years agodocs: re-add instructions on setting wal_level for rsync use
Bruce Momjian [Wed, 20 Sep 2017 13:36:19 +0000 (09:36 -0400)]
docs:   re-add instructions on setting wal_level for rsync use

This step was erroneously removed four days ago by me.

Reported-by: Magnus via IM
Backpatch-through: 9.5

7 years agoMention need for --no-inc-recursive in rsync command
Magnus Hagander [Wed, 20 Sep 2017 12:09:05 +0000 (14:09 +0200)]
Mention need for --no-inc-recursive in rsync command

Since rsync 3.0.0 (released in 2008), the default way to enumerate
changes was changed in a way that makes it less likely that the hardlink
sync mode works. Since the whole point of the documented procedure is
for the hardlinks to work, change our docs to suggest using the
backwards compatibility switch.

7 years agodoc: add example of % substitution for connection URIs
Bruce Momjian [Tue, 19 Sep 2017 16:23:18 +0000 (12:23 -0400)]
doc:  add example of % substitution for connection URIs

Reported-by: Zhou Digoal
Discussion: https://postgr.es/m/20170912133722.25637.91@wrigleys.postgresql.org

Backpatch-through: 10

7 years agoStamp 10rc1. REL_10_RC1
Tom Lane [Mon, 18 Sep 2017 21:28:38 +0000 (17:28 -0400)]
Stamp 10rc1.

7 years agoFixed ECPG to correctly handle out-of-scope cursor declarations with pointers
Michael Meskes [Mon, 11 Sep 2017 19:10:36 +0000 (21:10 +0200)]
Fixed ECPG to correctly handle out-of-scope cursor declarations with pointers
or array variables.

7 years agoFix, or at least ameliorate, bugs in logicalrep_worker_launch().
Tom Lane [Mon, 18 Sep 2017 15:39:44 +0000 (11:39 -0400)]
Fix, or at least ameliorate, bugs in logicalrep_worker_launch().

If we failed to get a background worker slot, the code just walked
away from the logicalrep-worker slot it already had, leaving that
looking like the worker is still starting up.  This led to an indefinite
hang in subscription startup, as reported by Thomas Munro.  We must
release the slot on failure.

Also fix a thinko: we must capture the worker slot's generation before
releasing LogicalRepWorkerLock the first time, else testing to see if
it's changed is pretty meaningless.

BTW, the CHECK_FOR_INTERRUPTS() in WaitForReplicationWorkerAttach is a
ticking time bomb, even without considering the possibility of elog(ERROR)
in one of the other functions it calls.  Really, this entire business needs
a redesign with some actual thought about error recovery.  But for now
I'm just band-aiding the case observed in testing.

Back-patch to v10 where this code was added.

Discussion: https://postgr.es/m/CAEepm=2bP3TBMFBArP6o20AZaRduWjMnjCjt22hSdnA-EvrtCw@mail.gmail.com

7 years agoUpdate some dead external links in the documentation
Peter Eisentraut [Mon, 18 Sep 2017 15:09:15 +0000 (11:09 -0400)]
Update some dead external links in the documentation

7 years agoRemove dead external links from documentation
Peter Eisentraut [Mon, 18 Sep 2017 14:41:48 +0000 (10:41 -0400)]
Remove dead external links from documentation

7 years agoTranslation updates
Peter Eisentraut [Mon, 18 Sep 2017 13:09:36 +0000 (09:09 -0400)]
Translation updates

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

7 years agoFix DROP SUBSCRIPTION hang
Peter Eisentraut [Mon, 18 Sep 2017 01:37:02 +0000 (21:37 -0400)]
Fix DROP SUBSCRIPTION hang

When ALTER SUBSCRIPTION DISABLE is run in the same transaction before
DROP SUBSCRIPTION, the latter will hang because workers will still be
running, not having seen the DISABLE committed, and DROP SUBSCRIPTION
will wait until the workers have vacated the replication origin slots.

Previously, DROP SUBSCRIPTION killed the logical replication workers
immediately only if it was going to drop the replication slot, otherwise
it scheduled the worker killing for the end of the transaction, as a
result of 7e174fa793a2df89fe03d002a5087ef67abcdde8.  This, however,
causes the present problem.  To fix, kill the workers immediately in all
cases.  This covers all cases: A subscription that doesn't have a
replication slot must be disabled.  It was either disabled in the same
transaction, or it was already disabled before the current transaction,
but then there shouldn't be any workers left and this won't make a
difference.

Reported-by: Arseny Sher <a.sher@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/87mv6av84w.fsf%40ars-thinkpad

7 years agoDoc: update v10 release notes through today.
Tom Lane [Sun, 17 Sep 2017 21:04:21 +0000 (17:04 -0400)]
Doc: update v10 release notes through today.

Add item about number of times statement-level triggers will be fired.
Rearrange the compatibility items into (what seems to me) a less
random ordering.

7 years agoAllow rel_is_distinct_for() to look through RelabelType below OpExpr.
Tom Lane [Sun, 17 Sep 2017 19:28:51 +0000 (15:28 -0400)]
Allow rel_is_distinct_for() to look through RelabelType below OpExpr.

This lets it do the right thing for, eg, varchar columns.
Back-patch to 9.5 where this logic appeared.

David Rowley, per report from Kim Rose Carlsen

Discussion: https://postgr.es/m/VI1PR05MB17091F9A9876528055D6A827C76D0@VI1PR05MB1709.eurprd05.prod.outlook.com

7 years agoFix possible dangling pointer dereference in trigger.c.
Tom Lane [Sun, 17 Sep 2017 18:50:01 +0000 (14:50 -0400)]
Fix possible dangling pointer dereference in trigger.c.

AfterTriggerEndQuery correctly notes that the query_stack could get
repalloc'd during a trigger firing, but it nonetheless passes the address
of a query_stack entry to afterTriggerInvokeEvents, so that if such a
repalloc occurs, afterTriggerInvokeEvents is already working with an
obsolete dangling pointer while it scans the rest of the events.  Oops.
The only code at risk is its "delete_ok" cleanup code, so we can
prevent unsafe behavior by passing delete_ok = false instead of true.

However, that could have a significant performance penalty, because the
point of passing delete_ok = true is to not have to re-scan possibly
a large number of dead trigger events on the next time through the loop.
There's more than one way to skin that cat, though.  What we can do is
delete all the "chunks" in the event list except the last one, since
we know all events in them must be dead.  Deleting the chunks is work
we'd have had to do later in AfterTriggerEndQuery anyway, and it ends
up saving rescanning of just about the same events we'd have gotten
rid of with delete_ok = true.

In v10 and HEAD, we also have to be careful to mop up any per-table
after_trig_events pointers that would become dangling.  This is slightly
annoying, but I don't think that normal use-cases will traverse this code
path often enough for it to be a performance problem.

It's pretty hard to hit this in practice because of the unlikelihood
of the query_stack getting resized at just the wrong time.  Nonetheless,
it's definitely a live bug of ancient standing, so back-patch to all
supported branches.

Discussion: https://postgr.es/m/2891.1505419542@sss.pgh.pa.us

7 years agoEnsure that BEFORE STATEMENT triggers fire the right number of times.
Tom Lane [Sun, 17 Sep 2017 16:16:38 +0000 (12:16 -0400)]
Ensure that BEFORE STATEMENT triggers fire the right number of times.

Commit 0f79440fb introduced mechanism to keep AFTER STATEMENT triggers
from firing more than once per statement, which was formerly possible
if more than one FK enforcement action had to be applied to a given
table.  Add a similar mechanism for BEFORE STATEMENT triggers, so that
we don't have the unexpected situation of firing BEFORE STATEMENT
triggers more often than AFTER STATEMENT.

As with the previous patch, back-patch to v10.

Discussion: https://postgr.es/m/22315.1505584992@sss.pgh.pa.us

7 years agoDoc: add example of transition table use in a trigger.
Tom Lane [Sat, 16 Sep 2017 19:31:26 +0000 (15:31 -0400)]
Doc: add example of transition table use in a trigger.

I noticed that there were exactly no complete examples of use of
a transition table in a trigger function, and no clear description
of just how you'd do it either.  Improve that.

7 years agoFix SQL-spec incompatibilities in new transition table feature.
Tom Lane [Sat, 16 Sep 2017 17:20:32 +0000 (13:20 -0400)]
Fix SQL-spec incompatibilities in new transition table feature.

The standard says that all changes of the same kind (insert, update, or
delete) caused in one table by a single SQL statement should be reported
in a single transition table; and by that, they mean to include foreign key
enforcement actions cascading from the statement's direct effects.  It's
also reasonable to conclude that if the standard had wCTEs, they would say
that effects of wCTEs applying to the same table as each other or the outer
statement should be merged into one transition table.  We weren't doing it
like that.

Hence, arrange to merge tuples from multiple update actions into a single
transition table as much as we can.  There is a problem, which is that if
the firing of FK enforcement triggers and after-row triggers with
transition tables is interspersed, we might need to report more tuples
after some triggers have already seen the transition table.  It seems like
a bad idea for the transition table to be mutable between trigger calls.
There's no good way around this without a major redesign of the FK logic,
so for now, resolve it by opening a new transition table each time this
happens.

Also, ensure that AFTER STATEMENT triggers fire just once per statement,
or once per transition table when we're forced to make more than one.
Previous versions of Postgres have allowed each FK enforcement query
to cause an additional firing of the AFTER STATEMENT triggers for the
referencing table, but that's certainly not per spec.  (We're still
doing multiple firings of BEFORE STATEMENT triggers, though; is that
something worth changing?)

Also, forbid using transition tables with column-specific UPDATE triggers.
The spec requires such transition tables to show only the tuples for which
the UPDATE trigger would have fired, which means maintaining multiple
transition tables or else somehow filtering the contents at readout.
Maybe someday we'll bother to support that option, but it looks like a
lot of trouble for a marginal feature.

The transition tables are now managed by the AfterTriggers data structures,
rather than being directly the responsibility of ModifyTable nodes.  This
removes a subtransaction-lifespan memory leak introduced by my previous
band-aid patch 3c4359521.

In passing, refactor the AfterTriggers data structures to reduce the
management overhead for them, by using arrays of structs rather than
several parallel arrays for per-query-level and per-subtransaction state.

I failed to resist the temptation to do some copy-editing on the SGML
docs about triggers, above and beyond merely documenting the effects
of this patch.

Back-patch to v10, because we don't want the semantics of transition
tables to change post-release.

Patch by me, with help and review from Thomas Munro.

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

7 years agodocs: clarify pg_upgrade docs regarding standbys and rsync
Bruce Momjian [Sat, 16 Sep 2017 15:58:00 +0000 (11:58 -0400)]
docs:  clarify pg_upgrade docs regarding standbys and rsync

Document that rsync is an _optional_ way to upgrade standbys, suggest
rsync option --dry-run, and mention a way of upgrading one standby from
another using rsync.  Also clarify some instructions by specifying if
they operate on the old or new clusters.

Reported-by: Stephen Frost, Magnus Hagander
Discussion: https://postgr.es/m/20170914191250.GB6595@momjian.us

Backpatch-through: 9.5

7 years agoAfter a MINVALUE/MAXVALUE bound, allow only more of the same.
Robert Haas [Sat, 16 Sep 2017 01:15:55 +0000 (21:15 -0400)]
After a MINVALUE/MAXVALUE bound, allow only more of the same.

In the old syntax, which used UNBOUNDED, we had a similar restriction,
but commit d363d42bb9a4399a0207bd3b371c966e22e06bd3, which changed the
syntax, eliminated it.  Put it back.

Patch by me, reviewed by Dean Rasheed.

Discussion: http://postgr.es/m/CA+Tgmobs+pLPC27tS3gOpEAxAffHrq5w509cvkwTf9pF6cWYbg@mail.gmail.com

7 years agoApply pg_get_serial_sequence() to identity column sequences as well
Peter Eisentraut [Fri, 15 Sep 2017 18:04:51 +0000 (14:04 -0400)]
Apply pg_get_serial_sequence() to identity column sequences as well

Bug: #14813

7 years agoAdd missing tags to GetCommandLogLevel.
Robert Haas [Thu, 14 Sep 2017 20:25:19 +0000 (16:25 -0400)]
Add missing tags to GetCommandLogLevel.

Otherwise, log_statement = 'ddl' causes errors if those statement
types are used.

Michael Paquier, reviewed by Ashutosh Sharma

Discussion: http://postgr.es/m/CAB7nPqStC3HkE76Q1MnHsVd1vF1Td9zXApzYadzDMyLMRkkGrw@mail.gmail.com

7 years agoFix inconsistent capitalization.
Robert Haas [Thu, 14 Sep 2017 15:11:12 +0000 (11:11 -0400)]
Fix inconsistent capitalization.

Amit Langote

Discussion: http://postgr.es/m/a83a0899-19f5-594c-9aac-3ba0f16989a1@lab.ntt.co.jp

7 years agoSet partitioned_rels appropriately when UNION ALL is used.
Robert Haas [Thu, 14 Sep 2017 14:43:44 +0000 (10:43 -0400)]
Set partitioned_rels appropriately when UNION ALL is used.

In most cases, this omission won't matter, because the appropriate
locks will have been acquired during parse/plan or by AcquireExecutorLocks.
But it's a bug all the same.

Report by Ashutosh Bapat.  Patch by me, reviewed by Amit Langote.

Discussion: http://postgr.es/m/CAFjFpRdHb_ZnoDTuBXqrudWXh3H1ibLkr6nHsCFT96fSK4DXtA@mail.gmail.com

7 years agoProperly check interrupts in execScan.c.
Andres Freund [Thu, 14 Sep 2017 08:53:10 +0000 (01:53 -0700)]
Properly check interrupts in execScan.c.

During the development of d47cfef711 the CFI()s in ExecScan() were
moved back and forth, ending up in the wrong place. Thus queries that
largely spend their time in ExecScan(), and have neither projection
nor a qual, can't be cancelled in a timely manner.

Reported-By: Jeff Janes
Author: Andres Freund
Discussion: https://postgr.es/m/CAMkU=1weDXp8eLLPt9SO1LEUsJYYK9cScaGhLKpuN+WbYo9b5g@mail.gmail.com
Backpatch: 10, as d47cfef711

7 years agoFix ordering in pg_dump of GRANTs
Stephen Frost [Thu, 14 Sep 2017 00:04:43 +0000 (20:04 -0400)]
Fix ordering in pg_dump of GRANTs

The order in which GRANTs are output is important as GRANTs which have
been GRANT'd by individuals via WITH GRANT OPTION GRANTs have to come
after the GRANT which included the WITH GRANT OPTION.  This happens
naturally in the backend during normal operation as we only change
existing ACLs in-place, only add new ACLs to the end, and when removing
an ACL we remove any which depend on it also.

Also, adjust the comments in acl.h to make this clear.

Unfortunately, the updates to pg_dump to handle initial privileges
involved pulling apart ACLs and then combining them back together and
could end up putting them back together in an invalid order, leading to
dumps which wouldn't restore.

Fix this by adjusting the queries used by pg_dump to ensure that the
ACLs are rebuilt in the same order in which they were originally.

Back-patch to 9.6 where the changes for initial privileges were done.

7 years agoChanged order of statements and added an additiona MSVC safeguard to make ecpg
Michael Meskes [Sat, 26 Aug 2017 17:07:25 +0000 (19:07 +0200)]
Changed order of statements and added an additiona MSVC safeguard to make ecpg
thread test cases work on Windows.

7 years agoMake setlocale in ECPG test cases thread aware on Windows.
Michael Meskes [Sat, 26 Aug 2017 10:57:21 +0000 (12:57 +0200)]
Make setlocale in ECPG test cases thread aware on Windows.

Fix threaded test cases on Windows not to crash in setlocale() which can be
global or local to a thread on Windows.

Author: Christian Ullrich

7 years agodoc: Remove incorrect SCRAM protocol documentation
Peter Eisentraut [Wed, 13 Sep 2017 14:10:34 +0000 (10:10 -0400)]
doc: Remove incorrect SCRAM protocol documentation

The documentation claimed that one should send
"pg_same_as_startup_message" as the user name in the SCRAM messages, but
this did not match the actual implementation, so remove it.

7 years agodocs: adjust "link mode" mention in pg_upgrade streaming steps
Bruce Momjian [Wed, 13 Sep 2017 13:22:18 +0000 (09:22 -0400)]
docs:  adjust "link mode" mention in pg_upgrade streaming steps

Backpatch-through: 9.5

7 years agodocs: improve pg_upgrade standby instructions
Bruce Momjian [Wed, 13 Sep 2017 13:11:28 +0000 (09:11 -0400)]
docs:  improve pg_upgrade standby instructions

This makes it clear that pg_upgrade standby upgrade instructions should
only be used in link mode, adds examples, and explains how rsync works
with links.

Reported-by: Andreas Joseph Krogh
Discussion: https://postgr.es/m/VisenaEmail.6c.c0e592c5af4ef0a2.15e785dcb61@tc7-visena

Backpatch-through: 9.5

7 years agoImprove error message in WAL sender
Peter Eisentraut [Wed, 13 Sep 2017 12:31:03 +0000 (08:31 -0400)]
Improve error message in WAL sender

The previous error message when attempting to run a general SQL command
in a physical replication WAL sender was a bit sloppy.

Reported-by: Fujii Masao <masao.fujii@gmail.com>
7 years agodocs: improve pg_upgrade rsync instructions
Bruce Momjian [Tue, 12 Sep 2017 17:17:52 +0000 (13:17 -0400)]
docs:  improve pg_upgrade rsync instructions

This explains how rsync accomplishes updating standby servers and
clarifies the instructions.

Reported-by: Andreas Joseph Krogh
Discussion: https://postgr.es/m/VisenaEmail.10.2b4049e43870bd16.15d898d696f@tc7-visena

Backpatch-through: 9.5

7 years agoFix RecursiveCopy.pm to cope with disappearing files.
Tom Lane [Tue, 12 Sep 2017 02:02:58 +0000 (22:02 -0400)]
Fix RecursiveCopy.pm to cope with disappearing files.

When copying from an active database tree, it's possible for files to be
deleted after we see them in a readdir() scan but before we can open them.
(Once we've got a file open, we don't expect any further errors from it
getting unlinked, though.)  Tweak RecursiveCopy so it can cope with this
case, so as to avoid irreproducible test failures.

Back-patch to 9.6 where this code was added.  In v10 and HEAD, also
remove unused "use RecursiveCopy" in one recovery test script.

Michael Paquier and Tom Lane

Discussion: https://postgr.es/m/24621.1504924323@sss.pgh.pa.us

7 years agoPG 10 release notes: change trigger transition tables
Bruce Momjian [Mon, 11 Sep 2017 23:56:44 +0000 (19:56 -0400)]
PG 10 release notes:  change trigger transition tables

Add attribution of trigger transition tables for Thomas Munro.

Reported-by: Thomas Munro
Discussion: https://postgr.es/m/CAEepm=2bDFgr4ut+1-QjKQY4MA=5ek8Ap3nyB19y2tpTL6xxtA@mail.gmail.com

Backpatch-through: 10

7 years agoPG 10 release notes: update PL/Tcl functions item
Bruce Momjian [Mon, 11 Sep 2017 23:43:49 +0000 (19:43 -0400)]
PG 10 release notes:  update PL/Tcl functions item

Update attribution of PL/Tcl functions item from Jim Nasby to Karl
Lehenbauer.

Reported-by: Jim Nasby
Discussion: https://postgr.es/m/ed42f3d6-4251-dabc-747f-1ff936763b2b@nasby.net

Backpatch-through: 10

7 years agoTranslation updates
Peter Eisentraut [Mon, 11 Sep 2017 16:49:35 +0000 (12:49 -0400)]
Translation updates

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 0c1fdae472e52197eb0e5ccdd2cfdd3654f76834

7 years agoMessage style fixes
Peter Eisentraut [Mon, 11 Sep 2017 15:20:47 +0000 (11:20 -0400)]
Message style fixes

7 years agoQuick-hack fix for foreign key cascade vs triggers with transition tables.
Tom Lane [Sun, 10 Sep 2017 18:59:56 +0000 (14:59 -0400)]
Quick-hack fix for foreign key cascade vs triggers with transition tables.

AFTER triggers using transition tables crashed if they were fired due
to a foreign key ON CASCADE update.  This is because ExecEndModifyTable
flushes the transition tables, on the assumption that any trigger that
could need them was already fired during ExecutorFinish.  Normally
that's true, because we don't allow transition-table-using triggers
to be deferred.  However, foreign key CASCADE updates force any
triggers on the referencing table to be deferred to the outer query
level, by means of the EXEC_FLAG_SKIP_TRIGGERS flag.  I don't recall
all the details of why it's like that and am pretty loath to redesign
it right now.  Instead, just teach ExecEndModifyTable to skip destroying
the TransitionCaptureState when that flag is set.  This will allow the
transition table data to survive until end of the current subtransaction.

This isn't a terribly satisfactory solution, because (1) we might be
leaking the transition tables for much longer than really necessary,
and (2) as things stand, an AFTER STATEMENT trigger will fire once per
RI updating query, ie once per row updated or deleted in the referenced
table.  I suspect that is not per SQL spec.  But redesigning this is a
research project that we're certainly not going to get done for v10.
So let's go with this hackish answer for now.

In passing, tweak AfterTriggerSaveEvent to not save the transition_capture
pointer into the event record for a deferrable trigger.  This is not
necessary to fix the current bug, but it avoids letting dangling pointers
to long-gone transition tables persist in the trigger event queue.  That's
at least a safety feature.  It might also allow merging shared trigger
states in more cases than before.

I added a regression test that demonstrates the crash on unpatched code,
and also exposes the behavior of firing the AFTER STATEMENT triggers
once per row update.

Per bug #14808 from Philippe Beaudoin.  Back-patch to v10.

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

7 years agopg_upgrade: Message style fixes
Peter Eisentraut [Sat, 9 Sep 2017 21:32:10 +0000 (17:32 -0400)]
pg_upgrade: Message style fixes

7 years agoFix uninitialized-variable bug.
Tom Lane [Fri, 8 Sep 2017 23:04:32 +0000 (19:04 -0400)]
Fix uninitialized-variable bug.

map_partition_varattnos() failed to set its found_whole_row output
parameter if the given expression list was NIL.  This seems to be
a pre-existing bug that chanced to be exposed by commit 6f6b99d13.
It might be unreachable in v10, but I have little faith in that
proposition, so back-patch.

Per buildfarm.