]> granicus.if.org Git - postgresql/log
postgresql
8 years agoFix unwanted flushing of libpq's input buffer when socket EOF is seen.
Tom Lane [Thu, 12 Nov 2015 18:03:52 +0000 (13:03 -0500)]
Fix unwanted flushing of libpq's input buffer when socket EOF is seen.

In commit 210eb9b743c0645d I centralized libpq's logic for closing down
the backend communication socket, and made the new pqDropConnection
routine always reset the I/O buffers to empty.  Many of the call sites
previously had not had such code, and while that amounted to an oversight
in some cases, there was one place where it was intentional and necessary
*not* to flush the input buffer: pqReadData should never cause that to
happen, since we probably still want to process whatever data we read.

This is the true cause of the problem Robert was attempting to fix in
c3e7c24a1d60dc6a, namely that libpq no longer reported the backend's final
ERROR message before reporting "server closed the connection unexpectedly".
But that only accidentally fixed it, by invoking parseInput before the
input buffer got flushed; and very likely there are timing scenarios
where we'd still lose the message before processing it.

To fix, pass a flag to pqDropConnection to tell it whether to flush the
input buffer or not.  On review I think flushing is actually correct for
every other call site.

Back-patch to 9.3 where the problem was introduced.  In HEAD, also improve
the comments added by c3e7c24a1d60dc6a.

8 years agolibpq: Notice errors a backend may have sent just before dying.
Robert Haas [Thu, 12 Nov 2015 14:12:18 +0000 (09:12 -0500)]
libpq: Notice errors a backend may have sent just before dying.

At least since the introduction of Hot Standby, the backend has
sometimes sent fatal errors even when no client query was in
progress, assuming that the client would receive it.  However,
pqHandleSendFailure was not in sync with this assumption, and
only tries to catch notices and notifies.  Add a parseInput call
to the loop there to fix.

Andres Freund suggested the fix.  Comments are by me.
Reviewed by Michael Paquier.

8 years agoMake idle backends exit if the postmaster dies.
Robert Haas [Thu, 12 Nov 2015 14:00:33 +0000 (09:00 -0500)]
Make idle backends exit if the postmaster dies.

Letting backends continue to run if the postmaster has exited prevents
PostgreSQL from being restarted, which in many environments is
catastrophic.  Worse, if some other backend crashes, we no longer have
any protection against shared memory corruption.  So, arrange for them
to exit instead.  We don't want to expend many cycles on this, but
including postmaster death in the set of things that we wait for when
a backend is idle seems cheap enough.

Rajeev Rastogi and Robert Haas

8 years agoProvide readfuncs support for custom scans.
Robert Haas [Thu, 12 Nov 2015 12:40:31 +0000 (07:40 -0500)]
Provide readfuncs support for custom scans.

Commit a0d9f6e434bb56f7e5441b7988f3982feead33b3 added this support for
all other plan node types; this fills in the gap.

Since TextOutCustomScan complicates this and is pretty well useless,
remove it.

KaiGai Kohei, with some modifications by me.

8 years agoDo a round of copy-editing on the 9.5 release notes.
Tom Lane [Thu, 12 Nov 2015 00:19:14 +0000 (19:19 -0500)]
Do a round of copy-editing on the 9.5 release notes.

Also fill in the previously empty "major enhancements" list.  YMMV as to
which items should make the cut, but it's past time we had something more
than a placeholder here.

(I meant to get this done before beta2 was wrapped, but got distracted by
PDF build problems.  Better late than never.)

8 years agoImprove documentation around autovacuum-related storage parameters.
Tom Lane [Wed, 11 Nov 2015 22:13:38 +0000 (17:13 -0500)]
Improve documentation around autovacuum-related storage parameters.

