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.)
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
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.
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.
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.
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
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
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
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.
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.
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.
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.
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.
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.
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.
We should ignore output_columns unless it's greater than zero.
A zero means we couldn't get any information from ioctl(TIOCGWINSZ);
in that case the expected behavior is to print the data at native width,
not to wrap it at the smallest possible value. print_aligned_text()
gets this consideration right, but print_aligned_vertical() lost track
of this detail somewhere along the line.
This area was rather heavily whacked around in 6513633b9 and follow-on
commits, and it was showing it, because the logic to calculate the
allowable data width in wrapped expanded mode had only the vaguest
relationship to the logic that was actually printing the data. It was
not very close to being right about the conditions requiring overhead
columns to be added. Aside from being wrong, it was pretty unreadable
and under-commented. Rewrite it so it corresponds to what the printing
code actually does.
In passing, remove a couple of dead tests in the printing logic, too.
Per a complaint from Jeff Janes, though this doesn't look much like his
patch because it fixes a number of other corner-case bogosities too.
One such fix that's visible in the regression test results is that
although the code was attempting to enforce a minimum data width of
3 columns, it sometimes left less space than that available.
In commit 8abb3cda0ddc00a0ab98977a1633a95b97068d4e I attempted to cache
the expression state trees constructed for domain CHECK constraints for
the life of the backend (assuming the domain's constraints don't get
redefined). However, this turns out not to work very well, because
execQual.c will run those state trees with ecxt_per_query_memory pointing
to a query-lifespan context, and in some situations we'll end up with
pointers into that context getting stored into the state trees. This
happens in particular with SQL-language functions, as reported by
Emre Hasegeli, but there are many other cases.
To fix, keep only the expression plan trees for domain CHECK constraints
in the typcache's data structure, and revert to performing ExecInitExpr
(at least) once per query to set up expression state trees in the query's
context.
Eventually it'd be nice to undo this, but that will require some careful
thought about memory management for expression state trees, and it seems
far too late for any such redesign in 9.5. This way is still much more
efficient than what happened before 8abb3cda0.
Failure to initially palloc the comboCids array, or to realloc it bigger
when needed, left combocid's data structures in an inconsistent state that
would cause trouble if the top transaction continues to execute. Noted
while examining a user complaint about the amount of memory used for this.
(There's not much we can do about that, but it does point up that repalloc
failure has a non-negligible chance of occurring here.)
In HEAD/9.5, also avoid possible invocation of memcpy() with a null pointer
in SerializeComboCIDState; cf commit 13bba0227.
PQhost() can return NULL in non-error situations, namely when a Unix-socket
connection has been selected by default. That behavior is a tad debatable
perhaps, but for the moment we should make sure that psql copes with it.
Unfortunately, do_connect() failed to: it could pass a NULL pointer to
strcmp(), resulting in crashes on most platforms. This was reported as a
security issue by ChenQin of Topsec Security Team, but the consensus of
the security list is that it's just a garden-variety bug with no security
implications.
For paranoia's sake, I made the keep_password test not trust PQuser or
PQport either, even though I believe those will never return NULL given
a valid PGconn.
Back-patch to all supported branches.
Also fix getErrorText() to return the right error string on failure.
This behavior now matches that of other operating systems.
Report by Noah Misch
Backpatch through 9.1
The POSIX standard for tar headers requires archive member sizes to be
printed in octal with at most 11 digits, limiting the representable file
size to 8GB. However, GNU tar and apparently most other modern tars
support a convention in which oversized values can be stored in base-256,
allowing any practical file to be a tar member. Adopt this convention
to remove two limitations:
* pg_dump with -Ft output format failed if the contents of any one table
exceeded 8GB.
* pg_basebackup failed if the data directory contained any file exceeding
8GB. (This would be a fatal problem for installations configured with a
table segment size of 8GB or more, and it has also been seen to fail when
large core dump files exist in the data directory.)
File sizes under 8GB are still printed in octal, so that no compatibility
issues are created except in cases that would have failed entirely before.
In addition, this patch fixes several bugs in the same area:
* In 9.3 and later, we'd defined tarCreateHeader's file-size argument as
size_t, which meant that on 32-bit machines it would write a corrupt tar
header for file sizes between 4GB and 8GB, even though no error was raised.
This broke both "pg_dump -Ft" and pg_basebackup for such cases.
* pg_restore from a tar archive would fail on tables of size between 4GB
and 8GB, on machines where either "size_t" or "unsigned long" is 32 bits.
This happened even with an archive file not affected by the previous bug.
* pg_basebackup would fail if there were files of size between 4GB and 8GB,
even on 64-bit machines.
* In 9.3 and later, "pg_basebackup -Ft" failed entirely, for any file size,
on 64-bit big-endian machines.
In view of these potential data-loss bugs, back-patch to all supported
branches, even though removal of the documented 8GB limit might otherwise
be considered a new feature rather than a bug fix.
The previous way of reconstructing check constraints was to do a separate
"ALTER TABLE ONLY tab ADD CONSTRAINT" for each table in an inheritance
hierarchy. However, that way has no hope of reconstructing the check
constraints' own inheritance properties correctly, as pointed out in
bug #13779 from Jan Dirk Zijlstra. What we should do instead is to do
a regular "ALTER TABLE", allowing recursion, at the topmost table that
has a particular constraint, and then suppress the work queue entries
for inherited instances of the constraint.
Annoyingly, we'd tried to fix this behavior before, in commit 5ed6546cf,
but we failed to notice that it wasn't reconstructing the pg_constraint
field values correctly.
As long as I'm touching pg_get_constraintdef_worker anyway, tweak it to
always schema-qualify the target table name; this seems like useful backup
to the protections installed by commit 5f173040.
In HEAD/9.5, get rid of get_constraint_relation_oids, which is now unused.
(I could alternatively have modified it to also return conislocal, but that
seemed like a pretty single-purpose API, so let's not pretend it has some
other use.) It's unused in the back branches as well, but I left it in
place just in case some third-party code has decided to use it.
In HEAD/9.5, also rename pg_get_constraintdef_string to
pg_get_constraintdef_command, as the previous name did nothing to explain
what that entry point did differently from others (and its comment was
equally useless). Again, that change doesn't seem like material for
back-patching.
I did a bit of re-pgindenting in tablecmds.c in HEAD/9.5, as well.
Otherwise, back-patch to all supported branches.
Some versions of Perl export a macro named HS_KEY. This creates a
conflict in contrib/hstore_plperl against hstore's macro of the same
name. The most future-proof solution seems to be to rename our macro;
I chose HSTORE_KEY. For consistency, rename HS_VAL and related macros
similarly.
Back-patch to 9.5. contrib/hstore_plperl doesn't exist before that
so there is no need to worry about the conflict in older releases.
Per reports from Marco Atzeri and Mike Blackwell.
Silly mistake in my commit 09cecdf285ea9f51, reported by Erik Rijkers.
The fact that the buildfarm didn't find this implies that we are not
testing Perl builds that lack MULTIPLICITY, which is a bit disturbing
from a coverage standpoint. Until today I'd have said nobody cared
about such configurations anymore; but maybe not.
The target is now named 'bincheck' rather than 'tapcheck' so that it
reflects what is checked instead of the test mechanism. Some of the
logic is improved, making it easier to add further sets of TAP based
tests in future. Also, the environment setting logic is imrpoved.
As discussed on -hackers a couple of months ago.
At least one of the names was, due to a function renaming late in the
development of ON CONFLICT, wrong. Since including function names in
error messages is against the message style guide anyway, remove them
from the messages.
Discussion: CAM3SWZT8paz=usgMVHm0XOETkQvzjRtAUthATnmaHQQY0obnGw@mail.gmail.com
Backpatch: 9.5, where ON CONFLICT was introduced
Per buildfarm member anchovy, 2.6.0 exists in the wild now.
Hopefully it works with Postgres; if not, we'll have to do something
about that, but in any case claiming it's "too old" is pretty silly.
div_var_fast() postpones propagating carries in the same way as mul_var(),
so it has the same corner-case overflow risk we fixed in 246693e5ae8a36f0,
namely that the size of the carries has to be accounted for when setting
the threshold for executing a carry propagation step. We've not devised
a test case illustrating the brokenness, but the required fix seems clear
enough. Like the previous fix, back-patch to all active branches.
Dean Rasheed
This back-ports commit 13d856e177e69083 and assorted followon patches
into 9.4 and 9.5. 9.5 and HEAD are now substantially identical in all
the files touched by this commit, except that 010_pg_basebackup.pl has
a few more tests related to the new --slot option. 9.4 has many fewer
TAP tests, but the test infrastructure files are substantially the same,
with the exception that 9.4 lacks the single-tmp-install infrastructure
introduced in 9.5 (commit dcae5faccab64776).
The primary motivation for this patch is to ensure that TAP test case
fixes can be back-patched without hazards of the kind seen in commits
34557f544/06dd4b44f. In principle it should also make the world safe
for running the TAP tests in the buildfarm in these branches; although
we might want to think about back-porting dcae5faccab64776 to 9.4 if
we're going to do that for real, because the TAP tests are quite disk
space hungry without it.
Michael Paquier did the back-porting work; original patches were by
him and assorted other people.
Since commit 11e131854f8231a21613f834c40fe9d046926387, ruleutils.c has
attempted to ensure that each RTE in a query or plan tree has a unique
alias name. However, the code that was added for this could be quite slow,
even as bad as O(N^3) if N identical RTE names must be replaced, as noted
by Jeff Janes. Improve matters by building a transient hash table within
set_rtable_names. The hash table in itself reduces the cost of detecting a
duplicate from O(N) to O(1), and we can save another factor of N by storing
the number of de-duplicated names already created for each entry, so that
we don't have to re-try names already created. This way is probably a bit
slower overall for small range tables, but almost by definition, such cases
should not be a performance problem.
In principle the same problem applies to the column-name-de-duplication
code; but in practice that seems to be less of a problem, first because
N is limited since we don't support extremely wide tables, and second
because duplicate column names within an RTE are fairly rare, so that in
practice the cost is more like O(N^2) not O(N^3). It would be very much
messier to fix the column-name code, so for now I've left that alone.
An independent problem in the same area was that the de-duplication code
paid no attention to the identifier length limit, and would happily produce
identifiers that were longer than NAMEDATALEN and wouldn't be unique after
truncation to NAMEDATALEN. This could result in dump/reload failures, or
perhaps even views that silently behaved differently than before. We can
fix that by shortening the base name as needed. Fix it for both the
relation and column name cases.
In passing, check for interrupts in set_rtable_names, just in case it's
still slow enough to be an issue.
Back-patch to 9.3 where this code was introduced.
Normally ruleutils prints a whole-row Var as "foo.*". We already knew that
that doesn't work at top level of a SELECT list, because the parser would
treat the "*" as a directive to expand the reference into separate columns,
not a whole-row Var. However, Joshua Yanovski points out in bug #13776
that the same thing happens at top level of a ROW() construct; and some
nosing around in the parser shows that the same is true in VALUES().
Hence, apply the same workaround already devised for the SELECT-list case,
namely to add a forced cast to the appropriate rowtype in these cases.
(The alternative of just printing "foo" was rejected because it is
difficult to avoid ambiguity against plain columns named "foo".)
Back-patch to all supported branches.
The sepgsql docs included a comment that PG doesn't support RLS. That
is only true for versions prior to 9.5.
Update the docs for 9.5 and master to say that PG supports RLS but that
sepgsql does not yet.
Pointed out by Heikki.
Back-patch to 9.5
Having the script prompt for passwords over and over was a preexisting
problem when it processed multiple databases or when it processed
multiple analyze stages, but the parallel mode introduced in commit
a179232047 made it worse.
Fix the annoyance by keeping a copy of the password used by the first
connection that requires one. Since users can (currently) only have a
single password, there's no need for more complex arrangements (such as
remembering one password per database).
Per bug #13741 reported by Eric Brown. Patch authored and
cross-reviewed by Haribabu Kommi and Michael Paquier, slightly tweaked
by Álvaro Herrera.
Discussion: http://www.postgresql.org/message-id/20151027193919.931.54948@wrigleys.postgresql.org
Backpatch to 9.5, where parallel vacuumdb was introduced.
