]> granicus.if.org Git - postgresql/log
postgresql
10 years agoFix *-qualification of named parameters in SQL-language functions.
Tom Lane [Mon, 3 Feb 2014 19:46:57 +0000 (14:46 -0500)]
Fix *-qualification of named parameters in SQL-language functions.

Given a composite-type parameter named x, "$1.*" worked fine, but "x.*"
not so much.  This has been broken since named parameter references were
added in commit 9bff0780cf5be2193a5bad0d3df2dbe143085264, so patch back
to 9.2.  Per bug #9085 from Hardy Falk.

10 years agoFix makefile syntax.
Andrew Dunstan [Sun, 2 Feb 2014 00:52:11 +0000 (19:52 -0500)]
Fix makefile syntax.

10 years agoFix some wide-character bugs in the text-search parser.
Tom Lane [Sat, 1 Feb 2014 23:27:01 +0000 (18:27 -0500)]
Fix some wide-character bugs in the text-search parser.

In p_isdigit and other character class test functions generated by the
p_iswhat macro, the code path for non-C locales with multibyte encodings
contained a bogus pointer cast that would accidentally fail to malfunction
if types wchar_t and wint_t have the same width.  Apparently that is true
on most platforms, but not on recent Cygwin releases.  Remove the cast,
as it seems completely unnecessary (I think it arose from a false analogy
to the need to cast to unsigned char when dealing with the <ctype.h>
functions).  Per bug #8970 from Marco Atzeri.

In the same functions, the code path for C locale with a multibyte encoding
simply ANDed each wide character with 0xFF before passing it to the
corresponding <ctype.h> function.  This could result in false positive
answers for some non-ASCII characters, so use a range test instead.
Noted by me while investigating Marco's complaint.

Also, remove some useless though not actually buggy maskings and casts
in the hand-coded p_isalnum and p_isalpha functions, which evidently
got tested a bit more carefully than the macro-generated functions.

10 years agofix whitespace
Andrew Dunstan [Sat, 1 Feb 2014 21:30:18 +0000 (16:30 -0500)]
fix whitespace

10 years agoFix some more bugs in signal handlers and process shutdown logic.
Tom Lane [Sat, 1 Feb 2014 21:21:04 +0000 (16:21 -0500)]
Fix some more bugs in signal handlers and process shutdown logic.

WalSndKill was doing things exactly backwards: it should first clear
MyWalSnd (to stop signal handlers from touching MyWalSnd->latch),
then disown the latch, and only then mark the WalSnd struct unused by
clearing its pid field.

Also, WalRcvSigUsr1Handler and worker_spi_sighup failed to preserve
errno, which is surely a requirement for any signal handler.

Per discussion of recent buildfarm failures.  Back-patch as far
as the relevant code exists.

10 years agoDon't use deprecated dllwrap on Cygwin.
Andrew Dunstan [Sat, 1 Feb 2014 21:13:46 +0000 (16:13 -0500)]
Don't use deprecated dllwrap on Cygwin.

The preferred method is to use "cc -shared", and this allows binaries
to be rebased if required, unlike dllwrap.

Backpatch to 9.0 where we have buildfarm coverage.

There are still some issues with Cygwin, especially modern Cygwin, but
this helps us get closer to good support.

Marco Atzeri.

10 years agoCopy the libpq DLL to the bin directory on Mingw and Cygwin.
Andrew Dunstan [Sat, 1 Feb 2014 20:16:18 +0000 (15:16 -0500)]
Copy the libpq DLL to the bin directory on Mingw and Cygwin.

This has long been done by the MSVC build system, and has caused
confusion in the past when programs like psql have failed to start
because they can't find the DLL. If it's in the same directory as it now
will be they will find it.

Backpatch to all live branches.

10 years agoClear MyProc and MyProcSignalState before they become invalid.
Robert Haas [Sat, 1 Feb 2014 02:31:08 +0000 (21:31 -0500)]
Clear MyProc and MyProcSignalState before they become invalid.

Evidence from buildfarm member crake suggests that the new test_shm_mq
module is routinely crashing the server due to the arrival of a SIGUSR1
after the shared memory segment has been unmapped.  Although processes
using the new dynamic background worker facilities are more likely to
receive a SIGUSR1 around this time, the problem is also possible on older
branches, so I'm back-patching the parts of this change that apply to
older branches as far as they apply.

It's already generally the case that code checks whether these pointers
are NULL before deferencing them, so the important thing is mostly to
make sure that they do get set to NULL before they become invalid.  But
in master, there's one case in procsignal_sigusr1_handler that lacks a
NULL guard, so add that.

Patch by me; review by Tom Lane.

10 years agoFix potential coredump on bad locale value in pg_upgrade.
Tom Lane [Thu, 30 Jan 2014 23:10:06 +0000 (18:10 -0500)]
Fix potential coredump on bad locale value in pg_upgrade.

Thinko in error report (and a typo in the message text, too).  We're
failing anyway, but it would be good to print something useful first.
Noted while reviewing a patch to make pg_upgrade's locale code laxer.

10 years agoFix unsafe references to errno within error messaging logic.
Tom Lane [Thu, 30 Jan 2014 01:04:05 +0000 (20:04 -0500)]
Fix unsafe references to errno within error messaging logic.

Various places were supposing that errno could be expected to hold still
within an ereport() nest or similar contexts.  This isn't true necessarily,
though in some cases it accidentally failed to fail depending on how the
compiler chanced to order the subexpressions.  This class of thinko
explains recent reports of odd failures on clang-built versions, typically
missing or inappropriate HINT fields in messages.

Problem identified by Christian Kruse, who also submitted the patch this
commit is based on.  (I fixed a few issues in his patch and found a couple
of additional places with the same disease.)

Back-patch as appropriate to all supported branches.

10 years agoFix bugs in PQhost().
Fujii Masao [Thu, 23 Jan 2014 14:01:06 +0000 (23:01 +0900)]
Fix bugs in PQhost().

In the platform that doesn't support Unix-domain socket, when
neither host nor hostaddr are specified, the default host
'localhost' is used to connect to the server and PQhost() must
return that, but it didn't. This patch fixes PQhost() so that
it returns the default host in that case.

Also this patch fixes PQhost() so that it doesn't return
Unix-domain socket directory path in the platform that doesn't
support Unix-domain socket.

Back-patch to all supported versions.

10 years agoAllow type_func_name_keywords in even more places
Stephen Frost [Wed, 22 Jan 2014 03:49:22 +0000 (22:49 -0500)]
Allow type_func_name_keywords in even more places