These were discussed in three different sections of the manual, which
unsurprisingly had diverged over time; and the descriptions of individual
variables lacked stylistic consistency even within each section (and
frequently weren't in very good English anyway).  Clean up the mess, and
remove some of the redundant information in hopes that future additions
will be less likely to re-introduce inconsistency.  For instance I see
no need for maintenance.sgml to include its very own list of all the
autovacuum storage parameters, especially since that list was already
incomplete.

8 years agoBe more noisy about "wrong number of nailed relations" initfile problems.
Tom Lane [Wed, 11 Nov 2015 18:39:21 +0000 (13:39 -0500)]
Be more noisy about "wrong number of nailed relations" initfile problems.

In commit 5d1ff6bd559ea8df1b7302e245e690b01b9a4fa4 I added some logic to
relcache.c to try to ensure that the regression tests would fail if we
made a mistake about which relations belong in the relcache init files.
I'm quite sure I tested that, but I must have done so only for the
non-shared-catalog case, because a report from Adam Brightwell showed that
the regression tests still pass just fine if we bollix the shared-catalog
init file in the way this code was supposed to catch.  The reason is that
that file gets loaded before we do client authentication, so the WARNING
is not sent to the client, only to the postmaster log, where it's far too
easily missed.

The least Rube Goldbergian answer to this is to put an Assert(false)
after the elog(WARNING).  That will certainly get developers' attention,
while not breaking production builds' ability to recover from corner
cases with similar symptoms.

Since this is only of interest to developers, there seems no need for
a back-patch, even though the previous commit went into all branches.

8 years agoGenerate parallel sequential scan plans in simple cases.
Robert Haas [Wed, 11 Nov 2015 14:02:52 +0000 (09:02 -0500)]
Generate parallel sequential scan plans in simple cases.

Add a new flag, consider_parallel, to each RelOptInfo, indicating
whether a plan for that relation could conceivably be run inside of
a parallel worker.  Right now, we're pretty conservative: for example,
it might be possible to defer applying a parallel-restricted qual
in a worker, and later do it in the leader, but right now we just
don't try to parallelize access to that relation.  That's probably
the right decision in most cases, anyway.

Using the new flag, generate parallel sequential scan plans for plain
baserels, meaning that we now have parallel sequential scan in
PostgreSQL.  The logic here is pretty unsophisticated right now: the
costing model probably isn't right in detail, and we can't push joins
beneath Gather nodes, so the number of plans that can actually benefit
from this is pretty limited right now.  Lots more work is needed.
Nevertheless, it seems time to enable this functionality so that all
this code can actually be tested easily by users and developers.

Note that, if you wish to test this functionality, it will be
necessary to set max_parallel_degree to a value greater than the
default of 0.  Once a few more loose ends have been tidied up here, we
might want to consider changing the default value of this GUC, but
I'm leaving it alone for now.

Along the way, fix a bug in cost_gather: the previous coding thought
that a Gather node's transfer overhead should be costed on the basis of
the relation size rather than the number of tuples that actually need
to be passed off to the leader.

Patch by me, reviewed in earlier versions by Amit Kapila.

8 years agoMake sequential scans parallel-aware.
Robert Haas [Wed, 11 Nov 2015 13:57:52 +0000 (08:57 -0500)]
Make sequential scans parallel-aware.

In addition, this path fills in a number of missing bits and pieces in
the parallel infrastructure.  Paths and plans now have a parallel_aware
flag indicating whether whatever parallel-aware logic they have should
be engaged.  It is believed that we will need this flag for a number of
path/plan types, not just sequential scans, which is why the flag is
generic rather than part of the SeqScan structures specifically.
Also, execParallel.c now gives parallel nodes a chance to initialize
their PlanState nodes from the DSM during parallel worker startup.

Amit Kapila, with a fair amount of adjustment by me.  Review of previous
patch versions by Haribabu Kommi and others.

8 years agoAdd outfuncs.c support for GatherPath.
Robert Haas [Wed, 11 Nov 2015 11:29:03 +0000 (06:29 -0500)]
Add outfuncs.c support for GatherPath.

I dunno how commit 3bd909b220930f21d6e15833a17947be749e7fde missed
this, but it evidently did.

8 years agoDocs: fix misleading example.
Tom Lane [Wed, 11 Nov 2015 03:11:39 +0000 (22:11 -0500)]
Docs: fix misleading example.

Commit 8457d0beca731bf0 introduced an example which, while not incorrect,
failed to exhibit the behavior it meant to describe, as a result of omitting
an E'' prefix that needed to be there.  Noticed and fixed by Peter Geoghegan.

I (tgl) failed to resist the temptation to wordsmith nearby text a bit
while at it.

8 years agoAdd missing "static" qualifier.
Tom Lane [Tue, 10 Nov 2015 23:24:18 +0000 (18:24 -0500)]
Add missing "static" qualifier.

Per buildfarm member pademelon.

8 years agoImprove our workaround for 'TeX capacity exceeded' in building PDF files.
Tom Lane [Tue, 10 Nov 2015 20:59:59 +0000 (15:59 -0500)]
Improve our workaround for 'TeX capacity exceeded' in building PDF files.

In commit a5ec86a7c787832d28d5e50400ec96a5190f2555 I wrote a quick hack
that reduced the number of TeX string pool entries created while converting
our documentation to PDF form.  That held the fort for awhile, but as of
HEAD we're back up against the same limitation.  It turns out that the
original coding of \FlowObjectSetup actually results in *three* string pool
entries being generated for every "flow object" (that is, potential
cross-reference target) in the documentation, and my previous hack only got
rid of one of them.  With a little more care, we can reduce the string
count to one per flow object plus one per actually-cross-referenced flow
object (about 115000 + 5000 as of current HEAD); that should work until
the documentation volume roughly doubles from where it is today.

As a not-incidental side benefit, this change also causes pdfjadetex to
stop emitting unreferenced hyperlink anchors (bookmarks) into the PDF file.
It had been making one willy-nilly for every flow object; now it's just one
per actually-cross-referenced object.  This results in close to a 2X
savings in PDF file size.  We will still want to run the output through
"jpdftweak" to get it to be compressed; but we no longer need removal of
unreferenced bookmarks, so we might be able to find a quicker tool for
that step.

Although the failure only affects HEAD and US-format output at the moment,
9.5 cannot be more than a few pages short of failing likewise, so it
will inevitably fail after a few rounds of minor-version release notes.
I don't have a lot of faith that we'll never hit the limit in the older
branches; and anyway it would be nice to get rid of jpdftweak across the
board.  Therefore, back-patch to all supported branches.

8 years agoComment update.
Robert Haas [Mon, 9 Nov 2015 18:48:50 +0000 (13:48 -0500)]
Comment update.

Adjust to account for 5fc4c26db5120bd90348b6ee3101fcddfdf54800.

Etsuro Fujita

8 years agoFix rebasing mistake in nodeGather.c
Robert Haas [Mon, 9 Nov 2015 15:49:24 +0000 (10:49 -0500)]
Fix rebasing mistake in nodeGather.c

The patches committed as 6e71dd7ce9766582da453f493bc371d64977282f
and 3a1f8611f2582df0a16bcd35caed2e1526387643 were developed in
parallel but dependent on each other in a way that I failed to
notice.

This patch to fix the problem was prepared by Amit Kapila.

8 years agoAdd a dummy return statement to TupleQueueRemap.
Robert Haas [Mon, 9 Nov 2015 15:45:32 +0000 (10:45 -0500)]
Add a dummy return statement to TupleQueueRemap.

This is unreachable for multiple reasons, but per Amit Kapila the
Windows compiler he is using still thinks we can get there.

8 years agoAdd paragraph about ON CONFLICT interaction with partitioning.
Andres Freund [Mon, 9 Nov 2015 04:08:56 +0000 (05:08 +0100)]
Add paragraph about ON CONFLICT interaction with partitioning.

Author: Peter Geoghegan and Andres Freund
Discussion: CAM3SWZScpWzQ-7EJC77vwqzZ1GO8GNmURQ1QqDQ3wRn7AbW1Cg@mail.gmail.com,
    CAHGQGwFUCWwSU7dtc2aRdRk73ztyr_jY5cPOyts+K8xKJ92X4Q@mail.gmail.com
Backpatch: 9.5, where UPSERT was introduced

8 years agoSet replication origin when decoding commit records.
Andres Freund [Sun, 8 Nov 2015 22:01:53 +0000 (23:01 +0100)]
Set replication origin when decoding commit records.

By accident the replication origin was not set properly in
DecodeCommit(). That's bad because the origin is passed to the output
plugins origin filter, and accessible from the output plugin via
ReorderBufferTXN->origin_id.  Accessing the origin of individual changes
worked before the fix, which is why this wasn't notices earlier.

Reported-By: Craig Ringer
Author: Craig Ringer
Discussion: CAMsr+YFhBJLp=qfSz3-J+0P1zLkE8zNXM2otycn20QRMx380gw@mail.gmail.com
Backpatch: 9.5, where replication origins where introduced

8 years agoDon't connect() to a wildcard address in test_postmaster_connection().
Noah Misch [Sun, 8 Nov 2015 22:28:53 +0000 (17:28 -0500)]
Don't connect() to a wildcard address in test_postmaster_connection().

At least OpenBSD, NetBSD, and Windows don't support it.  This repairs
pg_ctl for listen_addresses='0.0.0.0' and listen_addresses='::'.  Since
pg_ctl prefers to test a Unix-domain socket, Windows users are most
likely to need this change.  Back-patch to 9.1 (all supported versions).
This could change pg_ctl interaction with loopback-interface firewall
rules.  Therefore, in 9.4 and earlier (released branches), activate the
change only on known-affected platforms.

Reported (bug #13611) and designed by Kondo Yuta.

8 years agoRemove set-but-not-used variables.
Robert Haas [Sun, 8 Nov 2015 01:25:32 +0000 (20:25 -0500)]
Remove set-but-not-used variables.

Reported by both Peter Eisentraunt and Kevin Grittner.

8 years agoUpdate 9.5 release notes through today.
Tom Lane [Sat, 7 Nov 2015 22:09:04 +0000 (17:09 -0500)]
Update 9.5 release notes through today.

8 years agoAdd "xid <> xid" and "xid <> int4" operators.
Tom Lane [Sat, 7 Nov 2015 21:40:15 +0000 (16:40 -0500)]
Add "xid <> xid" and "xid <> int4" operators.

The corresponding "=" operators have been there a long time, and not
having their negators is a bit of a nuisance.

Michael Paquier

8 years agoRename PQsslAttributes() to PQsslAttributeNames(), and const-ify fully.
Tom Lane [Sat, 7 Nov 2015 21:13:49 +0000 (16:13 -0500)]
Rename PQsslAttributes() to PQsslAttributeNames(), and const-ify fully.

Per discussion, the original name was a bit misleading, and
PQsslAttributeNames() seems more apropos.  It's not quite too late to
change this in 9.5, so let's change it while we can.

Also, make sure that the pointer array is const, not only the pointed-to
strings.

Minor documentation wordsmithing while at it.

Lars Kanis, slight adjustments by me

8 years agoFix enforcement of restrictions inside regexp lookaround constraints.
Tom Lane [Sat, 7 Nov 2015 17:43:24 +0000 (12:43 -0500)]
Fix enforcement of restrictions inside regexp lookaround constraints.

Lookahead and lookbehind constraints aren't allowed to contain backrefs,
and parentheses within them are always considered non-capturing.  Or so
says the manual.  But the regexp parser forgot about these rules once
inside a parenthesized subexpression, so that constructs like (\w)(?=(\1))
were accepted (but then not correctly executed --- a case like this acted
like (\w)(?=\w), without any enforcement that the two \w's match the same
text).  And in (?=((foo))) the innermost parentheses would be counted as
capturing parentheses, though no text would ever be captured for them.

To fix, properly pass down the "type" argument to the recursive invocation
of parse().

Back-patch to all supported branches; it was agreed that silent
misexecution of such patterns is worse than throwing an error, even though
new errors in minor releases are generally not desirable.

8 years agoTry to convince gcc that TupleQueueRemap never falls off the end.
Robert Haas [Sat, 7 Nov 2015 04:04:21 +0000 (23:04 -0500)]
Try to convince gcc that TupleQueueRemap never falls off the end.

Without this, MacOS gcc version 4.2.1 isn't convinced.

8 years agoWhen completing ALTER INDEX .. SET, add an equals sign also.
Robert Haas [Sat, 7 Nov 2015 03:59:47 +0000 (22:59 -0500)]
When completing ALTER INDEX .. SET, add an equals sign also.

Jeff Janes

8 years agoModify tqueue infrastructure to support transient record types.
Robert Haas [Fri, 6 Nov 2015 21:58:45 +0000 (16:58 -0500)]
Modify tqueue infrastructure to support transient record types.

Commit 4a4e6893aa080b9094dadbe0e65f8a75fee41ac6, which introduced this
mechanism, failed to account for the fact that the RECORD pseudo-type
uses transient typmods that are only meaningful within a single
backend.  Transferring such tuples without modification between two
cooperating backends does not work.  This commit installs a system
for passing the tuple descriptors over the same shm_mq being used to
send the tuples themselves.  The two sides might not assign the same
transient typmod to any given tuple descriptor, so we must also
substitute the appropriate receiver-side typmod for the one used by
the sender.  That adds some CPU overhead, but still seems better than
being unable to pass records between cooperating parallel processes.

Along the way, move the logic for handling multiple tuple queues from
tqueue.c to nodeGather.c; tqueue.c now provides a TupleQueueReader,
which reads from a single queue, rather than a TupleQueueFunnel, which
potentially reads from multiple queues.  This change was suggested
previously as a way to make sure that nodeGather.c rather than tqueue.c
had policy control over the order in which to read from queues, but
it wasn't clear to me until now how good an idea it was.  typmod
mapping needs to be performed separately for each queue, and it is
much simpler if the tqueue.c code handles that and leaves multiplexing
multiple queues to higher layers of the stack.

8 years agoRemove unnecessary cast in previous commit.
Robert Haas [Fri, 6 Nov 2015 17:17:31 +0000 (12:17 -0500)]
Remove unnecessary cast in previous commit.

Noted by Kyotaro Horiguchi, who also reviewed the previous patch, but
I failed to notice his review before committing.

8 years agoAdd sort support routine for the UUID data type.
Robert Haas [Fri, 6 Nov 2015 17:10:42 +0000 (12:10 -0500)]
Add sort support routine for the UUID data type.

This introduces a simple encoding scheme to produce abbreviated keys:
pack as many bytes of each UUID as will fit into a Datum.  On
little-endian machines, a byteswap is also performed; the abbreviated
comparator can therefore just consist of a simple 3-way unsigned integer
comparison.

The purpose of this change is to speed up sorting data on a column
of type UUID.

Peter Geoghegan

8 years agoSet include_realm=1 default in parse_hba_line
Stephen Frost [Fri, 6 Nov 2015 16:18:27 +0000 (11:18 -0500)]
Set include_realm=1 default in parse_hba_line

With include_realm=1 being set down in parse_hba_auth_opt, if multiple
options are passed on the pg_hba line, such as:

host all     all    0.0.0.0/0    gss include_realm=0 krb_realm=XYZ.COM

We would mistakenly reset include_realm back to 1.  Instead, we need to
set include_realm=1 up in parse_hba_line, prior to parsing any of the
additional options.

Discovered by Jeff McCormick during testing.

Bug introduced by 9a08841.

Back-patch to 9.5

8 years agopg_size_pretty: Format negative values similar to positive ones.
Robert Haas [Fri, 6 Nov 2015 16:03:02 +0000 (11:03 -0500)]
pg_size_pretty: Format negative values similar to positive ones.

Previously, negative values were always displayed in bytes, regardless
of how large they were.

Adrian Vondendriesch, reviewed by Julien Rouhaud and myself

8 years agoDocument interaction of bgworkers with LISTEN/NOTIFY.
Robert Haas [Fri, 6 Nov 2015 05:31:03 +0000 (00:31 -0500)]
Document interaction of bgworkers with LISTEN/NOTIFY.

Thomas Munro and Robert Haas, reviewed by Haribabu Kommi

8 years agoFix erroneous hash calculations in gin_extract_jsonb_path().
Tom Lane [Thu, 5 Nov 2015 23:15:48 +0000 (18:15 -0500)]
Fix erroneous hash calculations in gin_extract_jsonb_path().

The jsonb_path_ops code calculated hash values inconsistently in some cases
involving nested arrays and objects.  This would result in queries possibly
not finding entries that they should find, when using a jsonb_path_ops GIN
index for the search.  The problem cases involve JSONB values that contain
both scalars and sub-objects at the same nesting level, for example an
array containing both scalars and sub-arrays.  To fix, reset the current
stack->hash after processing each value or sub-object, not before; and
don't try to be cute about the outermost level's initial hash.

Correcting this means that existing jsonb_path_ops indexes may now be
inconsistent with the new hash calculation code.  The symptom is the same
--- searches not finding entries they should find --- but the specific
rows affected are likely to be different.  Users will need to REINDEX
jsonb_path_ops indexes to make sure that all searches work as expected.

Per bug #13756 from Daniel Cheng.  Back-patch to 9.4 where the faulty
logic was introduced.

8 years agoFix memory leaks in PL/Python.
Tom Lane [Thu, 5 Nov 2015 18:52:30 +0000 (13:52 -0500)]
Fix memory leaks in PL/Python.

Previously, plpython was in the habit of allocating a lot of stuff in
TopMemoryContext, and it was very slipshod about making sure that stuff
got cleaned up; in particular, use of TopMemoryContext as fn_mcxt for
function calls represents an unfixable leak, since we generally don't
know what the called function might have allocated in fn_mcxt.  This
results in session-lifespan leakage in certain usage scenarios, as for
example in a case reported by Ed Behn back in July.

To fix, get rid of all the retail allocations in TopMemoryContext.
All long-lived allocations are now made in sub-contexts that are
associated with specific objects (either pl/python procedures, or
Python-visible objects such as cursors and plans).  We can clean these
up when the associated object is deleted.

I went so far as to get rid of PLy_malloc completely.  There were a
couple of places where it could still have been used safely, but on
the whole it was just an invitation to bad coding.

Haribabu Kommi, based on a draft patch by Heikki Linnakangas;
some further work by me

8 years agoPass extra data to bgworkers, and use this to fix parallel contexts.
Robert Haas [Thu, 5 Nov 2015 17:05:38 +0000 (12:05 -0500)]
Pass extra data to bgworkers, and use this to fix parallel contexts.

Up until now, the total amount of data that could be passed to a
background worker at startup was one datum, which can be a small as
4 bytes on some systems.  That's enough to pass a dsm_handle or an
array index, but not much else.  Add a bgw_extra flag to the
BackgroundWorker struct, allowing up to 128 bytes to be passed to
a new worker on any platform.

Use this to fix a problem I recently discovered with the parallel
context machinery added in 9.5: the master assigns each worker an
array index, and each worker subsequently assigns itself an array
index, and there's nothing to guarantee that the two sets of indexes
match, leading to chaos.

Normally, I would not back-patch the change to add bgw_extra, since it
is basically a feature addition.  However, since 9.5 is still in beta
and there seems to be no other sensible way to repair the broken
parallel context machinery, back-patch to 9.5.  Existing background
worker code can ignore the bgw_extra field without a problem, but
might need to be recompiled since the structure size has changed.

Report and patch by me.  Review by Amit Kapila.

8 years agoImprove implementation of GEQO's init_tour() function.
Tom Lane [Thu, 5 Nov 2015 15:46:14 +0000 (10:46 -0500)]
Improve implementation of GEQO's init_tour() function.

Rather than filling a temporary array and then copying values to the
output array, we can generate the required random permutation in-place
using the Fisher-Yates shuffle algorithm.  This is shorter as well as
more efficient than before.  It's pretty unlikely that anyone would
notice a speed improvement, but shorter code is better.

Nathan Wagner, edited a bit by me

8 years agoUpdate spelling of COPY options
Peter Eisentraut [Thu, 5 Nov 2015 02:01:26 +0000 (21:01 -0500)]
Update spelling of COPY options

The preferred spelling was changed from FORCE QUOTE to FORCE_QUOTE and
the like, but some code was still referring to the old spellings.

8 years agoAdd regression tests for remote execution of extension operators/functions.
Tom Lane [Wed, 4 Nov 2015 17:03:30 +0000 (12:03 -0500)]
Add regression tests for remote execution of extension operators/functions.

Rather than relying on other extensions to be available for installation,
let's just add some test objects to the postgres_fdw extension itself
within the regression script.

8 years agoAllow postgres_fdw to ship extension funcs/operators for remote execution.
Tom Lane [Tue, 3 Nov 2015 23:42:02 +0000 (18:42 -0500)]
Allow postgres_fdw to ship extension funcs/operators for remote execution.

The user can whitelist specified extension(s) in the foreign server's
options, whereupon we will treat immutable functions and operators of those
extensions as candidates to be sent for remote execution.

Whitelisting an extension in this way basically promises that the extension
exists on the remote server and behaves compatibly with the local instance.
We have no way to prove that formally, so we have to rely on the user to
get it right.  But this seems like something that people can usually get
right in practice.

We might in future allow functions and operators to be whitelisted
individually, but extension granularity is a very convenient special case,
so it got done first.

The patch as-committed lacks any regression tests, which is unfortunate,
but introducing dependencies on other extensions for testing purposes
would break "make installcheck" scenarios, which is worse.  I have some
ideas about klugy ways around that, but it seems like material for a
separate patch.  For the moment, leave the problem open.

Paul Ramsey, hacked up a bit more by me

8 years agoImprove comments about abbreviation abort.
Robert Haas [Tue, 3 Nov 2015 19:11:49 +0000 (14:11 -0500)]
Improve comments about abbreviation abort.

Peter Geoghegan

8 years agopostgres_fdw: Add ORDER BY to some remote SQL queries.
Robert Haas [Tue, 3 Nov 2015 17:46:06 +0000 (12:46 -0500)]
postgres_fdw: Add ORDER BY to some remote SQL queries.

If the join problem's entire ORDER BY clause can be pushed to the
remote server, consider a path that adds this ORDER BY clause.  If
use_remote_estimate is on, we cost this path using an additional
remote EXPLAIN.  If not, we just estimate that the path costs 20%
more, which is intended to be large enough that we won't request a
remote sort when it's not helpful, but small enough that we'll have
the remote side do the sort when in doubt.  In some cases, the remote
sort might actually be free, because the remote query plan might
happen to produce output that is ordered the way we need, but without
remote estimates we have no way of knowing that.

It might also be useful to request sorted output from the remote side
if it enables an efficient merge join, but this patch doesn't attempt
to handle that case.

Ashutosh Bapat with revisions by me.  Also reviewed by Fabrízio de Royes
Mello and Jeevan Chalke.

8 years agoRemove obsolete advice about doubling backslashes in regex escapes.
Tom Lane [Tue, 3 Nov 2015 16:57:56 +0000 (11:57 -0500)]
Remove obsolete advice about doubling backslashes in regex escapes.

Standard-conforming literals have been the default for long enough that
it no longer seems necessary to go out of our way to tell people to write
regex escapes illegibly.

8 years agoCode + docs review for unicode linestyle patch.
Tom Lane [Tue, 3 Nov 2015 16:49:21 +0000 (11:49 -0500)]
Code + docs review for unicode linestyle patch.

Fix some brain fade in commit a2dabf0e1dda93c8: erroneous variable names
in docs, rearrangements that made sentences less clear not more so,
undocumented and poorly-chosen-anyway API behaviors of subroutines,
bad grammar in error messages, copy-and-paste faults.

Albe Laurenz and Tom Lane

8 years agoshm_mq: Third attempt at fixing nowait behavior in shm_mq_receive.
Robert Haas [Tue, 3 Nov 2015 14:12:52 +0000 (09:12 -0500)]
shm_mq: Third attempt at fixing nowait behavior in shm_mq_receive.

Commit a1480ec1d3bacb9acb08ec09f22bc25bc033115b purported to fix the
problems with commit b2ccb5f4e6c81305386edb34daf7d1d1e1ee112a, but it
didn't completely fix them.  The problem is that the checks were
performed in the wrong order, leading to a race condition.  If the
sender attached, sent a message, and detached after the receiver
called shm_mq_get_sender and before the receiver called
shm_mq_counterparty_gone, we'd incorrectly return SHM_MQ_DETACHED
before all messages were read.  Repair by reversing the order of
operations, and add a long comment explaining why this new logic is
(hopefully) correct.

8 years agoCorrect tiny inaccuracy in strxfrm cache comment.
Robert Haas [Tue, 3 Nov 2015 13:32:22 +0000 (08:32 -0500)]
Correct tiny inaccuracy in strxfrm cache comment.

Peter Geoghegan

8 years agoRemove some more dead Alpha-specific code.
Tom Lane [Tue, 3 Nov 2015 00:37:51 +0000 (19:37 -0500)]
Remove some more dead Alpha-specific code.

8 years agoFix problems with ParamListInfo serialization mechanism.
Robert Haas [Mon, 2 Nov 2015 23:11:29 +0000 (18:11 -0500)]
Fix problems with ParamListInfo serialization mechanism.

Commit d1b7c1ffe72e86932b5395f29e006c3f503bc53d introduced a mechanism
for serializing a ParamListInfo structure to be passed to a parallel
worker.  However, this mechanism failed to handle external expanded
values, as pointed out by Noah Misch.  Repair.

Moreover, plpgsql_param_fetch requires adjustment because the
serialization mechanism needs it to skip evaluating unused parameters
just as we would do when it is called from copyParamList, but params
== estate->paramLI in that case.  To fix, make the bms_is_member test
in that function unconditional.

Finally, have setup_param_list set a new ParamListInfo field,
paramMask, to the parameters actually used in the expression, so that
we don't try to fetch those that are not needed when serializing a
parameter list.  This isn't necessary for correctness, but it makes
the performance of the parallel executor code comparable to what we
do for cases involving cursors.

Design suggestions and extensive review by Noah Misch.  Patch by me.

8 years agoAdd RMV to list of commands taking AE lock.
Kevin Grittner [Mon, 2 Nov 2015 12:23:10 +0000 (06:23 -0600)]
Add RMV to list of commands taking AE lock.

Backpatch to 9.3, where it was initially omitted.

Craig Ringer, with minor adjustment by Kevin Grittner

8 years agoFix serialization anomalies due to race conditions on INSERT.
Kevin Grittner [Sat, 31 Oct 2015 19:43:34 +0000 (14:43 -0500)]
Fix serialization anomalies due to race conditions on INSERT.

On insert the CheckForSerializableConflictIn() test was performed
before the page(s) which were going to be modified had been locked
(with an exclusive buffer content lock).  If another process
acquired a relation SIReadLock on the heap and scanned to a page on
which an insert was going to occur before the page was so locked,
a rw-conflict would be missed, which could allow a serialization
anomaly to be missed.  The window between the check and the page
lock was small, so the bug was generally not noticed unless there
was high concurrency with multiple processes inserting into the
same table.

This was reported by Peter Bailis as bug #11732, by Sean Chittenden
as bug #13667, and by others.

The race condition was eliminated in heap_insert() by moving the
check down below the acquisition of the buffer lock, which had been
the very next statement.  Because of the loop locking and unlocking
multiple buffers in heap_multi_insert() a check was added after all
inserts were completed.  The check before the start of the inserts
was left because it might avoid a large amount of work to detect a
serialization anomaly before performing the all of the inserts and
the related WAL logging.

While investigating this bug, other SSI bugs which were even harder
to hit in practice were noticed and fixed, an unnecessary check
(covered by another check, so redundant) was removed from
heap_update(), and comments were improved.

Back-patch to all supported branches.

Kevin Grittner and Thomas Munro

8 years agoImplement lookbehind constraints in our regular-expression engine.
Tom Lane [Fri, 30 Oct 2015 23:14:19 +0000 (19:14 -0400)]
Implement lookbehind constraints in our regular-expression engine.

A lookbehind constraint is like a lookahead constraint in that it consumes
no text; but it checks for existence (or nonexistence) of a match *ending*
at the current point in the string, rather than one *starting* at the
current point.  This is a long-requested feature since it exists in many
other regex libraries, but Henry Spencer had never got around to
implementing it in the code we use.

Just making it work is actually pretty trivial; but naive copying of the
logic for lookahead constraints leads to code that often spends O(N^2) time
to scan an N-character string, because we have to run the match engine
from string start to the current probe point each time the constraint is
checked.  In typical use-cases a lookbehind constraint will be written at
the start of the regex and hence will need to be checked at every character
--- so O(N^2) work overall.  To fix that, I introduced a third copy of the
core DFA matching loop, paralleling the existing longest() and shortest()
loops.  This version, matchuntil(), can suspend and resume matching given
a couple of pointers' worth of storage space.  So we need only run it
across the string once, stopping at each interesting probe point and then
resuming to advance to the next one.

I also put in an optimization that simplifies one-character lookahead and
lookbehind constraints, such as "(?=x)" or "(?<!\w)", into AHEAD and BEHIND
constraints, which already existed in the engine.  This avoids the overhead
of the LACON machinery entirely for these rather common cases.

The net result is that lookbehind constraints run a factor of three or so
slower than Perl's for multi-character constraints, but faster than Perl's
for one-character constraints ... and they work fine for variable-length
constraints, which Perl gives up on entirely.  So that's not bad from a
competitive perspective, and there's room for further optimization if
anyone cares.  (In reality, raw scan rate across a large input string is
probably not that big a deal for Postgres usage anyway; so I'm happy if
it's linear.)

8 years agodoc: security_barrier option is a Boolean, not a string.
Robert Haas [Fri, 30 Oct 2015 11:18:55 +0000 (12:18 +0100)]
doc: security_barrier option is a Boolean, not a string.

Mistake introduced by commit 5bd91e3a835b5d5499fee5f49fc7c0c776fe63dd.

Hari Babu

8 years agoUpdate parallel executor support to reuse the same DSM.
Robert Haas [Fri, 30 Oct 2015 09:43:00 +0000 (10:43 +0100)]
Update parallel executor support to reuse the same DSM.

Commit b0b0d84b3d663a148022e900ebfc164284a95f55 purported to make it
possible to relaunch workers using the same parallel context, but it had
an unpleasant race condition: we might reinitialize after the workers
have sent their last control message but before they have dettached the
DSM, leaving to crashes.  Repair by introducing a new ParallelContext
operation, ReinitializeParallelDSM.

Adjust execParallel.c to use this new support, so that we can rescan a
Gather node by relaunching workers but without needing to recreate the
DSM.

Amit Kapila, with some adjustments by me.  Extracted from latest parallel
sequential scan patch.

8 years agoFix typo in bgworker.c
Robert Haas [Fri, 30 Oct 2015 09:35:33 +0000 (10:35 +0100)]
Fix typo in bgworker.c

8 years agoDocs: add example clarifying use of nested JSON containment.
Tom Lane [Thu, 29 Oct 2015 22:54:35 +0000 (18:54 -0400)]
Docs: add example clarifying use of nested JSON containment.

Show how this can be used in practice to make queries simpler and more
flexible.  Also, draw an explicit contrast to the existence operator,
which doesn't work that way.

Peter Geoghegan and Tom Lane

8 years agoRemove some remains from Alpha support removal
Peter Eisentraut [Thu, 29 Oct 2015 20:40:14 +0000 (16:40 -0400)]
Remove some remains from Alpha support removal

8 years agoMessage style improvements
Peter Eisentraut [Thu, 29 Oct 2015 00:23:53 +0000 (20:23 -0400)]
Message style improvements

Message style, plurals, quoting, spelling, consistency with similar
messages

8 years agoAdd missing serial comma, for consistency.
Robert Haas [Wed, 28 Oct 2015 11:19:14 +0000 (12:19 +0100)]
Add missing serial comma, for consistency.

Amit Langote, per Etsuro Fujita

8 years agoFix incorrect message in ATWrongRelkindError.
Robert Haas [Wed, 28 Oct 2015 10:44:47 +0000 (11:44 +0100)]
Fix incorrect message in ATWrongRelkindError.

Mistake introduced by commit 3bf3ab8c563699138be02f9dc305b7b77a724307.

Etsuro Fujita

8 years agoFix secondary expected output for commit_ts test
Alvaro Herrera [Wed, 28 Oct 2015 02:02:04 +0000 (23:02 -0300)]
Fix secondary expected output for commit_ts test

Per red wall in buildfarm

8 years agoMake Gather node projection-capable.
Robert Haas [Tue, 27 Oct 2015 23:27:58 +0000 (00:27 +0100)]
Make Gather node projection-capable.

The original Gather code failed to mark a Gather node as not able to
do projection, but it couldn't, even though it did call initialize its
projection info via ExecAssignProjectionInfo.  There doesn't seem to
be any good reason for this node not to have projection capability,
so clean things up so that it does.  Without this, plans using Gather
nodes might need to carry extra Result nodes to do projection.

8 years agoDocument BRIN's inclusion opclass framework
Alvaro Herrera [Tue, 27 Oct 2015 22:03:15 +0000 (19:03 -0300)]
Document BRIN's inclusion opclass framework

Backpatch to 9.5 -- this should have been part of b0b7be61337, but we
didn't have 38b03caebc5de either at the time.

Author: Emre Hasegeli
Revised by: Ian Barwick
Discussion:
 http://www.postgresql.org/message-id/CAE2gYzyB39Q9up_-TO6FKhH44pcAM1x6n_Cuj15qKoLoFihUVg@mail.gmail.com
 http://www.postgresql.org/message-id/562DA711.3020305@2ndquadrant.com

8 years agoFix BRIN free space computations
Alvaro Herrera [Tue, 27 Oct 2015 21:17:55 +0000 (18:17 -0300)]
Fix BRIN free space computations

A bug in the original free space computation made it possible to
return a page which wasn't actually able to fit the item.  Since the
insertion code isn't prepared to deal with PageAddItem failing, a PANIC
resulted ("failed to add BRIN tuple [to new page]").  Add a macro to
encapsulate the correct computation, and use it in
brin_getinsertbuffer's callers before calling that routine, to raise an
early error.

I became aware of the possiblity of a problem in this area while working
on ccc4c074994d734.  There's no archived discussion about it, but it's
easy to reproduce a problem in the unpatched code with something like

CREATE TABLE t (a text);
CREATE INDEX ti ON t USING brin (a) WITH (pages_per_range=1);

for length in `seq 8000 8196`
do
psql -f - <<EOF
TRUNCATE TABLE t;
INSERT INTO t VALUES ('z'), (repeat('a', $length));
EOF
done

Backpatch to 9.5, where BRIN was introduced.

8 years agoCleanup commit timestamp module activaction, again
Alvaro Herrera [Tue, 27 Oct 2015 18:06:50 +0000 (15:06 -0300)]
Cleanup commit timestamp module activaction, again

Further tweak commit_ts.c so that on a standby the state is completely
consistent with what that in the master, rather than behaving
differently in the cases that the settings differ.  Now in standby and
master the module should always be active or inactive in lockstep.

Author: Petr Jelínek, with some further tweaks by Álvaro Herrera.

Backpatch to 9.5, where commit timestamps were introduced.

Discussion: http://www.postgresql.org/message-id/5622BF9D.2010409@2ndquadrant.com

8 years agoMeasure string lengths only once
Alvaro Herrera [Tue, 27 Oct 2015 16:20:40 +0000 (13:20 -0300)]
Measure string lengths only once

Bernd Helmle complained that CreateReplicationSlot() was assigning the
same value to the same variable twice, so we could remove one of them.
Code inspection reveals that we can actually remove both assignments:
according to the author the assignment was there for beauty of the
strlen line only, and another possible fix to that is to put the strlen
in its own line, so do that.

To be consistent within the file, refactor all duplicated strlen()
calls, which is what we do elsewhere in the backend anyway.  In
basebackup.c, snprintf already returns the right length; no need for
strlen afterwards.

Backpatch to 9.4, where replication slots were introduced, to keep code
identical.  Some of this is older, but the patch doesn't apply cleanly
and it's only of cosmetic value anyway.

Discussion: http://www.postgresql.org/message-id/BE2FD71DEA35A2287EA5F018@eje.credativ.lan

8 years agoshm_mq: Repair breakage from previous commit.
Robert Haas [Fri, 23 Oct 2015 02:01:11 +0000 (22:01 -0400)]
shm_mq: Repair breakage from previous commit.

If the counterparty writes some data into the queue and then detaches,
it's wrong to return SHM_MQ_DETACHED right away.  If we do that, we
fail to read whatever was written.

8 years agoAdd two missing cases to ATWrongRelkindError.
Robert Haas [Thu, 22 Oct 2015 21:00:53 +0000 (17:00 -0400)]
Add two missing cases to ATWrongRelkindError.

This way, we produce a better error message if someone tries to do
something like ALTER INDEX .. ALTER COLUMN .. SET STORAGE.

Amit Langote

8 years agoshm_mq: Fix failure to notice a dead counterparty when nowait is used.
Robert Haas [Thu, 22 Oct 2015 20:33:30 +0000 (16:33 -0400)]
shm_mq: Fix failure to notice a dead counterparty when nowait is used.

The shm_mq mechanism was intended to optionally notice when the process
on the other end of the queue fails to attach to the queue.  It does
this by allowing the user to pass a BackgroundWorkerHandle; if the
background worker in question is launched and dies without attaching
to the queue, then we know it never will.  This logic works OK in
blocking mode, but when called with nowait = true we fail to notice
that this has happened due to an asymmetry in the logic.  Repair.

Reported off-list by Rushabh Lathia.  Patch by me.

8 years agoFix typos in comments.
Robert Haas [Thu, 22 Oct 2015 18:51:49 +0000 (14:51 -0400)]
Fix typos in comments.

CharSyam

8 years agodoc: Add advice on updating checkpoint_segments to max_wal_size
Peter Eisentraut [Thu, 22 Oct 2015 17:59:58 +0000 (13:59 -0400)]
doc: Add advice on updating checkpoint_segments to max_wal_size

with suggestion from Michael Paquier

8 years agoRemove redundant CREATEUSER/NOCREATEUSER options in CREATE ROLE et al.
Tom Lane [Thu, 22 Oct 2015 16:33:51 +0000 (09:33 -0700)]
Remove redundant CREATEUSER/NOCREATEUSER options in CREATE ROLE et al.

Once upon a time we did not have a separate CREATEROLE privilege, and
CREATEUSER effectively meant SUPERUSER.  When we invented CREATEROLE
(in 8.1) we also added SUPERUSER so as to have a less confusing keyword
for this role property.  However, we left CREATEUSER in place as a
deprecated synonym for SUPERUSER, because of backwards-compatibility
concerns.  It's still there and is still confusing people, as for example
in bug #13694 from Justin Catterson.  9.6 will be ten years or so later,
which surely ought to be long enough to end the deprecation and just
remove these old keywords.  Hence, do so.

8 years agoFix a couple of bugs in recent parallelism-related commits.
Robert Haas [Thu, 22 Oct 2015 14:49:20 +0000 (10:49 -0400)]
Fix a couple of bugs in recent parallelism-related commits.

Commit 816e336f12ecabdc834d4cc31bcf966b2dd323dc added the wrong error
check to async.c; sending restrictions is restricted to the leader,
not altogether unsafe.

Commit 3bd909b220930f21d6e15833a17947be749e7fde added ExecShutdownNode
to traverse the planstate tree and call shutdown functions, but made
a Gather node, the only node that actually has such a function, abort
the tree traversal, which is wrong.

8 years agoAdd header comments to execParallel.c and nodeGather.c.
Robert Haas [Thu, 22 Oct 2015 14:37:24 +0000 (10:37 -0400)]
Add header comments to execParallel.c and nodeGather.c.

Patch by me, per a note from Simon Riggs.  Reviewed by Amit Kapila
and Amit Langote.

8 years agodoc: Improve markup and fine-tune replication protocol documentation
Peter Eisentraut [Thu, 22 Oct 2015 02:31:56 +0000 (22:31 -0400)]
doc: Improve markup and fine-tune replication protocol documentation

8 years agoFix incorrect translation of minus-infinity datetimes for json/jsonb.
Tom Lane [Tue, 20 Oct 2015 18:06:24 +0000 (11:06 -0700)]
Fix incorrect translation of minus-infinity datetimes for json/jsonb.

Commit bda76c1c8cfb1d11751ba6be88f0242850481733 caused both plus and
minus infinity to be rendered as "infinity", which is not only wrong
but inconsistent with the pre-9.4 behavior of to_json().  Fix that by
duplicating the coding in date_out/timestamp_out/timestamptz_out more
closely.  Per bug #13687 from Stepan Perlov.  Back-patch to 9.4, like
the previous commit.

In passing, also re-pgindent json.c, since it had gotten a bit messed up by
recent patches (and I was already annoyed by indentation-related problems
in back-patching this fix ...)

8 years agodoc: Move documentation of max_wal_size to better position
Peter Eisentraut [Tue, 20 Oct 2015 17:33:39 +0000 (13:33 -0400)]
doc: Move documentation of max_wal_size to better position

8 years agoFix incorrect comment in plannodes.h
Robert Haas [Tue, 20 Oct 2015 15:11:35 +0000 (11:11 -0400)]
Fix incorrect comment in plannodes.h

Etsuro Fujita

8 years agoRemove duplicate word.
Robert Haas [Tue, 20 Oct 2015 14:29:19 +0000 (10:29 -0400)]
Remove duplicate word.

Amit Langote

8 years agoTab complete CREATE EXTENSION .. VERSION.
Robert Haas [Tue, 20 Oct 2015 14:27:20 +0000 (10:27 -0400)]
Tab complete CREATE EXTENSION .. VERSION.

Jeff Janes

8 years agoPut back ssl_renegotiation_limit parameter, but only allow 0.
Robert Haas [Tue, 20 Oct 2015 13:56:04 +0000 (09:56 -0400)]
Put back ssl_renegotiation_limit parameter, but only allow 0.

Per a report from Shay Rojansky, Npgsql sends ssl_renegotiation_limit=0
in the startup packet because it does not support renegotiation; other
clients which have not attempted to support renegotiation might well
behave similarly.  The recent removal of this parameter forces them to
break compatibility with either current PostgreSQL versions, or
previous ones.  Per discussion, the best solution is to accept the
parameter but only allow a value of 0.

Shay Rojansky, edited a little by me.

8 years agoBe a bit more rigorous about how we cache strcoll and strxfrm results.
Robert Haas [Tue, 20 Oct 2015 13:27:50 +0000 (09:27 -0400)]
Be a bit more rigorous about how we cache strcoll and strxfrm results.

Commit 0e57b4d8bd9674adaf5747421b3255b85e385534 contained some clever
logic that attempted to make sure that we couldn't get confused about
whether the last thing we cached was a strcoll() result or a strxfrm()
result, but it wasn't quite clever enough, because we can perform
further abbreviations after having already performed some comparisons.
Introduce an explicit flag in the hopes of making this watertight.

Peter Geoghegan, reviewed by me.

8 years agoRemove obsolete comment.
Robert Haas [Tue, 20 Oct 2015 13:15:13 +0000 (09:15 -0400)]
Remove obsolete comment.

Peter Geoghegan

8 years agoEschew "RESET statement_timeout" in tests.
Noah Misch [Tue, 20 Oct 2015 04:37:22 +0000 (00:37 -0400)]
Eschew "RESET statement_timeout" in tests.

Instead, use transaction abort.  Given an unlucky bout of latency, the
timeout would cancel the RESET itself.  Buildfarm members gharial,
lapwing, mereswine, shearwater, and sungazer witness that.  Back-patch
to 9.1 (all supported versions).  The query_canceled test still could
timeout before entering its subtransaction; for whatever reason, that
has yet to happen on the buildfarm.

8 years agoFix incorrect handling of lookahead constraints in pg_regprefix().
Tom Lane [Mon, 19 Oct 2015 20:54:53 +0000 (13:54 -0700)]
Fix incorrect handling of lookahead constraints in pg_regprefix().

pg_regprefix was doing nothing with lookahead constraints, which would
be fine if it were the right kind of nothing, but it isn't: we have to
terminate our search for a fixed prefix, not just pretend the LACON arc
isn't there.  Otherwise, if the current state has both a LACON outarc and a
single plain-color outarc, we'd falsely conclude that the color represents
an addition to the fixed prefix, and generate an extracted index condition
that restricts the indexscan too much.  (See added regression test case.)

Terminating the search is conservative: we could traverse the LACON arc
(thus assuming that the constraint can be satisfied at runtime) and then
examine the outarcs of the linked-to state.  But that would be a lot more
work than it seems worth, because writing a LACON followed by a single
plain character is a pretty silly thing to do.

This makes a difference only in rather contrived cases, but it's a bug,
so back-patch to all supported branches.

8 years agoAdd a C API for parallel heap scans.
Robert Haas [Fri, 16 Oct 2015 21:25:02 +0000 (17:25 -0400)]
Add a C API for parallel heap scans.

Using this API, one backend can set up a ParallelHeapScanDesc to
which multiple backends can then attach.  Each tuple in the relation
will be returned to exactly one of the scanning backends.  Only
forward scans are supported, and rescans must be carefully
coordinated.

This is not exposed to the planner or executor yet.

The original version of this code was written by me.  Amit Kapila
reviewed it, tested it, and improved it, including adding support for
synchronized scans, per review comments from Jeff Davis.  Extensive
testing of this and related patches was performed by Haribabu Kommi.
Final cleanup of this patch by me.

8 years agoAllow a parallel context to relaunch workers.
Robert Haas [Fri, 16 Oct 2015 21:18:05 +0000 (17:18 -0400)]
Allow a parallel context to relaunch workers.

This may allow some callers to avoid the overhead involved in tearing
down a parallel context and then setting up a new one, which means
releasing the DSM and then allocating and populating a new one.  I
suspect we'll want to revise the Gather node to make use of this new
capability, but even if not it may be useful elsewhere and requires
very little additional code.

8 years agoMiscellaneous cleanup of regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 19:52:12 +0000 (15:52 -0400)]
Miscellaneous cleanup of regular-expression compiler.

Revert our previous addition of "all" flags to copyins() and copyouts();
they're no longer needed, and were never anything but an unsightly hack.

Improve a couple of infelicities in the REG_DEBUG code for dumping
the NFA data structure, including adding code to count the total
number of states and arcs.

Add a couple of missed error checks.

Add some more documentation in the README file, and some regression tests
illustrating cases that exceeded the state-count limit and/or took
unreasonable amounts of time before this set of patches.

Back-patch to all supported branches.

8 years agoImprove memory-usage accounting in regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 19:36:16 +0000 (15:36 -0400)]
Improve memory-usage accounting in regular-expression compiler.

This code previously counted the number of NFA states it created, and
complained if a limit was exceeded, so as to prevent bizarre regex patterns
from consuming unreasonable time or memory.  That's fine as far as it went,
but the code paid no attention to how many arcs linked those states.  Since
regexes can be contrived that have O(N) states but will need O(N^2) arcs
after fixempties() processing, it was still possible to blow out memory,
and take a long time doing it too.  To fix, modify the bookkeeping to count
space used by both states and arcs.

I did not bother with including the "color map" in the accounting; it
can only grow to a few megabytes, which is not a lot in comparison to
what we're allowing for states+arcs (about 150MB on 64-bit machines
or half that on 32-bit machines).

