postgresql.git
9 years agodoc: Fix markup and improve placeholder names
Peter Eisentraut [Thu, 3 Dec 2015 15:20:54 +0000 (10:20 -0500)]
doc: Fix markup and improve placeholder names

9 years agoFix behavior of printTable() and friends with externally-invoked pager.
Tom Lane [Wed, 2 Dec 2015 23:20:34 +0000 (18:20 -0500)]
Fix behavior of printTable() and friends with externally-invoked pager.

The formatting modes that depend on knowledge of the terminal window width
did not work right when printing a query result that's been fetched in
sections (as a result of FETCH_SIZE).  ExecQueryUsingCursor() would force
use of the pager as soon as there's more than one result section, and then
print.c would see an output file pointer that's not stdout and incorrectly
conclude that the terminal window width isn't relevant.

This has been broken all along for non-expanded "wrapped" output format,
and as of 9.5 the issue affects expanded mode as well.  The problem also
caused "\pset expanded auto" mode to invariably *not* switch to expanded
output in a segmented result, which seems to me to be exactly backwards.

To fix, we need to pass down an "is_pager" flag to inform the print.c
subroutines that some calling level has already replaced stdout with a
pager pipe, so they should (a) not do that again and (b) nonetheless honor
the window size.  (Notably, this makes the first is_pager test in
print_aligned_text() not be dead code anymore.)

This patch is a bit invasive because there are so many existing calls of
printQuery()/printTable(), but fortunately all but a couple can just pass
"false" for the added parameter.

Back-patch to 9.5 but no further.  Given the lack of field complaints,
it's not clear that we should change the behavior in stable branches.
Also, the API change for printQuery()/printTable() might possibly break
third-party code, again something we don't like to do in stable branches.
However, it's not quite too late to do this in 9.5, and with the larger
scope of the problem there, it seems worth doing.

9 years agoMake gincostestimate() cope with hypothetical GIN indexes.
Tom Lane [Tue, 1 Dec 2015 21:24:34 +0000 (16:24 -0500)]
Make gincostestimate() cope with hypothetical GIN indexes.

We tried to fetch statistics data from the index metapage, which does not
work if the index isn't actually present.  If the index is hypothetical,
instead extrapolate some plausible internal statistics based on the index
page count provided by the index-advisor plugin.

There was already some code in gincostestimate() to invent internal stats
in this way, but since it was only meant as a stopgap for pre-9.1 GIN
indexes that hadn't been vacuumed since upgrading, it was pretty crude.
If we want it to support index advisors, we should try a little harder.
A small amount of testing says that it's better to estimate the entry pages
as 90% of the index, not 100%.  Also, estimating the number of entries
(keys) as equal to the heap tuple count could be wildly wrong in either
direction.  Instead, let's estimate 100 entries per entry page.

Perhaps someday somebody will want the index advisor to be able to provide
these numbers more directly, but for the moment this should serve.

Problem report and initial patch by Julien Rouhaud; modified by me to
invent less-bogus internal statistics.  Back-patch to all supported
branches, since we've supported index advisors since 9.0.

9 years agoFurther tweaking of print_aligned_vertical().
Tom Lane [Tue, 1 Dec 2015 19:47:13 +0000 (14:47 -0500)]
Further tweaking of print_aligned_vertical().

Don't force the data width to extend all the way to the right margin if it
doesn't need to.  This reverts the behavior in non-wrapping cases to be
what it was in 9.4.  Also, make the logic that ensures the data line width
is at least equal to the record-header line width a little less obscure.

In passing, avoid possible calculation of log10(0).  Probably that's
harmless, given the lack of field complaints, but it seems risky:
conversion of NaN to an integer isn't well defined.

9 years agoUse "g" not "f" format in ecpg's PGTYPESnumeric_from_double().
Tom Lane [Tue, 1 Dec 2015 16:42:25 +0000 (11:42 -0500)]
Use "g" not "f" format in ecpg's PGTYPESnumeric_from_double().