A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed
type_func_name_keywords to be used in more places, including role
identifiers.  Unfortunately, that commit missed out on cases where
name_list was used for lists-of-roles, eg: for DROP ROLE.  This
resulted in the unfortunate situation that you could CREATE a role
with a type_func_name_keywords-allowed identifier, but not DROP it
(directly- ALTER could be used to rename it to something which
could be DROP'd).

This extends allowing type_func_name_keywords to places where role
lists can be used.

Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was.

10 years agoTweak parse location assignment for CURRENT_DATE and related constructs.
Tom Lane [Tue, 21 Jan 2014 21:34:35 +0000 (16:34 -0500)]
Tweak parse location assignment for CURRENT_DATE and related constructs.

All these constructs generate parse trees consisting of a Const and
a run-time type coercion (perhaps a FuncExpr or a CoerceViaIO).  Modify
the raw parse output so that we end up with the original token's location
attached to the type coercion node while the Const has location -1;
before, it was the other way around.  This makes no difference in terms
of what exprLocation() will say about the parse tree as a whole, so it
should not have any user-visible impact.  The point of changing it is that
we do not want contrib/pg_stat_statements to treat these constructs as
replaceable constants.  It will do the right thing if the Const has
location -1 rather than a valid location.

This is a pretty ugly hack, but then this code is ugly already; we should
someday replace this translation with special-purpose parse node(s) that
would allow ruleutils.c to reconstruct the original query text.

(See also commit 5d3fcc4c2e137417ef470d604fee5e452b22f6a7, which also
hacked location assignment rules for the benefit of pg_stat_statements.)

Back-patch to 9.2 where pg_stat_statements grew the ability to recognize
replaceable constants.

Kyotaro Horiguchi

10 years agoAllow SET TABLESPACE to database default
Stephen Frost [Sat, 18 Jan 2014 23:41:52 +0000 (18:41 -0500)]
Allow SET TABLESPACE to database default

We've always allowed CREATE TABLE to create tables in the database's default
tablespace without checking for CREATE permissions on that tablespace.
Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE
didn't pick up on that exception.

This changes ALTER TABLE ... SET TABLESPACE to allow the database's default
tablespace without checking for CREATE rights on that tablespace, just as
CREATE TABLE works today.  Users could always do this through a series of
commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so
let's fix the oversight in SET TABLESPACE's original implementation.

10 years agoFix client-only installation
Peter Eisentraut [Sat, 18 Jan 2014 04:08:22 +0000 (23:08 -0500)]
Fix client-only installation

The psql Makefile was not creating $(datadir) before installing
psqlrc.sample there.

In most cases, the directory would be created in some other way, but for
the documented from-source client-only installation procedure, it could
fail.

Reported-by: Mike Blackwell <mike.blackwell@rrd.com>
10 years agoImprove FILES section of psql reference page.
Tom Lane [Wed, 15 Jan 2014 00:27:57 +0000 (19:27 -0500)]
Improve FILES section of psql reference page.

Primarily, explain where to find the system-wide psqlrc file, per recent
gripe from John Sutton.  Do some general wordsmithing and improve the
markup, too.

Also adjust psqlrc.sample so its comments about file location are somewhat
trustworthy.  (Not sure why we bother with this file when it's empty,
but whatever.)

Back-patch to 9.2 where the startup file naming scheme was last changed.

10 years agoFix multiple bugs in index page locking during hot-standby WAL replay.
Tom Lane [Tue, 14 Jan 2014 22:34:54 +0000 (17:34 -0500)]
Fix multiple bugs in index page locking during hot-standby WAL replay.

In ordinary operation, VACUUM must be careful to take a cleanup lock on
each leaf page of a btree index; this ensures that no indexscans could
still be "in flight" to heap tuples due to be deleted.  (Because of
possible index-tuple motion due to concurrent page splits, it's not enough
to lock only the pages we're deleting index tuples from.)  In Hot Standby,
the WAL replay process must likewise lock every leaf page.  There were
several bugs in the code for that:

* The replay scan might come across unused, all-zero pages in the index.
While btree_xlog_vacuum itself did the right thing (ie, nothing) with
such pages, xlogutils.c supposed that such pages must be corrupt and
would throw an error.  This accounts for various reports of replication
failures with "PANIC: WAL contains references to invalid pages".  To
fix, add a ReadBufferMode value that instructs XLogReadBufferExtended
not to complain when we're doing this.

* btree_xlog_vacuum performed the extra locking if standbyState ==
STANDBY_SNAPSHOT_READY, but that's not the correct test: we won't open up
for hot standby queries until the database has reached consistency, and
we don't want to do the extra locking till then either, for fear of reading
corrupted pages (which bufmgr.c would complain about).  Fix by exporting a
new function from xlog.c that will report whether we're actually in hot
standby replay mode.

* To ensure full coverage of the index in the replay scan, btvacuumscan
would emit a dummy WAL record for the last page of the index, if no
vacuuming work had been done on that page.  However, if the last page
of the index is all-zero, that would result in corruption of said page,
since the functions called on it weren't prepared to handle that case.
There's no need to lock any such pages, so change the logic to target
the last normal leaf page instead.

The first two of these bugs were diagnosed by Andres Freund, the other one
by me.  Fixes based on ideas from Heikki Linnakangas and myself.

This has been wrong since Hot Standby was introduced, so back-patch to 9.0.

10 years agoFix possible buffer overrun in contrib/pg_trgm.
Tom Lane [Mon, 13 Jan 2014 18:07:17 +0000 (13:07 -0500)]
Fix possible buffer overrun in contrib/pg_trgm.

Allow for the possibility that folding a string to lower case makes it
longer (due to replacing a character with a longer multibyte character).
This doesn't change the number of trigrams that will be extracted, but
it does affect the required size of an intermediate buffer in
generate_trgm().  Per bug #8821 from Ufuk Kayserilioglu.

Also install some checks that the input string length is not so large
as to cause overflow in the calculations of palloc request sizes.

Back-patch to all supported versions.

10 years agoFix calculation of ISMN check digit.
Heikki Linnakangas [Mon, 13 Jan 2014 13:42:35 +0000 (15:42 +0200)]
Fix calculation of ISMN check digit.

This has always been broken, so back-patch to all supported versions.

Fabien COELHO

10 years agoFix possible crashes due to using elog/ereport too early in startup.
Tom Lane [Sat, 11 Jan 2014 21:35:34 +0000 (16:35 -0500)]
Fix possible crashes due to using elog/ereport too early in startup.

Per reports from Andres Freund and Luke Campbell, a server failure during
set_pglocale_pgservice results in a segfault rather than a useful error
message, because the infrastructure needed to use ereport hasn't been
initialized; specifically, MemoryContextInit hasn't been called.
One known cause of this is starting the server in a directory it
doesn't have permission to read.

We could try to prevent set_pglocale_pgservice from using anything that
depends on palloc or elog, but that would be messy, and the odds of future
breakage seem high.  Moreover there are other things being called in main.c
that look likely to use palloc or elog too --- perhaps those things
shouldn't be there, but they are there today.  The best solution seems to
be to move the call of MemoryContextInit to very early in the backend's
real main() function.  I've verified that an elog or ereport occurring
immediately after that is now capable of sending something useful to
stderr.

I also added code to elog.c to print something intelligible rather than
just crashing if MemoryContextInit hasn't created the ErrorContext.
This could happen if MemoryContextInit itself fails (due to malloc
failure), and provides some future-proofing against someone trying to
sneak in new code even earlier in server startup.

Back-patch to all supported branches.  Since we've only heard reports of
this type of failure recently, it may be that some recent change has made
it more likely to see a crash of this kind; but it sure looks like it's
broken all the way back.

10 years agoFix compute_scalar_stats() for case that all values exceed WIDTH_THRESHOLD.
Tom Lane [Sat, 11 Jan 2014 18:41:41 +0000 (13:41 -0500)]
Fix compute_scalar_stats() for case that all values exceed WIDTH_THRESHOLD.

The standard typanalyze functions skip over values whose detoasted size
exceeds WIDTH_THRESHOLD (1024 bytes), so as to limit memory bloat during
ANALYZE.  However, we (I think I, actually :-() failed to consider the
possibility that *every* non-null value in a column is too wide.  While
compute_minimal_stats() seems to behave reasonably anyway in such a case,
compute_scalar_stats() just fell through and generated no pg_statistic
entry at all.  That's unnecessarily pessimistic: we can still produce
valid stanullfrac and stawidth values in such cases, since we do include
too-wide values in the average-width calculation.  Furthermore, since the
general assumption in this code is that too-wide values are probably all
distinct from each other, it seems reasonable to set stadistinct to -1
("all distinct").

Per complaint from Kadri Raudsepp.  This has been like this since roughly
neolithic times, so back-patch to all supported branches.

10 years agoFix descriptor output in ECPG.
Michael Meskes [Thu, 9 Jan 2014 14:41:51 +0000 (15:41 +0100)]
Fix descriptor output in ECPG.

While working on most platforms the old way sometimes created alignment
problems. This should fix it. Also the regresion tests were updated to test for
the reported case.

Report and fix by MauMau <maumau307@gmail.com>

10 years agoFix "cannot accept a set" error when only some arms of a CASE return a set.
Tom Lane [Thu, 9 Jan 2014 01:18:13 +0000 (20:18 -0500)]
Fix "cannot accept a set" error when only some arms of a CASE return a set.

In commit c1352052ef1d4eeb2eb1d822a207ddc2d106cb13, I implemented an
optimization that assumed that a function's argument expressions would
either always return a set (ie multiple rows), or always not.  This is
wrong however: we allow CASE expressions in which some arms return a set
of some type and others just return a scalar of that type.  There may be
other examples as well.  To fix, replace the run-time test of whether an
argument returned a set with a static precheck (expression_returns_set).
This adds a little bit of query startup overhead, but it seems barely
measurable.

Per bug #8228 from David Johnston.  This has been broken since 8.0,
so patch all supported branches.

10 years agoFix pause_at_recovery_target + recovery_target_inclusive combination.
Heikki Linnakangas [Wed, 8 Jan 2014 21:06:03 +0000 (23:06 +0200)]
Fix pause_at_recovery_target + recovery_target_inclusive combination.

If pause_at_recovery_target is set, recovery pauses *before* applying the
target record, even if recovery_target_inclusive is set. If you then
continue with pg_xlog_replay_resume(), it will apply the target record
before ending recovery. In other words, if you log in while it's paused
and verify that the database looks OK, ending recovery changes its state
again, possibly destroying data that you were tring to salvage with PITR.

Backpatch to 9.1, this has been broken since pause_at_recovery_target was
added.

10 years agoFix bug in determining when recovery has reached consistency.
Heikki Linnakangas [Wed, 8 Jan 2014 09:39:55 +0000 (11:39 +0200)]
Fix bug in determining when recovery has reached consistency.

When starting WAL replay from an online checkpoint, the last replayed WAL
record variable was initialized using the checkpoint record's location, even
though the records between the REDO location and the checkpoint record had
not been replayed yet. That was noted as "slightly confusing" but harmless
in the comment, but in some cases, it fooled CheckRecoveryConsistency to
incorrectly conclude that we had already reached a consistent state
immediately at the beginning of WAL replay. That caused the system to accept
read-only connections in hot standby mode too early, and also PANICs with
message "WAL contains references to invalid pages".

Fix by initializing the variables to the REDO location instead.

In 9.2 and above, change CheckRecoveryConsistency() to use
lastReplayedEndRecPtr variable when checking if backup end location has
been reached. It was inconsistently using EndRecPtr for that check, but
lastReplayedEndRecPtr when checking min recovery point. It made no
difference before this patch, because in all the places where
CheckRecoveryConsistency was called the two variables were the same, but
it was always an accident waiting to happen, and would have been wrong
after this patch anyway.

Report and analysis by Tomonari Katsumata, bug #8686. Backpatch to 9.0,
where hot standby was introduced.

10 years agoUpdate copyright for 2014
Bruce Momjian [Tue, 7 Jan 2014 21:05:29 +0000 (16:05 -0500)]
Update copyright for 2014

Update all files in head, and files COPYRIGHT and legal.sgml in all back
branches.

10 years agoMove permissions check from do_pg_start_backup to pg_start_backup
Magnus Hagander [Tue, 7 Jan 2014 16:47:52 +0000 (17:47 +0100)]
Move permissions check from do_pg_start_backup to pg_start_backup

And the same for do_pg_stop_backup. The code in do_pg_* is not allowed
to access the catalogs. For manual base backups, the permissions
check can be handled in the calling function, and for streaming
base backups only users with the required permissions can get past
the authentication step in the first place.

Reported by Antonin Houska, diagnosed by Andres Freund

10 years agoAvoid including tablespaces inside PGDATA twice in base backups
Magnus Hagander [Tue, 7 Jan 2014 16:04:40 +0000 (17:04 +0100)]
Avoid including tablespaces inside PGDATA twice in base backups

If a tablespace was crated inside PGDATA it was backed up both as part
of the PGDATA backup and as the backup of the tablespace. Avoid this
by skipping any directory inside PGDATA that contains one of the active
tablespaces.

Dimitri Fontaine and Magnus Hagander

10 years agoFix translatability markings in psql, and add defenses against future bugs.
Tom Lane [Sat, 4 Jan 2014 21:05:23 +0000 (16:05 -0500)]
Fix translatability markings in psql, and add defenses against future bugs.

Several previous commits have added columns to various \d queries without
updating their translate_columns[] arrays, leading to potentially incorrect
translations in NLS-enabled builds.  Offenders include commit 893686762
(added prosecdef to \df+), c9ac00e6e (added description to \dc+) and
3b17efdfd (added description to \dC+).  Fix those cases back to 9.3 or
9.2 as appropriate.

Since this is evidently more easily missed than one would like, in HEAD
also add an Assert that the supplied array is long enough.  This requires
an API change for printQuery(), so it seems inappropriate for back
branches, but presumably all future changes will be tested in HEAD anyway.

In HEAD and 9.3, also clean up a whole lot of sloppiness in the emitted
SQL for \dy (event triggers): lack of translatability due to failing to
pass words-to-be-translated through gettext_noop(), inadequate schema
qualification, and sloppy formatting resulting in unnecessarily ugly
-E output.

Peter Eisentraut and Tom Lane, per bug #8702 from Sergey Burladyan

10 years agoDo not use an empty hostname.
Michael Meskes [Wed, 1 Jan 2014 11:24:19 +0000 (12:24 +0100)]
Do not use an empty hostname.

When trying to connect to a given database libecpg should not try using an
empty hostname if no hostname was given.

10 years agoDon't attempt to limit target database for pg_restore.
Kevin Grittner [Sun, 29 Dec 2013 21:18:38 +0000 (15:18 -0600)]
Don't attempt to limit target database for pg_restore.

There was an apparent attempt to limit the target database for
pg_restore to version 7.1.0 or later.  Due to a leading zero this
was interpreted as an octal number, which allowed targets with
version numbers down to 2.87.36.  The lowest actual release above
that was 6.0.0, so that was effectively the limit.

Since the success of the restore attempt will depend primarily on
on what statements were generated by the dump run, we don't want
pg_restore trying to guess whether a given target should be allowed
based on version number.  Allow a connection to any version.  Since
it is very unlikely that anyone would be using a recent version of
pg_restore to restore to a pre-6.0 database, this has little to no
practical impact, but it makes the code less confusing to read.

Issue reported and initial patch suggestion from Joel Jacobson
based on an article by Andrey Karpov reporting on issues found by
PVS-Studio static code analyzer.  Final patch based on analysis by
Tom Lane.  Back-patch to all supported branches.

10 years agoProperly detect invalid JSON numbers when generating JSON.
Andrew Dunstan [Fri, 27 Dec 2013 22:21:27 +0000 (17:21 -0500)]
Properly detect invalid JSON numbers when generating JSON.

Instead of looking for characters that aren't valid in JSON numbers, we
simply pass the output string through the JSON number parser, and if it
fails the string is quoted. This means among other things that money and
domains over money will be quoted correctly and generate valid JSON.

Fixes bug #8676 reported by Anderson Cristian da Silva.

Backpatched to 9.2 where JSON generation was introduced.

10 years agoFix misplaced right paren bugs in pgstatfuncs.c.
Kevin Grittner [Fri, 27 Dec 2013 21:41:02 +0000 (15:41 -0600)]
Fix misplaced right paren bugs in pgstatfuncs.c.

The bug would only show up if the C sockaddr structure contained
zero in the first byte for a valid address; otherwise it would
fail to fail, which is probably why it went unnoticed for so long.

Patch submitted by Joel Jacobson after seeing an article by Andrey
Karpov in which he reports finding this through static code
analysis using PVS-Studio.  While I was at it I moved a definition
of a local variable referenced in the buggy code to a more local
context.

Backpatch to all supported branches.

10 years agoAdd "SHIFT_JIS" as an accepted encoding name for locale checking.
Tatsuo Ishii [Sun, 15 Dec 2013 01:33:06 +0000 (10:33 +0900)]
Add "SHIFT_JIS" as an accepted encoding name for locale checking.

When locale is "ja_JP.SJIS", nl_langinfo(CODESET) returns "SHIFT_JIS"
on some platforms, at least on RedHat Linux. So the encoding/locale
match table (encoding_match_list) needs the entry. Otherwise client
encoding is set to SQL_ASCII.

Back patch to all supported branches.

10 years agoFix inherited UPDATE/DELETE with UNION ALL subqueries.
Tom Lane [Sat, 14 Dec 2013 22:34:00 +0000 (17:34 -0500)]
Fix inherited UPDATE/DELETE with UNION ALL subqueries.

Fix an oversight in commit b3aaf9081a1a95c245fd605dcf02c91b3a5c3a29: we do
indeed need to process the planner's append_rel_list when copying RTE
subqueries, because if any of them were flattenable UNION ALL subqueries,
the append_rel_list shows which subquery RTEs were pulled up out of which
other ones.  Without this, UNION ALL subqueries aren't correctly inserted
into the update plans for inheritance child tables after the first one,
typically resulting in no update happening for those child table(s).
Per report from Victor Yegorov.

Experimentation with this case also exposed a fault in commit
a7b965382cf0cb30aeacb112572718045e6d4be7: if an inherited UPDATE/DELETE
was proven totally dummy by constraint exclusion, we might arrive at
add_rtes_to_flat_rtable with root->simple_rel_array being NULL.  This
should be interpreted as not having any RelOptInfos.  I chose to code
the guard as a check against simple_rel_array_size, so as to also
provide some protection against indexing off the end of the array.

Back-patch to 9.2 where the faulty code was added.

10 years agoAdd HOLD/RESUME_INTERRUPTS in HandleCatchupInterrupt/HandleNotifyInterrupt.
Tom Lane [Fri, 13 Dec 2013 19:05:19 +0000 (14:05 -0500)]
Add HOLD/RESUME_INTERRUPTS in HandleCatchupInterrupt/HandleNotifyInterrupt.

This prevents a possible longjmp out of the signal handler if a timeout
or SIGINT occurs while something within the handler has transiently set
ImmediateInterruptOK.  For safety we must hold off the timeout or cancel
error until we're back in mainline, or at least till we reach the end of
the signal handler when ImmediateInterruptOK was true at entry.  This
syncs these functions with the logic now present in handle_sig_alarm.

AFAICT there is no live bug here in 9.0 and up, because I don't think we
currently can wait for any heavyweight lock inside these functions, and
there is no other code (except read-from-client) that will turn on
ImmediateInterruptOK.  However, that was not true pre-9.0: in older
branches ProcessIncomingNotify might block trying to lock pg_listener, and
then a SIGINT could lead to undesirable control flow.  It might be all
right anyway given the relatively narrow code ranges in which NOTIFY
interrupts are enabled, but for safety's sake I'm back-patching this.

10 years agoFix ancient docs/comments thinko: XID comparison is mod 2^32, not 2^31.
Tom Lane [Thu, 12 Dec 2013 17:39:57 +0000 (12:39 -0500)]
Fix ancient docs/comments thinko: XID comparison is mod 2^32, not 2^31.

Pointed out by Gianni Ciolli.

10 years agoTweak placement of explicit ANALYZE commands in the regression tests.
Tom Lane [Wed, 11 Dec 2013 20:08:38 +0000 (15:08 -0500)]
Tweak placement of explicit ANALYZE commands in the regression tests.

Make the COPY test, which loads most of the large static tables used in
the tests, also explicitly ANALYZE those tables.  This allows us to get
rid of various ad-hoc, and rather redundant, ANALYZE commands that had
gotten stuck into various test scripts over time to ensure we got
consistent plan choices.  (We could have done a database-wide ANALYZE,
but that would cause stats to get attached to the small static tables
too, which results in plan changes compared to the historical behavior.
I'm not sure that's a good idea, so not going that far for now.)

Back-patch to 9.0, since 9.0 and 9.1 are currently sometimes failing
regression tests for lack of an "ANALYZE tenk1" in the subselect test.
There's no need for this in 8.4 since we didn't print any plans back
then.

10 years agoFix possible crash with nested SubLinks.
Tom Lane [Tue, 10 Dec 2013 21:10:24 +0000 (16:10 -0500)]
Fix possible crash with nested SubLinks.

An expression such as WHERE (... x IN (SELECT ...) ...) IN (SELECT ...)
could produce an invalid plan that results in a crash at execution time,
if the planner attempts to flatten the outer IN into a semi-join.
This happens because convert_testexpr() was not expecting any nested
SubLinks and would wrongly replace any PARAM_SUBLINK Params belonging
to the inner SubLink.  (I think the comment denying that this case could
happen was wrong when written; it's certainly been wrong for quite a long
time, since very early versions of the semijoin flattening logic.)

Per report from Teodor Sigaev.  Back-patch to all supported branches.

10 years agoFix performance regression in dblink connection speed.
Joe Conway [Sun, 8 Dec 2013 00:59:35 +0000 (16:59 -0800)]
Fix performance regression in dblink connection speed.

Previous commit e5de601267d98c5d60df6de8d436685c7105d149 modified dblink
to ensure client encoding matched the server. However the added
PQsetClientEncoding() call added significant overhead. Restore original
performance in the common case where client encoding already matches
server encoding by doing nothing in that case. Applies to all active
branches.

Issue reported and work sponsored by Zonar Systems.

10 years agoClear retry flags properly in replacement OpenSSL sock_write function.
Tom Lane [Thu, 5 Dec 2013 17:48:35 +0000 (12:48 -0500)]
Clear retry flags properly in replacement OpenSSL sock_write function.

Current OpenSSL code includes a BIO_clear_retry_flags() step in the
sock_write() function.  Either we failed to copy the code correctly, or
they added this since we copied it.  In any case, lack of the clear step
appears to be the cause of the server lockup after connection loss reported
in bug #8647 from Valentine Gogichashvili.  Assume that this is correct
coding for all OpenSSL versions, and hence back-patch to all supported
branches.

Diagnosis and patch by Alexander Kukushkin.

10 years agoFix full-page writes of internal GIN pages.
Heikki Linnakangas [Tue, 3 Dec 2013 20:13:16 +0000 (22:13 +0200)]
Fix full-page writes of internal GIN pages.

Insertion to a non-leaf GIN page didn't make a full-page image of the page,
which is wrong. The code used to do it correctly, but was changed (commit
853d1c3103fa961ae6219f0281885b345593d101) because the redo-routine didn't
track incomplete splits correctly when the page was restored from a full
page image. Of course, that was not right way to fix it, the redo routine
should've been fixed instead. The redo-routine was surreptitiously fixed
in 2010 (commit 4016bdef8aded77b4903c457050622a5a1815c16), so all we need
to do now is revert the code that creates the record to its original form.

This doesn't change the format of the WAL record.

Backpatch to all supported versions.

10 years agoFix crash in assign_collations_walker for EXISTS with empty SELECT list.
Tom Lane [Tue, 3 Dec 2013 01:28:53 +0000 (20:28 -0500)]
Fix crash in assign_collations_walker for EXISTS with empty SELECT list.

We (I think I, actually) forgot about this corner case while coding
collation resolution.  Per bug #8648 from Arjen Nienhuis.

10 years agoStamp 9.2.6. REL9_2_6
Tom Lane [Mon, 2 Dec 2013 21:00:18 +0000 (16:00 -0500)]
Stamp 9.2.6.

10 years agoUpdate release notes for 9.3.2, 9.2.6, 9.1.11, 9.0.15, 8.4.19.
Tom Lane [Mon, 2 Dec 2013 20:54:01 +0000 (15:54 -0500)]
Update release notes for 9.3.2, 9.2.6, 9.1.11, 9.0.15, 8.4.19.

10 years agoFix incomplete backpatch of pg_multixact truncation changes to <= 9.2
Alvaro Herrera [Mon, 2 Dec 2013 16:28:24 +0000 (13:28 -0300)]
Fix incomplete backpatch of pg_multixact truncation changes to <= 9.2

The backpatch of a95335b544d9c8377e9dc7a399d8e9a155895f82 to 9.2, 9.1
and 9.0 was incomplete, missing changes to xlog.c, primarily the call
to TrimMultiXact(). Testing presumably didn't show a problem without
these changes because TrimMultiXact() performs defense-in-depth work,
which is not strictly necessary.

It also missed moving StartupMultiXact() which would have been
problematic if a restartpoing happened in exactly the wrong moment,
causing a transient error.

Andres Freund

10 years agoTranslation updates
Peter Eisentraut [Mon, 2 Dec 2013 05:08:10 +0000 (00:08 -0500)]
Translation updates

10 years agoUpdate time zone data files to tzdata release 2013h.
Tom Lane [Sun, 1 Dec 2013 19:11:44 +0000 (14:11 -0500)]
Update time zone data files to tzdata release 2013h.

DST law changes in Argentina, Brazil, Jordan, Libya, Liechtenstein,
Morocco, Palestine.  New timezone abbreviations WIB, WIT, WITA for
Indonesia.

10 years agoBack-patch src/timezone/known_abbrevs.txt into all active branches.
Tom Lane [Sun, 1 Dec 2013 19:09:31 +0000 (14:09 -0500)]
Back-patch src/timezone/known_abbrevs.txt into all active branches.

Needed so that timezone data update patches can be cherry-picked
into older branches conveniently.

10 years agoFix pg_dumpall to work for databases flagged as read-only.
Kevin Grittner [Sat, 30 Nov 2013 17:33:54 +0000 (11:33 -0600)]
Fix pg_dumpall to work for databases flagged as read-only.

pg_dumpall's charter is to be able to recreate a database cluster's
contents in a virgin installation, but it was failing to honor that
contract if the cluster had any ALTER DATABASE SET
default_transaction_read_only settings.  By including a SET command
for the connection for each connection opened by pg_dumpall output,
errors are avoided and the source cluster is successfully
recreated.

There was discussion of whether to also set this for the connection
applying pg_dump output, but it was felt that it was both less
appropriate in that context, and far easier to work around.

Backpatch to all supported branches.

10 years agoTruncate pg_multixact/'s contents during crash recovery
Alvaro Herrera [Fri, 29 Nov 2013 14:26:41 +0000 (11:26 -0300)]
Truncate pg_multixact/'s contents during crash recovery

Commit 9dc842f08 of 8.2 era prevented MultiXact truncation during crash
recovery, because there was no guarantee that enough state had been
setup, and because it wasn't deemed to be a good idea to remove data
during crash recovery anyway.  Since then, due to Hot-Standby, streaming
replication and PITR, the amount of time a cluster can spend doing crash
recovery has increased significantly, to the point that a cluster may
even never come out of it.  This has made not truncating the content of
pg_multixact/ not defensible anymore.

To fix, take care to setup enough state for multixact truncation before
crash recovery starts (easy since checkpoints contain the required
information), and move the current end-of-recovery actions to a new
TrimMultiXact() function, analogous to TrimCLOG().

At some later point, this should probably done similarly to the way
clog.c is doing it, which is to just WAL log truncations, but we can't
do that for the back branches.

Back-patch to 9.0.  8.4 also has the problem, but since there's no hot
standby there, it's much less pressing.  In 9.2 and earlier, this patch
is simpler than in newer branches, because multixact access during
recovery isn't required.  Add appropriate checks to make sure that's not
happening.

Andres Freund

10 years agoFix assorted issues in pg_ctl's pgwin32_CommandLine().
Tom Lane [Fri, 29 Nov 2013 23:34:14 +0000 (18:34 -0500)]
Fix assorted issues in pg_ctl's pgwin32_CommandLine().

Ensure that the invocation command for postgres or pg_ctl runservice
double-quotes the executable's pathname; failure to do this leads to
trouble when the path contains spaces.

Also, ensure that the path ends in ".exe" in both cases and uses
backslashes rather than slashes as directory separators.  The latter issue
is reported to confuse some third-party tools such as Symantec Backup Exec.

Also, rewrite the function to avoid buffer overrun issues by using a
PQExpBuffer instead of a fixed-size static buffer.  Combinations of
very long executable pathnames and very long data directory pathnames
could have caused trouble before, for example.

Back-patch to all active branches, since this code has been like this
for a long while.

Naoya Anzai and Tom Lane, reviewed by Rajeev Rastogi

10 years agoBe sure to release proc->backendLock after SetupLockInTable() failure.
Tom Lane [Fri, 29 Nov 2013 22:35:15 +0000 (17:35 -0500)]
Be sure to release proc->backendLock after SetupLockInTable() failure.

The various places that transferred fast-path locks to the main lock table
neglected to release the PGPROC's backendLock if SetupLockInTable failed
due to being out of shared memory.  In most cases this is no big deal since
ensuing error cleanup would release all held LWLocks anyway.  But there are
some hot-standby functions that don't consider failure of
FastPathTransferRelationLocks to be a hard error, and in those cases this
oversight could lead to system lockup.  For consistency, make all of these
places look the same as FastPathTransferRelationLocks.

Noted while looking for the cause of Dan Wood's bugs --- this wasn't it,
but it's a bug anyway.

10 years agoFix latent(?) race condition in LockReleaseAll.
Tom Lane [Thu, 28 Nov 2013 17:17:53 +0000 (12:17 -0500)]
Fix latent(?) race condition in LockReleaseAll.

We have for a long time checked the head pointer of each of the backend's
proclock lists and skipped acquiring the corresponding locktable partition
lock if the head pointer was NULL.  This was safe enough in the days when
proclock lists were changed only by the owning backend, but it is pretty
questionable now that the fast-path patch added cases where backends add
entries to other backends' proclock lists.  However, we don't really wish
to revert to locking each partition lock every time, because in simple
transactions that would add a lot of useless lock/unlock cycles on
already-heavily-contended LWLocks.  Fortunately, the only way that another
backend could be modifying our proclock list at this point would be if it
was promoting a formerly fast-path lock of ours; and any such lock must be
one that we'd decided not to delete in the previous loop over the locallock
table.  So it's okay if we miss seeing it in this loop; we'd just decide
not to delete it again.  However, once we've detected a non-empty list,
we'd better re-fetch the list head pointer after acquiring the partition
lock.  This guards against possibly fetching a corrupt-but-non-null pointer
if pointer fetch/store isn't atomic.  It's not clear if any practical
architectures are like that, but we've never assumed that before and don't
wish to start here.  In any case, the situation certainly deserves a code
comment.

While at it, refactor the partition traversal loop to use a for() construct
instead of a while() loop with goto's.

Back-patch, just in case the risk is real and not hypothetical.

10 years agodoc: Put data types in alphabetical order
Peter Eisentraut [Thu, 28 Nov 2013 02:50:27 +0000 (21:50 -0500)]
doc: Put data types in alphabetical order

From: Andreas Karlsson <andreas@proxel.se>

10 years agoFix stale-pointer problem in fast-path locking logic.
Tom Lane [Wed, 27 Nov 2013 23:10:06 +0000 (18:10 -0500)]
Fix stale-pointer problem in fast-path locking logic.

When acquiring a lock in fast-path mode, we must reset the locallock
object's lock and proclock fields to NULL.  They are not necessarily that
way to start with, because the locallock could be left over from a failed
lock acquisition attempt earlier in the transaction.  Failure to do this
led to all sorts of interesting misbehaviors when LockRelease tried to
clean up no-longer-related lock and proclock objects in shared memory.
Per report from Dan Wood.

In passing, modify LockRelease to elog not just Assert if it doesn't find
lock and proclock objects for a formerly fast-path lock, matching the code
in FastPathGetRelationLockEntry and LockRefindAndRelease.  This isn't a
bug but it will help in diagnosing any future bugs in this area.

Also, modify FastPathTransferRelationLocks and FastPathGetRelationLockEntry
to break out of their loops over the fastpath array once they've found the
sole matching entry.  This was inconsistently done in some search loops
and not others.

Improve assorted related comments, too.

Back-patch to 9.2 where the fast-path mechanism was introduced.

10 years agoDon't update relfrozenxid if any pages were skipped.
Heikki Linnakangas [Wed, 27 Nov 2013 11:10:16 +0000 (13:10 +0200)]
Don't update relfrozenxid if any pages were skipped.

Vacuum recognizes that it can update relfrozenxid by checking whether it has
processed all pages of a relation. Unfortunately it performed that check
after truncating the dead pages at the end of the relation, and used the new
number of pages to decide whether all pages have been scanned. If the new
number of pages happened to be smaller or equal to the number of pages
scanned, it incorrectly decided that all pages were scanned.

This can lead to relfrozenxid being updated, even though some pages were
skipped that still contain old XIDs. That can lead to data loss due to xid
wraparounds with some rows suddenly missing. This likely has escaped notice
so far because it takes a large number (~2^31) of xids being used to see the
effect, while a full-table vacuum before that would fix the issue.

The incorrect logic was introduced by commit
b4b6923e03f4d29636a94f6f4cc2f5cf6298b8c8. Backpatch this fix down to 8.4,
like that commit.

Andres Freund, with some modifications by me.

10 years agoECPG: Fix searching for quoted cursor names case-sensitively.
Michael Meskes [Wed, 27 Nov 2013 10:02:13 +0000 (11:02 +0100)]
ECPG: Fix searching for quoted cursor names case-sensitively.

Patch by Böszörményi Zoltán <zb@cybertec.at>

10 years agoDocumentation fix for ecpg.
Michael Meskes [Wed, 27 Nov 2013 10:03:59 +0000 (11:03 +0100)]
Documentation fix for ecpg.

The latest fixes removed a limitation that was still in the docs, so Zoltan updated the docs, too.

10 years agoFix typo in release note.
Fujii Masao [Wed, 27 Nov 2013 04:45:41 +0000 (13:45 +0900)]
Fix typo in release note.

Backpatch to 9.1.

Josh Kupershmidt

10 years agoECPG: Make the preprocessor emit ';' if the variable type for a list of
Michael Meskes [Tue, 26 Nov 2013 16:12:39 +0000 (17:12 +0100)]
ECPG: Make the preprocessor emit ';' if the variable type for a list of
variables is varchar. This fixes this test case:

int main(void)
{
    exec sql begin declare section;
    varchar a[50], b[50];
    exec sql end declare section;

    return 0;
}

Since varchars are internally turned into custom structs and
the type name is emitted for these variable declarations,
the preprocessed code previously had:

struct varchar_1  { ... }  a _,_  struct varchar_2  { ... }  b ;

The comma in the generated C file was a syntax error.

There are no regression test changes since it's not exercised.

Patch by Boszormenyi Zoltan <zb@cybertec.at>

10 years agoECPG: Fix offset to NULL/size indicator array.
Michael Meskes [Tue, 26 Nov 2013 16:16:39 +0000 (17:16 +0100)]
ECPG: Fix offset to NULL/size indicator array.

Patch by Boszormenyi Zoltan <zb@cybertec.at>

10 years agoDefend against bad trigger definitions in contrib/lo's lo_manage() trigger.
Tom Lane [Sun, 24 Nov 2013 03:45:46 +0000 (22:45 -0500)]
Defend against bad trigger definitions in contrib/lo's lo_manage() trigger.

This function formerly crashed if called as a statement-level trigger,
or if a column-name argument wasn't given.

In passing, add the trigger name to all error messages from the function.
(None of them are expected cases, so this shouldn't pose any compatibility
risk.)

Marc Cousin, reviewed by Sawada Masahiko

10 years agoFix array slicing of int2vector and oidvector values.
Tom Lane [Sun, 24 Nov 2013 01:04:03 +0000 (20:04 -0500)]
Fix array slicing of int2vector and oidvector values.

The previous coding labeled expressions such as pg_index.indkey[1:3] as
being of int2vector type; which is not right because the subscript bounds
of such a result don't, in general, satisfy the restrictions of int2vector.
To fix, implicitly promote the result of slicing int2vector to int2[],
or oidvector to oid[].  This is similar to what we've done with domains
over arrays, which is a good analogy because these types are very much
like restricted domains of the corresponding regular-array types.

A side-effect is that we now also forbid array-element updates on such
columns, eg while "update pg_index set indkey[4] = 42" would have worked
before if you were superuser (and corrupted your catalogs irretrievably,
no doubt) it's now disallowed.  This seems like a good thing since, again,
some choices of subscripting would've led to results not satisfying the
restrictions of int2vector.  The case of an array-slice update was
rejected before, though with a different error message than you get now.
We could make these cases work in future if we added a cast from int2[]
to int2vector (with a cast function checking the subscript restrictions)
but it seems unlikely that there's any value in that.

Per report from Ronan Dunklau.  Back-patch to all supported branches
because of the crash risks involved.

10 years agoEnsure _dosmaperr() actually sets errno correctly.
Tom Lane [Sat, 23 Nov 2013 23:24:26 +0000 (18:24 -0500)]
Ensure _dosmaperr() actually sets errno correctly.

If logging is enabled, either ereport() or fprintf() might stomp on errno
internally, causing this function to return the wrong result.  That might
only end in a misleading error report, but in any code that's examining
errno to decide what to do next, the consequences could be far graver.

This has been broken since the very first version of this file in 2006
... it's a bit astonishing that we didn't identify this long ago.

Reported by Amit Kapila, though this isn't his proposed fix.

10 years agoAvoid potential buffer overflow crash
Peter Eisentraut [Sat, 23 Nov 2013 12:25:37 +0000 (07:25 -0500)]
Avoid potential buffer overflow crash

A pointer to a C string was treated as a pointer to a "name" datum and
passed to SPI_execute_plan().  This pointer would then end up being
passed through datumCopy(), which would try to copy the entire 64 bytes
of name data, thus running past the end of the C string.  Fix by
converting the string to a proper name structure.

Found by LLVM AddressSanitizer.

10 years agoFlatten join alias Vars before pulling up targetlist items from a subquery.
Tom Lane [Fri, 22 Nov 2013 19:37:29 +0000 (14:37 -0500)]
Flatten join alias Vars before pulling up targetlist items from a subquery.

pullup_replace_vars()'s decisions about whether a pulled-up replacement
expression needs to be wrapped in a PlaceHolderVar depend on the assumption
that what looks like a Var behaves like a Var.  However, if the Var is a
join alias reference, later flattening of join aliases might replace the
Var with something that's not a Var at all, and should have been wrapped.

To fix, do a forcible pass of flatten_join_alias_vars() on the subquery
targetlist before we start to copy items out of it.  We'll re-run that
processing on the pulled-up expressions later, but that's harmless.

Per report from Ken Tanzer; the added regression test case is based on his
example.  This bug has been there since the PlaceHolderVar mechanism was
invented, but has escaped detection because the circumstances that trigger
it are fairly narrow.  You need a flattenable query underneath an outer
join, which contains another flattenable query inside a join of its own,
with a dangerous expression (a constant or something else non-strict)
in that one's targetlist.

Having seen this, I'm wondering if it wouldn't be prudent to do all
alias-variable flattening earlier, perhaps even in the rewriter.
But that would probably not be a back-patchable change.

10 years agoFix quoting in help messages in uuid-ossp extension scripts.
Tom Lane [Fri, 22 Nov 2013 17:07:53 +0000 (12:07 -0500)]
Fix quoting in help messages in uuid-ossp extension scripts.

The command we're telling people to type needs to include double-quoting
around the unfortunately-chosen extension name.  Twiddle the textual
quoting so that it looks somewhat sane.  Per gripe from roadrunner6.

10 years agoFix Hot-Standby initialization of clog and subtrans.
Heikki Linnakangas [Fri, 22 Nov 2013 12:38:59 +0000 (14:38 +0200)]
Fix Hot-Standby initialization of clog and subtrans.

These bugs can cause data loss on standbys started with hot_standby=on at
the moment they start to accept read only queries, by marking committed
transactions as uncommited. The likelihood of such corruptions is small
unless the primary has a high transaction rate.

5a031a5556ff83b8a9646892715d7fef415b83c3 fixed bugs in HS's startup logic
by maintaining less state until at least STANDBY_SNAPSHOT_PENDING state
was reached, missing the fact that both clog and subtrans are written to
before that. This only failed to fail in common cases because the usage
of ExtendCLOG in procarray.c was superflous since clog extensions are
actually WAL logged.

f44eedc3f0f347a856eea8590730769125964597/I then tried to fix the missing
extensions of pg_subtrans due to the former commit's changes - which are
not WAL logged - by performing the extensions when switching to a state
> STANDBY_INITIALIZED and not performing xid assignments before that -
again missing the fact that ExtendCLOG is unneccessary - but screwed up
twice: Once because latestObservedXid wasn't updated anymore in that
state due to the earlier commit and once by having an off-by-one error in
the loop performing extensions. This means that whenever a
CLOG_XACTS_PER_PAGE (32768 with default settings) boundary was crossed
between the start of the checkpoint recovery started from and the first
xl_running_xact record old transactions commit bits in pg_clog could be
overwritten if they started and committed in that window.

Fix this mess by not performing ExtendCLOG() in HS at all anymore since
it's unneeded and evidently dangerous and by performing subtrans
extensions even before reaching STANDBY_SNAPSHOT_PENDING.

Analysis and patch by Andres Freund. Reported by Christophe Pettus.
Backpatch down to 9.0, like the previous commit that caused this.

10 years agoCount locked pages that don't need vacuuming as scanned.
Heikki Linnakangas [Mon, 18 Nov 2013 07:51:09 +0000 (09:51 +0200)]
Count locked pages that don't need vacuuming as scanned.

Previously, if VACUUM skipped vacuuming a page because it's pinned, it
didn't count that page as scanned. However, that meant that relfrozenxid
was not bumped up either, which prevented anti-wraparound vacuum from
doing its job.

Report by Миша Тюрин, analysis and patch by Sergey Burladyn and Jeff Janes.
Backpatch to 9.2, where the skip-locked-pages behavior was introduced.

10 years agoFix incorrect loop counts in tidbitmap.c.
Tom Lane [Fri, 15 Nov 2013 23:34:14 +0000 (18:34 -0500)]
Fix incorrect loop counts in tidbitmap.c.

A couple of places that should have been iterating over WORDS_PER_CHUNK
words were iterating over WORDS_PER_PAGE words instead.  This thinko
accidentally failed to fail, because (at least on common architectures
with default BLCKSZ) WORDS_PER_CHUNK is a bit less than WORDS_PER_PAGE,
and the extra words being looked at were always zero so nothing happened.
Still, it's a bug waiting to happen if anybody ever fools with the
parameters affecting TIDBitmap sizes, and it's a small waste of cycles
too.  So back-patch to all active branches.

Etsuro Fujita

10 years agoCompute correct em_nullable_relids in get_eclass_for_sort_expr().
Tom Lane [Fri, 15 Nov 2013 21:46:25 +0000 (16:46 -0500)]
Compute correct em_nullable_relids in get_eclass_for_sort_expr().

Bug #8591 from Claudio Freire demonstrates that get_eclass_for_sort_expr
must be able to compute valid em_nullable_relids for any new equivalence
class members it creates.  I'd worried about this in the commit message
for db9f0e1d9a4a0842c814a464cdc9758c3f20b96c, but claimed that it wasn't a
problem because multi-member ECs should already exist when it runs.  That
is transparently wrong, though, because this function is also called by
initialize_mergeclause_eclasses, which runs during deconstruct_jointree.
The example given in the bug report (which the new regression test item
is based upon) fails because the COALESCE() expression is first seen by
initialize_mergeclause_eclasses rather than process_equivalence.

Fixing this requires passing the appropriate nullable_relids set to
get_eclass_for_sort_expr, and it requires new code to compute that set
for top-level expressions such as ORDER BY, GROUP BY, etc.  We store
the top-level nullable_relids in a new field in PlannerInfo to avoid
computing it many times.  In the back branches, I've added the new
field at the end of the struct to minimize ABI breakage for planner
plugins.  There doesn't seem to be a good alternative to changing
get_eclass_for_sort_expr's API signature, though.  There probably aren't
any third-party extensions calling that function directly; moreover,
if there are, they probably need to think about what to pass for
nullable_relids anyway.

Back-patch to 9.2, like the previous patch in this area.

10 years agoClarify CREATE FUNCTION documentation about handling of typmods.
Tom Lane [Wed, 13 Nov 2013 18:26:33 +0000 (13:26 -0500)]
Clarify CREATE FUNCTION documentation about handling of typmods.

The previous text was a bit misleading, as well as unnecessarily vague
about what information would be discarded.  Per gripe from Craig Skinner.

11 years agoFix doc links in README file to work with new website layout
Magnus Hagander [Tue, 12 Nov 2013 11:53:32 +0000 (12:53 +0100)]
Fix doc links in README file to work with new website layout

Per report from Colin 't Hart

11 years agoFix failure with whole-row reference to a subquery.
Tom Lane [Mon, 11 Nov 2013 21:36:27 +0000 (16:36 -0500)]
Fix failure with whole-row reference to a subquery.

Simple oversight in commit 1cb108efb0e60d87e4adec38e7636b6e8efbeb57 ---
recursively examining a subquery output column is only sane if the
original Var refers to a single output column.  Found by Kevin Grittner.

11 years agoDon't abort pg_basebackup when receiving empty WAL block
Magnus Hagander [Mon, 11 Nov 2013 13:34:06 +0000 (14:34 +0100)]
Don't abort pg_basebackup when receiving empty WAL block

This can happen exactly at the switch of a logical WAL file
(segment number ending in FE), when running pg_basebackup connected
to a standby server, and would cause the backup to abort with
the error message "streaming header too small".

There is nothing wrong with an empty message, it's just unnecessary,
and the rest of the code can handle the case of an empty message,
so this patch just removes the error condition when the size is
exactly zero.

11 years agoFix race condition in GIN posting tree page deletion.
Heikki Linnakangas [Fri, 8 Nov 2013 20:21:42 +0000 (22:21 +0200)]
Fix race condition in GIN posting tree page deletion.

If a page is deleted, and reused for something else, just as a search is
following a rightlink to it from its left sibling, the search would continue
scanning whatever the new contents of the page are. That could lead to
incorrect query results, or even something more curious if the page is
reused for a different kind of a page.

To fix, modify the search algorithm to lock the next page before releasing
the previous one, and refrain from deleting pages from the leftmost branch
of the tree.

Add a new Concurrency section to the README, explaining why this works.
There is a lot more one could say about concurrency in GIN, but that's for
another patch.

Backpatch to all supported versions.

11 years agoMake contain_volatile_functions/contain_mutable_functions look into SubLinks.
Tom Lane [Fri, 8 Nov 2013 16:37:04 +0000 (11:37 -0500)]
Make contain_volatile_functions/contain_mutable_functions look into SubLinks.

This change prevents us from doing inappropriate subquery flattening in
cases such as dangerous functions hidden inside a sub-SELECT in the
targetlist of another sub-SELECT.  That could result in unexpected behavior
due to multiple evaluations of a volatile function, as in a recent
complaint from Etienne Dube.  It's been questionable from the very
beginning whether these functions should look into subqueries (as noted in
their comments), and this case seems to provide proof that they should.

Because the new code only descends into SubLinks, not SubPlans or
InitPlans, the change only affects the planner's behavior during
prepjointree processing and not later on --- for example, you can still get
it to use a volatile function in an indexqual if you wrap the function in
(SELECT ...).  That's a historical behavior, for sure, but it's reasonable
given that the executor's evaluation rules for subplans don't depend on
whether there are volatile functions inside them.  In any case, we need to
constrain the behavioral change as narrowly as we can to make this
reasonable to back-patch.

11 years agoFix subtly-wrong volatility checking in BeginCopyFrom().
Tom Lane [Fri, 8 Nov 2013 13:59:49 +0000 (08:59 -0500)]
Fix subtly-wrong volatility checking in BeginCopyFrom().

contain_volatile_functions() is best applied to the output of
expression_planner(), not its input, so that insertion of function
default arguments and constant-folding have been done.  (See comments
at CheckMutability, for instance.)  It's perhaps unlikely that anyone
will notice a difference in practice, but still we should do it properly.

In passing, change variable type from Node* to Expr* to reduce the net
number of casts needed.

Noted while perusing uses of contain_volatile_functions().

11 years agoBe more robust when strerror() doesn't give a useful result.
Tom Lane [Thu, 7 Nov 2013 21:33:25 +0000 (16:33 -0500)]
Be more robust when strerror() doesn't give a useful result.

Back-patch commits 8e68816cc2567642c6fcca4eaac66c25e0ae5ced and
8dace66e0735ca39b779922d02c24ea2686e6521 into the stable branches.
Buildfarm testing revealed no great portability surprises, and it
seems useful to have this robustness improvement in all branches.

11 years agoPrevent display of dropped columns in row constraint violation messages.
Tom Lane [Thu, 7 Nov 2013 19:41:43 +0000 (14:41 -0500)]
Prevent display of dropped columns in row constraint violation messages.

ExecBuildSlotValueDescription() printed "null" for each dropped column in
a row being complained of by ExecConstraints().  This has some sanity in
terms of the underlying implementation, but is of course pretty surprising
to users.  To fix, we must pass the target relation's descriptor to
ExecBuildSlotValueDescription(), because the slot descriptor it had been
using doesn't get labeled with attisdropped markers.

Per bug #8408 from Maxim Boguk.  Back-patch to 9.2 where the feature of
printing row values in NOT NULL and CHECK constraint violation messages
was introduced.

Michael Paquier and Tom Lane

11 years agoFix generation of MergeAppend plans for optimized min/max on expressions.
Tom Lane [Thu, 7 Nov 2013 18:13:19 +0000 (13:13 -0500)]
Fix generation of MergeAppend plans for optimized min/max on expressions.

Before jamming a desired targetlist into a plan node, one really ought to
make sure the plan node can handle projections, and insert a buffering
Result plan node if not.  planagg.c forgot to do this, which is a hangover
from the days when it only dealt with IndexScan plan types.  MergeAppend
doesn't project though, not to mention that it gets unhappy if you remove
its possibly-resjunk sort columns.  The code accidentally failed to fail
for cases in which the min/max argument was a simple Var, because the new
targetlist would be equivalent to the original "flat" tlist anyway.
For any more complex case, it's been broken since 9.1 where we introduced
the ability to optimize min/max using MergeAppend, as reported by Raphael
Bauduin.  Fix by duplicating the logic from grouping_planner that decides
whether we need a Result node.

In 9.2 and 9.1, this requires back-porting the tlist_same_exprs() function
introduced in commit 4387cf956b9eb13aad569634e0c4df081d76e2e3, else we'd
uselessly add a Result node in cases that worked before.  It's rather
tempting to back-patch that whole commit so that we can avoid extra Result
nodes in mainline cases too; but I'll refrain, since that code hasn't
really seen all that much field testing yet.

11 years agoSupport default arguments and named-argument notation for window functions.
Tom Lane [Wed, 6 Nov 2013 18:26:38 +0000 (13:26 -0500)]
Support default arguments and named-argument notation for window functions.

These things didn't work because the planner omitted to do the necessary
preprocessing of a WindowFunc's argument list.  Add the few dozen lines
of code needed to handle that.

Although this sounds like a feature addition, it's really a bug fix because
the default-argument case was likely to crash previously, due to lack of
checking of the number of supplied arguments in the built-in window
functions.  It's not a security issue because there's no way for a
non-superuser to create a window function definition with defaults that
refers to a built-in C function, but nonetheless people might be annoyed
that it crashes rather than producing a useful error message.  So
back-patch as far as the patch applies easily, which turns out to be 9.2.
I'll put a band-aid in earlier versions as a separate patch.

(Note that these features still don't work for aggregates, and fixing that
case will be harder since we represent aggregate arg lists as target lists
not bare expression lists.  There's no crash risk though because CREATE
AGGREGATE doesn't accept defaults, and we reject named-argument notation
when parsing an aggregate call.)

11 years agoImprove the error message given for modifying a window with frame clause.
Tom Lane [Wed, 6 Nov 2013 02:58:16 +0000 (21:58 -0500)]
Improve the error message given for modifying a window with frame clause.

For rather inscrutable reasons, SQL:2008 disallows copying-and-modifying a
window definition that has any explicit framing clause.  The error message
we gave for this only made sense if the referencing window definition
itself contains an explicit framing clause, which it might well not.
Moreover, in the context of an OVER clause it's not exactly obvious that
"OVER (windowname)" implies copy-and-modify while "OVER windowname" does
not.  This has led to multiple complaints, eg bug #5199 from Iliya
Krapchatov.  Change to a hopefully more intelligible error message, and
in the case where we have just "OVER (windowname)", add a HINT suggesting
that omitting the parentheses will fix it.  Also improve the related
documentation.  Back-patch to all supported branches.

11 years agoPrevent memory leaks from accumulating across printtup() calls.
Tom Lane [Sun, 3 Nov 2013 16:33:13 +0000 (11:33 -0500)]
Prevent memory leaks from accumulating across printtup() calls.

Historically, printtup() has assumed that it could prevent memory leakage
by pfree'ing the string result of each output function and manually
managing detoasting of toasted values.  This amounts to assuming that
datatype output functions never leak any memory internally; an assumption
we've already decided to be bogus elsewhere, for example in COPY OUT.
range_out in particular is known to leak multiple kilobytes per call, as
noted in bug #8573 from Godfried Vanluffelen.  While we could go in and fix
that leak, it wouldn't be very notationally convenient, and in any case
there have been and undoubtedly will again be other leaks in other output
functions.  So what seems like the best solution is to run the output
functions in a temporary memory context that can be reset after each row,
as we're doing in COPY OUT.  Some quick experimentation suggests this is
actually a tad faster than the retail pfree's anyway.

This patch fixes all the variants of printtup, except for debugtup()
which is used in standalone mode.  It doesn't seem worth worrying
about query-lifespan leaks in standalone mode, and fixing that case
would be a bit tedious since debugtup() doesn't currently have any
startup or shutdown functions.

While at it, remove manual detoast management from several other
output-function call sites that had copied it from printtup().  This
doesn't make a lot of difference right now, but in view of recent
discussions about supporting "non-flattened" Datums, we're going to
want that code gone eventually anyway.

Back-patch to 9.2 where range_out was introduced.  We might eventually
decide to back-patch this further, but in the absence of known major
leaks in older output functions, I'll refrain for now.

11 years agoChanged test case slightly so it doesn't have an unused typedef.
Michael Meskes [Sun, 3 Nov 2013 14:37:34 +0000 (15:37 +0100)]
Changed test case slightly so it doesn't have an unused typedef.

11 years agoRetry after buffer locking failure during SPGiST index creation.
Tom Lane [Sat, 2 Nov 2013 20:45:42 +0000 (16:45 -0400)]
Retry after buffer locking failure during SPGiST index creation.

The original coding thought this case was impossible, but it can happen
if the bgwriter or checkpointer processes decide to write out an index
page while creation is still proceeding, leading to a bogus "unexpected
spgdoinsert() failure" error.  Problem reported by Jonathan S. Katz.

Teodor Sigaev

11 years agoEnsure all files created for a single BufFile have the same resource owner.
Tom Lane [Fri, 1 Nov 2013 20:09:57 +0000 (16:09 -0400)]
Ensure all files created for a single BufFile have the same resource owner.

Callers expect that they only have to set the right resource owner when
creating a BufFile, not during subsequent operations on it.  While we could
insist this be fixed at the caller level, it seems more sensible for the
BufFile to take care of it.  Without this, some temp files belonging to
a BufFile can go away too soon, eg at the end of a subtransaction,
leading to errors or crashes.

Reported and fixed by Andres Freund.  Back-patch to all active branches.

11 years agoFix some odd behaviors when using a SQL-style simple GMT offset timezone.
Tom Lane [Fri, 1 Nov 2013 16:13:26 +0000 (12:13 -0400)]
Fix some odd behaviors when using a SQL-style simple GMT offset timezone.

Formerly, when using a SQL-spec timezone setting with a fixed GMT offset
(called a "brute force" timezone in the code), the session_timezone
variable was not updated to match the nominal timezone; rather, all code
was expected to ignore session_timezone if HasCTZSet was true.  This is
of course obviously fragile, though a search of the code finds only
timeofday() failing to honor the rule.  A bigger problem was that
DetermineTimeZoneOffset() supposed that if its pg_tz parameter was
pointer-equal to session_timezone, then HasCTZSet should override the
parameter.  This would cause datetime input containing an explicit zone
name to be treated as referencing the brute-force zone instead, if the
zone name happened to match the session timezone that had prevailed
before installing the brute-force zone setting (as reported in bug #8572).
The same malady could affect AT TIME ZONE operators.

To fix, set up session_timezone so that it matches the brute-force zone
specification, which we can do using the POSIX timezone definition syntax
"<abbrev>offset", and get rid of the bogus lookaside check in
DetermineTimeZoneOffset().  Aside from fixing the erroneous behavior in
datetime parsing and AT TIME ZONE, this will cause the timeofday() function
to print its result in the user-requested time zone rather than some
previously-set zone.  It might also affect results in third-party
extensions, if there are any that make use of session_timezone without
considering HasCTZSet, but in all cases the new behavior should be saner
than before.

Back-patch to all supported branches.

11 years agoPrevent using strncpy with src == dest in TupleDescInitEntry.
Tom Lane [Tue, 29 Oct 2013 00:49:32 +0000 (20:49 -0400)]
Prevent using strncpy with src == dest in TupleDescInitEntry.

The C and POSIX standards state that strncpy's behavior is undefined when
source and destination areas overlap.  While it remains dubious whether any
implementations really misbehave when the pointers are exactly equal, some
platforms are now starting to force the issue by complaining when an
undefined call occurs.  (In particular OS X 10.9 has been seen to dump core
here, though the exact set of circumstances needed to trigger that remain
elusive.  Similar behavior can be expected to be optional on Linux and
other platforms in the near future.)  So tweak the code to explicitly do
nothing when nothing need be done.

Back-patch to all active branches.  In HEAD, this also lets us get rid of
an exception in valgrind.supp.

Per discussion of a report from Matthias Schmitt.

11 years agoWork around NetBSD shell issue in pg_upgrade test script.
Andrew Dunstan [Mon, 28 Oct 2013 15:45:50 +0000 (11:45 -0400)]
Work around NetBSD shell issue in pg_upgrade test script.

The NetBSD shell apparently returns non-zero from an unset command if
the variable is already unset. This matters when, as in pg_upgrade's
test.sh, we are working under 'set -e'. To protect against this, we
first set the PG variables to an empty string before unsetting them
completely.

Error found on buildfarm member coypu, solution from Rémi Zara.

11 years agoFix two bugs in setting the vm bit of empty pages.
Heikki Linnakangas [Wed, 23 Oct 2013 11:03:54 +0000 (14:03 +0300)]
Fix two bugs in setting the vm bit of empty pages.

Use a critical section when setting the all-visible flag on an empty page,
and WAL-logging it. log_newpage_buffer() contains an assertion that it
must be called inside a critical section, and it's the right thing to do
when modifying a buffer anyway.

Also, the page should be marked dirty before calling log_newpage_buffer(),
per the comment in log_newpage_buffer() and src/backend/access/transam/README.

Patch by Andres Freund, in response to my report. Backpatch to 9.2, like
the patch that introduced these bugs (a6370fd9).

11 years agodoc: Remove i18ngurus.com link
Peter Eisentraut [Wed, 3 Jul 2013 00:32:09 +0000 (20:32 -0400)]
doc: Remove i18ngurus.com link

The web site is dead, and the Wayback Machine shows that it didn't have
much useful content before.

11 years agodocs: correct 9.1 and 9.2 release note mention of timeline switch fix
Bruce Momjian [Tue, 15 Oct 2013 14:34:04 +0000 (10:34 -0400)]
docs: correct 9.1 and 9.2 release note mention of timeline switch fix

Backpatch through 9.1.

KONDO Mitsumasa

11 years agodoc: fix typo in release notes
Bruce Momjian [Wed, 9 Oct 2013 12:44:52 +0000 (08:44 -0400)]
doc:  fix typo in release notes

Backpatch through 8.4

Per suggestion by Amit Langote

11 years agoStamp 9.2.5. REL9_2_5
Peter Eisentraut [Tue, 8 Oct 2013 03:16:13 +0000 (23:16 -0400)]
Stamp 9.2.5.

11 years agoRevert "Document support for VPATH builds of extensions."
Peter Eisentraut [Tue, 8 Oct 2013 02:42:26 +0000 (22:42 -0400)]
Revert "Document support for VPATH builds of extensions."

This reverts commit 565beb41bdd73611ddb26165b7f6bb8b6157e49d.

11 years agoRevert "Backpatch pgxs vpath build and installation fixes (v2)"
Peter Eisentraut [Tue, 8 Oct 2013 02:37:26 +0000 (22:37 -0400)]
Revert "Backpatch pgxs vpath build and installation fixes (v2)"

This reverts commit dd9abd3c995dbc4d32cfc97fde03fe3583e2717c.

pending resolution of
http://www.postgresql.org/message-id/1381193255.25702.4.camel@vanquo.pezone.net

11 years agodocs: update release notes for 8.4.18, 9.0.14, 9.1.10, 9.2.5, 9.3.1
Bruce Momjian [Tue, 8 Oct 2013 01:35:02 +0000 (21:35 -0400)]
docs:  update release notes for 8.4.18, 9.0.14, 9.1.10, 9.2.5, 9.3.1

11 years agoFix bugs in SSI tuple locking.
Heikki Linnakangas [Mon, 7 Oct 2013 20:57:40 +0000 (23:57 +0300)]
Fix bugs in SSI tuple locking.

1. In heap_hot_search_buffer(), the PredicateLockTuple() call is passed
wrong offset number. heapTuple->t_self is set to the tid of the first
tuple in the chain that's visited, not the one actually being read.

2. CheckForSerializableConflictIn() uses the tuple's t_ctid field
instead of t_self to check for exiting predicate locks on the tuple. If
the tuple was updated, but the updater rolled back, t_ctid points to the
aborted dead tuple.

Reported by Hannu Krosing. Backpatch to 9.1.