Looking at some of the larger real-world regexes captured in the Tcl
regression test suite suggests that the most that is likely to be needed
for regexes found in the wild is under 10MB, so I believe that the current
limit has enough headroom to make it okay to keep it as a hard-wired limit.

In connection with this, redefine REG_ETOOBIG as meaning "regular
expression is too complex"; the previous wording of "nfa has too many
states" was already somewhat inapropos because of the error code's use
for stack depth overrun, and it was not very user-friendly either.

Back-patch to all supported branches.

8 years agoImprove performance of pullback/pushfwd in regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 19:11:49 +0000 (15:11 -0400)]
Improve performance of pullback/pushfwd in regular-expression compiler.

The previous coding would create a new intermediate state every time it
wanted to interchange the ordering of two constraint arcs.  Certain regex
features such as \Y can generate large numbers of parallel constraint arcs,
and if we needed to reorder the results of that, we created unreasonable
numbers of intermediate states.  To improve matters, keep a list of
already-created intermediate states associated with the state currently
being considered by the outer loop; we can re-use such states to place all
the new arcs leading to the same destination or source.

I also took the trouble to redefine push() and pull() to have a less risky
API: they no longer delete any state or arc that the caller might possibly
have a pointer to, except for the specifically-passed constraint arc.
This reduces the risk of re-introducing the same type of error seen in
the failed patch for CVE-2007-4772.

