Alvaro Herrera [Fri, 12 Feb 2016 20:30:46 +0000 (17:30 -0300)]
pgbench: cleanup use of a "logfile" parameter
There is no reason to have the per-thread logfile file pointer as a
separate parameter in various functions: it's much simpler to put it in
the per-thread state struct instead, which is already being passed to
all functions that need the log file anyway. Change the callsites in
which it was used as a boolean to test whether logging is active, so
that they use the use_log global variable instead.
No backpatch, even though this exists since commit a887c486d5df of March
2010, because this is just for cleanliness' sake and the surrounding
code has been modified a lot recently anyway.
Alvaro Herrera [Fri, 12 Feb 2016 20:14:45 +0000 (17:14 -0300)]
pgbench: fix segfault with empty sql file
Commit 1d0c3b3f8a introduced a bug that causes pgbench to crash if an
empty script file is specified. Fix it by rejecting such files at
startup, which is the historical and intended behavior.
Reported-By: Jeff Janes
Discussion: https://www.postgresql.org/message-id/CAMkU=1zxKUbLPOt9hQWFp14pTc=V0cGo2GQBbn2GsK2Pu+8ZfA@mail.gmail.com
Tom Lane [Fri, 12 Feb 2016 19:19:57 +0000 (14:19 -0500)]
Still further tweaking of deadlock isolation tests.
It turns out that there is a second race condition in the new deadlock-hard
test: once the deadlock detector fires, it's uncertain whether step s7a8 or
step s8a1 will report first, because killing s8's transaction unblocks s7.
So far, s7 has only been seen to report first in CLOBBER_CACHE_ALWAYS
builds, but it's pretty reproducible there, and in theory it should
sometimes occur in normal builds too. If s7 were a bit slower than usual,
that could also break the test, since the existing expected-file assumes
that we'll see s7a8 report the first time we check it after s8a1 completes.
To fix, add a post-lock delay to s7a8.
Tom Lane [Fri, 12 Feb 2016 19:10:36 +0000 (14:10 -0500)]
isolationtester: don't repeat the is-it-waiting query when retrying a step.
If we're retrying a step, then we already decided it was blocked on a lock,
and there's no need to recheck that. The original coding of commit 38f8bdcac4982215beb9f65a19debecaf22fd470 resulted in a large number of
is-it-waiting queries when dealing with multiple concurrently-blocked
sessions, which is fairly pointless and also results in test failures in
CLOBBER_CACHE_ALWAYS builds, where the is-it-waiting query is quite slow.
This definition also permits appending pg_sleep() calls to steps where it's
needed to control the order of finish of concurrent steps. Before, that
did not work nicely because we'd decide that a step performing a sleep was
not blocked and hang up waiting for it to finish, rather than noticing the
completion of the concurrent step we're supposed to notice first.
In passing, revise handling of removal of completed waiting steps
to make it a bit less messy.
Robert Haas [Fri, 12 Feb 2016 14:31:16 +0000 (09:31 -0500)]
Introduce extensible node types.
An extensible node is always tagged T_Extensible, but the extnodename
field identifies it more specifically; it may also include arbitrary
private data. Extensible nodes can be copied, tested for equality,
serialized, and deserialized, but the core system doesn't know
anything about them otherwise. Some extensions may find it useful to
include these nodes in fdw_private or custom_private lists in lieu of
arm-wrestling their data into a format that the core code can
understand.
Along the way, so as not to burden the authors of such extensible
node types too much, expose the functions for writing serialized
tokens, and for serializing and deserializing bitmapsets.
KaiGai Kohei, per a design suggested by me. Reviewed by Andres Freund
and by me, and further edited by me.
Tom Lane [Fri, 12 Feb 2016 04:21:33 +0000 (23:21 -0500)]
Further tweaking of deadlock isolation tests.
The new deadlock-soft-2 test has a timing dependency too: it supposes
that isolationtester will detect step s1b as waiting before the deadlock
detector runs and grants it the lock. Adjust deadlock_timeout to ensure
that that's true even in CLOBBER_CACHE_ALWAYS builds, where the wait
detection query is quite slow. Per buildfarm member jaguarundi.
Tom Lane [Thu, 11 Feb 2016 22:52:03 +0000 (17:52 -0500)]
Refactor check_functional_grouping() to use get_primary_key_attnos().
If we ever get around to allowing functional dependency to be proven
from other things besides simple primary keys, this code will need to
be rethought, but that was true anyway. In the meantime, we might as
well not have two very-similar routines for scanning pg_constraint.
Tom Lane [Thu, 11 Feb 2016 22:34:59 +0000 (17:34 -0500)]
Remove GROUP BY columns that are functionally dependent on other columns.
If a GROUP BY clause includes all columns of a non-deferred primary key,
as well as other columns of the same relation, those other columns are
redundant and can be dropped from the grouping; the pkey is enough to
ensure that each row of the table corresponds to a separate group.
Getting rid of the excess columns will reduce the cost of the sorting or
hashing needed to implement GROUP BY, and can indeed remove the need for
a sort step altogether.
This seems worth testing for since many query authors are not aware of
the GROUP-BY-primary-key exception to the rule about queries not being
allowed to reference non-grouped-by columns in their targetlists or
HAVING clauses. Thus, redundant GROUP BY items are not uncommon. Also,
we can make the test pretty cheap in most queries where it won't help
by not looking up a rel's primary key until we've found that at least
two of its columns are in GROUP BY.
Tom Lane [Thu, 11 Feb 2016 20:51:28 +0000 (15:51 -0500)]
Move pg_constraint.h function declarations to new file pg_constraint_fn.h.
A pending patch requires exporting a function returning Bitmapset from
catalog/pg_constraint.c. As things stand, that would mean including
nodes/bitmapset.h in pg_constraint.h, which might be hazardous for the
client-side includability of that header. It's not entirely clear whether
any client-side code needs to include pg_constraint.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_constraint_fn.h, similarly to what
we've done for some other catalog header files.
Tom Lane [Thu, 11 Feb 2016 19:14:07 +0000 (14:14 -0500)]
Shift the responsibility for emitting "database system is shut down".
Historically this message has been emitted at the end of ShutdownXLOG().
That's not an insane place for it in a standalone backend, but in the
postmaster environment we've grown a fair amount of stuff that happens
later, including archiver/walsender shutdown, stats collector shutdown,
etc. Recent buildfarm experimentation showed that on slower machines
there could be many seconds' delay between finishing ShutdownXLOG() and
actual postmaster exit. That's fairly confusing, both for testing
purposes and for DBAs. Hence, move the code that prints this message
into UnlinkLockFiles(), so that it comes out just after we remove the
postmaster's pidfile. That is a more appropriate definition of "is shut
down" from the point of view of "pg_ctl stop", for example. In general,
removing the pidfile should be the last externally-visible action of
either a postmaster or a standalone backend; compare commit d73d14c271653dff10c349738df79ea03b85236c for instance. So this seems
like a reasonably future-proof approach.
Robert Haas [Thu, 11 Feb 2016 19:07:33 +0000 (14:07 -0500)]
Use separate lwlock tranches for buffer, lock, and predicate lock managers.
This finishes the work - spread across many commits over the last
several months - of putting each type of lock other than the named
individual locks into a separate tranche.
Tom Lane [Thu, 11 Feb 2016 16:59:11 +0000 (11:59 -0500)]
Make new deadlock isolation test more reproducible.
The original formulation of 4c9864b9b4d87d02f07f40bb27976da737afdcab
was extremely timing-sensitive, because it arranged for the deadlock
detector to be running (and possibly unblocking the current query)
at almost exactly the same time as isolationtester would be probing
to see if the query is blocked. The committed expected-file assumed
that the deadlock detection would finish first, but we see the opposite
on both fast and slow buildfarm animals. Adjust the deadlock timeout
settings to make it predictable that isolationtester *will* see the
query as waiting before deadlock detection unblocks it.
Tom Lane [Thu, 11 Feb 2016 16:30:46 +0000 (11:30 -0500)]
Code review for isolationtester changes.
Fix a few oversights in 38f8bdcac4982215beb9f65a19debecaf22fd470:
don't leak memory in run_permutation(), remember when we've issued
a cancel rather than issuing another one every 10ms,
fix some typos in comments.
Robert Haas [Thu, 11 Feb 2016 13:55:24 +0000 (08:55 -0500)]
Rename PGPROC fields related to group XID clearing again.
Commit 0e141c0fbb211bdd23783afa731e3eef95c9ad7a introduced a new
facility to reduce ProcArrayLock contention by clearing several XIDs
from the ProcArray under a single lock acquisition. The names
initially chosen were deemed not to be very good choices, so commit 4aec49899e5782247e134f94ce1c6ee926f88e1c renamed them. But now it
seems like we still didn't get it right. A pending patch wants to
add similar infrastructure for batching CLOG updates, so the names
need to be clear enough to allow a new set of structure members with
a related purpose.
Robert Haas [Thu, 11 Feb 2016 13:36:30 +0000 (08:36 -0500)]
Modify the isolation tester so that multiple sessions can wait.
This allows testing of deadlock scenarios. Scenarios that would
previously have been considered invalid are now simply taken as a
scenario in which more than one backend will wait.
Robert Haas [Thu, 11 Feb 2016 13:23:46 +0000 (08:23 -0500)]
Specify permutations for isolation tests with "invalid" permutations.
This is a necessary prerequisite for forthcoming changes to allow deadlock
scenarios to be tested by the isolation tester. It is also a good idea on
general principle, since these scenarios add no useful test coverage not
provided by other scenarios, but do to take time to execute.
Noah Misch [Thu, 11 Feb 2016 01:34:57 +0000 (20:34 -0500)]
In pg_rewind test suite, triple promote timeout to 90s.
Thirty seconds was not consistently enough for promotion to complete on
buildfarm members sungazer and tern. Experiments suggest 43s would have
been enough. Back-patch to 9.5, where pg_rewind was introduced.
Noah Misch [Thu, 11 Feb 2016 01:34:02 +0000 (20:34 -0500)]
Accept pg_ctl timeout from the PGCTLTIMEOUT environment variable.
Many automated test suites call pg_ctl. Buildfarm members axolotl,
hornet, mandrill, shearwater, sungazer and tern have failed when server
shutdown took longer than the pg_ctl default 60s timeout. This addition
permits slow hosts to easily raise the timeout without us editing a
--timeout argument into every test suite pg_ctl call. Back-patch to 9.1
(all supported versions) for the sake of automated testing.
Tom Lane [Thu, 11 Feb 2016 00:30:11 +0000 (19:30 -0500)]
Avoid use of sscanf() to parse ispell dictionary files.
It turns out that on FreeBSD-derived platforms (including OS X), the
*scanf() family of functions is pretty much brain-dead about multibyte
characters. In particular it will apply isspace() to individual bytes
of input even when those bytes are part of a multibyte character, thus
allowing false recognition of a field-terminating space.
We appear to have little alternative other than instituting a coding
rule that *scanf() is not to be used if the input string might contain
multibyte characters. (There was some discussion of relying on "%ls",
but that probably just moves the portability problem somewhere else,
and besides it doesn't fully prevent BSD *scanf() from using isspace().)
This patch is a down payment on that: it gets rid of use of sscanf()
to parse ispell dictionary files, which are certainly at great risk
of having a problem. The code is cleaner this way anyway, though
a bit longer.
In passing, improve a few comments.
Report and patch by Artur Zakirov, reviewed and somewhat tweaked by me.
Back-patch to all supported branches.
Robert Haas [Wed, 10 Feb 2016 14:58:09 +0000 (09:58 -0500)]
Code cleanup in the wake of recent LWLock refactoring.
As of commit c1772ad9225641c921545b35c84ee478c326b95e, there's no
longer any way of requesting additional LWLocks in the main tranche,
so we don't need NumLWLocks() or LWLockAssign() any more. Also,
some of the allocation counters that we had previously aren't needed
any more either.
Tom Lane [Wed, 10 Feb 2016 00:36:21 +0000 (19:36 -0500)]
Add still more chattiness in server shutdown.
Further investigation says that there may be some slow operations after
we've finished ShutdownXLOG(), so add some more log messages to try to
isolate that. This is all temporary code too.
Robert Haas [Tue, 9 Feb 2016 19:00:50 +0000 (14:00 -0500)]
postgres_fdw: Push down joins to remote servers.
If we've got a relatively straightforward join between two tables,
this pushes that join down to the remote server instead of fetching
the rows for each table and performing the join locally. Some cases
are not handled yet, such as SEMI and ANTI joins. Also, we don't
yet attempt to create presorted join paths or parameterized join
paths even though these options do get tried for a base relation
scan. Nevertheless, this seems likely to be a very significant win
in many practical cases.
Shigeru Hanada and Ashutosh Bapat, reviewed by Robert Haas, with
additional review at various points by Tom Lane, Etsuro Fujita,
KaiGai Kohei, and Jeevan Chalke.
Tom Lane [Tue, 9 Feb 2016 16:21:46 +0000 (11:21 -0500)]
Add more chattiness in server shutdown.
Early returns from the buildfarm show that there's a bit of a gap in the
logging I added in 3971f64843b02e4a: the portion of CreateCheckPoint()
after CheckPointGuts() can take a fair amount of time. Add a few more
log messages in that section of code. This too shall be reverted later.
Tom Lane [Mon, 8 Feb 2016 23:43:11 +0000 (18:43 -0500)]
Temporarily make pg_ctl and server shutdown a whole lot chattier.
This is a quick hack, due to be reverted when its purpose has been served,
to try to gather information about why some of the buildfarm critters
regularly fail with "postmaster does not shut down" complaints. Maybe they
are just really overloaded, but maybe something else is going on. Hence,
instrument pg_ctl to print the current time when it starts waiting for
postmaster shutdown and when it gives up, and add a lot of logging of the
current time in the server's checkpoint and shutdown code paths.
No attempt has been made to make this pretty. I'm not even totally sure
if it will build on Windows, but we'll soon find out.
Tom Lane [Mon, 8 Feb 2016 20:15:56 +0000 (15:15 -0500)]
Rename typedef "string" to "VarString".
Since pgindent treats typedef names as global, the original coding of b47b4dbf683f13e6 would have had rather nasty effects on the formatting
of other files in which "string" is used as a variable or field name.
Use a less generic name for this typedef, and rename some other
identifiers to match.
Tom Lane [Mon, 8 Feb 2016 15:25:40 +0000 (10:25 -0500)]
Fix some regex issues with out-of-range characters and large char ranges.
Previously, our regex code defined CHR_MAX as 0xfffffffe, which is a
bad choice because it is outside the range of type "celt" (int32).
Characters approaching that limit could lead to infinite loops in logic
such as "for (c = a; c <= b; c++)" where c is of type celt but the
range bounds are chr. Such loops will work safely only if CHR_MAX+1
is representable in celt, since c must advance to beyond b before the
loop will exit.
Fortunately, there seems no reason not to restrict CHR_MAX to 0x7ffffffe.
It's highly unlikely that Unicode will ever assign codes that high, and
none of our other backend encodings need characters beyond that either.
In addition to modifying the macro, we have to explicitly enforce character
range restrictions on the values of \u, \U, and \x escape sequences, else
the limit is trivially bypassed.
Also, the code for expanding case-independent character ranges in bracket
expressions had a potential integer overflow in its calculation of the
number of characters it could generate, which could lead to allocating too
small a character vector and then overwriting memory. An attacker with the
ability to supply arbitrary regex patterns could easily cause transient DOS
via server crashes, and the possibility for privilege escalation has not
been ruled out.
Quite aside from the integer-overflow problem, the range expansion code was
unnecessarily inefficient in that it always produced a result consisting of
individual characters, abandoning the knowledge that we had a range to
start with. If the input range is large, this requires excessive memory.
Change it so that the original range is reported as-is, and then we add on
any case-equivalent characters that are outside that range. With this
approach, we can bound the number of individual characters allowed without
sacrificing much. This patch allows at most 100000 individual characters,
which I believe to be more than the number of case pairs existing in
Unicode, so that the restriction will never be hit in practice.
It's still possible for range() to take awhile given a large character code
range, so also add statement-cancel detection to its loop. The downstream
function dovec() also lacked cancel detection, and could take a long time
given a large output from range().
Per fuzz testing by Greg Stark. Back-patch to all supported branches.
Fujii Masao [Mon, 8 Feb 2016 14:41:46 +0000 (23:41 +0900)]
Make GIN regression test stable.
Commit 7f46eaf added the regression test which checks that
gin_clean_pending_list() cleans up the GIN pending list and returns >0.
This usually works fine. But if autovacuum comes along and cleans
the list before gin_clean_pending_list() starts, the function may
return 0, and then the regression test may fail.
To fix the problem, this commit disables autovacuum on the target
index of gin_clean_pending_list() by setting autovacuum_enabled
reloption to off when creating the table.
Also this commit sets gin_pending_list_limit reloption to 4MB on
the target index. Otherwise when running "make installcheck" with
small gin_pending_list_limit GUC, insertions of data may trigger
the cleanup of pending list before gin_clean_pending_list() starts
and the function may return 0. This could cause the regression test
to fail.
Andres Freund [Mon, 8 Feb 2016 10:03:31 +0000 (11:03 +0100)]
Fix overeager pushdown of HAVING clauses when grouping sets are used.
In 61444bfb we started to allow HAVING clauses to be fully pushed down
into WHERE, even when grouping sets are in use. That turns out not to
work correctly, because grouping sets can "produce" NULLs, meaning that
filtering in WHERE and HAVING can have different results, even when no
aggregates or volatile functions are involved.
Instead only allow pushdown of empty grouping sets.
It'd be nice to do better, but the exact mechanics of deciding which
cases are safe are still being debated. It's important to give correct
results till we find a good solution, and such a solution might not be
appropriate for backpatching anyway.
Bug: #13863 Reported-By: 'wrb' Diagnosed-By: Dean Rasheed
Author: Andrew Gierth Reviewed-By: Dean Rasheed and Andres Freund
Discussion: 20160113183558.12989.56904@wrigleys.postgresql.org
Backpatch: 9.5, where grouping sets were introduced
Tom Lane [Sun, 7 Feb 2016 21:02:44 +0000 (16:02 -0500)]
Improve documentation about PRIMARY KEY constraints.
Get rid of the false implication that PRIMARY KEY is exactly equivalent to
UNIQUE + NOT NULL. That was more-or-less true at one time in our
implementation, but the standard doesn't say that, and we've grown various
features (many of them required by spec) that treat a pkey differently from
less-formal constraints. Per recent discussion on pgsql-general.
I failed to resist the temptation to do some other wordsmithing in the
same area.
Tom Lane [Sun, 7 Feb 2016 19:57:24 +0000 (14:57 -0500)]
Fix deparsing of ON CONFLICT arbiter WHERE clauses.
The parser doesn't allow qualification of column names appearing in
these clauses, but ruleutils.c would sometimes qualify them, leading
to dump/reload failures. Per bug #13891 from Onder Kalaci.
(In passing, make stanzas in ruleutils.c that save/restore varprefix
more consistent.)
Tom Lane [Sun, 7 Feb 2016 17:29:17 +0000 (12:29 -0500)]
ExecHashRemoveNextSkewBucket must physically copy tuples to main hashtable.
Commit 45f6240a8fa9d355 added an assumption in ExecHashIncreaseNumBatches
and ExecHashIncreaseNumBuckets that they could find all tuples in the main
hash table by iterating over the "dense storage" introduced by that patch.
However, ExecHashRemoveNextSkewBucket continued its old practice of simply
re-linking deleted skew tuples into the main table's hashchains. Hence,
such tuples got lost during any subsequent increase in nbatch or nbuckets,
and would never get joined, as reported in bug #13908 from Seth P.
I (tgl) think that the aforesaid commit has got multiple design issues
and should be reworked rather completely; but there is no time for that
right now, so band-aid the problem by making ExecHashRemoveNextSkewBucket
physically copy deleted skew tuples into the "dense storage" arena.
The added test case is able to exhibit the problem by means of fooling the
planner with a WHERE condition that it will underestimate the selectivity
of, causing the initial nbatch estimate to be too small.
Tomas Vondra and Tom Lane. Thanks to David Johnston for initial
investigation into the bug report.
Robert Haas [Sun, 7 Feb 2016 16:39:22 +0000 (11:39 -0500)]
Introduce a new GUC force_parallel_mode for testing purposes.
When force_parallel_mode = true, we enable the parallel mode restrictions
for all queries for which this is believed to be safe. For the subset of
those queries believed to be safe to run entirely within a worker, we spin
up a worker and run the query there instead of running it in the
original process. When force_parallel_mode = regress, make additional
changes to allow the regression tests to run cleanly even though parallel
workers have been injected under the hood.
Taken together, this facilitates both better user testing and better
regression testing of the parallelism code.
Robert Haas, with help from Amit Kapila and Rushabh Lathia.
Robert Haas [Sun, 7 Feb 2016 15:16:13 +0000 (10:16 -0500)]
Introduce group locking to prevent parallel processes from deadlocking.
For locking purposes, we now regard heavyweight locks as mutually
non-conflicting between cooperating parallel processes. There are some
possible pitfalls to this approach that are not to be taken lightly,
but it works OK for now and can be changed later if we find a better
approach. Without this, it's very easy for parallel queries to
silently self-deadlock if the user backend holds strong relation locks.
Robert Haas, with help from Amit Kapila. Thanks to Noah Misch and
Andres Freund for extensive discussion of possible issues with this
approach.
Tom Lane [Sun, 7 Feb 2016 04:11:28 +0000 (23:11 -0500)]
Improve speed of timestamp/time/date output functions.
It seems that sprintf(), at least in glibc's version, is unreasonably slow
compared to hand-rolled code for printing integers. Replacing most uses of
sprintf() in the datetime.c output functions with special-purpose code
turns out to give more than a 2X speedup in COPY of a table with a single
timestamp column; which is pretty impressive considering all the other
logic in that code path.
David Rowley and Andres Freund, reviewed by Peter Geoghegan and myself
Tom Lane [Sat, 6 Feb 2016 20:05:23 +0000 (15:05 -0500)]
Improve HJDEBUG code a bit.
Commit 30d7ae3c76d2de144232ae6ab328ca86b70e72c3 introduced an HJDEBUG
stanza that probably didn't compile at the time, and definitely doesn't
compile now, because it refers to a nonexistent variable. It doesn't seem
terribly useful anyway, so just get rid of it.
While I'm fooling with it, use %z modifier instead of the obsolete hack of
casting size_t to unsigned long, and include the HashJoinTable's address in
each printout so that it's possible to distinguish the activities of
multiple hashjoins occurring in one query.
Noted while trying to use HJDEBUG to investigate bug #13908. Back-patch
to 9.5, because code that doesn't compile is certainly not very helpful.
Noah Misch [Sat, 6 Feb 2016 01:22:51 +0000 (20:22 -0500)]
Force certain "pljava" custom GUCs to be PGC_SUSET.
Future PL/Java versions will close CVE-2016-0766 by making these GUCs
PGC_SUSET. This PostgreSQL change independently mitigates that PL/Java
vulnerability, helping sites that update PostgreSQL more frequently than
PL/Java. Back-patch to 9.1 (all supported versions).
Robert Haas [Fri, 5 Feb 2016 13:07:38 +0000 (08:07 -0500)]
Remove parallel-safety check from GetExistingLocalJoinPath.
Commit a104a017fc5f67ff5d9c374cd831ac3948a874c2 has this check because
I added it to the submitted patch before commit, but that was entirely
wrongheaded, as explained to me by Ashutosh Bapat, who also wrote this
patch.
Tom Lane [Fri, 5 Feb 2016 04:03:10 +0000 (23:03 -0500)]
Add num_nulls() and num_nonnulls() to count NULL arguments.
An example use-case is "CHECK(num_nonnulls(a,b,c) = 1)" to assert that
exactly one of a,b,c isn't NULL. The functions are variadic, so they
can also be pressed into service to count the number of null or nonnull
elements in an array.
Robert Haas [Fri, 5 Feb 2016 03:15:50 +0000 (22:15 -0500)]
postgres_fdw: Avoid possible misbehavior when RETURNING tableoid column only.
deparseReturningList ended up adding up RETURNING NULL to the code, but
code elsewhere saw an empty list of attributes and concluded that it
should not expect tuples from the remote side.
Etsuro Fujita and Robert Haas, reviewed by Thom Brown
Robert Haas [Thu, 4 Feb 2016 22:05:09 +0000 (17:05 -0500)]
Add some additional core functions to support join pushdown for FDWs.
GetExistingLocalJoinPath() is useful for handling EvalPlanQual rechecks
properly, and GetUserMappingById() is needed to make sure you're using
the right credentials.
Shigeru Hanada, Etsuro Fujita, Ashutosh Bapat, Robert Haas
Robert Haas [Thu, 4 Feb 2016 21:43:04 +0000 (16:43 -0500)]
Change the way that LWLocks for extensions are allocated.
The previous RequestAddinLWLocks() method had several disadvantages.
First, the locks would be in the main tranche; we've recently decided
that it's useful for LWLocks used for separate purposes to have
separate tranche IDs. Second, there wasn't any correlation between
what code called RequestAddinLWLocks() and what code called
LWLockAssign(); when multiple modules are in use, it could become
quite difficult to troubleshoot problems where LWLockAssign() ran out
of locks. To fix, create a concept of named LWLock tranches which
can be used either by extension or by core code.
Tom Lane [Thu, 4 Feb 2016 18:58:40 +0000 (13:58 -0500)]
Simplify syntax diagram for REINDEX.
Since there currently is only one possible parenthesized option, namely
VERBOSE, it's a bit pointless to show it with "{ } [, ... ]". The curly
braces are useless and therefore confusing, as seen in a recent question
from Karsten Hilbert. Remove the extra decoration for the time being;
we can put it back when and if REINDEX grows some more options.
Tom Lane [Thu, 4 Feb 2016 05:26:10 +0000 (00:26 -0500)]
In pg_dump, ensure that view triggers are processed after view rules.
If a view is split into CREATE TABLE + CREATE RULE to break a circular
dependency, then any triggers on the view must be dumped/reloaded after
the CREATE RULE; else the backend may reject the CREATE TRIGGER because
it's the wrong type of trigger for a plain table. This works all right
in plain dump/restore because of pg_dump's sorting heuristic that places
triggers after rules. However, when using parallel restore, the ordering
must be enforced by a dependency --- and we didn't have one.
Fixing this is a mere matter of adding an addObjectDependency() call,
except that we need to be able to find all the triggers belonging to the
view relation, and there was no easy way to do that. Add fields to
pg_dump's TableInfo struct to remember where the associated TriggerInfo
struct(s) are.
Per bug report from Dennis Kögel. The failure can be exhibited at least
as far back as 9.1, so back-patch to all supported branches.
Robert Haas [Wed, 3 Feb 2016 19:17:35 +0000 (14:17 -0500)]
Extend sortsupport for text to more opclasses.
Have varlena.c expose an interface that allows the char(n), bytea, and
bpchar types to piggyback on a now-generalized SortSupport for text.
This pushes a little more knowledge of the bpchar/char(n) type into
varlena.c than might be preferred, but that seems like the approach
that creates least friction. Also speed things up for index builds
that use text_pattern_ops or varchar_pattern_ops.
This patch does quite a bit of renaming, but it seems likely to be
worth it, so as to avoid future confusion about the fact that this code
is now more generally used than the old names might have suggested.
Peter Geoghegan, reviewed by Álvaro Herrera and Andreas Karlsson,
with small tweaks by me.
Robert Haas [Wed, 3 Feb 2016 17:46:18 +0000 (12:46 -0500)]
Allow parallel custom and foreign scans.
This patch doesn't put the new infrastructure to use anywhere, and
indeed it's not clear how it could ever be used for something like
postgres_fdw which has to send an SQL query and wait for a reply,
but there might be FDWs or custom scan providers that are CPU-bound,
so let's give them a way to join club parallel.
Tom Lane [Wed, 3 Feb 2016 17:03:50 +0000 (12:03 -0500)]
Make hstore_to_jsonb_loose match hstore_to_json_loose on what's a number.
Commit e09996ff8dee3f70 removed some ad-hoc code in hstore_to_json_loose
that determined whether an hstore value string looked like a number,
in favor of calling the JSON parser's is-it-a-number code. However,
it neglected the fact that the exact same code appeared in
hstore_to_jsonb_loose.
This is not a bug, exactly, because the requirements on the two functions
are not the same: hstore_to_json_loose must accept only syntactically legal
JSON numbers as numbers, or it will produce invalid JSON output, as per bug
#12070 which spawned the prior commit. But hstore_to_jsonb_loose could
accept anything that numeric_in will eat, other than Inf and NaN.
Nonetheless it seems surprising and arbitrary that the two functions don't
use the same rules for what is a number versus what is a string; especially
since they did use the same rules before the aforesaid commit. For one
thing, that means that doing hstore_to_json_loose and then casting to jsonb
can produce results different from doing just hstore_to_jsonb_loose.
Hence, change hstore_to_jsonb_loose's logic to match hstore_to_json_loose,
ie, hstore values are treated as numbers when they match the JSON syntax
for numbers.
No back-patch, since this is more in the nature of a definitional change
than a bug fix.
Remove duplicate assignment. This part by Ashutosh Bapat.
Remove now-obsolete comment. This part by me, although the pending
join pushdown patch does something similar, and for the same reason:
there's no reason to keep two lists of the things in the fdw_private
structure that have to be kept in sync with each other.
Robert Haas [Wed, 3 Feb 2016 15:38:50 +0000 (10:38 -0500)]
Remove CustomPath's TextOutCustomPath method.
You can't really do anything useful with this in the form it currently
exists; among other problems, there's no way to reread whatever
information might be produced when the path is output. Work is
underway to replace this with a more useful and more general system of
extensible nodes, but let's start by getting rid of this bit.
Tom Lane [Wed, 3 Feb 2016 06:39:08 +0000 (01:39 -0500)]
Fix IsValidJsonNumber() to notice trailing non-alphanumeric garbage.
Commit e09996ff8dee3f70 was one brick shy of a load: it didn't insist
that the detected JSON number be the whole of the supplied string.
This allowed inputs such as "2016-01-01" to be misdetected as valid JSON
numbers. Per bug #13906 from Dmitry Ryabov.
In passing, be more wary of zero-length input (I'm not sure this can
happen given current callers, but better safe than sorry), and do some
minor cosmetic cleanup.
Peter Eisentraut [Tue, 17 Nov 2015 11:46:17 +0000 (06:46 -0500)]
Add support for systemd service notifications
Insert sd_notify() calls at server start and stop for integration with
systemd. This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
Reviewed-by: Pavel Stěhule <pavel.stehule@gmail.com>
Peter Eisentraut [Tue, 17 Nov 2015 11:47:18 +0000 (06:47 -0500)]
Improve error reporting when location specified by postgres -D does not exist
Previously, the first error seen would be that postgresql.conf does not
exist. But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
Tom Lane [Tue, 2 Feb 2016 20:26:21 +0000 (15:26 -0500)]
Remove printQueryOpt.quote field.
This field was included in the original definition of the printQueryOpt
struct in commit a45195a191eec367, but it was not used anywhere in that
commit, nor since then. Spotted by Dickson S. Guedes.
Alvaro Herrera [Tue, 2 Feb 2016 18:20:02 +0000 (19:20 +0100)]
Don't test for system columns on join relations
create_foreignscan_plan needs to know whether any system columns are
requested from a relation (this flag is needed by ForeignNext during
execution). However, for join relations this is a pointless test,
because it's not possible to request system columns from them, so
remove the check.
Author: Etsuro Fujita
Discussion: http://www.postgresql.org/message-id/56AA0FC5.9000207@lab.ntt.co.jp Reviewed-by: David Rowley, Robert Haas
Tom Lane [Tue, 2 Feb 2016 16:52:27 +0000 (11:52 -0500)]
Remove unnecessary "implementation of FOO operator" DESCR() entries.
Apparently at least one committer hasn't gotten the word that these do not
need to be maintained by hand, since initdb will create them automatically.
Noted while fixing bug #13905.
No catversion bump since the post-initdb state is exactly the same either
way. I don't see a need for back-patch, either.
Tom Lane [Tue, 2 Feb 2016 16:39:50 +0000 (11:39 -0500)]
Fix pg_description entries for jsonb_to_record() and jsonb_to_recordset().
All the other jsonb function descriptions refer to the arguments as being
"jsonb", but these two said "json". Make it consistent. Per bug #13905
from Petru Florin Mihancea.
No catversion bump --- we can't force one in the back branches, and this
isn't very critical anyway.
Teodor Sigaev [Tue, 2 Feb 2016 12:20:33 +0000 (15:20 +0300)]
Fix lossy KNN GiST when ordering operator returns non-float8 value.
KNN GiST with recheck flag should return to executor the same type as ordering
operator, GiST detects this type by looking to return type of function which
implements ordering operator. But occasionally detecting code works after
replacing ordering operator function to distance support function.
Distance support function always returns float8, so, detecting code get float8
instead of actual return type of ordering operator.
Built-in opclasses don't have ordering operator which doesn't return
non-float8 value, so, tests are impossible here, at least now.
Backpatch to 9.5 where lozzy KNN was introduced.
Author: Alexander Korotkov
Report by: Artur Zakirov
Alvaro Herrera [Mon, 1 Feb 2016 14:55:33 +0000 (15:55 +0100)]
pgbench: allow per-script statistics
Provide per-script statistical info (count of transactions executed
under that script, average latency for the whole script) after a
multi-script run, adding an intermediate level of detail to existing
global stats and per-command stats.
Fujii Masao [Mon, 1 Feb 2016 13:19:51 +0000 (22:19 +0900)]
Various fixes to "ALTER ... SET/RESET" tab completions
Add
- ALTER SYSTEM SET/RESET ... -> GUC variables
- ALTER TABLE ... SET WITH -> OIDS
- ALTER DATABASE/FUNCTION/ROLE/USER ... SET/RESET -> GUC variables
- ALTER DATABASE/FUNCTION/ROLE/USER ... SET ... -> FROM CURRENT/TO
- ALTER DATABASE/FUNCTION/ROLE/USER ... SET ... TO/= -> possible values
Author: Fujii Masao Reviewed-by: Michael Paquier, Masahiko Sawada
Michael Meskes [Mon, 1 Feb 2016 12:10:40 +0000 (13:10 +0100)]
Make sure ecpg header files do not have a comment lasting several lines, one of
which is a preprocessor directive. This leads ecpg to incorrectly parse the comment as nested.
Robert Haas [Sat, 30 Jan 2016 15:32:38 +0000 (10:32 -0500)]
postgres_fdw: More preliminary refactoring for upcoming join pushdown.
The code that generates a complete SQL query for a given foreign relation
was repeated in two places, and they didn't quite agree: the EXPLAIN case
left out the locking clause. Centralize the code so we get the same
behavior everywhere, and adjust calling conventions and which functions
are static vs. extern accordingly . Centralize the code so we get the same
behavior everywhere, and adjust calling conventions and which functions
are static vs. extern accordingly.
Ashutosh Bapat, reviewed and slightly adjusted by me.
Robert Haas [Fri, 29 Jan 2016 13:10:47 +0000 (08:10 -0500)]
Migrate PGPROC's backendLock into PGPROC itself, using a new tranche.
Previously, each PGPROC's backendLock was part of the main tranche,
and the PGPROC just contained a pointer. Now, the actual LWLock is
part of the PGPROC.
As with previous, similar patches, this makes it significantly easier
to identify these lwlocks in LWLOCK_STATS or Trace_lwlocks output
and improves modularity.
Author: Ildus Kurbangaliev Reviewed-by: Amit Kapila, Robert Haas
Tom Lane [Fri, 29 Jan 2016 09:28:02 +0000 (10:28 +0100)]
Fix incorrect pattern-match processing in psql's \det command.
listForeignTables' invocation of processSQLNamePattern did not match up
with the other ones that handle potentially-schema-qualified names; it
failed to make use of pg_table_is_visible() and also passed the name
arguments in the wrong order. Bug seems to have been aboriginal in commit 0d692a0dc9f0e532. It accidentally sort of worked as long as you didn't
inquire too closely into the behavior, although the silliness was later
exposed by inconsistencies in the test queries added by 59efda3e50ca4de6
(which I probably should have questioned at the time, but didn't).
Per bug #13899 from Reece Hart. Patch by Reece Hart and Tom Lane.
Back-patch to all affected branches.