]> granicus.if.org Git - postgresql/log
postgresql
11 years agoMinor docs wordsmithing.
Tom Lane [Fri, 7 Jun 2013 04:08:02 +0000 (00:08 -0400)]
Minor docs wordsmithing.

Swap the order of a couple of phrases to clarify what the adjective
"subsequent" applies to.

Joshua Tolley

11 years agoFix typo in comment.
Heikki Linnakangas [Thu, 6 Jun 2013 15:25:26 +0000 (18:25 +0300)]
Fix typo in comment.

11 years agoEnsure that XLOG_HEAP2_VISIBLE always targets an initialized page.
Robert Haas [Thu, 6 Jun 2013 14:15:45 +0000 (10:15 -0400)]
Ensure that XLOG_HEAP2_VISIBLE always targets an initialized page.

Andres Freund

11 years agoBackport log_newpage_buffer.
Robert Haas [Thu, 6 Jun 2013 14:14:46 +0000 (10:14 -0400)]
Backport log_newpage_buffer.

Andres' fix for XLOG_HEAP2_VISIBLE on unitialized pages requires
this.

11 years agopg_upgrade: document that --link should be used with --check
Bruce Momjian [Thu, 6 Jun 2013 14:13:54 +0000 (10:13 -0400)]
pg_upgrade:  document that --link should be used with --check
Backpatch to 9.2.

11 years agoPrevent pushing down WHERE clauses into unsafe UNION/INTERSECT nests.
Tom Lane [Thu, 6 Jun 2013 03:44:08 +0000 (23:44 -0400)]
Prevent pushing down WHERE clauses into unsafe UNION/INTERSECT nests.

The planner is aware that it mustn't push down upper-level quals into
subqueries if the quals reference subquery output columns that contain
set-returning functions or volatile functions, or are non-DISTINCT outputs
of a DISTINCT ON subquery.  However, it missed making this check when
there were one or more levels of UNION or INTERSECT above the dangerous
expression.  This could lead to "set-valued function called in context that
cannot accept a set" errors, as seen in bug #8213 from Eric Soroos, or to
silently wrong answers in the other cases.

To fix, refactor the checks so that we make the column-is-unsafe checks
during subquery_is_pushdown_safe(), which already has to recursively
inspect all arms of a set-operation tree.  This makes
qual_is_pushdown_safe() considerably simpler, at the cost that we will
spend some cycles checking output columns that possibly aren't referenced
in any upper qual.  But the cases where this code gets executed at all
are already nontrivial queries, so it's unlikely anybody will notice any
slowdown of planning.

This has been broken since commit 05f916e6add9726bf4ee046e4060c1b03c9961f2,
which makes the bug over ten years old.  A bit surprising nobody noticed it
before now.

11 years agoPut analyze_keyword back in explain_option_name production.
Tom Lane [Wed, 5 Jun 2013 17:32:53 +0000 (13:32 -0400)]
Put analyze_keyword back in explain_option_name production.

In commit 2c92edad48796119c83d7dbe6c33425d1924626d, I broke "EXPLAIN
(ANALYZE)" syntax, because I mistakenly thought that ANALYZE/ANALYSE were
only partially reserved and thus would be included in NonReservedWord;
but actually they're fully reserved so they still need to be called out
here.