Back-patch to all supported branches.

8 years agoImprove performance of fixempties() pass in regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 18:58:10 +0000 (14:58 -0400)]
Improve performance of fixempties() pass in regular-expression compiler.

The previous coding took something like O(N^4) time to fully process a
chain of N EMPTY arcs.  We can't really do much better than O(N^2) because
we have to insert about that many arcs, but we can do lots better than
what's there now.  The win comes partly from using mergeins() to amortize
de-duplication of arcs across multiple source states, and partly from
exploiting knowledge of the ordering of arcs for each state to avoid
looking at arcs we don't need to consider during the scan.  We do have
to be a bit careful of the possible reordering of arcs introduced by
the sort-merge coding of the previous commit, but that's not hard to
deal with.

Back-patch to all supported branches.

8 years agoFix O(N^2) performance problems in regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 18:43:17 +0000 (14:43 -0400)]
Fix O(N^2) performance problems in regular-expression compiler.

Change the singly-linked in-arc and out-arc lists to be doubly-linked,
so that arc deletion is constant time rather than having worst-case time
proportional to the number of other arcs on the connected states.

Modify the bulk arc transfer operations copyins(), copyouts(), moveins(),
moveouts() so that they use a sort-and-merge algorithm whenever there's
more than a small number of arcs to be copied or moved.  The previous
method is O(N^2) in the number of arcs involved, because it performs
duplicate checking independently for each copied arc.  The new method may
change the ordering of existing arcs for the destination state, but nothing
really cares about that.