The previous coding could overrun the provided buffer size for a very large
input, or lose precision for a very small input.  Adopt the methodology
that's been in use in the equivalent backend code for a long time.

Per private report from Bas van Schaik.  Back-patch to all supported
branches.

9 years agoFurther adjustment to psql's print_aligned_vertical() function.
Tom Lane [Tue, 1 Dec 2015 16:07:29 +0000 (11:07 -0500)]
Further adjustment to psql's print_aligned_vertical() function.

We should ignore output_columns unless it's greater than zero.
A zero means we couldn't get any information from ioctl(TIOCGWINSZ);
in that case the expected behavior is to print the data at native width,
not to wrap it at the smallest possible value.  print_aligned_text()
gets this consideration right, but print_aligned_vertical() lost track
of this detail somewhere along the line.

9 years agoRework wrap-width calculation in psql's print_aligned_vertical() function.
Tom Lane [Mon, 30 Nov 2015 22:53:32 +0000 (17:53 -0500)]
Rework wrap-width calculation in psql's print_aligned_vertical() function.

This area was rather heavily whacked around in 6513633b9 and follow-on
commits, and it was showing it, because the logic to calculate the
allowable data width in wrapped expanded mode had only the vaguest
relationship to the logic that was actually printing the data.  It was
not very close to being right about the conditions requiring overhead
columns to be added.  Aside from being wrong, it was pretty unreadable
and under-commented.  Rewrite it so it corresponds to what the printing
code actually does.

In passing, remove a couple of dead tests in the printing logic, too.

Per a complaint from Jeff Janes, though this doesn't look much like his
patch because it fixes a number of other corner-case bogosities too.
One such fix that's visible in the regression test results is that
although the code was attempting to enforce a minimum data width of
3 columns, it sometimes left less space than that available.

9 years agoAvoid caching expression state trees for domain constraints across queries.
Tom Lane [Sun, 29 Nov 2015 23:18:42 +0000 (18:18 -0500)]
Avoid caching expression state trees for domain constraints across queries.

In commit 8abb3cda0ddc00a0ab98977a1633a95b97068d4e I attempted to cache
the expression state trees constructed for domain CHECK constraints for
the life of the backend (assuming the domain's constraints don't get
redefined).  However, this turns out not to work very well, because
execQual.c will run those state trees with ecxt_per_query_memory pointing
to a query-lifespan context, and in some situations we'll end up with
pointers into that context getting stored into the state trees.  This
happens in particular with SQL-language functions, as reported by
Emre Hasegeli, but there are many other cases.

To fix, keep only the expression plan trees for domain CHECK constraints
in the typcache's data structure, and revert to performing ExecInitExpr
(at least) once per query to set up expression state trees in the query's
context.

Eventually it'd be nice to undo this, but that will require some careful
thought about memory management for expression state trees, and it seems
far too late for any such redesign in 9.5.  This way is still much more
efficient than what happened before 8abb3cda0.

9 years agoFix failure to consider failure cases in GetComboCommandId().
Tom Lane [Thu, 26 Nov 2015 18:23:02 +0000 (13:23 -0500)]
Fix failure to consider failure cases in GetComboCommandId().

Failure to initially palloc the comboCids array, or to realloc it bigger
when needed, left combocid's data structures in an inconsistent state that
would cause trouble if the top transaction continues to execute.  Noted
while examining a user complaint about the amount of memory used for this.
(There's not much we can do about that, but it does point up that repalloc
failure has a non-negligible chance of occurring here.)

In HEAD/9.5, also avoid possible invocation of memcpy() with a null pointer
in SerializeComboCIDState; cf commit 13bba0227.

9 years agoBe more paranoid about null return values from libpq status functions.
Tom Lane [Wed, 25 Nov 2015 22:31:53 +0000 (17:31 -0500)]
Be more paranoid about null return values from libpq status functions.