A nicer solution would be to demote these words to type_func_name_keyword
status (they can't be less than that because of "VACUUM [ANALYZE] ColId").
While that works fine so far as the core grammar is concerned, it breaks
ECPG's grammar for reasons I don't have time to isolate at the moment.
So do this for the time being.

Per report from Kevin Grittner.  Back-patch to 9.0, like the previous
commit.

11 years agodoc: Add IDs to link targets used by phpPgAdmin
Peter Eisentraut [Wed, 8 May 2013 01:23:21 +0000 (21:23 -0400)]
doc: Add IDs to link targets used by phpPgAdmin

Karl O. Pinc

11 years agoProvide better message when CREATE EXTENSION can't find a target schema.
Tom Lane [Tue, 4 Jun 2013 21:22:29 +0000 (17:22 -0400)]
Provide better message when CREATE EXTENSION can't find a target schema.

The new message (and SQLSTATE) matches the corresponding error cases in
namespace.c.

This was thought to be a "can't happen" case when extension.c was written,
so we didn't think hard about how to report it.  But it definitely can
happen in 9.2 and later, since we no longer require search_path to contain
any valid schema names.  It's probably also possible in 9.1 if search_path
came from a noninteractive source.  So, back-patch to all releases
containing this code.

Per report from Sean Chittenden, though this isn't exactly his patch.

11 years agoAdd ARM64 (aarch64) support to s_lock.h.
Tom Lane [Tue, 4 Jun 2013 19:42:02 +0000 (15:42 -0400)]
Add ARM64 (aarch64) support to s_lock.h.

Use the same gcc atomic functions as we do on newer ARM chips.
(Basically this is a copy and paste of the __arm__ code block,
but omitting the SWPB option since that definitely won't work.)

Back-patch to 9.2.  The patch would work further back, but we'd also
need to update config.guess/config.sub in older branches to make them
build out-of-the-box, and there hasn't been demand for it.

Mark Salter

11 years agoFix memory leak in LogStandbySnapshot().
Tom Lane [Tue, 4 Jun 2013 18:58:52 +0000 (14:58 -0400)]
Fix memory leak in LogStandbySnapshot().

The array allocated by GetRunningTransactionLocks() needs to be pfree'd
when we're done with it.  Otherwise we leak some memory during each
checkpoint, if wal_level = hot_standby.  This manifests as memory bloat
in the checkpointer process, or in bgwriter in versions before we made
the checkpointer separate.

Reported and fixed by Naoya Anzai.  Back-patch to 9.0 where the issue
was introduced.

In passing, improve comments for GetRunningTransactionLocks(), and add
an Assert that we didn't overrun the palloc'd array.

11 years agoAdd semicolons to eval'd strings to hide a minor Perl behavioral change.
Tom Lane [Mon, 3 Jun 2013 18:19:32 +0000 (14:19 -0400)]
Add semicolons to eval'd strings to hide a minor Perl behavioral change.

"eval q{foo}" used to complain that the error was on line 2 of the eval'd
string, because eval internally tacked on "\n;" so that the end of the
erroneous command was indeed on line 2.  But as of Perl 5.18 it more
sanely says that the error is on line 1.  To avoid Perl-version-dependent
regression test results, use "eval q{foo;}" instead in the two places
where this matters.  Per buildfarm.

Since people might try to use newer Perl versions with older PG releases,
back-patch as far as 9.0 where these test cases were added.

11 years agoAllow type_func_name_keywords in some places where they weren't before.
Tom Lane [Mon, 3 Jun 2013 00:09:26 +0000 (20:09 -0400)]
Allow type_func_name_keywords in some places where they weren't before.

This change makes type_func_name_keywords less reserved than they were
before, by allowing them for role names, language names, EXPLAIN and COPY
options, and SET values for GUCs; which are all places where few if any
actual keywords could appear instead, so no new ambiguities are introduced.

The main driver for this change is to allow "COPY ... (FORMAT BINARY)"
to work without quoting the word "binary".  That is an inconsistency that
has been complained of repeatedly over the years (at least by Pavel Golub,
Kurt Lidl, and Simon Riggs); but we hadn't thought of any non-ugly solution
until now.

Back-patch to 9.0 where the COPY (FORMAT BINARY) syntax was introduced.

11 years agoDocument auto_explain.log_timing.
Robert Haas [Wed, 29 May 2013 11:11:21 +0000 (07:11 -0400)]
Document auto_explain.log_timing.

Tomas Vondra

11 years agoDocumentation fix for ALTER TYPE .. RENAME
Stephen Frost [Mon, 27 May 2013 15:12:54 +0000 (11:12 -0400)]
Documentation fix for ALTER TYPE .. RENAME

The documentation for ALTER TYPE .. RENAME claimed to support a
RESTRICT/CASCADE option at the 'type' level, which wasn't implemented
and doesn't make a whole lot of sense to begin with.  What is supported,
and previously undocumented, is

ALTER TYPE .. RENAME ATTRIBUTE .. RESTRICT/CASCADE.

I've updated the documentation and back-patched this to 9.1 where it was
first introduced.

11 years agoFix typo in comment.
Robert Haas [Thu, 23 May 2013 15:34:30 +0000 (11:34 -0400)]
Fix typo in comment.

Pavan Deolasee

11 years agoPrint line number correctly in COPY.
Heikki Linnakangas [Thu, 23 May 2013 11:49:59 +0000 (07:49 -0400)]
Print line number correctly in COPY.

When COPY uses the multi-insert method to insert a batch of tuples into the
heap at a time, incorrect line number was printed if something went wrong in
inserting the index tuples (primary key failure, for exampl), or processing
after row triggers.

Fixes bug #8173 reported by Lloyd Albin. Backpatch to 9.2, where the multi-
insert code was added.

11 years agoFix fd.c to preserve errno where needed.
Tom Lane [Thu, 16 May 2013 19:04:38 +0000 (15:04 -0400)]
Fix fd.c to preserve errno where needed.

PathNameOpenFile failed to ensure that the correct value of errno was
returned to its caller after a failure (because it incorrectly supposed
that free() can never change errno).  In some cases this would result
in a user-visible failure because an expected ENOENT errno was replaced
with something else.  Bogus EINVAL failures have been observed on OS X,
for example.

There were also a couple of places that could mangle an important value
of errno if FDDEBUG was defined.  While the usefulness of that debug
support is highly debatable, we might as well make it safe to use,
so add errno save/restore logic to the DO_DB macro.

Per bug #8167 from Nelson Minar, diagnosed by RhodiumToad.
Back-patch to all supported branches.

11 years agoFix handling of OID wraparound while in standalone mode.
Tom Lane [Mon, 13 May 2013 19:40:16 +0000 (15:40 -0400)]
Fix handling of OID wraparound while in standalone mode.

If OID wraparound should occur while in standalone mode (unlikely but
possible), we want to advance the counter to FirstNormalObjectId not
FirstBootstrapObjectId.  Otherwise, user objects might be created with OIDs
in the system-reserved range.  That isn't immediately harmful but it poses
a risk of conflicts during future pg_upgrade operations.

Noted by Andres Freund.  Back-patch to all supported branches, since all of
them are supported sources for pg_upgrade operations.

11 years agoUpdate CREATE FUNCTION documentation about argument names.
Tom Lane [Sat, 11 May 2013 16:07:47 +0000 (12:07 -0400)]
Update CREATE FUNCTION documentation about argument names.

The 9.2 patch that added argument name support in SQL-language functions
missed updating a parenthetical comment about that in the CREATE FUNCTION
reference page.  Noted by Erwin Brandstetter.

11 years agoGuard against input_rows == 0 in estimate_num_groups().
Tom Lane [Fri, 10 May 2013 21:15:35 +0000 (17:15 -0400)]
Guard against input_rows == 0 in estimate_num_groups().

This case doesn't normally happen, because the planner usually clamps
all row estimates to at least one row; but I found that it can arise
when dealing with relations excluded by constraints.  Without a defense,
estimate_num_groups() can return zero, which leads to divisions by zero
inside the planner as well as assertion failures in the executor.

An alternative fix would be to change set_dummy_rel_pathlist() to make
the size estimate for a dummy relation 1 row instead of 0, but that seemed
pretty ugly; and probably someday we'll want to drop the convention that
the minimum rowcount estimate is 1 row.

Back-patch to 8.4, as the problem can be demonstrated that far back.

11 years agoFix pgp_pub_decrypt() so it works for secret keys with passwords.
Tom Lane [Fri, 10 May 2013 17:06:52 +0000 (13:06 -0400)]
Fix pgp_pub_decrypt() so it works for secret keys with passwords.

Per report from Keith Fiske.

Marko Kreen

11 years agoFix management of fn_extra caching during repeated GiST index scans.
Tom Lane [Fri, 10 May 2013 03:08:25 +0000 (23:08 -0400)]
Fix management of fn_extra caching during repeated GiST index scans.

Commit d22a09dc70f9830fa78c1cd1a3a453e4e473d354 introduced official support
for GiST consistentFns that want to cache data using the FmgrInfo fn_extra
pointer: the idea was to preserve the cached values across gistrescan(),
whereas formerly they'd been leaked.  However, there was an oversight in
that, namely that multiple scan keys might reference the same column's
consistentFn; the code would result in propagating the same cache value
into multiple scan keys, resulting in crashes or wrong answers.  Use a
separate array instead to ensure that each scan key keeps its own state.

Per bug #8143 from Joel Roller.  Back-patch to 9.2 where the bug was
introduced.

11 years agoUse pg_dump's --quote-all-identifiers option in pg_upgrade.
Tom Lane [Thu, 9 May 2013 21:34:40 +0000 (17:34 -0400)]
Use pg_dump's --quote-all-identifiers option in pg_upgrade.

This helps guard against changes in the set of reserved keywords from
one version to another.  In theory it should only be an issue if we
de-reserve a keyword in a newer release, since that can create the type
of problem shown in bug #8128.

Back-patch to 9.1 where the --quote-all-identifiers option was added.

11 years agoRevert "Fix permission tests for views/tables proven empty by constraint exclusion."
Tom Lane [Wed, 8 May 2013 21:01:19 +0000 (17:01 -0400)]
Revert "Fix permission tests for views/tables proven empty by constraint exclusion."

This reverts commit 15b0421002624919c62ae3c6574af2a8452bf6c4.  Per
complaint from Robert Haas, that patch caused crashes on appendrel
cases, and it didn't fix all forms of the problem anyway.  Consensus
is we'll leave this problem alone in the back branches, at least
for now.

11 years agodocs: log_line_prefix session id fix
Bruce Momjian [Sat, 4 May 2013 17:15:54 +0000 (13:15 -0400)]
docs:  log_line_prefix session id fix

Restore 4-byte designation for docs.  Fix 9.3 doc query to properly pad
to four digits.

Backpatch to all active branches

Per suggestions from Ian Lawrence Barwick

11 years agodocs: fix log_line_prefix session id docs
Bruce Momjian [Sat, 4 May 2013 15:05:16 +0000 (11:05 -0400)]
docs:  fix log_line_prefix session id docs

Backpatch to 9.2.

Report from Ian Lawrence Barwick

11 years agoFix thinko in comment.
Heikki Linnakangas [Thu, 2 May 2013 15:08:43 +0000 (18:08 +0300)]
Fix thinko in comment.

WAL segment means a 16 MB physical WAL file; this comment meant a logical
4 GB log file.

Amit Langote. Apply to backbranches only, as the comment is gone in master.

11 years agoFix permission tests for views/tables proven empty by constraint exclusion.
Tom Lane [Wed, 1 May 2013 22:26:58 +0000 (18:26 -0400)]
Fix permission tests for views/tables proven empty by constraint exclusion.

A view defined as "select <something> where false" had the curious property
that the system wouldn't check whether users had the privileges necessary
to select from it.  More generally, permissions checks could be skipped
for tables referenced in sub-selects or views that were proven empty by
constraint exclusion (although some quick testing suggests this seldom
happens in cases of practical interest).  This happened because the planner
failed to include rangetable entries for such tables in the finished plan.

This was noticed in connection with erroneous handling of materialized
views, but actually the issue is quite unrelated to matviews.  Therefore,
revert commit 200ba1667b3a8d7a9d559d2f05f83d209c9d8267 in favor of a more
direct test for the real problem.

Back-patch to 9.2 where the bug was introduced (by commit
7741dd6590073719688891898e85f0cb73453159).

11 years agoInstall recycled WAL segments with current timeline ID during recovery.
Heikki Linnakangas [Tue, 30 Apr 2013 13:31:21 +0000 (16:31 +0300)]
Install recycled WAL segments with current timeline ID during recovery.

This is a follow-up to the earlier fix, which changed the recycling logic
to recycle WAL segments under the current recovery target timeline. That
turns out to be a bad idea, because installing a recycled segment with
a TLI higher than what we're recovering at the moment means that the recovery
logic will find the recycled WAL segment and try to replay it. It will fail,
but but the mere presence of such a WAL segment will mask any other, real,
file with the same log/seg, but smaller TLI.

Per report from Mitsumasa Kondo. Apply to 9.1 and 9.2, like the previous
fix. Master was already doing this differently; this patch makes 9.1 and
9.2 to do the same thing as master.

11 years agoPostpone creation of pathkeys lists to fix bug #8049.
Tom Lane [Mon, 29 Apr 2013 18:49:16 +0000 (14:49 -0400)]
Postpone creation of pathkeys lists to fix bug #8049.

This patch gets rid of the concept of, and infrastructure for,
non-canonical PathKeys; we now only ever create canonical pathkey lists.

The need for non-canonical pathkeys came from the desire to have
grouping_planner initialize query_pathkeys and related pathkey lists before
calling query_planner.  However, since query_planner didn't actually *do*
anything with those lists before they'd been made canonical, we can get rid
of the whole mess by just not creating the lists at all until the point
where we formerly canonicalized them.

There are several ways in which we could implement that without making
query_planner itself deal with grouping/sorting features (which are
supposed to be the province of grouping_planner).  I chose to add a
callback function to query_planner's API; other alternatives would have
required adding more fields to PlannerInfo, which while not bad in itself
would create an ABI break for planner-related plugins in the 9.2 release
series.  This still breaks ABI for anything that calls query_planner
directly, but it seems somewhat unlikely that there are any such plugins.

I had originally conceived of this change as merely a step on the way to
fixing bug #8049 from Teun Hoogendoorn; but it turns out that this fixes
that bug all by itself, as per the added regression test.  The reason is
that now get_eclass_for_sort_expr is adding the ORDER BY expression at the
end of EquivalenceClass creation not the start, and so anything that is in
a multi-member EquivalenceClass has already been created with correct
em_nullable_relids.  I am suspicious that there are related scenarios in
which we still need to teach get_eclass_for_sort_expr to compute correct
nullable_relids, but am not eager to risk destabilizing either 9.2 or 9.3
to fix bugs that are only hypothetical.  So for the moment, do this and
stop here.

Back-patch to 9.2 but not to earlier branches, since they don't exhibit
this bug for lack of join-clause-movement logic that depends on
em_nullable_relids being correct.  (We might have to revisit that choice
if any related bugs turn up.)  In 9.2, don't change the signature of
make_pathkeys_for_sortclauses nor remove canonicalize_pathkeys, so as
not to risk more plugin breakage than we have to.

11 years agoEnsure ANALYZE phase is not skipped because of canceled truncate.
Kevin Grittner [Mon, 29 Apr 2013 18:05:56 +0000 (13:05 -0500)]
Ensure ANALYZE phase is not skipped because of canceled truncate.

Patch b19e4250b45e91c9cbdd18d35ea6391ab5961c8d attempted to
preserve existing behavior regarding statistics generation in the
case that a truncation attempt was canceled due to lock conflicts.
It failed to do this accurately in two regards: (1) autovacuum had
previously generated statistics if the truncate attempt failed to
initially get the lock rather than having started the attempt, and
(2) the VACUUM ANALYZE command had always generated statistics.

Both of these changes were unintended, and are reverted by this
patch.  On review, there seems to be consensus that the previous
failure to generate statistics when the truncate was terminated
was more an unfortunate consequence of how that effort was
previously terminated than a feature we want to keep; so this
patch generates statistics even when an autovacuum truncation
attempt terminates early.  Another unintended change which is kept
on the basis that it is an improvement is that when a VACUUM
command is truncating, it will the new heuristic for avoiding
blocking other processes, rather than keeping an
AccessExclusiveLock on the table for however long the truncation
takes.

Per multiple reports, with some renaming per patch by Jeff Janes.

Backpatch to 9.0, where problem was created.

11 years agoEnsure that user created rows in extension tables get dumped if the table is explicit...
Joe Conway [Fri, 26 Apr 2013 18:54:14 +0000 (11:54 -0700)]
Ensure that user created rows in extension tables get dumped if the table is explicitly requested, either with a -t/--table switch of the table itself, or by -n/--schema switch of the schema containing the extension table. Patch reviewed by Vibhor Kumar and Dimitri Fontaine.

Backpatched to 9.1 when the extension management facility was added.

11 years agoAvoid deadlock between concurrent CREATE INDEX CONCURRENTLY commands.
Tom Lane [Thu, 25 Apr 2013 20:58:10 +0000 (16:58 -0400)]
Avoid deadlock between concurrent CREATE INDEX CONCURRENTLY commands.

There was a high probability of two or more concurrent C.I.C. commands
deadlocking just before completion, because each would wait for the others
to release their reference snapshots.  Fix by releasing the snapshot
before waiting for other snapshots to go away.

Per report from Paul Hinze.  Back-patch to all active branches.

11 years agoFix typo in comment.
Heikki Linnakangas [Thu, 25 Apr 2013 11:03:10 +0000 (14:03 +0300)]
Fix typo in comment.

Peter Geoghegan

11 years agodoc: Fix syntax in example
Peter Eisentraut [Mon, 22 Apr 2013 02:16:12 +0000 (22:16 -0400)]
doc: Fix syntax in example

LANGUAGE 'plpgsql' no longer works.  The single quotes need to be
removed.

Erwin Brandstetter

11 years agoFix longstanding race condition in plancache.c.
Tom Lane [Sat, 20 Apr 2013 20:59:27 +0000 (16:59 -0400)]
Fix longstanding race condition in plancache.c.

When creating or manipulating a cached plan for a transaction control
command (particularly ROLLBACK), we must not perform any catalog accesses,
since we might be in an aborted transaction.  However, plancache.c busily
saved or examined the search_path for every cached plan.  If we were
unlucky enough to do this at a moment where the path's expansion into
schema OIDs wasn't already cached, we'd do some catalog accesses; and with
some more bad luck such as an ill-timed signal arrival, that could lead to
crashes or Assert failures, as exhibited in bug #8095 from Nachiket Vaidya.
Fortunately, there's no real need to consider the search path for such
commands, so we can just skip the relevant steps when the subject statement
is a TransactionStmt.  This is somewhat related to bug #5269, though the
failure happens during initial cached-plan creation rather than
revalidation.

This bug has been there since the plan cache was invented, so back-patch
to all supported branches.

11 years agoUpdate the description for the graphical installers
Magnus Hagander [Wed, 10 Apr 2013 19:37:49 +0000 (21:37 +0200)]
Update the description for the graphical installers

Remove references to "one click", as we're not supposed to call
them that anymore.

11 years agoIn isolationtester, retry after EINTR return from select(2).
Tom Lane [Sun, 7 Apr 2013 02:28:53 +0000 (22:28 -0400)]
In isolationtester, retry after EINTR return from select(2).

Per report from Jaime Casanova.  Very curious that no one else has seen
this failure ... but the code is clearly wrong as-is.

11 years agoImprove documentation about the relationship of extensions and schemas.
Tom Lane [Fri, 5 Apr 2013 02:37:29 +0000 (22:37 -0400)]
Improve documentation about the relationship of extensions and schemas.

There's been some confusion expressed about this point, so clarify.
Extended version of a patch by David Wheeler.

11 years agopsql: fix startup crash caused by PSQLRC containing a tilde
Bruce Momjian [Thu, 4 Apr 2013 16:56:21 +0000 (12:56 -0400)]
psql:  fix startup crash caused by PSQLRC containing a tilde

'strdup' the PSQLRC environment variable value before calling a routine
that might free() it.

Backpatch to 9.2, where the bug first appeared.

11 years agoFix crash on compiling a regular expression with more than 32k colors.
Heikki Linnakangas [Thu, 4 Apr 2013 16:04:57 +0000 (19:04 +0300)]
Fix crash on compiling a regular expression with more than 32k colors.

Throw an error instead.

Backpatch to all supported branches.

11 years agoCalculate # of semaphores correctly with --disable-spinlocks.
Heikki Linnakangas [Thu, 4 Apr 2013 13:31:44 +0000 (16:31 +0300)]
Calculate # of semaphores correctly with --disable-spinlocks.

The old formula didn't take into account that each WAL sender process needs
a spinlock. We had also already exceeded the fixed number of spinlocks
reserved for misc purposes (10). Bump that to 30.

Backpatch to 9.0, where WAL senders were introduced. If I counted correctly,
9.0 had exactly 10 predefined spinlocks, and 9.1 exceeded that, but bump the
limit in 9.0 too because 10 is uncomfortably close to the edge.

11 years agoAvoid updating our PgBackendStatus entry when track_activities is off.
Tom Lane [Wed, 3 Apr 2013 18:13:34 +0000 (14:13 -0400)]
Avoid updating our PgBackendStatus entry when track_activities is off.

The point of turning off track_activities is to avoid this reporting
overhead, but a thinko in commit 4f42b546fd87a80be30c53a0f2c897acb826ad52
caused pgstat_report_activity() to perform half of its updates anyway.
Fix that, and also make sure that we clear all the now-disabled fields
when transitioning to the non-reporting state.

11 years agoMinor robustness improvements for isolationtester.
Tom Lane [Wed, 3 Apr 2013 01:15:45 +0000 (21:15 -0400)]
Minor robustness improvements for isolationtester.

Notice and complain about PQcancel() failures.  Also, don't dump core if
an error PGresult doesn't contain severity and message subfields, as it
might not if it was generated by libpq itself.  (We have a longstanding
TODO item to improve that, but in the meantime isolationtester had better
cope.)

I tripped across the latter item while investigating a trouble report on
buildfarm member spoonbill.  As for the former, there's no evidence that
PQcancel failure is actually involved in spoonbill's problem, but it still
seems like a bad idea to ignore an error return code.

11 years agoStamp 9.2.4. REL9_2_4
Tom Lane [Mon, 1 Apr 2013 18:20:36 +0000 (14:20 -0400)]
Stamp 9.2.4.

11 years agoUpdate release notes for 9.2.4, 9.1.9, 9.0.13, 8.4.17.
Tom Lane [Mon, 1 Apr 2013 18:11:16 +0000 (14:11 -0400)]
Update release notes for 9.2.4, 9.1.9, 9.0.13, 8.4.17.

Security: CVE-2013-1899, CVE-2013-1901

11 years agoFix insecure parsing of server command-line switches.
Tom Lane [Mon, 1 Apr 2013 18:00:59 +0000 (14:00 -0400)]
Fix insecure parsing of server command-line switches.

An oversight in commit e710b65c1c56ca7b91f662c63d37ff2e72862a94 allowed
database names beginning with "-" to be treated as though they were secure
command-line switches; and this switch processing occurs before client
authentication, so that even an unprivileged remote attacker could exploit
the bug, needing only connectivity to the postmaster's port.  Assorted
exploits for this are possible, some requiring a valid database login,
some not.  The worst known problem is that the "-r" switch can be invoked
to redirect the process's stderr output, so that subsequent error messages
will be appended to any file the server can write.  This can for example be
used to corrupt the server's configuration files, so that it will fail when
next restarted.  Complete destruction of database tables is also possible.

Fix by keeping the database name extracted from a startup packet fully
separate from command-line switches, as had already been done with the
user name field.

The Postgres project thanks Mitsumasa Kondo for discovering this bug,
Kyotaro Horiguchi for drafting the fix, and Noah Misch for recognizing
the full extent of the danger.

Security: CVE-2013-1899

11 years agoMake REPLICATION privilege checks test current user not authenticated user.
Tom Lane [Mon, 1 Apr 2013 17:09:29 +0000 (13:09 -0400)]
Make REPLICATION privilege checks test current user not authenticated user.

The pg_start_backup() and pg_stop_backup() functions checked the privileges
of the initially-authenticated user rather than the current user, which is
wrong.  For example, a user-defined index function could successfully call
these functions when executed by ANALYZE within autovacuum.  This could
allow an attacker with valid but low-privilege database access to interfere
with creation of routine backups.  Reported and fixed by Noah Misch.

Security: CVE-2013-1901

11 years agoTranslation updates
Peter Eisentraut [Mon, 1 Apr 2013 03:46:49 +0000 (23:46 -0400)]
Translation updates

11 years agoIgnore extra subquery outputs in set_subquery_size_estimates().
Tom Lane [Sun, 31 Mar 2013 22:33:01 +0000 (18:33 -0400)]
Ignore extra subquery outputs in set_subquery_size_estimates().

In commit 0f61d4dd1b4f95832dcd81c9688dac56fd6b5687, I added code to copy up
column width estimates for each column of a subquery.  That code supposed
that the subquery couldn't have any output columns that didn't correspond
to known columns of the current query level --- which is true when a query
is parsed from scratch, but the assumption fails when planning a view that
depends on another view that's been redefined (adding output columns) since
the upper view was made.  This results in an assertion failure or even a
crash, as per bug #8025 from lindebg.  Remove the Assert and instead skip
the column if its resno is out of the expected range.

11 years agoTranslation updates
Alvaro Herrera [Sun, 31 Mar 2013 19:41:54 +0000 (16:41 -0300)]
Translation updates

11 years agopg_upgrade: don't copy/link files for invalid indexes
Bruce Momjian [Sun, 31 Mar 2013 02:20:53 +0000 (22:20 -0400)]
pg_upgrade:  don't copy/link files for invalid indexes

Now that pg_dump no longer dumps invalid indexes, per commit
683abc73dff549e94555d4020dae8d02f32ed78b, have pg_upgrade also skip
them.  Previously pg_upgrade threw an error if invalid indexes existed.

Backpatch to 9.2, 9.1, and 9.0 (where pg_upgrade was added to git)

11 years agoAvoid moving data directory in upgrade testing.
Andrew Dunstan [Sat, 30 Mar 2013 16:54:36 +0000 (12:54 -0400)]
Avoid moving data directory in upgrade testing.

Windows sometimes gets upset if we rename a large directory and then try
to use the old name quickly, as seen in occasional buildfarm failures.
So we avoid that by building the old version in the intended
destination in the first place instead of renaming it, similar to the
change made for the same reason in commit b7f8465c.

11 years agoDocument encode(bytea, 'escape')'s behavior correctly.
Tom Lane [Fri, 29 Mar 2013 03:14:58 +0000 (23:14 -0400)]
Document encode(bytea, 'escape')'s behavior correctly.

I changed this in commit fd15dba543247eb1ce879d22632b9fdb4c230831, but
missed the fact that the SGML documentation of the function specified
exactly what it did.  Well, one of the two places where it's specified
documented that --- probably I looked at the other place and thought
nothing needed to be done.  Sync the two places where encode() and
decode() are described.

11 years agoMust check indisready not just indisvalid when dumping from 9.2 server.
Tom Lane [Fri, 29 Mar 2013 02:09:20 +0000 (22:09 -0400)]
Must check indisready not just indisvalid when dumping from 9.2 server.

9.2 uses a kluge representation of "indislive"; we have to account for
that when examining pg_index.  Simplest solution is to check indisready
for 9.0 and 9.1 as well; that's harmless though unnecessary, so it's
not worth making a version distinction for.

Fixes oversight in commit 683abc73dff549e94555d4020dae8d02f32ed78b,
as noted by Andres Freund.

11 years agoUpdate time zone data files to tzdata release 2013b.
Tom Lane [Thu, 28 Mar 2013 19:25:54 +0000 (15:25 -0400)]
Update time zone data files to tzdata release 2013b.

DST law changes in Chile, Haiti, Morocco, Paraguay, some Russian areas.
Historical corrections for numerous places.

11 years agoReset OpenSSL randomness state in each postmaster child process.
Tom Lane [Wed, 27 Mar 2013 22:50:25 +0000 (18:50 -0400)]
Reset OpenSSL randomness state in each postmaster child process.

Previously, if the postmaster initialized OpenSSL's PRNG (which it will do
when ssl=on in postgresql.conf), the same pseudo-random state would be
inherited by each forked child process.  The problem is masked to a
considerable extent if the incoming connection uses SSL encryption, but
when it does not, identical pseudo-random state is made available to
functions like contrib/pgcrypto.  The process's PID does get mixed into any
requested random output, but on most systems that still only results in 32K
or so distinct random sequences available across all Postgres sessions.
This might allow an attacker who has database access to guess the results
of "secure" operations happening in another session.

To fix, forcibly reset the PRNG after fork().  Each child process that has
need for random numbers from OpenSSL's generator will thereby be forced to
go through OpenSSL's normal initialization sequence, which should provide
much greater variability of the sequences.  There are other ways we might
do this that would be slightly cheaper, but this approach seems the most
future-proof against SSL-related code changes.

This has been assigned CVE-2013-1900, but since the issue and the patch
have already been publicized on pgsql-hackers, there's no point in trying
to hide this commit.

Back-patch to all supported branches.

Marko Kreen

11 years agoFix buffer pin leak in heap update redo routine.
Heikki Linnakangas [Wed, 27 Mar 2013 19:51:27 +0000 (21:51 +0200)]
Fix buffer pin leak in heap update redo routine.

In a heap update, if the old and new tuple were on different pages, and the
new page no longer existed (because it was subsequently truncated away by
vacuum), heap_xlog_update forgot to release the pin on the old buffer. This
bug was introduced by the "Fix multiple problems in WAL replay" patch,
commit 3bbf668de9f1bc172371681e80a4e769b6d014c8 (on master branch).

With full_page_writes=off, this triggered an "incorrect local pin count"
error later in replay, if the old page was vacuumed.

This fixes bug #7969, reported by Yunong Xiao. Backpatch to 9.0, like the
commit that introduced this bug.

11 years agoIgnore invalid indexes in pg_dump.
Tom Lane [Tue, 26 Mar 2013 21:43:23 +0000 (17:43 -0400)]
Ignore invalid indexes in pg_dump.

Dumping invalid indexes can cause problems at restore time, for example
if the reason the index creation failed was because it tried to enforce
a uniqueness condition not satisfied by the table's data.  Also, if the
index creation is in fact still in progress, it seems reasonable to
consider it to be an uncommitted DDL change, which pg_dump wouldn't be
expected to dump anyway.

Back-patch to all active versions, and teach them to ignore invalid
indexes in servers back to 8.2, where the concept was introduced.

Michael Paquier

11 years agoIn base backup, only include our own tablespace version directory.
Heikki Linnakangas [Mon, 25 Mar 2013 18:19:22 +0000 (20:19 +0200)]
In base backup, only include our own tablespace version directory.

If you have clusters of different versions pointing to the same tablespace
location, we would incorrectly include all the data belonging to the other
versions, too.

Fixes bug #7986, reported by Sergey Burladyan.

11 years agoAdd a server version check to pg_basebackup and pg_receivexlog.
Heikki Linnakangas [Mon, 25 Mar 2013 09:02:55 +0000 (11:02 +0200)]
Add a server version check to pg_basebackup and pg_receivexlog.

These programs don't work against 9.0 or earlier servers, so check that when
the connection is made. That's better than a cryptic error message you got
before.

Also, these programs won't work with a 9.3 server, because the WAL streaming
protocol was changed in a non-backwards-compatible way. As a general rule,
we don't make any guarantee that an old client will work with a new server,
so check that. However, allow a 9.1 client to connect to a 9.2 server, to
avoid breaking environments that currently work; a 9.1 client happens to
work with a 9.2 server, even though we didn't make any great effort to
ensure that.

This patch is for the 9.1 and 9.2 branches, I'll commit a similar patch to
master later. Although this isn't a critical bug fix, it seems safe enough
to back-patch. The error message you got when connecting to a 9.3devel
server without this patch was cryptic enough to warrant backpatching.

11 years agoUpdate time zone abbreviation lists for changes missed since 2006.
Tom Lane [Sat, 23 Mar 2013 23:16:42 +0000 (19:16 -0400)]
Update time zone abbreviation lists for changes missed since 2006.

Most (all?) of Russia has moved to what's effectively year-round daylight
savings time, so that the "standard" zone names now mean an hour later
than they used to.  Update that, notably changing MSK as per recent
complaint from Sergey Konoplev, but also CHOT, GET, IRKT, KGT, KRAT,
MAGT, NOVT, OMST, VLAT, YAKT, YEKT.  The corresponding DST abbreviations
are presumably now obsolete, but I left them in place with their old
definitions, just to reduce any possible breakage from this change.

Also add VOLT (Europe/Volgograd), which for some reason we never had
before, as well as MIST (Antarctica/Macquarie), and fix obsolete
definitions of MAWT, TKT, and WST.

11 years agoAvoid renaming data directory during MSVC upgrade testing.
Andrew Dunstan [Sat, 23 Mar 2013 20:31:01 +0000 (16:31 -0400)]
Avoid renaming data directory during MSVC upgrade testing.

This appears to cause some intermittent file system problems
on Windows 8. Instead, set up the old data directory in its
intended final location to start with.

11 years agoDon't put <indexterm> before <term> in <varlistentry> items.
Tom Lane [Sat, 23 Mar 2013 18:06:37 +0000 (14:06 -0400)]
Don't put <indexterm> before <term> in <varlistentry> items.

Doing that results in a broken index entry in PDF output.  We had only
a few like that, which is probably why nobody noticed before.
Standardize on putting the <term> first.

Josh Kupershmidt

11 years agoFix contrib/dblink to handle inconsistent DateStyle/IntervalStyle safely.
Tom Lane [Fri, 22 Mar 2013 19:22:21 +0000 (15:22 -0400)]
Fix contrib/dblink to handle inconsistent DateStyle/IntervalStyle safely.

If the remote database's settings of these GUCs are different from ours,
ambiguous datetime values may be read incorrectly.  To fix, temporarily
adopt the remote server's settings while we ingest a query result.

This is not a complete fix, since it doesn't do anything about ambiguous
values in commands sent to the remote server; but there seems little we
can do about that end of it given dblink's entirely textual API for
transmitted commands.

Back-patch to 9.2.  The hazard exists in all versions, but this patch
would need more work to apply before 9.2.  Given the lack of field
complaints about this issue, it doesn't seem worth the effort at present.

Daniel Farina and Tom Lane

11 years agoImprove documentation of EXTRACT(WEEK).
Tom Lane [Mon, 18 Mar 2013 17:34:21 +0000 (13:34 -0400)]
Improve documentation of EXTRACT(WEEK).

The docs showed that early-January dates can be considered part of the
previous year for week-counting purposes, but failed to say explicitly
that late-December dates can also be considered part of the next year.
Fix that, and add a cross-reference to the "isoyear" field.  Per bug
#7967 from Pawel Kobylak.

11 years agoFix race condition in DELETE RETURNING.
Tom Lane [Sun, 10 Mar 2013 23:18:44 +0000 (19:18 -0400)]
Fix race condition in DELETE RETURNING.

When RETURNING is specified, ExecDelete would return a virtual-tuple slot
that could contain pointers into an already-unpinned disk buffer.  Another
process could change the buffer contents before we get around to using the
data, resulting in garbage results or even a crash.  This seems of fairly
low probability, which may explain why there are no known field reports of
the problem, but it's definitely possible.  Fix by forcing the result slot
to be "materialized" before we release pin on the disk buffer.

Back-patch to 9.0; in earlier branches there is no bug because
ExecProcessReturning sent the tuple to the destination immediately.  Also,
this is already fixed in HEAD as part of the writable-foreign-tables patch
(where the fix is necessary for DELETE RETURNING to work at all with
postgres_fdw).

11 years agoFix infinite-loop risk in fixempties() stage of regex compilation.
Tom Lane [Thu, 7 Mar 2013 16:51:08 +0000 (11:51 -0500)]
Fix infinite-loop risk in fixempties() stage of regex compilation.

The previous coding of this function could get into situations where it
would never terminate, because successive passes would re-add EMPTY arcs
that had been removed by the previous pass.  Rewrite the function
completely using a new algorithm that is guaranteed to terminate, and
also seems to be usually faster than the old one.  Per Tcl bugs 3604074
and 3606683.

Tom Lane and Don Porter

11 years agoFix tli history file fetching, broken by the archive after crash recevery patch.
Heikki Linnakangas [Thu, 7 Mar 2013 10:18:41 +0000 (12:18 +0200)]
Fix tli history file fetching, broken by the archive after crash recevery patch.

If we were about to enter archive recovery after crash recovery, we scanned
the archive for the latest tli history file, and set the recovery target
timeline to that. However, when we actually tried to read the history file,
we would not fetch the file from the archive, because we were not in archive
recovery yet.

To fix, make readTimeLineHistory and existsTimeLineHistory to always fetch
the file from archive if archive recovery is requested, even if we're not in
archive recovery yet.

Backpatch to 9.2. Mitsumasa KONDO

11 years agoFurther fix to the mode where we enter archive recovery after crash recovery.
Heikki Linnakangas [Thu, 7 Mar 2013 10:12:33 +0000 (12:12 +0200)]
Further fix to the mode where we enter archive recovery after crash recovery.

I missed to returns in the middle of ReadRecord function in my previous fix.
If a WAL file was not found at all during crash recovery, XLogPageRead would
return 'false', and ReadRecord would return without entering archive recovery.

9.2 only. In master, the code is structured differently and does not have this
problem.

Kyotaro HORIGUCHI, Mitsumasa KONDO and me.

11 years agoFix message typo.
Andrew Dunstan [Wed, 6 Mar 2013 14:53:38 +0000 (09:53 -0500)]
Fix message typo.

11 years agoFix to_char() to use ASCII-only case-folding rules where appropriate.
Tom Lane [Tue, 5 Mar 2013 18:02:35 +0000 (13:02 -0500)]
Fix to_char() to use ASCII-only case-folding rules where appropriate.

formatting.c used locale-dependent case folding rules in some code paths
where the result isn't supposed to be locale-dependent, for example
to_char(timestamp, 'DAY').  Since the source data is always just ASCII
in these cases, that usually didn't matter ... but it does matter in
Turkish locales, which have unusual treatment of "i" and "I".  To confuse
matters even more, the misbehavior was only visible in UTF8 encoding,
because in single-byte encodings we used pg_toupper/pg_tolower which
don't have locale-specific behavior for ASCII characters.  Fix by providing
intentionally ASCII-only case-folding functions and using these where
appropriate.  Per bug #7913 from Adnan Dursun.  Back-patch to all active
branches, since it's been like this for a long time.

11 years agoFix overflow check in tm2timestamp (this time for sure).
Tom Lane [Mon, 4 Mar 2013 20:13:31 +0000 (15:13 -0500)]
Fix overflow check in tm2timestamp (this time for sure).

I fixed this code back in commit 841b4a2d5, but didn't think carefully
enough about the behavior near zero, which meant it improperly rejected
1999-12-31 24:00:00.  Per report from Magnus Hagander.

11 years agoFix SQL function execution to be safe with long-lived FmgrInfos.
Tom Lane [Sun, 3 Mar 2013 22:40:04 +0000 (17:40 -0500)]
Fix SQL function execution to be safe with long-lived FmgrInfos.

fmgr_sql had been designed on the assumption that the FmgrInfo it's called
with has only query lifespan.  This is demonstrably unsafe in connection
with range types, as shown in bug #7881 from Andrew Gierth.  Fix things
so that we re-generate the function's cache data if the (sub)transaction
it was made in is no longer active.

Back-patch to 9.2.  This might be needed further back, but it's not clear
whether the case can realistically arise without range types, so for now
I'll desist from back-patching further.

11 years agodoc: A few awkward phrasing fixes
Peter Eisentraut [Sun, 3 Mar 2013 13:49:49 +0000 (08:49 -0500)]
doc: A few awkward phrasing fixes

Josh Kupershmidt

11 years agoExclude utils/probes.h and pg_trace.h from cpluspluscheck
Peter Eisentraut [Sat, 2 Mar 2013 03:43:47 +0000 (22:43 -0500)]
Exclude utils/probes.h and pg_trace.h from cpluspluscheck

They can include sys/sdt.h from SystemTap, which itself contains C++
code and so won't compile with a C++ compiler under extern "C" linkage.

11 years agoEliminate memory leaks in plperl's spi_prepare() function.
Tom Lane [Sat, 2 Mar 2013 02:33:38 +0000 (21:33 -0500)]
Eliminate memory leaks in plperl's spi_prepare() function.

Careless use of TopMemoryContext for I/O function data meant that repeated
use of spi_prepare and spi_freeplan would leak memory at the session level,
as per report from Christian Schröder.  In addition, spi_prepare
leaked a lot of transient data within the current plperl function's SPI
Proc context, which would be a problem for repeated use of spi_prepare
within a single plperl function call; and it wasn't terribly careful
about releasing permanent allocations in event of an error, either.

In passing, clean up some copy-and-pasteos in query-lookup error messages.

Alex Hunsaker and Tom Lane

11 years agoAdd missing error check in regexp parser.
Tom Lane [Wed, 27 Feb 2013 15:40:10 +0000 (10:40 -0500)]
Add missing error check in regexp parser.

parseqatom() failed to check for an error return (NULL result) from its
recursive call to parsebranch(), and in consequence could crash with a
null-pointer dereference after an error return.  This bug has been there
since day one, but wasn't noticed before, probably because most error cases
in parsebranch() didn't actually lead to returning NULL.  Add the missing
error check, and also tweak parsebranch() to exit in a less indirect
fashion after a call to parseqatom() fails.

Report by Tomasz Karlik, fix by me.

11 years agodoc: Fix markup typo
Peter Eisentraut [Mon, 25 Feb 2013 22:58:14 +0000 (17:58 -0500)]
doc: Fix markup typo

11 years agodoc: Remove PostgreSQL version number from xml2 deprecation notice
Peter Eisentraut [Sun, 24 Feb 2013 20:38:07 +0000 (15:38 -0500)]
doc: Remove PostgreSQL version number from xml2 deprecation notice

It is obviously no longer true.

11 years agoCorrect tense in log message
Peter Eisentraut [Sun, 24 Feb 2013 04:30:14 +0000 (23:30 -0500)]
Correct tense in log message

11 years agoAdd quotes to messages
Peter Eisentraut [Sat, 23 Feb 2013 04:33:07 +0000 (23:33 -0500)]
Add quotes to messages

11 years agoFix thinko in previous commit.
Heikki Linnakangas [Fri, 22 Feb 2013 11:07:02 +0000 (13:07 +0200)]
Fix thinko in previous commit.

We must still initialize minRecoveryPoint if we start straight with archive
recovery, e.g when recovering from a normal base backup taken with
pg_start/stop_backup. Otherwise we never consider the system consistent.

11 years agoIf recovery.conf is created after "pg_ctl stop -m i", do crash recovery.
Heikki Linnakangas [Fri, 22 Feb 2013 09:43:04 +0000 (11:43 +0200)]
If recovery.conf is created after "pg_ctl stop -m i", do crash recovery.

If you create a base backup using an atomic filesystem snapshot, and try to
perform PITR starting from that base backup, or if you just kill a master
server and create recovery.conf to put it into standby mode, we don't know
how far we need to recover before reaching consistency. Normally in crash
recovery, we replay all the WAL present in pg_xlog, and assume that we're
consistent after that. And normally in archive recovery, minRecoveryPoint,
backupEndRequired, or backupEndPoint is set in the control file, indicating
how far we need to replay to reach consistency. But if the server was
previously up and running normally, and you kill -9 it or take an atomic
filesystem snapshot, none of those fields are set in the control file.

The solution is to perform crash recovery first, replaying all the WAL in
pg_xlog. After that's done, we assume that the system is consistent like in
normal crash recovery, and switch to archive recovery mode after that.

Per report from Kyotaro HORIGUCHI. In his scenario, recovery.conf was
created after "pg_ctl stop -m i". I'm not sure we need to support that exact
scenario, but we should support backing up using a filesystem snapshot,
which looks identical.

This issue goes back to at least 9.0, where hot standby was introduced and
we started to track when consistency is reached. In 9.1 and 9.2, we would
open up for hot standby too early, and queries could briefly see an
inconsistent state. But 9.2 made it more visible, as we started to PANIC if
we see a reference to a non-existing page during recovery, if we've already
reached consistency. This is a fairly big patch, so back-patch to 9.2 only,
where the issue is more visible. We can consider back-patching further after
this has received some more testing in 9.2 and master.

11 years agoFix pg_dumpall with database names containing =
Heikki Linnakangas [Wed, 20 Feb 2013 15:08:54 +0000 (17:08 +0200)]
Fix pg_dumpall with database names containing =

If a database name contained a '=' character, pg_dumpall failed. The problem
was in the way pg_dumpall passes the database name to pg_dump on the
command line. If it contained a '=' character, pg_dump would interpret it
as a libpq connection string instead of a plain database name.

To fix, pass the database name to pg_dump as a connection string,
"dbname=foo", with the database name escaped if necessary.

Back-patch to all supported branches.

11 years agoDon't pass NULL to fprintf, if a bogus connection string is given to pg_dump.
Heikki Linnakangas [Wed, 20 Feb 2013 14:22:47 +0000 (16:22 +0200)]
Don't pass NULL to fprintf, if a bogus connection string is given to pg_dump.

Back-patch to all supported branches.

11 years agoBetter fix for "unarchived WAL files get deleted on crash recovery" bug.
Heikki Linnakangas [Fri, 15 Feb 2013 17:33:31 +0000 (19:33 +0200)]
Better fix for "unarchived WAL files get deleted on crash recovery" bug.

Revert my earlier fix for the bug that unarchived WAL files get deleted on
crash recovery, commit c9cc7e05c6d82a9781883a016c70d95aa4923122. We create
a .done file for files streamed or restored from archive, so the WAL file
recycling logic used during normal operation works just as well during
archive recovery.

Per Fujii Masao's suggestion.

11 years agoDon't delete unarchived WAL files during crash recovery.
Heikki Linnakangas [Fri, 15 Feb 2013 15:25:16 +0000 (17:25 +0200)]
Don't delete unarchived WAL files during crash recovery.

Bug reported by Jehan-Guillaume (ioguix) de Rorthais. This was introduced
with the change to keep WAL files restored from archive in pg_xlog, in 9.2.

11 years agoFix contrib/pg_trgm's similarity() function for trigram-free strings.
Tom Lane [Wed, 13 Feb 2013 19:07:13 +0000 (14:07 -0500)]
Fix contrib/pg_trgm's similarity() function for trigram-free strings.

Cases such as similarity('', '') produced a NaN result due to computing
0/0.  Per discussion, make it return zero instead.

This appears to be the basic cause of bug #7867 from Michele Baravalle,
although it remains unclear why her installation doesn't think Cyrillic
letters are letters.

Back-patch to all active branches.

11 years agoFix bogus when-to-deregister-from-listener-array logic.
Tom Lane [Wed, 13 Feb 2013 17:48:11 +0000 (12:48 -0500)]
Fix bogus when-to-deregister-from-listener-array logic.

Since a backend adds itself to the global listener array during
Exec_ListenPreCommit, it's inappropriate for it to remove itself during
Exec_UnlistenCommit or Exec_UnlistenAllCommit --- that leads to failure
when committing a transaction that did UNLISTEN then LISTEN, since we end
up not registered though we should be.  (This leads to missing later
notifications, or to Assert failures in assert-enabled builds.)  Instead
deal with deregistering at the bottom of AtCommit_Notify, when we know the
final state of the listenChannels list.

Also, simplify the representation of registration status by replacing the
transient backendHasExecutedInitialListen flag with an amRegisteredListener
flag.

Per report from Greg Sabino Mullane.  Back-patch to 9.0, where the problem
was introduced during the LISTEN/NOTIFY rewrite.

11 years agoFurther cleanup of gistsplit.c.
Tom Lane [Sun, 10 Feb 2013 21:21:32 +0000 (16:21 -0500)]
Further cleanup of gistsplit.c.

After further reflection I was unconvinced that the existing coding is
guaranteed to return valid union datums in every code path for multi-column
indexes.  Fix that by forcing a gistunionsubkey() call at the end of the
recursion.  Having done that, we can remove some clearly-redundant calls
elsewhere.  This should be a little faster for multi-column indexes (since
the previous coding would uselessly do such a call for each column while
unwinding the recursion), as well as much harder to break.

Also, simplify the handling of cases where one side or the other of a
primary split contains only don't-care tuples.  The previous coding used a
very ugly hack in removeDontCares() that essentially forced one random
tuple to be treated as non-don't-care, providing a random initial choice of
seed datum for the secondary split.  It seems unlikely that that method
will give better-than-random splits.  Instead, treat such a split as
degenerate and just let the next column determine the split, the same way
that we handle fully degenerate cases where the two sides produce identical
union datums.

11 years agoRemove useless picksplit-doesn't-support-secondary-split log spam.
Tom Lane [Sun, 10 Feb 2013 18:07:45 +0000 (13:07 -0500)]
Remove useless picksplit-doesn't-support-secondary-split log spam.

This LOG message was put in over five years ago with the evident
expectation that we'd make all GiST opclasses support secondary split
directly.  However, no such thing ever happened, and indeed the number of
opclasses supporting it decreased to zero in 9.2.  The reason is that
improving on the default implementation isn't that easy --- the
opclass-specific code that did exist, before 9.2, doesn't appear to have
been any improvement over the default.

Hence, remove the message altogether.  There's certainly no point in
nagging users about this in released branches, but I doubt that we'll
ever implement complete opclass-specific support anyway.

11 years agoRemove vestigial secondary-split support in gist_box_picksplit().
Tom Lane [Sun, 10 Feb 2013 17:40:16 +0000 (12:40 -0500)]
Remove vestigial secondary-split support in gist_box_picksplit().

Not only is this implementation of secondary-split not better than the
default implementation in gistsplit.c, it's actually worse.  The gistsplit.c
code at least looks to see if switching the left and right sides would make
a better merge with the previously-split tuples, while this doesn't.

In any case it's rather useless to support secondary split only in an edge
case.  There used to be more complete support for it here (in chooseLR()),
but that was removed in commit 7f3bd86843e5aad84585a57d3f6b80db3c609916.
It appears to me though that the chooseLR() code was really isomorphic to
the default implementation, since it was still based on choosing the cheaper
way of adding two sub-split vectors that had been chosen without regard to
the primary split initially.  I think an implementation of secondary split
that could beat the default implementation would have to be pretty fully
integrated into the split algorithm, not plastered on at the end.

Back-patch to 9.2, but not further; previous branches have the chooseLR()
code which I don't feel a great need to mess with.  This is mainly so we
just have two behaviors and not three among the various branches (IOW, this
patch is cleanup for commit 7f3bd86843e5aad84585a57d3f6b80db3c609916's
incomplete removal of secondary-split support).

11 years agoDocument and clean up gistsplit.c.
Tom Lane [Sun, 10 Feb 2013 16:58:23 +0000 (11:58 -0500)]
Document and clean up gistsplit.c.

Improve comments, rename some variables and functions, slightly simplify
a couple of APIs, in an attempt to make this code readable by people other
than its original author.

Even though this is essentially just cosmetic, back-patch to all active
branches, because otherwise it's going to make back-patching future fixes
in this file very painful.

11 years agoFix gist_box_same and gist_point_consistent to handle fuzziness correctly.
Tom Lane [Fri, 8 Feb 2013 23:03:23 +0000 (18:03 -0500)]
Fix gist_box_same and gist_point_consistent to handle fuzziness correctly.

While there's considerable doubt that we want fuzzy behavior in the
geometric operators at all (let alone as currently implemented), nobody is
stepping forward to redesign that stuff.  In the meantime it behooves us
to make sure that index searches agree with the behavior of the underlying
operators.  This patch fixes two problems in this area.

First, gist_box_same was using fuzzy equality, but it really needs to use
exact equality to prevent not-quite-identical upper index keys from being
treated as identical, which for example would prevent an existing upper
key from being extended by an amount less than epsilon.  This would result
in inconsistent indexes.  (The next release notes will need to recommend
that users reindex GiST indexes on boxes, polygons, circles, and points,
since all four opclasses use gist_box_same.)

Second, gist_point_consistent used exact comparisons for upper-page
comparisons in ~= searches, when it needs to use fuzzy comparisons to
ensure it finds all matches; and it used fuzzy comparisons for point <@ box
searches, when it needs to use exact comparisons because that's what the
<@ operator (rather inconsistently) does.

The added regression test cases illustrate all three misbehaviors.

Back-patch to all active branches.  (8.4 did not have GiST point_ops,
but it still seems prudent to apply the gist_box_same patch to it.)

Alexander Korotkov, reviewed by Noah Misch

11 years agoFix performance issue in EXPLAIN (ANALYZE, TIMING OFF).
Tom Lane [Fri, 8 Feb 2013 03:53:06 +0000 (22:53 -0500)]
Fix performance issue in EXPLAIN (ANALYZE, TIMING OFF).

Commit af7914c6627bcf0b0ca614e9ce95d3f8056602bf, which added the TIMING
option to EXPLAIN, had an oversight: if the TIMING option is disabled
then control in InstrStartNode() goes through an elog(DEBUG2) call, which
typically does nothing but takes a noticeable amount of time to do it.
Tweak the logic to avoid that.

In HEAD, also change the elog(DEBUG2)'s in instrument.c to elog(ERROR).
It's not very clear why they weren't like that to begin with, but this
episode shows that not complaining more vociferously about misuse is
likely to do little except allow bugs to remain hidden.

While at it, adjust some code that was making possibly-dangerous
assumptions about flag bits being in the rightmost byte of the
instrument_options word.

Problem reported by Pavel Stehule (via Tomas Vondra).

11 years agoMake contrib/btree_gist's GiST penalty function a bit saner.
Tom Lane [Fri, 8 Feb 2013 00:14:08 +0000 (19:14 -0500)]
Make contrib/btree_gist's GiST penalty function a bit saner.

The previous coding supposed that the first differing bytes in two varlena
datums must have the same sign difference as their overall comparison
result.  This is obviously bogus for text strings in non-C locales, and
probably wrong for numeric, and even for bytea I think it was wrong on
machines where char is signed.  When the assumption failed, the function
could deliver a zero or negative penalty in situations where such a result
is quite ridiculous, leading the core GiST code to make very bad page-split
decisions.

To fix, take the absolute values of the byte-level differences.  Also,
switch the code to using unsigned char not just char, so that the behavior
will be consistent whether char is signed or not.

Per investigation of a trouble report from Tomas Vondra.  Back-patch to all
supported branches.

11 years agoFix erroneous range-union logic for varlena types in contrib/btree_gist.
Tom Lane [Thu, 7 Feb 2013 23:22:27 +0000 (18:22 -0500)]
Fix erroneous range-union logic for varlena types in contrib/btree_gist.

gbt_var_bin_union() failed to do the right thing when the existing range
needed to be widened at both ends rather than just one end.  This could
result in an invalid index in which keys that are present would not be
found by searches, because the searches would not think they need to
descend to the relevant leaf pages.  This error affected all the varlena
datatypes supported by btree_gist (text, bytea, bit, numeric).

Per investigation of a trouble report from Tomas Vondra.  (There is also
an issue in gbt_var_penalty(), but that should only result in inefficiency
not wrong answers.  I'm committing this separately so that we have a git
state in which it can be tested that bad penalty results don't produce
invalid indexes.)  Back-patch to all supported branches.

11 years agoRepair bugs in GiST page splitting code for multi-column indexes.
Tom Lane [Thu, 7 Feb 2013 22:44:10 +0000 (17:44 -0500)]
Repair bugs in GiST page splitting code for multi-column indexes.

When considering a non-last column in a multi-column GiST index,
gistsplit.c tries to improve on the split chosen by the opclass-specific
pickSplit function by considering penalties for the next column.  However,
there were two bugs in this code: it failed to recompute the union keys for
the leftmost index columns, even though these might well change after
reassigning tuples; and it included the old union keys in the recomputation
for the columns it did recompute, so that those keys couldn't get smaller
even if they should.  The first problem could result in an invalid index
in which searches wouldn't find index entries that are in fact present;
the second would make the index less efficient to search.

Both of these errors were caused by misuse of gistMakeUnionItVec, whose
API was designed in a way that just begged such errors to be made.  There
is no situation in which it's safe or useful to compute the union keys for
a subset of the index columns, and there is no caller that wants any
previous union keys to be included in the computation; so the undocumented
choice to treat the union keys as in/out rather than pure output parameters
is a waste of code as well as being dangerous.

Hence, rather than just making a minimal patch, I've changed the API of
gistMakeUnionItVec to remove the "startkey" parameter (it now always
processes all index columns) and treat the attr/isnull arrays as purely
output parameters.

In passing, also get rid of a couple of unnecessary and dangerous uses
of static variables in gistutil.c.  It's remarkable that the one in
gistMakeUnionKey hasn't given us portability troubles before now, because
in addition to posing a re-entrancy hazard, it was unsafely assuming that
a static char[] array would have at least Datum alignment.

Per investigation of a trouble report from Tomas Vondra.  (There are also
some bugs in contrib/btree_gist to be fixed, but that seems like material
for a separate patch.)  Back-patch to all supported branches.