Provide another bulk arc copying method mergeins(), which is unused as
of this commit but is needed for the next one.  It basically is like
copyins(), but the source arcs might not all come from the same state.

Replace the O(N^2) bubble-sort algorithm used in carcsort() with a qsort()
call.

These changes greatly improve the performance of regex compilation for
large or complex regexes, at the cost of extra space for arc storage during
compilation.  The original tradeoff was probably fine when it was made, but
now we care more about speed and less about memory consumption.

Back-patch to all supported branches.

8 years agoFix regular-expression compiler to handle loops of constraint arcs.
Tom Lane [Fri, 16 Oct 2015 18:14:40 +0000 (14:14 -0400)]
Fix regular-expression compiler to handle loops of constraint arcs.

It's possible to construct regular expressions that contain loops of
constraint arcs (that is, ^ $ AHEAD BEHIND or LACON arcs).  There's no use
in fully traversing such a loop at execution, since you'd just end up in
the same NFA state without having consumed any input.  Worse, such a loop
leads to infinite looping in the pullback/pushfwd stage of compilation,
because we keep pushing or pulling the same constraints around the loop
in a vain attempt to move them to the pre or post state.  Such looping was
previously recognized in CVE-2007-4772; but the fix only handled the case
of trivial single-state loops (that is, a constraint arc leading back to
its source state) ... and not only that, it was incorrect even for that
case, because it broke the admittedly-not-very-clearly-stated API contract
of the pull() and push() subroutines.  The first two regression test cases
added by this commit exhibit patterns that result in assertion failures
because of that (though there seem to be no ill effects in non-assert
builds).  The other new test cases exhibit multi-state constraint loops;
in an unpatched build they will run until the NFA state-count limit is
exceeded.

