Fix encoding handling in xml binary input function. If the XML header didn't
specify an encoding explicitly, we used to treat it as being in database
encoding when we parsed it, but then perform a UTF-8 -> database encoding
conversion on it, which was completely bogus. It's now consistently treated as
UTF-8.
Tom Lane [Thu, 3 Sep 2009 22:11:13 +0000 (22:11 +0000)]
Make LOAD of an already-loaded library into a no-op, instead of attempting
to unload and re-load the library.
The difficulty with unloading a library is that we haven't defined safe
protocols for doing so. In particular, there's no safe mechanism for
getting out of a "hook" function pointer unless libraries are unloaded
in reverse order of loading. And there's no mechanism at all for undefining
a custom GUC variable, so GUC would be left with a pointer to an old value
that might or might not still be valid, and very possibly wouldn't be in
the same place anymore.
While the unload and reload behavior had some usefulness in easing
development of new loadable libraries, it's of no use whatever to normal
users, so just disabling it isn't giving up that much. Someday we might
care to expend the effort to develop safe unload protocols; but even if
we did, there'd be little certainty that every third-party loadable module
was following them, so some security restrictions would still be needed.
Back-patch to 8.2; before that, LOAD was superuser-only anyway.
Security: unprivileged users could crash backend. CVE not assigned yet
Tom Lane [Thu, 3 Sep 2009 22:08:14 +0000 (22:08 +0000)]
Disallow RESET ROLE and RESET SESSION AUTHORIZATION inside security-definer
functions.
This extends the previous patch that forbade SETting these variables inside
security-definer functions. RESET is equally a security hole, since it
would allow regaining privileges of the caller; furthermore it can trigger
Assert failures and perhaps other internal errors, since the code is not
expecting these variables to change in such contexts. The previous patch
did not cover this case because assign hooks don't really have enough
information, so move the responsibility for preventing this into guc.c.
Problem discovered by Heikki Linnakangas.
Security: no CVE assigned yet, extends CVE-2007-6600
Tom Lane [Thu, 3 Sep 2009 18:48:21 +0000 (18:48 +0000)]
Install a workaround for a longstanding gcc bug that allows SIGFPE traps
to occur for division by zero, even though the code is carefully avoiding
that. All available evidence is that the only functions affected are
int24div, int48div, and int28div, so patch just those three functions to
include a "return" after the ereport() call.
Backpatch to 8.4 so that the fix can be tested in production builds.
For older branches our recommendation will continue to be to use -O1
on affected platforms (which are mostly non-mainstream anyway).
Tom Lane [Wed, 2 Sep 2009 17:52:33 +0000 (17:52 +0000)]
Fix subquery pullup to wrap a PlaceHolderVar around the entire RowExpr
that's generated for a whole-row Var referencing the subquery, when the
subquery is in the nullable side of an outer join. The previous coding
instead put PlaceHolderVars around the elements of the RowExpr. The effect
was that when the outer join made the subquery outputs go to null, the
whole-row Var produced ROW(NULL,NULL,...) rather than just NULL. There
are arguments afoot about whether those things ought to be semantically
indistinguishable, but for the moment they are not entirely so, and the
planner needs to take care that its machinations preserve the difference.
Per bug #5025.
Making this feasible required refactoring ResolveNew() to allow more caller
control over what is substituted for a Var. I chose to make ResolveNew()
a wrapper around a new general-purpose function replace_rte_variables().
I also fixed the ancient bogosity that ResolveNew might fail to set
a query's hasSubLinks field after inserting a SubLink in it. Although
all current callers make sure that happens anyway, we've had bugs of that
sort before, and it seemed like a good time to install a proper solution.
Back-patch to 8.4. The problem can be demonstrated clear back to 8.0,
but the fix would be too invasive in earlier branches; not to mention
that people may be depending on the subtly-incorrect behavior. The
8.4 series is new enough that fixing this probably won't cause complaints,
but it might in older branches. Also, 8.4 shows the incorrect behavior
in more cases than older branches do, because it is able to flatten
subqueries in more cases.
Tom Lane [Wed, 2 Sep 2009 02:40:59 +0000 (02:40 +0000)]
Fix pg_ctl's readfile() to not go into infinite loop on an empty file
(could happen if either postgresql.conf or postmaster.opts is empty).
It's been broken since the C version was written for 8.0, so patch
all the way back.
initdb's copy of the function is broken in the same way, but it's
less important there since the input files should never be empty.
Patch that in HEAD only, and also fix some cosmetic differences that
crept into that copy of the function.
Tom Lane [Sun, 30 Aug 2009 16:53:37 +0000 (16:53 +0000)]
Remove duplicate variable initializations identified by clang static checker.
One of these represents a nontrivial bug (a promptly-leaked palloc), so
backpatch.
Tom Lane [Thu, 27 Aug 2009 20:08:12 +0000 (20:08 +0000)]
Modify the definition of window-function PARTITION BY and ORDER BY clauses
so that their elements are always taken as simple expressions over the
query's input columns. It originally seemed like a good idea to make them
act exactly like GROUP BY and ORDER BY, right down to the SQL92-era behavior
of accepting output column names or numbers. However, that was not such a
great idea, for two reasons:
1. It permits circular references, as exhibited in bug #5018: the output
column could be the one containing the window function itself. (We actually
had a regression test case illustrating this, but nobody thought twice about
how confusing that would be.)
2. It doesn't seem like a good idea for, eg, "lead(foo) OVER (ORDER BY foo)"
to potentially use two completely different meanings for "foo".
Accordingly, narrow down the behavior of window clauses to use only the
SQL99-compliant interpretation that the expressions are simple expressions.
Alvaro Herrera [Thu, 27 Aug 2009 17:19:31 +0000 (17:19 +0000)]
Fix handling of autovacuum reloptions.
In the original coding, setting a single reloption would cause default
values to be used for all the other reloptions. This is a problem
particularly for autovacuum reloptions.
In the checkpoint written at the end of archive recovery, the WAL page header
was incorrectly initialized with timeline ID 0. That rendered the WAL page
unrecoverable, making a subsequent archive recovery stop at that point.
ThisTimeLineID needs to be initialized before calling AdvanceXLInsertBuffer().
This fixes bug #5011 reported by James Bardin. Backpatch to 8.4, as the bug
was introduced by the changes to use of bgwriter for writing the
end-of-archive-recovery checkpoint. Patch by Tom Lane.
Tom Lane [Mon, 24 Aug 2009 20:08:40 +0000 (20:08 +0000)]
Try to make silent_mode behave somewhat reasonably.
Instead of sending stdout/stderr to /dev/null after forking away from the
terminal, send them to postmaster.log within the data directory. Since
this opens the door to indefinite logfile bloat, recommend even more
strongly that log output be redirected when using silent_mode.
Move the postmaster's initial calls of load_hba() and load_ident() down
to after we have started the log collector, if we are going to. This
is so that errors reported by them will appear in the "usual" place.
Reclassify silent_mode as a LOGGING_WHERE, not LOGGING_WHEN, parameter,
since it's got absolutely nothing to do with the latter category.
In passing, fix some obsolete references to -S ... this option hasn't
had that switch letter for a long time.
Back-patch to 8.4, since as of 8.4 load_hba() and load_ident() are more
picky (and thus more likely to fail) than they used to be. This entire
change was driven by a complaint about those errors disappearing into
the bit bucket.
Alvaro Herrera [Mon, 24 Aug 2009 17:23:17 +0000 (17:23 +0000)]
Avoid calling kill() in a postmaster signal handler.
This causes problems when the system load is high, per report from Zdenek
Kotala in <1250860954.1239.114.camel@localhost>; instead of calling kill
directly, have the signal handler set a flag which is checked in ServerLoop.
This way, the handler can return before being called again by a subsequent
signal sent from the autovacuum launcher. Also, increase the sleep in the
launcher in this failure path to 1 second.
Backpatch to 8.3, which is when the signalling between autovacuum
launcher/postmaster was introduced.
Also, add a couple of ReleasePostmasterChildSlot calls in error paths; this
part backpatched to 8.4 which is when the child slot stuff was introduced.
Tom Lane [Mon, 24 Aug 2009 16:18:19 +0000 (16:18 +0000)]
Fix inclusions of readline/editline header files so that we only attempt to
#include the version of history.h that is in the same directory as the
readline.h we are using. This avoids problems in some scenarios where both
readline and editline are installed. Report and patch by Zdenek Kotala.
Tom Lane [Mon, 24 Aug 2009 02:18:40 +0000 (02:18 +0000)]
Fix a violation of WAL coding rules in the recent patch to include an
"all tuples visible" flag in heap page headers. The flag update *must*
be applied before calling XLogInsert, but heap_update and the tuple
moving routines in VACUUM FULL were ignoring this rule. A crash and
replay could therefore leave the flag incorrectly set, causing rows
to appear visible in seqscans when they should not be. This might explain
recent reports of data corruption from Jeff Ross and others.
In passing, do a bit of editorialization on comments in visibilitymap.c.
Tom Lane [Sun, 23 Aug 2009 18:26:15 +0000 (18:26 +0000)]
Tweak ExecIndexEvalRuntimeKeys to forcibly detoast any toasted comparison
values before they get passed to the index access method. This avoids
repeated detoastings that will otherwise ensue as the comparison value
is examined by various index support functions. We have seen a couple of
reports of cases where repeated detoastings result in an order-of-magnitude
slowdown, so it seems worth adding a bit of extra logic to prevent this.
I had previously proposed trying to avoid duplicate detoastings in general,
but this fix takes care of what seems the most important case in practice
with very little effort or risk.
Back-patch to 8.4 so that the PostGIS folk won't have to wait a year to
have this fix in a production release. (The issue exists further back,
of course, but the code's diverged enough to make backpatching further a
higher-risk action. Also it appears that the possible gains may be limited
in prior releases because of different handling of lossy operators.)
Tom Lane [Tue, 18 Aug 2009 21:23:21 +0000 (21:23 +0000)]
Fix overflow for INTERVAL 'x ms' where x is more than a couple million,
and integer datetimes are in use. Per bug report from Hubert Depesz
Lubaczewski.
Tom Lane [Sun, 16 Aug 2009 19:55:28 +0000 (19:55 +0000)]
Fix imprecise documentation of random(): it never returns 1.0.
This was changed in 8.2 but the documentation was not corrected.
Per gripe from Sam Mason.
Tom Lane [Sun, 16 Aug 2009 18:14:39 +0000 (18:14 +0000)]
Fix incorrect encoding-aware name truncation in makeArrayTypeName().
truncate_identifier won't do anything if the passed-in strlen is already
less than NAMEDATALEN, which it always would be given the strlcpy usage.
This has been broken since the arrays-of-composite-types code went in.
Arguably truncate_identifier is suffering from excessive optimization
and should always process the string, but for the moment I'll take the
more localized patch.
Tom Lane [Thu, 13 Aug 2009 16:53:15 +0000 (16:53 +0000)]
Put back adjust_appendrel_attrs()'s code for dealing with RestrictInfo.
I mistakenly removed it last month, thinking it was no longer needed ---
but it is still needed for dealing with joininfo lists. Fortunately this
bit of brain fade hadn't made it into any released versions yet.
Tom Lane [Wed, 12 Aug 2009 18:23:55 +0000 (18:23 +0000)]
Fix old bug in log_autovacuum_min_duration code: it was relying on being able
to access a Relation entry it had just closed. I happened to be testing with
CLOBBER_CACHE_ALWAYS, which made this a guaranteed core dump (at least on
machines where sprintf %s isn't forgiving of a NULL pointer). It's probably
quite unlikely that it would fail in the field, but a bug is a bug. Fix by
moving the relation_close call down past the logging action.
Magnus Hagander [Tue, 11 Aug 2009 11:51:22 +0000 (11:51 +0000)]
Reserve the shared memory region during backend startup on Windows, so
that memory allocated by starting third party DLLs doesn't end up
conflicting with it.
Hopefully this solves the long-time issue with "could not reattach
to shared memory" errors on Win32.
Patch from Tsutomu Yamada and me, based on idea from Trevor Talbot.
Tom Lane [Fri, 7 Aug 2009 20:16:16 +0000 (20:16 +0000)]
Try to defend against the possibility that libpq is still in COPY_IN state
when we reach the post-COPY "pump it dry" error recovery code that was added
2006-11-24. Per a report from Neil Best, there is at least one code path
in which this occurs, leading to an infinite loop in code that's supposed
to be making it more robust not less so. A reasonable response seems to be
to call PQputCopyEnd() again, so let's try that.
Back-patch to all versions that contain the cleanup loop.
Tom Lane [Fri, 7 Aug 2009 19:29:57 +0000 (19:29 +0000)]
rm_cleanup functions need to be allowed to write WAL entries. This oversight
appears to explain the recent reports of "PANIC: cannot make new WAL entries
during recovery".
Fast shutdown stop should forcibly disconnect any active backends, even
if a smart shutdown is already in progress. Backpatch to 8.3, this was broken
in the patch that introduced "dead-end backends".
Per report by Itagaki Takahiro, patch by Fujii Masao.
Tom Lane [Wed, 29 Jul 2009 22:19:23 +0000 (22:19 +0000)]
Fix time_part and timetz_part (ie, EXTRACT() for those datatypes) to
include a fractional part in the output for MILLISECOND and SECOND cases,
rather than truncating the source value. This is what the float-timestamp
code has always done, and it was clearly the code author's intent to do
the same for integer timestamps, but he forgot about integer division in C.
The other datatypes supported by EXTRACT() already do this correctly.
Backpatch to 8.4, so that the default (integer) behavior of that branch will
match the default (float) behavior of older branches. Arguably we should
patch further back, but it's possible that applications are expecting the
broken behavior in older branches. 8.4 is new enough that expectations
shouldn't be too settled.
Tom Lane [Wed, 29 Jul 2009 15:57:16 +0000 (15:57 +0000)]
Fix a thinko introduced into CountActiveBackends by a recent patch:
we should ignore NULL array entries, not non-NULL ones. This had the
effect of disabling commit_delay, and could have caused a crash in the
rare race condition the patch was intended to fix.
Bug report and diagnosis by Jeff Janes, in bug #4952.
Tom Lane [Thu, 23 Jul 2009 20:45:34 +0000 (20:45 +0000)]
In a non-hashed Agg node, reset the "aggcontext" at group boundaries, instead
of individually pfree'ing pass-by-reference transition values. This should
be at least as fast as the prior coding, and it has the major advantage of
clearing out any working data an aggregate function may have stored in or
underneath the aggcontext. This avoids memory leakage when an aggregate
such as array_agg() is used in GROUP BY mode. Per report from Chris Spotts.
Back-patch to 8.4. In principle the problem could arise in prior versions,
but since they didn't have array_agg the issue seems not critical.
Tom Lane [Thu, 23 Jul 2009 17:42:13 +0000 (17:42 +0000)]
Fix another thinko in join_is_legal's handling of semijoins: we have to test
for the case that the semijoin was implemented within either input by
unique-ifying its RHS before we test to see if it appears to match the current
join situation. The previous coding would select semijoin logic in situations
where we'd already unique-ified the RHS and joined it to some unrelated
relation(s), and then came to join it to the semijoin's LHS. That still gave
the right answer as far as the semijoin itself was concerned, but would lead
to incorrectly examining only an arbitrary one of the matchable rows from the
unrelated relation(s). The cause of this thinko was incorrect unification of
the pre-8.4 logic for IN joins and OUTER joins --- the comparable case for
outer joins can be handled after making the match test, but that's because
there is nothing like the unique-ification escape hatch for outer joins.
Per bug #4934 from Benjamin Reed.
Tom Lane [Tue, 21 Jul 2009 02:02:51 +0000 (02:02 +0000)]
Fix another semijoin-ordering bug. We already knew that we couldn't
reorder a semijoin into or out of the righthand side of another semijoin,
but actually it doesn't work to reorder it into or out of the righthand
side of a left or antijoin, either. Per bug #4906 from Mathieu Fenniak.
This was sloppy thinking on my part. This identity does work:
( A left join B on (Pab) ) semijoin C on (Pac)
==
( A semijoin C on (Pac) ) left join B on (Pab)
but I failed to see that that doesn't mean this does:
( A left join B on (Pab) ) semijoin C on (Pbc)
!=
A left join ( B semijoin C on (Pbc) ) on (Pab)
Install src/include/utils/fmgroids.h on VPATH builds too.
The original coding was not dealing specially with this file being a symlink,
with the end result that it was not installed in VPATH builds. Oddly enough,
the clean target does know about it ...
Tom Lane [Sun, 19 Jul 2009 20:32:56 +0000 (20:32 +0000)]
Fix a thinko in join_is_legal: when we decide we can implement a semijoin
by unique-ifying the RHS and then inner-joining to some other relation,
that is not grounds for violating the RHS of some other outer join.
Noticed while regression-testing new GEQO code, which will blindly follow
any path that join_is_legal says is legal, and then complain later if that
leads to a dead end.
I'm not certain that this can result in any visible failure in 8.4: the
mistake may always be masked by the fact that subsequent attempts to join
the rest of the RHS of the other join will fail. But I'm not certain it
can't, either, and it's definitely not operating as intended. So back-patch.
The added regression test depends on the new no-failures-allowed logic
that I'm about to commit in GEQO, so no point back-patching that.
Tom Lane [Sat, 18 Jul 2009 19:15:50 +0000 (19:15 +0000)]
Fix error cleanup failure caused by 8.4 changes in plpgsql to try to avoid
memory leakage in error recovery. We were calling FreeExprContext, and
therefore invoking ExprContextCallback callbacks, in both normal and error
exits from subtransactions. However this isn't very safe, as shown in
recent trouble report from Frank van Vugt, in which releasing a tupledesc
refcount failed. It's also unnecessary, since the resources that callbacks
might wish to release should be cleaned up by other error recovery mechanisms
(ie the resource owners). We only really want FreeExprContext to release
memory attached to the exprcontext in the error-exit case. So, add a bool
parameter to FreeExprContext to tell it not to call the callbacks.
A more general solution would be to pass the isCommit bool parameter on to
the callbacks, so they could do only safe things during error exit. But
that would make the patch significantly more invasive and possibly break
third-party code that registers ExprContextCallback callbacks. We might want
to do that later in HEAD, but for now I'll just do what seems reasonable to
back-patch.
Tom Lane [Fri, 17 Jul 2009 23:19:59 +0000 (23:19 +0000)]
Repair bug #4926 "too few pathkeys for mergeclauses". This example shows
that the sanity checking I added to create_mergejoin_plan() in 8.3 was a
few bricks shy of a load: the mergeclauses could reference pathkeys in a
noncanonical order such as x,y,x, not only cases like x,x,y which is all
that the code had allowed for. The odd cases only turn up when using
redundant clauses in an outer join condition, which is why no one had
noticed before.
Tom Lane [Tue, 14 Jul 2009 15:37:55 +0000 (15:37 +0000)]
Do a conditional SPI_push/SPI_pop when replanning a query in
RevalidateCachedPlan. This is to avoid a "SPI_ERROR_CONNECT" failure when
the planner calls a SPI-using function and we are already inside one.
The alternative fix is to expect callers of RevalidateCachedPlan to do this,
which seems likely to result in additional hard-to-detect bugs of omission.
Per reports from Frank van Vugt and Marek Lewczuk.
Back-patch to 8.3. It's much harder to trigger the bug in 8.3, due to a
smaller set of cases in which plans can be invalidated, but it could happen.
(I think perhaps only a SI reset event could make 8.3 fail here, but that's
certainly within the realm of possibility.)
Tom Lane [Sat, 11 Jul 2009 04:09:40 +0000 (04:09 +0000)]
Fix set_rel_width() to do something reasonable with non-Var items in a
RelOptInfo targetlist. It used to be that the only possibility other than
a Var was a RowExpr representing a whole-row child Var, but as of 8.4's
expanded ability to flatten appendrel members, we can get arbitrary expressions
in there. Use the expression's type info and get_typavgwidth() to produce
an at-least-marginally-sane result. Note that get_typavgwidth()'s fallback
estimate (32 bytes) is the same as what was here before, so there will be
no behavioral change for RowExprs. Noted while looking at recent gripe
about constant quals pushed down to FunctionScan appendrel members ...
not only were we failing to recognize the constant qual, we were getting
the width estimate wrong :-(
Tom Lane [Fri, 10 Jul 2009 00:32:06 +0000 (00:32 +0000)]
Fix xslt_process() to ensure that it inserts a NULL terminator after the
last pair of parameter name/value strings, even when there are MAXPARAMS
of them. Aboriginal bug in contrib/xml2, noted while studying bug #4912
(though I'm not sure whether there's something else involved in that
report).
This might be thought a security issue, since it's a potential backend
crash; but considering that untrustworthy users shouldn't be allowed
to get their hands on xslt_process() anyway, it's probably not worth
getting excited about.
Tom Lane [Wed, 8 Jul 2009 18:55:42 +0000 (18:55 +0000)]
Remove no-longer-necessary transmission of postmaster's LC_COLLATE and
LC_CTYPE settings to children via BackendParameters. Per discussion,
the postmaster is now just using system defaults anyway, so we might as
well save a few cycles during backend startup.
Need to use pg_perm_setlocale when setting LC_CTYPE and LC_COLLATE at startup.
Otherwise, the LC_CTYPE/COLLATE setting gets reverted when using plperl, which
leads to incorrect query results and index corruption.
This was accidentally broken in the per-database locale patch in 8.4. Pointed
out by Andrew Gierth.
Tom Lane [Wed, 8 Jul 2009 17:22:01 +0000 (17:22 +0000)]
Add missing HOUR TO SECOND option to list of possible INTERVAL field sets,
as noted by Sebastien Flaesch. Also update the claim that we simply throw
away fields outside this set --- that got changed later to only discard
less-significant fields.
Fix ancient bug in handling of to_char modifier 'TH', when used with HH.
In what seems like an oversight, we used to treat 'TH' the same as lowercase
'th', but only with HH/HH12.
Tom Lane [Mon, 6 Jul 2009 18:26:39 +0000 (18:26 +0000)]
Fix set_append_rel_pathlist() to deal intelligently with cases where
substituting a child rel's output expressions into the appendrel's restriction
clauses yields a pseudoconstant restriction. We might be able to skip scanning
that child rel entirely (if we get constant FALSE), or generate a one-time
filter. 8.3 more or less accidentally generated plans that weren't completely
stupid in these cases, but that was only because an extra recursive level of
subquery_planner() always occurred and allowed const-simplification to happen.
8.4's ability to pull up appendrel members with non-Var outputs exposes the
fact that we need to work harder here. Per gripe from Sergey Burladyan.
Tom Lane [Mon, 6 Jul 2009 02:58:48 +0000 (02:58 +0000)]
Per SQL spec (in particular, the grammar in SQL:2008 7.13) we should allow
parentheses around the <query expression body> that follows a WITH clause, eg
with cte(foo) as ( values(0) ) ((select foo from cte));
This seems to be just an oversight/thinko in gram.y. Noted while
experimenting with bug #4902.
Tom Lane [Mon, 6 Jul 2009 02:16:14 +0000 (02:16 +0000)]
Fix handling of changed-Param signaling for CteScan plan nodes. We were using
the "cteParam" as a proxy for the possibility that the underlying CTE plan
depends on outer-level variables or Params, but that doesn't work very well
because it sometimes causes calling subqueries to be treated as SubPlans when
they could be InitPlans. This is inefficient and also causes the outright
failure exhibited in bug #4902. Instead, leave the cteParam out of it and
copy the underlying CTE plan's extParams directly. Per bug #4902 from
Marko Tiikkaja.
Tom Lane [Sat, 27 Jun 2009 21:06:46 +0000 (21:06 +0000)]
Revert addition of "o" to tar options. This was intended to fix bug #4883,
but the cure appears to be worse than the disease. It turns out that GNU
tar versions 1.14.x misinterpret -o as --same-owner, not --no-same-owner,
leading to exactly the wrong behavior for both root and nonroot users.
While that bug has been fixed for nearly five years, these tar versions
are still found in the wild, notably in OS X 10.4. Given that #4883 was
the first complaint we'd heard, it's definitely not worth fixing at the
risk of breaking things for other users. Perhaps revisit at a later date
when we're not up against a release deadline.
Tom Lane [Fri, 26 Jun 2009 20:29:04 +0000 (20:29 +0000)]
Cleanup and code review for the patch that made bgwriter active during
archive recovery. Invent a separate state variable and inquiry function
for XLogInsertAllowed() to clarify some tests and make the management of
writing the end-of-recovery checkpoint less klugy. Fix several places
that were incorrectly testing InRecovery when they should be looking at
RecoveryInProgress or XLogInsertAllowed (because they will now be executed
in the bgwriter not startup process). Clarify handling of bad LSNs passed
to XLogFlush during recovery. Use a spinlock for setting/testing
SharedRecoveryInProgress. Improve quite a lot of comments.
Tom Lane [Thu, 25 Jun 2009 23:07:15 +0000 (23:07 +0000)]
Add __attribute__((format_arg(1))) to the declaration of err_gettext(),
to restore gcc's ability to crosscheck format arguments within elog.c.
Noted in a test compilation with -Wformat-nonliteral enabled.
Fix some serious bugs in archive recovery, now that bgwriter is active
during it:
When bgwriter is active, the startup process can't perform mdsync() correctly
because it won't see the fsync requests accumulated in bgwriter's private
pendingOpsTable. Therefore make bgwriter responsible for the end-of-recovery
checkpoint as well, when it's active.
When bgwriter is active (= archive recovery), the startup process must not
accumulate fsync requests to its own pendingOpsTable, since bgwriter won't
see them there when it performs restartpoints. Make startup process drop its
pendingOpsTable when bgwriter is launched to avoid that.
Update minimum recovery point one last time when leaving archive recovery.
It won't be updated by the end-of-recovery checkpoint because XLogFlush()
sees us as out of recovery already.
The code to unlink dropped relations in FinishPreparedTransaction() was
acting like runs inside WAL recovery, but it doesn't. I must've copy-pasted
this from a redo-function in the relation forks patch. Noticed by Tom Lane
while he was looking through callers of smgrdounlink().
Disable pg_standby -l option because the backend doesn't expect the recovered
file to be a symlink. We tried to fix this issue with an earlier server-side
patch, but it didn't fix the whole issue.
The same bug is present in older releases as well, but the 8.4 train is
about to leave the station, and I'm not sure if have consensus on whether
we can remove the -l option in back-branches or do we need to attempt a
server-side fix to make symlinking safe.
Patch by Simon Riggs, per discussion on bug identified by Fujii Masao.
Tom Lane [Tue, 23 Jun 2009 16:25:02 +0000 (16:25 +0000)]
Fix an ancient error in dist_ps (distance from point to line segment), which
a number of other geometric operators also depend on. It miscalculated the
slope of the perpendicular to the given line segment anytime that slope was
other than 0, infinite, or +/-1. In some cases the error would be masked
because the true closest point on the line segment was one of its endpoints
rather than the intersection point, but in other cases it could give an
arbitrarily bad answer. Per bug #4872 from Nick Roosevelt.
Bug goes clear back to Berkeley days, so patch all supported branches.
Make a couple of cosmetic adjustments while at it.
Tom Lane [Tue, 23 Jun 2009 03:46:00 +0000 (03:46 +0000)]
Fix the makefiles to fail cleanly if Perl is needed but not present. This
used to work as intended, but got broken some time ago (a quoted empty string
is not an empty string), and got broken some more by the changes to generate
ecpg's preproc.y automatically. Given all the unprotected uses of $(PERL)
elsewhere, it seems best to make use of the $(missing) script rather than
trying to ensure each such use is protected individually. Also fix various
bits of documentation that omitted to mention Perl as a requirement for
building from a CVS pull. Per a complaint from Robert Haas.
Tom Lane [Mon, 22 Jun 2009 20:04:28 +0000 (20:04 +0000)]
For bulk write operations (eg COPY IN), use a ring buffer of 16MB instead
of the 256KB limit originally enforced by a patch committed 2008-11-06.
Per recent test results, the smaller size resulted in an undesirable decrease
in bulk data loading speed, due to COPY processing frequently getting blocked
for WAL flushing. This area might need more tweaking later, but this setting
seems to be good enough for 8.4.
Tom Lane [Mon, 22 Jun 2009 17:54:30 +0000 (17:54 +0000)]
Make to_timestamp and friends skip leading spaces before an integer field,
even when not in FM mode. This improves compatibility with Oracle and with
our pre-8.4 behavior, as per bug #4862.
Brendan Jurd
Add a couple of regression test cases for this. In passing, get rid of the
labeling of the individual test cases; doesn't seem to be good for anything
except causing extra work when inserting a test...