PQhost() can return NULL in non-error situations, namely when a Unix-socket
connection has been selected by default.  That behavior is a tad debatable
perhaps, but for the moment we should make sure that psql copes with it.
Unfortunately, do_connect() failed to: it could pass a NULL pointer to
strcmp(), resulting in crashes on most platforms.  This was reported as a
security issue by ChenQin of Topsec Security Team, but the consensus of
the security list is that it's just a garden-variety bug with no security
implications.

For paranoia's sake, I made the keep_password test not trust PQuser or
PQport either, even though I believe those will never return NULL given
a valid PGconn.

Back-patch to all supported branches.

9 years agopg_upgrade: fix CopyFile() on Windows to fail on file existence
Bruce Momjian [Tue, 24 Nov 2015 22:18:28 +0000 (17:18 -0500)]
pg_upgrade:  fix CopyFile() on Windows to fail on file existence

Also fix getErrorText() to return the right error string on failure.
This behavior now matches that of other operating systems.

Report by Noah Misch

Backpatch through 9.1

9 years agodoc: Some improvements on CREATE POLICY and ALTER POLICY documentation
Peter Eisentraut [Tue, 24 Nov 2015 02:36:57 +0000 (21:36 -0500)]
doc: Some improvements on CREATE POLICY and ALTER POLICY documentation

9 years agoClarify pg_rewind connection requirements.
Teodor Sigaev [Mon, 23 Nov 2015 16:30:36 +0000 (19:30 +0300)]
Clarify pg_rewind connection requirements.

Per http://www.postgresql.org/message-id/flat/564C4CE6.9000509@postgrespro.ru
Pavel Luzanov <p.luzanov@postgrespro.ru>

9 years agodoc: Add more documentation about wal_retrieve_retry_interval
Peter Eisentraut [Mon, 23 Nov 2015 14:13:44 +0000 (09:13 -0500)]
doc: Add more documentation about wal_retrieve_retry_interval

from Michael Paquier

9 years agoAdopt the GNU convention for handling tar-archive members exceeding 8GB.
Tom Lane [Sun, 22 Nov 2015 01:21:32 +0000 (20:21 -0500)]
Adopt the GNU convention for handling tar-archive members exceeding 8GB.

The POSIX standard for tar headers requires archive member sizes to be
printed in octal with at most 11 digits, limiting the representable file
size to 8GB.  However, GNU tar and apparently most other modern tars
support a convention in which oversized values can be stored in base-256,
allowing any practical file to be a tar member.  Adopt this convention
to remove two limitations:
* pg_dump with -Ft output format failed if the contents of any one table
exceeded 8GB.
* pg_basebackup failed if the data directory contained any file exceeding
8GB.  (This would be a fatal problem for installations configured with a
table segment size of 8GB or more, and it has also been seen to fail when
large core dump files exist in the data directory.)

File sizes under 8GB are still printed in octal, so that no compatibility
issues are created except in cases that would have failed entirely before.

In addition, this patch fixes several bugs in the same area:

* In 9.3 and later, we'd defined tarCreateHeader's file-size argument as
size_t, which meant that on 32-bit machines it would write a corrupt tar
header for file sizes between 4GB and 8GB, even though no error was raised.
This broke both "pg_dump -Ft" and pg_basebackup for such cases.

* pg_restore from a tar archive would fail on tables of size between 4GB
and 8GB, on machines where either "size_t" or "unsigned long" is 32 bits.
This happened even with an archive file not affected by the previous bug.

* pg_basebackup would fail if there were files of size between 4GB and 8GB,
even on 64-bit machines.

* In 9.3 and later, "pg_basebackup -Ft" failed entirely, for any file size,
on 64-bit big-endian machines.

In view of these potential data-loss bugs, back-patch to all supported
branches, even though removal of the documented 8GB limit might otherwise
be considered a new feature rather than a bug fix.