To fix, remove the code added for CVE-2007-4772, and instead create a
general-purpose constraint-loop-breaking phase of regex compilation that
executes before we do pullback/pushfwd.  Since we never need to traverse
a constraint loop fully, we can just break the loop at any chosen spot,
if we add clone states that can replicate any sequence of arc transitions
that would've traversed just part of the loop.

Also add some commentary clarifying why we have to have all these
machinations in the first place.

This class of problems has been known for some time --- we had a report
from Marc Mamin about two years ago, for example, and there are related
complaints in the Tcl bug tracker.  I had discussed a fix of this kind
off-list with Henry Spencer, but didn't get around to doing something
about it until the issue was rediscovered by Greg Stark recently.

Back-patch to all supported branches.

8 years agoRemove volatile qualifiers from proc.c and procarray.c
Robert Haas [Fri, 16 Oct 2015 18:20:36 +0000 (14:20 -0400)]
Remove volatile qualifiers from proc.c and procarray.c

Prior to commit 0709b7ee72e4bc71ad07b7120acd117265ab51d0, access to
variables within a spinlock-protected critical section had to be done
through a volatile pointer, but that should no longer be necessary.

Michael Paquier

8 years agoRemove volatile qualifiers from dynahash.c, shmem.c, and sinvaladt.c
Robert Haas [Fri, 16 Oct 2015 18:12:20 +0000 (14:12 -0400)]
Remove volatile qualifiers from dynahash.c, shmem.c, and sinvaladt.c

