Tatsuo Ishii [Tue, 5 Jan 2016 23:01:40 +0000 (08:01 +0900)]
Fix typo in create_transform.sgml.
Tom Lane [Tue, 5 Jan 2016 20:47:05 +0000 (15:47 -0500)]
Sort $(wildcard) output where needed for reproducible build output.
The order of inclusion of .o files makes a difference in linker output;
not a functional difference, but still a bitwise difference, which annoys
some packagers who would like reproducible builds.
Report and patch by Christoph Berg
Alvaro Herrera [Tue, 5 Jan 2016 20:25:12 +0000 (17:25 -0300)]
Make pg_receivexlog silent with 9.3 and older servers
A pointless and confusing error message is shown to the user when
attempting to identify a 9.3 or older remote server with a 9.5/9.6
pg_receivexlog, because the return signature of IDENTIFY_SYSTEM was
changed in 9.4. There's no good reason for the warning message, so
shuffle code around to keep it quiet.
(pg_recvlogical is also affected by this commit, but since it obviously
cannot work with 9.3 that doesn't actually matter much.)
Backpatch to 9.5.
Reported by Marco Nenciarini, who also wrote the initial patch. Further
tweaked by Robert Haas and Fujii Masao; reviewed by Michael Paquier and
Craig Ringer.
Tom Lane [Tue, 5 Jan 2016 17:35:18 +0000 (12:35 -0500)]
Add to_regnamespace() and to_regrole() to the documentation.
Commits
cb9fa802b32b222b and
0c90f6769de6a60f added these functions,
but did not bother with documentation.
Tom Lane [Mon, 4 Jan 2016 21:29:34 +0000 (16:29 -0500)]
Stamp 9.5.0.
Tom Lane [Mon, 4 Jan 2016 20:11:44 +0000 (15:11 -0500)]
Docs: provide a concrete discussion and example for RLS race conditions.
Commit
43cd468cf01007f3 added some wording to create_policy.sgml purporting
to warn users against a race condition of the sort that had been noted some
time ago by Peter Geoghegan. However, that warning was far too vague to be
useful (or at least, I completely failed to grasp what it was on about).
Since the problem case occurs with a security design pattern that lots of
people are likely to try to use, we need to be as clear as possible about
it. Provide a concrete example in the main-line docs in place of the
original warning.
Tom Lane [Mon, 4 Jan 2016 17:21:31 +0000 (12:21 -0500)]
Adjust behavior of row_security GUC to match the docs.
Some time back we agreed that row_security=off should not be a way to
bypass RLS entirely, but only a way to get an error if it was being
applied. However, the code failed to act that way for table owners.
Per discussion, this is a must-fix bug for 9.5.0.
Adjust the logic in rls.c to behave as expected; also, modify the
error message to be more consistent with the new interpretation.
The regression tests need minor corrections as well. Also update
the comments about row_security in ddl.sgml to be correct. (The
official description of the GUC in config.sgml is already correct.)
I failed to resist the temptation to do some other very minor
cleanup as well, such as getting rid of a duplicate extern declaration.
Robert Haas [Mon, 4 Jan 2016 15:12:37 +0000 (10:12 -0500)]
Fix typo in comment.
Masahiko Sawada
Peter Eisentraut [Mon, 4 Jan 2016 13:18:48 +0000 (08:18 -0500)]
Translation updates
Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
3b0ccc27cf917446ea0a6c680b70534cfcaba81e
Tom Lane [Mon, 4 Jan 2016 06:53:24 +0000 (01:53 -0500)]
Fix regrole and regnamespace output functions to do quoting, too.
We discussed this but somehow failed to implement it...
Tom Lane [Mon, 4 Jan 2016 06:03:53 +0000 (01:03 -0500)]
Fix regrole and regnamespace types to honor quoting like other reg* types.
Aside from any consistency arguments, this is logically necessary because
the I/O functions for these types also handle numeric OID values. Without
a quoting rule it is impossible to distinguish numeric OIDs from role or
namespace names that happen to contain only digits.
Also change the to_regrole and to_regnamespace functions to dequote their
arguments. While not logically essential, this seems like a good idea
since the other to_reg* functions do it. Anyone who really wants raw
lookup of an uninterpreted name can fall back on the time-honored solution
of (SELECT oid FROM pg_namespace WHERE nspname = whatever).
Report and patch by Jim Nasby, reviewed by Michael Paquier
Tom Lane [Mon, 4 Jan 2016 01:53:35 +0000 (20:53 -0500)]
Fix bogus lock release in RemovePolicyById and RemoveRoleFromObjectPolicy.
Can't release the AccessExclusiveLock on the target table until commit.
Otherwise there is a race condition whereby other backends might service
our cache invalidation signals before they can actually see the updated
catalog rows.
Just to add insult to injury, RemovePolicyById was closing the rel (with
incorrect lock drop) and then passing the now-dangling rel pointer to
CacheInvalidateRelcache. Probably the reason this doesn't fall over on
CLOBBER_CACHE buildfarm members is that some outer level of the DROP logic
is still holding the rel open ... but it'd have bit us on the arse
eventually, no doubt.
Tom Lane [Mon, 4 Jan 2016 01:04:11 +0000 (20:04 -0500)]
Do some copy-editing on the docs for row-level security.
Clarifications, markup improvements, corrections of misleading or
outright wrong statements.
Tom Lane [Sun, 3 Jan 2016 21:26:38 +0000 (16:26 -0500)]
Guard against null arguments in binary_upgrade_create_empty_extension().
The CHECK_IS_BINARY_UPGRADE macro is not sufficient security protection
if we're going to dereference pass-by-reference arguments before it.
But in any case we really need to explicitly check PG_ARGISNULL for all
the arguments of a non-strict function, not only the ones we expect null
values for.
Oversight in commits
30982be4e5019684e1772dd9170aaa53f5a8e894 and
f92fc4c95ddcc25978354a8248d3df22269201bc. Found by Andreas Seltenreich.
(The other usages in pg_upgrade_support.c seem safe.)
Tom Lane [Sun, 3 Jan 2016 21:03:42 +0000 (16:03 -0500)]
Do some copy-editing on the docs for replication origins.
Minor grammar and markup improvements.
Tom Lane [Sun, 3 Jan 2016 20:33:12 +0000 (15:33 -0500)]
Do a final round of copy-editing on the 9.5 release notes.
Tom Lane [Sun, 3 Jan 2016 18:56:29 +0000 (13:56 -0500)]
Fix treatment of *lpNumberOfBytesRecvd == 0: that's a completion condition.
pgwin32_recv() has treated a non-error return of zero bytes from WSARecv()
as being a reason to block ever since the current implementation was
introduced in commit
a4c40f140d23cefb. However, so far as one can tell
from Microsoft's documentation, that is just wrong: what it means is
graceful connection closure (in stream protocols) or receipt of a
zero-length message (in message protocols), and neither case should result
in blocking here. The only reason the code worked at all was that control
then fell into the retry loop, which did *not* treat zero bytes specially,
so we'd get out after only wasting some cycles. But as of 9.5 we do not
normally reach the retry loop and so the bug is exposed, as reported by
Shay Rojansky and diagnosed by Andres Freund.
Remove the unnecessary test on the byte count, and rearrange the code
in the retry loop so that it looks identical to the initial sequence.
Back-patch to 9.5. The code is wrong all the way back, AFAICS, but
since it's relatively harmless in earlier branches we'll leave it alone.
Tom Lane [Sun, 3 Jan 2016 00:04:45 +0000 (19:04 -0500)]
Teach pg_dump to quote reloption values safely.
Commit
c7e27becd2e6eb93 fixed this on the backend side, but we neglected
the fact that several code paths in pg_dump were printing reloptions
values that had not gotten massaged by ruleutils. Apply essentially the
same quoting logic in those places, too.
Tom Lane [Sat, 2 Jan 2016 21:24:50 +0000 (16:24 -0500)]
Fix overly-strict assertions in spgtextproc.c.
spg_text_inner_consistent is capable of reconstructing an empty string
to pass down to the next index level; this happens if we have an empty
string coming in, no prefix, and a dummy node label. (In practice, what
is needed to trigger that is insertion of a whole bunch of empty-string
values.) Then, we will arrive at the next level with in->level == 0
and a non-NULL (but zero length) in->reconstructedValue, which is valid
but the Assert tests weren't expecting it.
Per report from Andreas Seltenreich. This has no impact in non-Assert
builds, so should not be a problem in production, but back-patch to
all affected branches anyway.
In passing, remove a couple of useless variable initializations and
shorten the code by not duplicating DatumGetPointer() calls.
Tom Lane [Sat, 2 Jan 2016 20:29:03 +0000 (15:29 -0500)]
Adjust back-branch release note description of commits
a2a718b22 et al.
As pointed out by Michael Paquier, recovery_min_apply_delay didn't exist
in 9.0-9.3, making the release note text not very useful. Instead make it
talk about recovery_target_xid, which did exist then.
9.0 is already out of support, but we can fix the text in the newer
branches' copies of its release notes.
Bruce Momjian [Sat, 2 Jan 2016 18:33:39 +0000 (13:33 -0500)]
Update copyright for 2016
Backpatch certain files through 9.1
Tom Lane [Fri, 1 Jan 2016 20:27:53 +0000 (15:27 -0500)]
Teach flatten_reloptions() to quote option values safely.
flatten_reloptions() supposed that it didn't really need to do anything
beyond inserting commas between reloption array elements. However, in
principle the value of a reloption could be nearly anything, since the
grammar allows a quoted string there. Any restrictions on it would come
from validity checking appropriate to the particular option, if any.
A reloption value that isn't a simple identifier or number could thus lead
to dump/reload failures due to syntax errors in CREATE statements issued
by pg_dump. We've gotten away with not worrying about this so far with
the core-supported reloptions, but extensions might allow reloption values
that cause trouble, as in bug #13840 from Kouhei Sutou.
To fix, split the reloption array elements explicitly, and then convert
any value that doesn't look like a safe identifier to a string literal.
(The details of the quoting rule could be debated, but this way is safe
and requires little code.) While we're at it, also quote reloption names
if they're not safe identifiers; that may not be a likely problem in the
field, but we might as well try to be bulletproof here.
It's been like this for a long time, so back-patch to all supported
branches.
Kouhei Sutou, adjusted some by me
Tom Lane [Fri, 1 Jan 2016 18:42:21 +0000 (13:42 -0500)]
Add some more defenses against silly estimates to gincostestimate().
A report from Andy Colson showed that gincostestimate() was not being
nearly paranoid enough about whether to believe the statistics it finds in
the index metapage. The problem is that the metapage stats (other than the
pending-pages count) are only updated by VACUUM, and in the worst case
could still reflect the index's original empty state even when it has grown
to many entries. We attempted to deal with that by scaling up the stats to
match the current index size, but if nEntries is zero then scaling it up
still gives zero. Moreover, the proportion of pages that are entry pages
vs. data pages vs. pending pages is unlikely to be estimated very well by
scaling if the index is now orders of magnitude larger than before.
We can improve matters by expanding the use of the rule-of-thumb estimates
I introduced in commit
7fb008c5ee59b040: if the index has grown by more
than a cutoff amount (here set at 4X growth) since VACUUM, then use the
rule-of-thumb numbers instead of scaling. This might not be exactly right
but it seems much less likely to produce insane estimates.
I also improved both the scaling estimate and the rule-of-thumb estimate
to account for numPendingPages, since it's reasonable to expect that that
is accurate in any case, and certainly pages that are in the pending list
are not either entry or data pages.
As a somewhat separate issue, adjust the estimation equations that are
concerned with extra fetches for partial-match searches. These equations
suppose that a fraction partialEntries / numEntries of the entry and data
pages will be visited as a consequence of a partial-match search. Now,
it's physically impossible for that fraction to exceed one, but our
estimate of partialEntries is mostly bunk, and our estimate of numEntries
isn't exactly gospel either, so we could arrive at a silly value. In the
example presented by Andy we were coming out with a value of 100, leading
to insane cost estimates. Clamp the fraction to one to avoid that.
Like the previous patch, back-patch to all supported branches; this
problem can be demonstrated in one form or another in all of them.
Tom Lane [Fri, 1 Jan 2016 18:00:13 +0000 (13:00 -0500)]
Split out pg_operator.h function declarations to new file pg_operator_fn.h.
Commit
a2e35b53c39b2a27 added an #include of catalog/objectaddress.h to
pg_operator.h, making it impossible for client-side code to #include
pg_operator.h. It's not entirely clear whether any client-side code needs
to include pg_operator.h, but it seems prudent to assume that there is some
such code somewhere. Therefore, split off the function definitions into a
new file pg_operator_fn.h, similarly to what we've done for some other
catalog header files.
Back-patch of part of commit
0dab5ef39b3d9d86.
Tom Lane [Thu, 31 Dec 2015 22:59:10 +0000 (17:59 -0500)]
Add a comment noting that FDWs don't have to implement EXCEPT or LIMIT TO.
postgresImportForeignSchema pays attention to IMPORT's EXCEPT and LIMIT TO
options, but only as an efficiency hack, not for correctness' sake. The
FDW documentation does explain that, but someone using postgres_fdw.c
as a coding guide might not remember it, so let's add a comment here.
Per question from Regina Obe.
Tom Lane [Tue, 29 Dec 2015 21:45:47 +0000 (16:45 -0500)]
Put back one copyObject() in rewriteTargetView().
Commit
6f8cb1e23485bd6d tried to centralize rewriteTargetView's copying
of a target view's Query struct. However, it ignored the fact that the
jointree->quals field was used twice. This only accidentally failed to
fail immediately because the same ChangeVarNodes mutation is applied in
both cases, so that we end up with logically identical expression trees
for both uses (and, as the code stands, the second ChangeVarNodes call
actually does nothing). However, we end up linking *physically*
identical expression trees into both an RTE's securityQuals list and
the WithCheckOption list. That's pretty dangerous, mainly because
prepsecurity.c is utterly cavalier about further munging such structures
without copying them first.
There may be no live bug in HEAD as a consequence of the fact that we apply
preprocess_expression in between here and prepsecurity.c, and that will
make a copy of the tree anyway. Or it may just be that the regression
tests happen to not trip over it. (I noticed this only because things
fell over pretty badly when I tried to relocate the planner's call of
expand_security_quals to before expression preprocessing.) In any case
it's very fragile because if anyone tried to make the securityQuals and
WithCheckOption trees diverge before we reach preprocess_expression, it
would not work. The fact that the current code will preprocess
securityQuals and WithCheckOptions lists at completely different times in
different query levels does nothing to increase my trust that that can't
happen.
In view of the fact that 9.5.0 is almost upon us and the aforesaid commit
has seen exactly zero field testing, the prudent course is to make an extra
copy of the quals so that the behavior is not different from what has been
in the field during beta.
Joe Conway [Mon, 28 Dec 2015 20:35:16 +0000 (12:35 -0800)]
Rename (new|old)estCommitTs to (new|old)estCommitTsXid
The variables newestCommitTs and oldestCommitTs sound as if they are
timestamps, but in fact they are the transaction Ids that correspond
to the newest and oldest timestamps rather than the actual timestamps.
Rename these variables to reflect that they are actually xids: to wit
newestCommitTsXid and oldestCommitTsXid respectively. Also modify
related code in a similar fashion, particularly the user facing output
emitted by pg_controldata and pg_resetxlog.
Complaint and patch by me, review by Tom Lane and Alvaro Herrera.
Backpatch to 9.5 where these variables were first introduced.
Alvaro Herrera [Mon, 28 Dec 2015 18:28:19 +0000 (15:28 -0300)]
Document brin_summarize_new_pages
Pointer out by Jeff Janes
Tom Lane [Mon, 28 Dec 2015 17:09:00 +0000 (12:09 -0500)]
Document the exponentiation operator as associating left to right.
Common mathematical convention is that exponentiation associates right to
left. We aren't going to change the parser for this, but we could note
it in the operator's description. (It's already noted in the operator
precedence/associativity table, but users might not look there.)
Per bug #13829 from Henrik Pauli.
Alvaro Herrera [Mon, 28 Dec 2015 16:45:03 +0000 (13:45 -0300)]
doc: pg_committs -> pg_commit_ts
Reported by: Alain Laporte (#13836)
Tom Lane [Mon, 28 Dec 2015 16:04:42 +0000 (11:04 -0500)]
Update documentation about pseudo-types.
Tone down an overly strong statement about which pseudo-types PLs are
likely to allow. Add "event_trigger" to the list, as well as
"pg_ddl_command" in 9.5/HEAD. Back-patch to 9.3 where event_trigger
was added.
Alvaro Herrera [Mon, 28 Dec 2015 13:50:35 +0000 (10:50 -0300)]
Fix translation domain in pg_basebackup
For some reason, we've been overlooking the fact that pg_receivexlog
and pg_recvlogical are using wrong translation domains all along,
so their output hasn't ever been translated. The right domain is
pg_basebackup, not their own executable names.
Noticed by Ioseph Kim, who's been working on the Korean translation.
Backpatch pg_receivexlog to 9.2 and pg_recvlogical to 9.4.
Alvaro Herrera [Sun, 27 Dec 2015 16:03:19 +0000 (13:03 -0300)]
Add forgotten CHECK_FOR_INTERRUPT calls in pgcrypto's crypt()
Both Blowfish and DES implementations of crypt() can take arbitrarily
long time, depending on the number of rounds specified by the caller;
make sure they can be interrupted.
Author: Andreas Karlsson
Reviewer: Jeff Janes
Backpatch to 9.1.
Tom Lane [Sat, 26 Dec 2015 17:56:09 +0000 (12:56 -0500)]
Fix brin_summarize_new_values() to check index type and ownership.
brin_summarize_new_values() did not check that the passed OID was for
an index at all, much less that it was a BRIN index, and would fail in
obscure ways if it wasn't (possibly damaging data first?). It also
lacked any permissions test; by analogy to VACUUM, we should only allow
the table's owner to summarize.
Noted by Jeff Janes, fix by Michael Paquier and me
Tom Lane [Thu, 24 Dec 2015 16:38:31 +0000 (11:38 -0500)]
Remove unnecessary row ordering dependency in pg_rewind test suite.
t/002_databases.pl was expecting to see a specific physical order of the
rows in pg_database. I broke that in HEAD with commit
01e386a325549b77,
but I'd say it's a pretty fragile test methodology in any case, so fix
it in 9.5 as well.
Tom Lane [Thu, 24 Dec 2015 15:50:03 +0000 (10:50 -0500)]
Docs: fix erroneously-given function name.
pg_replication_session_is_setup() exists nowhere; apparently this is
meant to refer to pg_replication_origin_session_is_setup().
Adrien Nayrat
Tom Lane [Thu, 24 Dec 2015 15:42:58 +0000 (10:42 -0500)]
Fix factual and grammatical errors in comments for struct _tableInfo.
Amit Langote, further adjusted by me
Tom Lane [Wed, 23 Dec 2015 20:45:43 +0000 (15:45 -0500)]
Improve handling of password reuse in src/bin/scripts programs.
This reverts most of commit
83dec5a71 in favor of having connectDatabase()
store the possibly-reusable password in a static variable, similar to the
coding we've had for a long time in pg_dump's version of that function.
To avoid possible problems with unwanted password reuse, make callers
specify whether it's reasonable to attempt to re-use the password.
This is a wash for cases where re-use isn't needed, but it is far simpler
for callers that do want that. Functionally there should be no difference.
Even though we're past RC1, it seems like a good idea to back-patch this
into 9.5, like the prior commit. Otherwise, if there are any third-party
users of connectDatabase(), they'll have to deal with an API change in
9.5 and then another one in 9.6.
Michael Paquier
Tom Lane [Wed, 23 Dec 2015 19:25:31 +0000 (14:25 -0500)]
In pg_dump, remember connection passwords no matter how we got them.
When pg_dump prompts the user for a password, it remembers the password
for possible re-use by parallel worker processes. However, libpq might
have extracted the password from a connection string originally passed
as "dbname". Since we don't record the original form of dbname but
break it down to host/port/etc, the password gets lost. Fix that by
retrieving the actual password from the PGconn.
(It strikes me that this whole approach is rather broken, as it will also
lose other information such as options that might have been present in
the connection string. But we'll leave that problem for another day.)
In passing, get rid of rather silly use of malloc() for small fixed-size
arrays.
Back-patch to 9.3 where parallel pg_dump was introduced.
Report and fix by Zeus Kronion, adjusted a bit by Michael Paquier and me
Robert Haas [Tue, 22 Dec 2015 18:57:18 +0000 (13:57 -0500)]
Comment improvements for abbreviated keys.
Peter Geoghegan and Robert Haas
Stephen Frost [Mon, 21 Dec 2015 15:34:20 +0000 (10:34 -0500)]
Make viewquery a copy in rewriteTargetView()
Rather than expect the Query returned by get_view_query() to be
read-only and then copy bits and pieces of it out, simply copy the
entire structure when we get it. This addresses an issue where
AcquireRewriteLocks, which is called by acquireLocksOnSubLinks(),
scribbles on the parsetree passed in, which was actually an entry
in relcache, leading to segfaults with certain view definitions.
This also future-proofs us a bit for anyone adding more code to this
path.
The acquireLocksOnSubLinks() was added in commit
c3e0ddd40.
Back-patch to 9.3 as that commit was.
Tom Lane [Sun, 20 Dec 2015 23:29:51 +0000 (18:29 -0500)]
Remove silly completion for "DELETE FROM tabname ...".
psql offered USING, WHERE, and SET in this context, but SET is not a valid
possibility here. Seems to have been a thinko in commit
f5ab0a14ea83eb6c
which added DELETE's USING option.
Peter Eisentraut [Sun, 20 Dec 2015 16:50:04 +0000 (11:50 -0500)]
psql: Review of new help output strings
Tom Lane [Sat, 19 Dec 2015 21:55:14 +0000 (16:55 -0500)]
Add missing COSTS OFF to EXPLAIN commands in rowsecurity.sql.
Commit
e5e11c8cc added a bunch of EXPLAIN statements without COSTS OFF
to the regression tests. This is contrary to project policy since it
results in unnecessary platform dependencies in the output (it's just
luck that we didn't get buildfarm failures from it). Per gripe from
Mike Wilson.
Andres Freund [Sat, 19 Dec 2015 16:37:11 +0000 (17:37 +0100)]
Fix tab completion for ALTER ... TABLESPACE ... OWNED BY.
Previously the completion used the wrong word to match 'BY'. This was
introduced brokenly, in b2de2a. While at it, also add completion of
IN TABLESPACE ... OWNED BY and fix comments referencing nonexistent
syntax.
Reported-By: Michael Paquier
Author: Michael Paquier and Andres Freund
Discussion: CAB7nPqSHDdSwsJqX0d2XzjqOHr==HdWiubCi4L=Zs7YFTUne8w@mail.gmail.com
Backpatch: 9.4, like the commit introducing the bug
Robert Haas [Fri, 18 Dec 2015 18:24:51 +0000 (13:24 -0500)]
pgbench: Change terminology from "threshold" to "parameter".
Per a recommendation from Tomas Vondra, it's more helpful to refer to
the value that determines how skewed a Gaussian or exponential
distribution is as a parameter rather than a threshold.
Since it's not quite too late to get this right in 9.5, where it was
introduced, back-patch this. Most of the patch changes only comments
and documentation, but a few pgbench messages are altered to match.
Fabien Coelho, reviewed by Michael Paquier and by me.
Robert Haas [Fri, 18 Dec 2015 17:17:35 +0000 (12:17 -0500)]
Fix copy-and-paste error in logical decoding callback.
This could result in the error context misidentifying where the error
actually occurred.
Craig Ringer
Tom Lane [Fri, 18 Dec 2015 01:21:42 +0000 (20:21 -0500)]
Remove unreferenced function declarations.
datapagemap_create() and datapagemap_destroy() were declared extern,
but they don't actually exist anywhere. Per YUriy Zhuravlev and
Michael Paquier.
Tom Lane [Thu, 17 Dec 2015 21:55:23 +0000 (16:55 -0500)]
Fix improper initialization order for readline.
Turns out we must set rl_basic_word_break_characters *before* we call
rl_initialize() the first time, because it will quietly copy that value
elsewhere --- but only on the first call. (Love these undocumented
dependencies.) I broke this yesterday in commit
2ec477dc8108339d;
like that commit, back-patch to all active branches. Per report from
Pavel Stehule.
Alvaro Herrera [Thu, 17 Dec 2015 17:25:41 +0000 (14:25 -0300)]
Rework internals of changing a type's ownership
This is necessary so that REASSIGN OWNED does the right thing with
composite types, to wit, that it also alters ownership of the type's
pg_class entry -- previously, the pg_class entry remained owned by the
original user, which caused later other failures such as the new owner's
inability to use ALTER TYPE to rename an attribute of the affected
composite. Also, if the original owner is later dropped, the pg_class
entry becomes owned by a non-existant user which is bogus.
To fix, create a new routine AlterTypeOwner_oid which knows whether to
pass the request to ATExecChangeOwner or deal with it directly, and use
that in shdepReassignOwner rather than calling AlterTypeOwnerInternal
directly. AlterTypeOwnerInternal is now simpler in that it only
modifies the pg_type entry and recurses to handle a possible array type;
higher-level tasks are handled by either AlterTypeOwner directly or
AlterTypeOwner_oid.
I took the opportunity to add a few more objects to the test rig for
REASSIGN OWNED, so that more cases are exercised. Additional ones could
be added for superuser-only-ownable objects (such as FDWs and event
triggers) but I didn't want to push my luck by adding a new superuser to
the tests on a backpatchable bug fix.
Per bug #13666 reported by Chris Pacejo.
Backpatch to 9.5.
(I would back-patch this all the way back, except that it doesn't apply
cleanly in 9.4 and earlier because
59367fdf9 wasn't backpatched. If we
decide that we need this in earlier branches too, we should backpatch
both.)
Tom Lane [Wed, 16 Dec 2015 21:58:55 +0000 (16:58 -0500)]
Cope with Readline's failure to track SIGWINCH events outside of input.
It emerges that libreadline doesn't notice terminal window size change
events unless they occur while collecting input. This is easy to stumble
over if you resize the window while using a pager to look at query output,
but it can be demonstrated without any pager involvement. The symptom is
that queries exceeding one line are misdisplayed during subsequent input
cycles, because libreadline has the wrong idea of the screen dimensions.
The safest, simplest way to fix this is to call rl_reset_screen_size()
just before calling readline(). That causes an extra ioctl(TIOCGWINSZ)
for every command; but since it only happens when reading from a tty, the
performance impact should be negligible. A more valid objection is that
this still leaves a tiny window during entry to readline() wherein delivery
of SIGWINCH will be missed; but the practical consequences of that are
probably negligible. In any case, there doesn't seem to be any good way to
avoid the race, since readline exposes no functions that seem safe to call
from a generic signal handler --- rl_reset_screen_size() certainly isn't.
It turns out that we also need an explicit rl_initialize() call, else
rl_reset_screen_size() dumps core when called before the first readline()
call.
rl_reset_screen_size() is not present in old versions of libreadline,
so we need a configure test for that. (rl_initialize() is present at
least back to readline 4.0, so we won't bother with a test for it.)
We would need a configure test anyway since libedit's emulation of
libreadline doesn't currently include such a function. Fortunately,
libedit seems not to have any corresponding bug.
Merlin Moncure, adjusted a bit by me
Tom Lane [Tue, 15 Dec 2015 22:25:44 +0000 (17:25 -0500)]
Stamp 9.5rc1.
Tom Lane [Tue, 15 Dec 2015 21:57:23 +0000 (16:57 -0500)]
Document use of Subject Alternative Names in SSL server certificates.
Commit
acd08d764 did not bother with updating the documentation.
Tom Lane [Tue, 15 Dec 2015 21:42:18 +0000 (16:42 -0500)]
Update 9.5 release notes through today.
Also do another round of copy-editing, and fix up remaining FIXME items.
Stephen Frost [Tue, 15 Dec 2015 15:08:14 +0000 (10:08 -0500)]
Improve CREATE POLICY documentation
Clarify that SELECT policies are now applied when SELECT rights
are required for a given query, even if the query is an UPDATE or
DELETE query. Pointed out by Noah.
Additionally, note the risk regarding concurrently open transactions
where a relation which controls access to the rows of another relation
are updated and the rows of the primary relation are also being
modified. Pointed out by Peter Geoghegan.
Back-patch to 9.5.
Stephen Frost [Tue, 15 Dec 2015 01:05:55 +0000 (20:05 -0500)]
Collect the global OR of hasRowSecurity flags for plancache
We carry around information about if a given query has row security or
not to allow the plancache to use that information to invalidate a
planned query in the event that the environment changes.
Previously, the flag of one of the subqueries was simply being copied
into place to indicate if the query overall included RLS components.
That's wrong as we need the global OR of all subqueries. Fix by
changing the code to match how fireRIRules works, which is results
in OR'ing all of the flags.
Noted by Tom.
Back-patch to 9.5 where RLS was introduced.
Tom Lane [Tue, 15 Dec 2015 00:22:50 +0000 (19:22 -0500)]
Add missing cleanup logic in pg_rewind/t/005_same_timeline.pl test.
Per Michael Paquier
Tom Lane [Mon, 14 Dec 2015 23:21:42 +0000 (18:21 -0500)]
pg_rewind: Don't error if the two clusters are already on the same timeline
This previously resulted in an error and a nonzero exit status, but
after discussion this should rather be a noop with a zero exit status.
This is a back-patch of commit
6b34e5563849edc12896bf5754e8fe7b88012697,
plus two changes from commit
e50cda78404d6400b1326a996a4fabb144871151
that teach pg_rewind to allow the initial control file states to be
DB_SHUTDOWNED_IN_RECOVERY as well as DB_SHUTDOWNED. That's necessary
to get the additional regression test case to pass, and the old behavior
seems like rather a foot-gun anyway.
Peter Eisentraut and Tom Lane
Alvaro Herrera [Mon, 14 Dec 2015 19:44:40 +0000 (16:44 -0300)]
Add missing CHECK_FOR_INTERRUPTS in lseg_inside_poly
Apparently, there are bugs in this code that cause it to loop endlessly.
That bug still needs more research, but in the meantime it's clear that
the loop is missing a check for interrupts so that it can be cancelled
timely.
Backpatch to 9.1 -- this has been missing since
49475aab8d0d.
Kevin Grittner [Mon, 14 Dec 2015 17:46:47 +0000 (11:46 -0600)]
Remove xmlparse(document '') test
This one test was behaving differently between the ubuntu fix for
CVE-2015-7499 and the base "expected" file. It's not worth having
yet another version of the expected file for this test, so drop it.
Perhaps at some point when all distros have settled down to the
same behavior on this test, it can be restored.
Problem found by me on libxml2 (2.9.1+dfsg1-3ubuntu4.6).
Solution suggested by Tom Lane.
Backpatch to 9.5, where the test was added.
Heikki Linnakangas [Mon, 14 Dec 2015 16:19:10 +0000 (18:19 +0200)]
Fix out-of-memory error handling in ParameterDescription message processing.
If libpq ran out of memory while constructing the result set, it would hang,
waiting for more data from the server, which might never arrive. To fix,
distinguish between out-of-memory error and not-enough-data cases, and give
a proper error message back to the client on OOM.
There are still similar issues in handling COPY start messages, but let's
handle that as a separate patch.
Michael Paquier, Amit Kapila and me. Backpatch to all supported versions.
Andres Freund [Mon, 14 Dec 2015 10:34:16 +0000 (11:34 +0100)]
Fix bug in SetOffsetVacuumLimit() triggered by find_multixact_start() failure.
Previously, if find_multixact_start() failed, SetOffsetVacuumLimit() would
install 0 into MultiXactState->offsetStopLimit if it previously succeeded.
Luckily, there are no known cases where find_multixact_start() will return
an error in 9.5 and above. But if it were to happen, for example due to
filesystem permission issues, it'd be somewhat bad: GetNewMultiXactId()
could continue allocating mxids even if close to a wraparound, or it could
erroneously stop allocating mxids, even if no wraparound is looming. The
wrong value would be corrected the next time SetOffsetVacuumLimit() is
called, or by a restart.
Reported-By: Noah Misch, although this is not his preferred fix
Discussion:
20151210140450[email protected]
Backpatch: 9.5, where the bug was introduced as part of 4f627f
Andres Freund [Mon, 14 Dec 2015 10:25:04 +0000 (11:25 +0100)]
Correct statement to actually be the intended assert statement.
e3f4cfc7 introduced a LWLockHeldByMe() call, without the corresponding
Assert() surrounding it.
Spotted by Coverity.
Backpatch: 9.1+, like the previous commit
Tom Lane [Mon, 14 Dec 2015 04:42:54 +0000 (23:42 -0500)]
Docs: document that psql's "\i -" means read from stdin.
This has worked that way for a long time, maybe always, but you would
not have known it from the documentation. Also back-patch the notes
I added to HEAD earlier today about behavior of the "-f -" switch,
which likewise have been valid for many releases.
Magnus Hagander [Sun, 13 Dec 2015 15:53:38 +0000 (16:53 +0100)]
Consistently set all fields in pg_stat_replication to null instead of 0
Previously the "sent" field would be set to 0 and all other xlog
pointers be set to NULL if there were no valid values (such as when
in a backup sending walsender).
Magnus Hagander [Sun, 13 Dec 2015 15:40:37 +0000 (16:40 +0100)]
Properly initialize write, flush and replay locations in walsender slots
These would leak random xlog positions if a walsender used for backup would
a walsender slot previously used by a replication walsender.
In passing also fix a couple of cases where the xlog pointer is directly
compared to zero instead of using XLogRecPtrIsInvalid, noted by
Michael Paquier.
Tom Lane [Sun, 13 Dec 2015 01:02:09 +0000 (20:02 -0500)]
Doc: update external URLs for PostGIS project.
Paul Ramsey
Andres Freund [Sat, 12 Dec 2015 13:19:19 +0000 (14:19 +0100)]
Fix ALTER TABLE ... SET TABLESPACE for unlogged relations.
Changing the tablespace of an unlogged relation did not WAL log the
creation and content of the init fork. Thus, after a standby is
promoted, unlogged relation cannot be accessed anymore, with errors
like:
ERROR: 58P01: could not open file "pg_tblspc/...": No such file or directory
Additionally the init fork was not synced to disk, independent of the
configured wal_level, a relatively small durability risk.
Investigation of that problem also brought to light that, even for
permanent relations, the creation of !main forks was not WAL logged,
i.e. no XLOG_SMGR_CREATE record were emitted. That mostly turns out not
to be a problem, because these files were created when the actual
relation data is copied; nonexistent files are not treated as an error
condition during replay. But that doesn't work for empty files, and
generally feels a bit haphazard. Luckily, outside init and main forks,
empty forks don't occur often or are not a problem.
Add the required WAL logging and syncing to disk.
Reported-By: Michael Paquier
Author: Michael Paquier and Andres Freund
Discussion:
20151210163230[email protected]
Backpatch: 9.1, where unlogged relations were introduced
Tom Lane [Sat, 12 Dec 2015 00:08:40 +0000 (19:08 -0500)]
Add an expected-file to match behavior of latest libxml2.
Recent releases of libxml2 do not provide error context reports for errors
detected at the very end of the input string. This appears to be a bug, or
at least an infelicity, introduced by the fix for libxml2's CVE-2015-7499.
We can hope that this behavioral change will get undone before too long;
but the security patch is likely to spread a lot faster/further than any
follow-on cleanup, which means this behavior is likely to be present in the
wild for some time to come. As a stopgap, add a variant regression test
expected-file that matches what you get with a libxml2 that acts this way.
Alvaro Herrera [Fri, 11 Dec 2015 21:39:09 +0000 (18:39 -0300)]
For REASSIGN OWNED for foreign user mappings
As reported in bug #13809 by Alexander Ashurkov, the code for REASSIGN
OWNED hadn't gotten word about user mappings. Deal with them in the
same way default ACLs do, which is to ignore them altogether; they are
handled just fine by DROP OWNED. The other foreign object cases are
already handled correctly by both commands.
Also add a REASSIGN OWNED statement to foreign_data test to exercise the
foreign data objects. (The changes are just before the "cleanup" phase,
so it shouldn't remove any existing live test.)
Reported by Alexander Ashurkov, then independently by Jaime Casanova.
Tom Lane [Fri, 11 Dec 2015 21:14:27 +0000 (16:14 -0500)]
Install our "missing" script where PGXS builds can find it.
This allows sane behavior in a PGXS build done on a machine where build
tools such as bison are missing.
Jim Nasby
Stephen Frost [Fri, 11 Dec 2015 21:12:36 +0000 (16:12 -0500)]
Handle policies during DROP OWNED BY
DROP OWNED BY handled GRANT-based ACLs but was not removing roles from
policies. Fix that by having DROP OWNED BY remove the role specified
from the list of roles the policy (or policies) apply to, or the entire
policy (or policies) if it only applied to the role specified.
As with ACLs, the DROP OWNED BY caller must have permission to modify
the policy or a WARNING is thrown and no change is made to the policy.
Tom Lane [Fri, 11 Dec 2015 20:52:16 +0000 (15:52 -0500)]
Get rid of the planner's LateralJoinInfo data structure.
I originally modeled this data structure on SpecialJoinInfo, but after
commit
acfcd45cacb6df23 that looks like a pretty poor decision.
All we really need is relid sets identifying laterally-referenced rels;
and most of the time, what we want to know about includes indirect lateral
references, a case the LateralJoinInfo data was unsuited to compute with
any efficiency. The previous commit redefined RelOptInfo.lateral_relids
as the transitive closure of lateral references, so that it easily supports
checking indirect references. For the places where we really do want just
direct references, add a new RelOptInfo field direct_lateral_relids, which
is easily set up as a copy of lateral_relids before we perform the
transitive closure calculation. Then we can just drop lateral_info_list
and LateralJoinInfo and the supporting code. This makes the planner's
handling of lateral references noticeably more efficient, and shorter too.
Such a change can't be back-patched into stable branches for fear of
breaking extensions that might be looking at the planner's data structures;
but it seems not too late to push it into 9.5, so I've done so.
Stephen Frost [Fri, 11 Dec 2015 20:44:03 +0000 (15:44 -0500)]
Handle dependencies properly in ALTER POLICY
ALTER POLICY hadn't fully considered partial policy alternation
(eg: change just the roles on the policy, or just change one of
the expressions) when rebuilding the dependencies. Instead, it
would happily remove all dependencies which existed for the
policy and then only recreate the dependencies for the objects
referred to in the specific ALTER POLICY command.
Correct that by extracting and building the dependencies for all
objects referenced by the policy, regardless of if they were
provided as part of the ALTER POLICY command or were already in
place as part of the pre-existing policy.
Tom Lane [Fri, 11 Dec 2015 19:22:20 +0000 (14:22 -0500)]
Still more fixes for planner's handling of LATERAL references.
More fuzz testing by Andreas Seltenreich exposed that the planner did not
cope well with chains of lateral references. If relation X references Y
laterally, and Y references Z laterally, then we will have to scan X on the
inside of a nestloop with Z, so for all intents and purposes X is laterally
dependent on Z too. The planner did not understand this and would generate
intermediate joins that could not be used. While that was usually harmless
except for wasting some planning cycles, under the right circumstances it
would lead to "failed to build any N-way joins" or "could not devise a
query plan" planner failures.
To fix that, convert the existing per-relation lateral_relids and
lateral_referencers relid sets into their transitive closures; that is,
they now show all relations on which a rel is directly or indirectly
laterally dependent. This not only fixes the chained-reference problem
but allows some of the relevant tests to be made substantially simpler
and faster, since they can be reduced to simple bitmap manipulations
instead of searches of the LateralJoinInfo list.
Also, when a PlaceHolderVar that is due to be evaluated at a join contains
lateral references, we should treat those references as indirect lateral
dependencies of each of the join's base relations. This prevents us from
trying to join any individual base relations to the lateral reference
source before the join is formed, which again cannot work.
Andreas' testing also exposed another oversight in the "dangerous
PlaceHolderVar" test added in commit
85e5e222b1dd02f1. Simply rejecting
unsafe join paths in joinpath.c is insufficient, because in some cases
we will end up rejecting *all* possible paths for a particular join, again
leading to "could not devise a query plan" failures. The restriction has
to be known also to join_is_legal and its cohort functions, so that they
will not select a join for which that will happen. I chose to move the
supporting logic into joinrels.c where the latter functions are.
Back-patch to 9.3 where LATERAL support was introduced.
Alvaro Herrera [Fri, 11 Dec 2015 17:30:43 +0000 (14:30 -0300)]
Fix commit timestamp initialization
This module needs explicit initialization in order to replay WAL records
in recovery, but we had broken this recently following changes to make
other (stranger) scenarios work correctly. To fix, rework the
initialization sequence so that it always takes place before WAL replay
commences for both master and standby.
I could have gone for a more localized fix that just added a "startup"
call for the master server, but it seemed better to restructure the
existing callers as well so that the whole thing made more sense. As a
drawback, there is more control logic in xlog.c now than previously, but
doing otherwise meant passing down the ControlFile flag, which seemed
uglier as a whole.
This also meant adding a check to not re-execute ActivateCommitTs if it
had already been called.
Reported by Fujii Masao.
Backpatch to 9.5.
Peter Eisentraut [Fri, 11 Dec 2015 03:05:27 +0000 (22:05 -0500)]
Improve some messages
Robert Haas [Thu, 10 Dec 2015 17:28:46 +0000 (12:28 -0500)]
Improve ALTER POLICY tab completion.
Complete "ALTER POLICY" with a policy name, as we do for DROP POLICY.
And, complete "ALTER POLICY polname ON" with a table name that has such
a policy, as we do for DROP POLICY, rather than with any table name
at all.
Masahiko Sawada
Robert Haas [Thu, 10 Dec 2015 16:13:24 +0000 (11:13 -0500)]
Fix typo.
Etsuro Fujita
Andres Freund [Thu, 10 Dec 2015 15:26:45 +0000 (16:26 +0100)]
Fix ON CONFLICT UPDATE bug breaking AFTER UPDATE triggers.
ExecOnConflictUpdate() passed t_ctid of the to-be-updated tuple to
ExecUpdate(). That's problematic primarily because of two reason: First
and foremost t_ctid could point to a different tuple. Secondly, and
that's what triggered the complaint by Stanislav, t_ctid is changed by
heap_update() to point to the new tuple version. The behavior of AFTER
UPDATE triggers was therefore broken, with NEW.* and OLD.* tuples
spuriously identical within AFTER UPDATE triggers.
To fix both issues, pass a pointer to t_self of a on-stack HeapTuple
instead.
Fixing this bug lead to one change in regression tests, which previously
failed due to the first issue mentioned above. There's a reasonable
expectation that test fails, as it updates one row repeatedly within one
INSERT ... ON CONFLICT statement. That is only possible if the second
update is triggered via ON CONFLICT ... SET, ON CONFLICT ... WHERE, or
by a WITH CHECK expression, as those are executed after
ExecOnConflictUpdate() does a visibility check. That could easily be
prohibited, but given it's allowed for plain UPDATEs and a rare corner
case, it doesn't seem worthwhile.
Reported-By: Stanislav Grozev
Author: Andres Freund and Peter Geoghegan
Discussion: CAA78GVqy1+LisN-8DygekD_Ldfy=BJLarSpjGhytOsgkpMavfQ@mail.gmail.com
Backpatch: 9.5, where ON CONFLICT was introduced
Andres Freund [Thu, 10 Dec 2015 15:25:12 +0000 (16:25 +0100)]
Fix bug leading to restoring unlogged relations from empty files.
At the end of crash recovery, unlogged relations are reset to the empty
state, using their init fork as the template. The init fork is copied to
the main fork without going through shared buffers. Unfortunately WAL
replay so far has not necessarily flushed writes from shared buffers to
disk at that point. In normal crash recovery, and before the
introduction of 'fast promotions' in
fd4ced523 / 9.3, the
END_OF_RECOVERY checkpoint flushes the buffers out in time. But with
fast promotions that's not the case anymore.
To fix, force WAL writes targeting the init fork to be flushed
immediately (using the new FlushOneBuffer() function). In 9.5+ that
flush can centrally be triggered from the code dealing with restoring
full page writes (XLogReadBufferForRedoExtended), in earlier releases
that responsibility is in the hands of XLOG_HEAP_NEWPAGE's replay
function.
Backpatch to 9.1, even if this currently is only known to trigger in
9.3+. Flushing earlier is more robust, and it is advantageous to keep
the branches similar.
Typical symptoms of this bug are errors like
'ERROR: index "..." contains unexpected zero page at block 0'
shortly after promoting a node.
Reported-By: Thom Brown
Author: Andres Freund and Michael Paquier
Discussion:
20150326175024[email protected]
Backpatch: 9.1-
Tom Lane [Thu, 10 Dec 2015 15:19:13 +0000 (10:19 -0500)]
Accept flex > 2.5.x on Windows, too.
Commit
32f15d05c fixed this in configure, but missed the similar check
in the MSVC scripts.
Michael Paquier, per report from Victor Wagner
Tom Lane [Wed, 9 Dec 2015 23:54:25 +0000 (18:54 -0500)]
Simplify LATERAL-related calculations within add_paths_to_joinrel().
While convincing myself that commit
7e19db0c09719d79 would solve both of
the problems recently reported by Andreas Seltenreich, I realized that
add_paths_to_joinrel's handling of LATERAL restrictions could be made
noticeably simpler and faster if we were to retain the minimum possible
parameterization for each joinrel (that is, the set of relids supplying
unsatisfied lateral references in it). We already retain that for
baserels, in RelOptInfo.lateral_relids, so we can use that field for
joinrels too.
This is a back-port of commit
edca44b1525b3d591263d032dc4fe500ea771e0e.
I originally intended not to back-patch that, but additional hacking
in this area turns out to be needed, making it necessary not optional
to compute lateral_relids for joinrels. In preparation for those fixes,
sync the relevant code with HEAD as much as practical. (I did not risk
rearranging fields of RelOptInfo in released branches, however.)
Robert Haas [Wed, 9 Dec 2015 19:11:58 +0000 (14:11 -0500)]
Remove redundant sentence.
Peter Geoghegan
Tom Lane [Tue, 8 Dec 2015 22:14:46 +0000 (17:14 -0500)]
Make failure to open psql's --log-file fatal.
Commit
344cdff2c made failure to open the target of --output fatal.
For consistency, the --log-file switch should behave similarly.
Like the previous commit, back-patch to 9.5 but no further.
Daniel Verite
Tom Lane [Tue, 8 Dec 2015 21:58:05 +0000 (16:58 -0500)]
Avoid odd portability problem in TestLib.pm's slurp_file function.
For unclear reasons, this function doesn't always read the expected data
in some old Perl versions. Rewriting it to avoid use of ARGV seems to
dodge the problem, and this version is clearer anyway if you ask me.
In passing, also improve error message in adjacent append_to_file function.
Robert Haas [Tue, 8 Dec 2015 17:31:03 +0000 (12:31 -0500)]
Allow foreign and custom joins to handle EvalPlanQual rechecks.
Commit
e7cb7ee14555cc9c5773e2c102efd6371f6f2005 provided basic
infrastructure for allowing a foreign data wrapper or custom scan
provider to replace a join of one or more tables with a scan.
However, this infrastructure failed to take into account the need
for possible EvalPlanQual rechecks, and ExecScanFetch would fail
an assertion (or just overwrite memory) if such a check was attempted
for a plan containing a pushed-down join. To fix, adjust the EPQ
machinery to skip some processing steps when scanrelid == 0, making
those the responsibility of scan's recheck method, which also has
the responsibility in this case of correctly populating the relevant
slot.
To allow foreign scans to gain control in the right place to make
use of this new facility, add a new, optional RecheckForeignScan
method. Also, allow a foreign scan to have a child plan, which can
be used to correctly populate the slot (or perhaps for something
else, but this is the only use currently envisioned).
KaiGai Kohei, reviewed by Robert Haas, Etsuro Fujita, and Kyotaro
Horiguchi.
Tom Lane [Mon, 7 Dec 2015 22:41:45 +0000 (17:41 -0500)]
Fix another oversight in checking if a join with LATERAL refs is legal.
It was possible for the planner to decide to join a LATERAL subquery to
the outer side of an outer join before the outer join itself is completed.
Normally that's fine because of the associativity rules, but it doesn't
work if the subquery contains a lateral reference to the inner side of the
outer join. In such a situation the outer join *must* be done first.
join_is_legal() missed this consideration and would allow the join to be
attempted, but the actual path-building code correctly decided that no
valid join path could be made, sometimes leading to planner errors such as
"failed to build any N-way joins".
Per report from Andreas Seltenreich. Back-patch to 9.3 where LATERAL
support was added.
Tom Lane [Sun, 6 Dec 2015 17:42:32 +0000 (12:42 -0500)]
Update xindex.sgml for recent additions to GIST opclass API.
Commit
d04c8ed9044ec added another support function to the GIST API,
but overlooked mentioning it in xindex.sgml's summary of index support
functions.
Anastasia Lubennikova
Tom Lane [Sat, 5 Dec 2015 18:23:48 +0000 (13:23 -0500)]
Create TestLib.pm's tempdir underneath tmp_check/, not out in the open.
This way, existing .gitignore entries and makefile clean actions will
automatically apply to the tempdir, should it survive a TAP test run
(which can happen if the user control-C's out of the run, for example).
Michael Paquier, per a complaint from me
Noah Misch [Sat, 5 Dec 2015 08:04:17 +0000 (03:04 -0500)]
Instruct Coverity using an assertion.
This should make Coverity deduce that plperl_call_perl_func() does not
dereference NULL argtypes. Back-patch to 9.5, where the affected code
was introduced.
Michael Paquier
Tom Lane [Fri, 4 Dec 2015 19:44:13 +0000 (14:44 -0500)]
Further improve documentation of the role-dropping process.
In commit
1ea0c73c2 I added a section to user-manag.sgml about how to drop
roles that own objects; but as pointed out by Stephen Frost, I neglected
that shared objects (databases or tablespaces) may need special treatment.
Fix that. Back-patch to supported versions, like the previous patch.
Alvaro Herrera [Thu, 3 Dec 2015 22:22:31 +0000 (19:22 -0300)]
Further tweak commit_timestamp behavior
As pointed out by Fujii Masao, we weren't quite there on a standby
behaving sanely: first because we were failing to acquire the correct
state in the case where no XLOG_PARAMETER_CHANGE message was sent
(because a checkpoint had already happened after the setting was changed
in the master, and then the standby was restarted); and second because
promoting the standby with the feature enabled failed to activate it if
the master had the feature disabled.
This patch fixes both those misbehaviors hopefully without
re-introducing any old problems.
Also change the hint emitted in a standby together with the error
message about the feature being disabled, to make it point out that the
place to chance the setting is the master. Otherwise, if the setting is
already enabled in the standby, it is very confusing to have it say that
the setting must be enabled ...
Authors: Álvaro Herrera, Petr Jelínek.
Backpatch to 9.5.
Tom Lane [Thu, 3 Dec 2015 19:29:07 +0000 (14:29 -0500)]
Clean up some psql issues around handling of the query output file.
Formerly, if "psql -o foo" failed to open the output file "foo", it would
print an error message but then carry on as though -o had not been
specified at all. This seems contrary to expectation: a program that
cannot open its output file normally fails altogether. Make psql do
exit(1) after reporting the error.
If "\o foo" failed to open "foo", it would print an error message but then
reset the output file to stdout, as if the argument had been omitted.
This is likewise pretty surprising behavior. Make it keep the previous
output state, instead.
psql keeps SIGPIPE interrupts disabled when it is writing to a pipe, either
a pipe specified by -o/\o or a transient pipe opened for purposes such as
using a pager on query output. The logic for this was too simple and could
sometimes re-enable SIGPIPE when a -o pipe was still active, thus possibly
leading to an unexpected psql crash later.
Fixing the last point required getting rid of the kluge in PrintQueryTuples
and ExecQueryUsingCursor whereby they'd transiently change the global
queryFout state, but that seems like good cleanup anyway.
Back-patch to 9.5 but not further; these are minor-enough issues that
changing the behavior in stable branches doesn't seem appropriate.
Peter Eisentraut [Thu, 3 Dec 2015 15:23:59 +0000 (10:23 -0500)]
psql: Improve spelling
Peter Eisentraut [Thu, 3 Dec 2015 15:20:54 +0000 (10:20 -0500)]
doc: Fix markup and improve placeholder names
Tom Lane [Wed, 2 Dec 2015 23:20:34 +0000 (18:20 -0500)]
Fix behavior of printTable() and friends with externally-invoked pager.
The formatting modes that depend on knowledge of the terminal window width
did not work right when printing a query result that's been fetched in
sections (as a result of FETCH_SIZE). ExecQueryUsingCursor() would force
use of the pager as soon as there's more than one result section, and then
print.c would see an output file pointer that's not stdout and incorrectly
conclude that the terminal window width isn't relevant.
This has been broken all along for non-expanded "wrapped" output format,
and as of 9.5 the issue affects expanded mode as well. The problem also
caused "\pset expanded auto" mode to invariably *not* switch to expanded
output in a segmented result, which seems to me to be exactly backwards.
To fix, we need to pass down an "is_pager" flag to inform the print.c
subroutines that some calling level has already replaced stdout with a
pager pipe, so they should (a) not do that again and (b) nonetheless honor
the window size. (Notably, this makes the first is_pager test in
print_aligned_text() not be dead code anymore.)
This patch is a bit invasive because there are so many existing calls of
printQuery()/printTable(), but fortunately all but a couple can just pass
"false" for the added parameter.
Back-patch to 9.5 but no further. Given the lack of field complaints,
it's not clear that we should change the behavior in stable branches.
Also, the API change for printQuery()/printTable() might possibly break
third-party code, again something we don't like to do in stable branches.
However, it's not quite too late to do this in 9.5, and with the larger
scope of the problem there, it seems worth doing.
Tom Lane [Tue, 1 Dec 2015 21:24:34 +0000 (16:24 -0500)]
Make gincostestimate() cope with hypothetical GIN indexes.
We tried to fetch statistics data from the index metapage, which does not
work if the index isn't actually present. If the index is hypothetical,
instead extrapolate some plausible internal statistics based on the index
page count provided by the index-advisor plugin.
There was already some code in gincostestimate() to invent internal stats
in this way, but since it was only meant as a stopgap for pre-9.1 GIN
indexes that hadn't been vacuumed since upgrading, it was pretty crude.
If we want it to support index advisors, we should try a little harder.
A small amount of testing says that it's better to estimate the entry pages
as 90% of the index, not 100%. Also, estimating the number of entries
(keys) as equal to the heap tuple count could be wildly wrong in either
direction. Instead, let's estimate 100 entries per entry page.
Perhaps someday somebody will want the index advisor to be able to provide
these numbers more directly, but for the moment this should serve.
Problem report and initial patch by Julien Rouhaud; modified by me to
invent less-bogus internal statistics. Back-patch to all supported
branches, since we've supported index advisors since 9.0.
Tom Lane [Tue, 1 Dec 2015 19:47:13 +0000 (14:47 -0500)]
Further tweaking of print_aligned_vertical().
Don't force the data width to extend all the way to the right margin if it
doesn't need to. This reverts the behavior in non-wrapping cases to be
what it was in 9.4. Also, make the logic that ensures the data line width
is at least equal to the record-header line width a little less obscure.
In passing, avoid possible calculation of log10(0). Probably that's
harmless, given the lack of field complaints, but it seems risky:
conversion of NaN to an integer isn't well defined.
Tom Lane [Tue, 1 Dec 2015 16:42:25 +0000 (11:42 -0500)]
Use "g" not "f" format in ecpg's PGTYPESnumeric_from_double().
The previous coding could overrun the provided buffer size for a very large
input, or lose precision for a very small input. Adopt the methodology
that's been in use in the equivalent backend code for a long time.
Per private report from Bas van Schaik. Back-patch to all supported
branches.