9 years agoFix handling of inherited check constraints in ALTER COLUMN TYPE (again).
Tom Lane [Fri, 20 Nov 2015 19:55:28 +0000 (14:55 -0500)]
Fix handling of inherited check constraints in ALTER COLUMN TYPE (again).

The previous way of reconstructing check constraints was to do a separate
"ALTER TABLE ONLY tab ADD CONSTRAINT" for each table in an inheritance
hierarchy.  However, that way has no hope of reconstructing the check
constraints' own inheritance properties correctly, as pointed out in
bug #13779 from Jan Dirk Zijlstra.  What we should do instead is to do
a regular "ALTER TABLE", allowing recursion, at the topmost table that
has a particular constraint, and then suppress the work queue entries
for inherited instances of the constraint.

Annoyingly, we'd tried to fix this behavior before, in commit 5ed6546cf,
but we failed to notice that it wasn't reconstructing the pg_constraint
field values correctly.

As long as I'm touching pg_get_constraintdef_worker anyway, tweak it to
always schema-qualify the target table name; this seems like useful backup
to the protections installed by commit 5f173040.

In HEAD/9.5, get rid of get_constraint_relation_oids, which is now unused.
(I could alternatively have modified it to also return conislocal, but that
seemed like a pretty single-purpose API, so let's not pretend it has some
other use.)  It's unused in the back branches as well, but I left it in
place just in case some third-party code has decided to use it.

In HEAD/9.5, also rename pg_get_constraintdef_string to
pg_get_constraintdef_command, as the previous name did nothing to explain
what that entry point did differently from others (and its comment was
equally useless).  Again, that change doesn't seem like material for
back-patching.

I did a bit of re-pgindenting in tablecmds.c in HEAD/9.5, as well.

Otherwise, back-patch to all supported branches.

9 years agoDodge a macro-name conflict with Perl.
Tom Lane [Thu, 19 Nov 2015 19:54:05 +0000 (14:54 -0500)]
Dodge a macro-name conflict with Perl.

Some versions of Perl export a macro named HS_KEY.  This creates a
conflict in contrib/hstore_plperl against hstore's macro of the same
name.  The most future-proof solution seems to be to rename our macro;
I chose HSTORE_KEY.  For consistency, rename HS_VAL and related macros
similarly.

Back-patch to 9.5.  contrib/hstore_plperl doesn't exist before that
so there is no need to worry about the conflict in older releases.

Per reports from Marco Atzeri and Mike Blackwell.

9 years agodoc: Clarify some things on pg_receivexlog reference page
Peter Eisentraut [Thu, 19 Nov 2015 19:19:04 +0000 (14:19 -0500)]
doc: Clarify some things on pg_receivexlog reference page

9 years agoFix thinko: errmsg -> ereport.
Tom Lane [Thu, 19 Nov 2015 19:16:39 +0000 (14:16 -0500)]
Fix thinko: errmsg -> ereport.

Silly mistake in my commit 09cecdf285ea9f51, reported by Erik Rijkers.

The fact that the buildfarm didn't find this implies that we are not
testing Perl builds that lack MULTIPLICITY, which is a bit disturbing
from a coverage standpoint.  Until today I'd have said nobody cared
about such configurations anymore; but maybe not.

9 years agofix a perl typo
Andrew Dunstan [Thu, 19 Nov 2015 07:42:02 +0000 (02:42 -0500)]
fix a perl typo

9 years agoUpdate docs for vcregress.pl bincheck changes
Andrew Dunstan [Thu, 19 Nov 2015 04:32:16 +0000 (23:32 -0500)]
Update docs for vcregress.pl bincheck changes

9 years agoImprove vcregress.pl's handling of tap tests for client programs
Andrew Dunstan [Thu, 19 Nov 2015 03:47:41 +0000 (22:47 -0500)]
Improve vcregress.pl's handling of tap tests for client programs