Prior to commit 0709b7ee72e4bc71ad07b7120acd117265ab51d0, access to
variables within a spinlock-protected critical section had to be done
through a volatile pointer, but that should no longer be necessary.

Thomas Munro

8 years agoRemove cautions about using volatile from spin.h.
Robert Haas [Fri, 16 Oct 2015 18:06:22 +0000 (14:06 -0400)]
Remove cautions about using volatile from spin.h.

Commit 0709b7ee72e4bc71ad07b7120acd117265ab51d0 obsoleted this comment
but neglected to update it.

Thomas Munro

8 years agoProhibit parallel query when the isolation level is serializable.
Robert Haas [Fri, 16 Oct 2015 15:58:27 +0000 (11:58 -0400)]
Prohibit parallel query when the isolation level is serializable.

In order for this to be safe, the code which hands true serializability
will need to taught that the SIRead locks taken by a parallel worker
pertain to the same transaction as those taken by the parallel leader.
Some further changes may be needed as well.  Until the necessary
adaptations are made, don't generate parallel plans in serializable
mode, and if a previously-generated parallel plan is used after
serializable mode has been activated, run it serially.

This fixes a bug in commit 7aea8e4f2daa4b39ca9d1309a0c4aadb0f7ed81b.

8 years agoRewrite interaction of parallel mode with parallel executor support.
Robert Haas [Fri, 16 Oct 2015 15:56:02 +0000 (11:56 -0400)]
Rewrite interaction of parallel mode with parallel executor support.