The target is now named 'bincheck' rather than 'tapcheck' so that it
reflects what is checked instead of the test mechanism. Some of the
logic is improved, making it easier to add further sets of TAP based
tests in future. Also, the environment setting logic is imrpoved.

As discussed on -hackers a couple of months ago.

9 years agoFix incomplete set_foreignscan_references handling for fdw_recheck_quals
Robert Haas [Thu, 19 Nov 2015 02:17:50 +0000 (21:17 -0500)]
Fix incomplete set_foreignscan_references handling for fdw_recheck_quals

KaiGai Kohei

9 years agoImprove ON CONFLICT documentation.
Andres Freund [Mon, 9 Nov 2015 23:02:49 +0000 (00:02 +0100)]
Improve ON CONFLICT documentation.

Author: Peter Geoghegan and Andres Freund
Discussion: CAM3SWZScpWzQ-7EJC77vwqzZ1GO8GNmURQ1QqDQ3wRn7AbW1Cg@mail.gmail.com
Backpatch: 9.5, where ON CONFLICT was introduced

9 years agoRemove function names from some elog() calls in heapam.c.
Andres Freund [Thu, 19 Nov 2015 00:25:58 +0000 (01:25 +0100)]
Remove function names from some elog() calls in heapam.c.

At least one of the names was, due to a function renaming late in the
development of ON CONFLICT, wrong. Since including function names in
error messages is against the message style guide anyway, remove them
from the messages.

Discussion: CAM3SWZT8paz=usgMVHm0XOETkQvzjRtAUthATnmaHQQY0obnGw@mail.gmail.com
Backpatch: 9.5, where ON CONFLICT was introduced

9 years agoAccept flex > 2.5.x in configure.
Tom Lane [Wed, 18 Nov 2015 22:45:05 +0000 (17:45 -0500)]
Accept flex > 2.5.x in configure.

Per buildfarm member anchovy, 2.6.0 exists in the wild now.
Hopefully it works with Postgres; if not, we'll have to do something
about that, but in any case claiming it's "too old" is pretty silly.

9 years agoFix possible internal overflow in numeric division.
Tom Lane [Tue, 17 Nov 2015 20:46:47 +0000 (15:46 -0500)]
Fix possible internal overflow in numeric division.

div_var_fast() postpones propagating carries in the same way as mul_var(),
so it has the same corner-case overflow risk we fixed in 246693e5ae8a36f0,
namely that the size of the carries has to be accounted for when setting
the threshold for executing a carry propagation step.  We've not devised
a test case illustrating the brokenness, but the required fix seems clear
enough.  Like the previous fix, back-patch to all active branches.

Dean Rasheed

9 years agoBack-patch fixes to make TAP tests work on Windows.
Tom Lane [Tue, 17 Nov 2015 19:10:24 +0000 (14:10 -0500)]
Back-patch fixes to make TAP tests work on Windows.

This back-ports commit 13d856e177e69083 and assorted followon patches
into 9.4 and 9.5.  9.5 and HEAD are now substantially identical in all
the files touched by this commit, except that 010_pg_basebackup.pl has
a few more tests related to the new --slot option.  9.4 has many fewer
TAP tests, but the test infrastructure files are substantially the same,
with the exception that 9.4 lacks the single-tmp-install infrastructure
introduced in 9.5 (commit dcae5faccab64776).

The primary motivation for this patch is to ensure that TAP test case
fixes can be back-patched without hazards of the kind seen in commits
34557f544/06dd4b44f.  In principle it should also make the world safe
for running the TAP tests in the buildfarm in these branches; although
we might want to think about back-porting dcae5faccab64776 to 9.4 if
we're going to do that for real, because the TAP tests are quite disk
space hungry without it.

Michael Paquier did the back-porting work; original patches were by
him and assorted other people.

9 years agoMessage style fix
Peter Eisentraut [Tue, 17 Nov 2015 11:53:07 +0000 (06:53 -0500)]
Message style fix

from Euler Taveira

9 years agoImprove message
Peter Eisentraut [Tue, 17 Nov 2015 03:26:32 +0000 (22:26 -0500)]
Improve message

9 years agoMessage improvements
Peter Eisentraut [Tue, 17 Nov 2015 02:16:42 +0000 (21:16 -0500)]
Message improvements

9 years agodoc: Fix commas and improve spacing
Peter Eisentraut [Mon, 16 Nov 2015 23:59:55 +0000 (18:59 -0500)]
doc: Fix commas and improve spacing

9 years agoSpeed up ruleutils' name de-duplication code, and fix overlength-name case.
Tom Lane [Mon, 16 Nov 2015 18:45:17 +0000 (13:45 -0500)]
Speed up ruleutils' name de-duplication code, and fix overlength-name case.

Since commit 11e131854f8231a21613f834c40fe9d046926387, ruleutils.c has
attempted to ensure that each RTE in a query or plan tree has a unique
alias name.  However, the code that was added for this could be quite slow,
even as bad as O(N^3) if N identical RTE names must be replaced, as noted
by Jeff Janes.  Improve matters by building a transient hash table within
set_rtable_names.  The hash table in itself reduces the cost of detecting a
duplicate from O(N) to O(1), and we can save another factor of N by storing
the number of de-duplicated names already created for each entry, so that
we don't have to re-try names already created.  This way is probably a bit
slower overall for small range tables, but almost by definition, such cases
should not be a performance problem.

In principle the same problem applies to the column-name-de-duplication
code; but in practice that seems to be less of a problem, first because
N is limited since we don't support extremely wide tables, and second
because duplicate column names within an RTE are fairly rare, so that in
practice the cost is more like O(N^2) not O(N^3).  It would be very much
messier to fix the column-name code, so for now I've left that alone.

An independent problem in the same area was that the de-duplication code
paid no attention to the identifier length limit, and would happily produce
identifiers that were longer than NAMEDATALEN and wouldn't be unique after
truncation to NAMEDATALEN.  This could result in dump/reload failures, or
perhaps even views that silently behaved differently than before.  We can
fix that by shortening the base name as needed.  Fix it for both the
relation and column name cases.

In passing, check for interrupts in set_rtable_names, just in case it's
still slow enough to be an issue.

Back-patch to 9.3 where this code was introduced.

9 years agoFix ruleutils.c's dumping of whole-row Vars in ROW() and VALUES() contexts.
Tom Lane [Sun, 15 Nov 2015 19:41:09 +0000 (14:41 -0500)]
Fix ruleutils.c's dumping of whole-row Vars in ROW() and VALUES() contexts.

Normally ruleutils prints a whole-row Var as "foo.*".  We already knew that
that doesn't work at top level of a SELECT list, because the parser would
treat the "*" as a directive to expand the reference into separate columns,
not a whole-row Var.  However, Joshua Yanovski points out in bug #13776
that the same thing happens at top level of a ROW() construct; and some
nosing around in the parser shows that the same is true in VALUES().
Hence, apply the same workaround already devised for the SELECT-list case,
namely to add a forced cast to the appropriate rowtype in these cases.
(The alternative of just printing "foo" was rejected because it is
difficult to avoid ambiguity against plain columns named "foo".)

Back-patch to all supported branches.

9 years agopg_upgrade: properly detect file copy failure on Windows
Bruce Momjian [Sat, 14 Nov 2015 16:47:11 +0000 (11:47 -0500)]
pg_upgrade:  properly detect file copy failure on Windows

Previously, file copy failures were ignored on Windows due to an
incorrect return value check.

Report by Manu Joye

Backpatch through 9.1

9 years agoCorrect sepgsql docs with regard to RLS
Stephen Frost [Fri, 13 Nov 2015 16:06:42 +0000 (11:06 -0500)]
Correct sepgsql docs with regard to RLS