In the previous coding, before returning from ExecutorRun, we'd shut
down all parallel workers.  This was dead wrong if ExecutorRun was
called with a non-zero tuple count; it had the effect of truncating
the query output.  To fix, give ExecutePlan control over whether to
enter parallel mode, and have it refuse to do so if the tuple count
is non-zero.  Rewrite the Gather logic so that it can cope with being
called outside parallel mode.

Commit 7aea8e4f2daa4b39ca9d1309a0c4aadb0f7ed81b is largely to blame
for this problem, though this patch modifies some subsequently-committed
code which relied on the guarantees it purported to make.

8 years agoMark more functions parallel-restricted or parallel-unsafe.
Robert Haas [Fri, 16 Oct 2015 15:48:48 +0000 (11:48 -0400)]
Mark more functions parallel-restricted or parallel-unsafe.

Commit 7aea8e4f2daa4b39ca9d1309a0c4aadb0f7ed81b was overoptimistic
about the degree of safety associated with running various functions
in parallel mode.  Functions that take a table name or OID as an
argument are at least parallel-restricted, because the table might be
temporary, and we currently don't allow parallel workers to touch
temporary tables.  Functions that take a query as an argument are
outright unsafe, because the query could be anything, including a
parallel-unsafe query.

Also, the queue of pending notifications is backend-private, so adding
to it from a worker doesn't behave correctly.  We could fix this by
transferring the worker's queue of pending notifications to the master
during worker cleanup, but that seems like more trouble than it's
worth for now.  In addition to adjusting the pg_proc.h markings, also
add an explicit check for this in async.c.

8 years agoFix a problem with parallel workers being unable to restore role.
Robert Haas [Fri, 16 Oct 2015 15:37:19 +0000 (11:37 -0400)]
Fix a problem with parallel workers being unable to restore role.

check_role() tries to verify that the user has permission to become the
requested role, but this is inappropriate in a parallel worker, which
needs to exactly recreate the master's authorization settings.  So skip
the check in that case.

This fixes a bug in commit 924bcf4f16d54c55310b28f77686608684734f42.

8 years agoInvalidate caches after cranking up a parallel worker transaction.
Robert Haas [Fri, 16 Oct 2015 15:31:23 +0000 (11:31 -0400)]
Invalidate caches after cranking up a parallel worker transaction.

Starting a parallel worker transaction changes our notion of which XIDs
are in-progress or committed, and our notion of the current command
counter ID.  Therefore, our view of these caches prior to starting
this transaction may no longer valid.  Defend against that by clearing
them.

This fixes a bug in commit 924bcf4f16d54c55310b28f77686608684734f42.

8 years agoFix order of arguments in ecpg generated typedef command.
Michael Meskes [Fri, 16 Oct 2015 15:29:05 +0000 (17:29 +0200)]
Fix order of arguments in ecpg generated typedef command.