The sepgsql docs included a comment that PG doesn't support RLS.  That
is only true for versions prior to 9.5.

Update the docs for 9.5 and master to say that PG supports RLS but that
sepgsql does not yet.

Pointed out by Heikki.

Back-patch to 9.5

9 years agovacuumdb: don't prompt for passwords over and over
Alvaro Herrera [Thu, 12 Nov 2015 21:05:23 +0000 (18:05 -0300)]
vacuumdb: don't prompt for passwords over and over

Having the script prompt for passwords over and over was a preexisting
problem when it processed multiple databases or when it processed
multiple analyze stages, but the parallel mode introduced in commit
a179232047 made it worse.

Fix the annoyance by keeping a copy of the password used by the first
connection that requires one.  Since users can (currently) only have a
single password, there's no need for more complex arrangements (such as
remembering one password per database).

Per bug #13741 reported by Eric Brown.  Patch authored and
cross-reviewed by Haribabu Kommi and Michael Paquier, slightly tweaked
by Álvaro Herrera.

Discussion: http://www.postgresql.org/message-id/20151027193919.931.54948@wrigleys.postgresql.org
Backpatch to 9.5, where parallel vacuumdb was introduced.

9 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.

9 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.)

9 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.

9 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.

9 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.

9 years agoStamp 9.5beta2. REL9_5_BETA2
Robert Haas [Mon, 9 Nov 2015 19:53:52 +0000 (14:53 -0500)]
Stamp 9.5beta2.

9 years agoTranslation updates
Peter Eisentraut [Mon, 9 Nov 2015 15:21:11 +0000 (10:21 -0500)]
Translation updates

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

9 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

9 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

9 years agoFix 9.5 version of previous commit to match its log message.
Noah Misch [Sun, 8 Nov 2015 22:40:19 +0000 (17:40 -0500)]
Fix 9.5 version of previous commit to match its log message.

9 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.

9 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.

9 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

9 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.

9 years agoSet include_realm=1 default in parse_hba_line
Stephen Frost [Fri, 6 Nov 2015 16:18:33 +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

9 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.

9 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.

9 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

9 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.

9 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

9 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.

9 years agoAdd RMV to list of commands taking AE lock.
Kevin Grittner [Mon, 2 Nov 2015 12:26:28 +0000 (06:26 -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

9 years agoFix serialization anomalies due to race conditions on INSERT.
Kevin Grittner [Sat, 31 Oct 2015 19:42:46 +0000 (14:42 -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

9 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

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

9 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

9 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

9 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

9 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

9 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

9 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

9 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.

9 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

9 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

9 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.

9 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

9 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.

9 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

9 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 ...)

9 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

9 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

9 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.

9 years agoFix back-patch of commit 8e3b4d9d40244c037bbc6e182ea3fabb9347d482.
Noah Misch [Tue, 20 Oct 2015 04:57:25 +0000 (00:57 -0400)]
Fix back-patch of commit 8e3b4d9d40244c037bbc6e182ea3fabb9347d482.

master emits an extra context message compared to 9.5 and earlier.

9 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.

9 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.

9 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.

9 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.

9 years agoImprove memory-usage accounting in regular-expression compiler.
Tom Lane [Fri, 16 Oct 2015 19:36:17 +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.

9 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.

9 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.

9 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.

9 years agoFix regular-expression compiler to handle loops of constraint arcs.
Tom Lane [Fri, 16 Oct 2015 18:14:41 +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.

9 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

9 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.

9 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.

9 years agoTighten up application of parallel mode checks.
Robert Haas [Fri, 16 Oct 2015 13:59:57 +0000 (09:59 -0400)]
Tighten up application of parallel mode checks.

Commit 924bcf4f16d54c55310b28f77686608684734f42 failed to enforce
parallel mode checks during the commit of a parallel worker, because
we exited parallel mode prior to ending the transaction so that we
could pop the active snapshot.  Re-establish parallel mode during
parallel worker commit.  Without this, it's far too easy for unsafe
actions during the pre-commit sequence to crash the server instead of
hitting the error checks as intended.

Just to be extra paranoid, adjust a couple of the sanity checks in
xact.c to check not only IsInParallelMode() but also
IsParallelWorker().

9 years agoTransfer current command counter ID to parallel workers.
Robert Haas [Fri, 16 Oct 2015 13:53:34 +0000 (09:53 -0400)]
Transfer current command counter ID to parallel workers.

Commit 924bcf4f16d54c55310b28f77686608684734f42 correctly forbade
parallel workers to modify the command counter while in parallel mode,
but it inexplicably neglected to actually transfer the current command
counter from leader to workers.  This can result in the workers seeing
a different set of tuples from the leader, which is bad.  Repair.

9 years agoDon't send protocol messages to a shm_mq that no longer exists.
Robert Haas [Fri, 16 Oct 2015 13:42:33 +0000 (09:42 -0400)]
Don't send protocol messages to a shm_mq that no longer exists.

Commit 2bd9e412f92bc6a68f3e8bcb18e04955cc35001d introduced a mechanism
for relaying protocol messages from a background worker to another
backend via a shm_mq.  However, there was no provision for shutting
down the communication channel.  Therefore, a protocol message sent
late in the shutdown sequence, such as a DEBUG message resulting from
cranking up log_min_messages, could crash the server.  To fix, install
an on_dsm_detach callback that disables sending messages to the shm_mq
when the associated DSM is detached.

9 years agoFix NULL handling in datum_to_jsonb().
Tom Lane [Thu, 15 Oct 2015 17:46:09 +0000 (13:46 -0400)]
Fix NULL handling in datum_to_jsonb().

The function failed to adhere to its specification that the "tcategory"
argument should not be examined when the input value is NULL.  This
resulted in a crash in some cases.  Per bug #13680 from Boyko Yordanov.

In passing, re-pgindent some recent changes in jsonb.c, and fix a rather
ungrammatical comment.

Diagnosis and patch by Michael Paquier, cosmetic changes by me

9 years agoAllow FDWs to push down quals without breaking EvalPlanQual rechecks.
Robert Haas [Thu, 15 Oct 2015 17:00:40 +0000 (13:00 -0400)]
Allow FDWs to push down quals without breaking EvalPlanQual rechecks.

This fixes a long-standing bug which was discovered while investigating
the interaction between the new join pushdown code and the EvalPlanQual
machinery: if a ForeignScan appears on the inner side of a paramaterized
nestloop, an EPQ recheck would re-return the original tuple even if
it no longer satisfied the pushed-down quals due to changed parameter
values.

This fix adds a new member to ForeignScan and ForeignScanState and a
new argument to make_foreignscan, and requires changes to FDWs which
push down quals to populate that new argument with a list of quals they
have chosen to push down.  Therefore, I'm only back-patching to 9.5,
even though the bug is not new in 9.5.

Etsuro Fujita, reviewed by me and by Kyotaro Horiguchi.

9 years agoFix bogus comments
Alvaro Herrera [Thu, 15 Oct 2015 15:20:15 +0000 (12:20 -0300)]
Fix bogus comments

Author: Amit Langote

9 years ago-- email subject limit -----------------------------------------
Bruce Momjian [Tue, 13 Oct 2015 22:25:32 +0000 (18:25 -0400)]
-- email subject limit -----------------------------------------
-- gitweb summary limit --------------------------
pg_upgrade:  reorder controldata checks to match program output

Also improve comment for how float8_pass_by_value is used.

Backpatch through 9.5

9 years agoImprove INSERT .. ON CONFLICT error message.
Robert Haas [Tue, 13 Oct 2015 19:33:07 +0000 (15:33 -0400)]
Improve INSERT .. ON CONFLICT error message.

Peter Geoghegan, reviewed by me.