Tom Lane [Mon, 29 Apr 2013 18:49:01 +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.
Kevin Grittner [Mon, 29 Apr 2013 18:05:26 +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.
Robert Haas [Mon, 29 Apr 2013 10:29:32 +0000 (06:29 -0400)]
Attempt to fix error recovery in COPY BOTH mode.
Previously, libpq and the backend had opposite ideas about whether
it was necessary for the client to send a CopyDone message after
receiving an ErrorResponse, making it impossible to cleanly exit
COPY BOTH mode. Fix libpq so that works correctly, adopting the
backend's notion that an ErrorResponse kills the copy in both
directions.
Adjust receivelog.c to avoid a degradation in the quality of the
resulting error messages. libpqwalreceiver.c is already doing
the right thing, so no adjustment needed there.
Add an explicit statement to the documentation explaining how
this part of the protocol is supposed to work, in the hopes of
avoiding future confusion in this area.
Since the consequences of all this confusion are very limited,
especially in the back-branches where no client ever attempts
to exit COPY BOTH mode without closing the connection entirely,
no back-patch.
Simon Riggs [Mon, 29 Apr 2013 08:05:27 +0000 (09:05 +0100)]
Introduce new page checksum algorithm and module.
Isolate checksum calculation to its own module, so that bufpage
knows little if anything about the details of the calculation.
This implementation is a modified FNV-1a hash checksum, details
of which are given in the new checksum.c header comments.
Basic implementation only, so we fix the output value.
Later related commits will add version numbers to pg_control,
compiler optimization flags and memory barriers.
Ants Aasma, reviewed by Jeff Davis and Simon Riggs
Tom Lane [Sun, 28 Apr 2013 04:18:45 +0000 (00:18 -0400)]
Editorialize a bit on new ProcessUtility() API.
Choose a saner ordering of parameters (adding a new input param after
the output params seemed a bit random), update the function's header
comment to match reality (cmon folks, is this really that hard?),
get rid of useless and sloppily-defined distinction between
PROCESS_UTILITY_SUBCOMMAND and PROCESS_UTILITY_GENERATED.
Tom Lane [Sun, 28 Apr 2013 03:11:28 +0000 (23:11 -0400)]
Fix unsafe event-trigger coding in ProcessUtility().
We mustn't run any of the event-trigger support code when handling
utility statements like START TRANSACTION or ABORT, because that code
may need to refresh event-trigger cache data, which requires being
inside a valid transaction. (This mistake explains the consistent
build failures exhibited by the CLOBBER_CACHE_ALWAYS buildfarm members,
as well as some irreproducible failures on other members.)
The least messy fix seems to be to break standard_ProcessUtility into two
functions, one that handles all the statements not supported by event
triggers, and one that contains the event-trigger support code and handles
the statements that are supported by event triggers.
This change also fixes several inconsistencies, such as four cases where
support had been installed for "ddl_event_start" but not "ddl_event_end"
triggers, plus the fact that InvokeDDLCommandEventTriggersIfSupported()
paid no mind to isCompleteQuery.
Tom Lane [Sat, 27 Apr 2013 21:48:57 +0000 (17:48 -0400)]
Incidental cleanup of matviews code.
Move checking for unscannable matviews into ExecOpenScanRelation, which is
a better place for it first because the open relation is already available
(saving a relcache lookup cycle), and second because this eliminates the
problem of telling the difference between rangetable entries that will or
will not be scanned by the query. In particular we can get rid of the
not-terribly-well-thought-out-or-implemented isResultRel field that the
initial matviews patch added to RangeTblEntry.
Also get rid of entirely unnecessary scannability check in the rewriter,
and a bogus decision about whether RefreshMatViewStmt requires a parse-time
snapshot.
catversion bump due to removal of a RangeTblEntry field, which changes
stored rules.
Tom Lane [Fri, 26 Apr 2013 19:48:24 +0000 (15:48 -0400)]
Fix collation assignment for aggregates with ORDER BY.
ORDER BY expressions were being treated the same as regular aggregate
arguments for purposes of collation determination, but really they should
not affect the aggregate's collation at all; only collations of the
aggregate's regular arguments should affect it.
In many cases this mistake would lead to incorrectly throwing a "collation
conflict" error; but in some cases the corrected code will silently assign
a different collation to the aggregate than before, for example
agg(foo ORDER BY bar COLLATE "x")
which will now use foo's collation rather than "x" for the aggregate.
Given this risk and the lack of field complaints about the issue, it
doesn't seem prudent to back-patch.
In passing, rearrange code in assign_collations_walker so that we don't
need multiple copies of the standard logic for computing collation of a
node with children. (Previously, CaseExpr duplicated the standard logic,
and we would have needed a third copy for Aggref without this change.)
Joe Conway [Fri, 26 Apr 2013 18:50:00 +0000 (11:50 -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.
Robert Haas [Fri, 26 Apr 2013 12:57:47 +0000 (08:57 -0400)]
libpq: Fix a few bits that didn't get the memo about COPY BOTH.
There's probably no real bug here at present, so not backpatching.
But it seems good to make these bits consistent with the rest of
libpq, so as to avoid future surprises.
Tom Lane [Thu, 25 Apr 2013 20:58:05 +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.
On non-Windows systems, sys/time.h was pulled in by portability/instr_time.h,
which pulled in time.h. We certainly should include time.h directly, since
we're using time(2), but the indirect include masked the problem on most
platforms.
Kevin Grittner [Wed, 24 Apr 2013 13:39:06 +0000 (08:39 -0500)]
Fix assertion failure for REFRESH MATERIALIZED VIEW in PL.
This was due to incomplete implementation of rowcount reporting
for RMV, which was due to initial waffling on whether it should
be provided. It seems unlikely to be a useful or universally
available number as more sophisticated techniques for maintaining
matviews are added, so remove the partial support rather than
completing it.
Per report of Jeevan Chalke, but with a different fix
Tom Lane [Sat, 20 Apr 2013 20:59:21 +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.
Peter Eisentraut [Sat, 20 Apr 2013 15:04:41 +0000 (11:04 -0400)]
Clean up references to SQL92
In most cases, these were just references to the SQL standard in
general. In a few cases, a contrast was made between SQL92 and later
standards -- those have been kept unchanged.
Tom Lane [Fri, 19 Apr 2013 20:14:56 +0000 (16:14 -0400)]
Improve error message when an FDW doesn't support WHERE CURRENT OF.
If an FDW fails to take special measures with a CurrentOfExpr, we will
end up trying to execute it as an ordinary qual, which was being treated
as a purely internal failure condition. Provide a more user-oriented
error message for such cases.
Remove some unused and seldom used fields from RelationAmInfo.
This saves some memory from each index relcache entry. At least on a 64-bit
machine, it saves just enough to shrink a typical relcache entry's memory
usage from 2k to 1k. That's nice if you have a lot of backends and a lot of
indexes.
Tom Lane [Mon, 15 Apr 2013 16:49:29 +0000 (12:49 -0400)]
Improve GiST index search performance for trigram regex queries.
The initial coding just descended the index if any of the target trigrams
were possibly present at the next level down. But actually we can apply
trigramsMatchGraph() so as to take advantage of AND requirements when there
are some. The input data might contain false positive matches, but that
can only result in a false positive result, not false negative, so it's
safe to do it this way.
Peter Eisentraut [Sun, 14 Apr 2013 03:42:42 +0000 (23:42 -0400)]
pg_ctl: Add idempotent option
This changes the behavior of the start and stop actions to exit
successfully if the server was already started or stopped.
This changes the default behavior of the start action: Before, if the
server was already running, it would print a message and succeed. Now,
that situation will result in an error. When running in idempotent
mode, no message is printed and pg_ctl exits successfully.
It was considered to just make the idempotent behavior the default and
only option, but pg_upgrade needs the old behavior.
Peter Eisentraut [Sat, 13 Apr 2013 02:45:51 +0000 (22:45 -0400)]
Fix sporadic rebuilds for .pc files
The build of .pc (pkg-config) files depends on all makefiles in use, and
in dependency tracking mode, the previous coding ended up including
/dev/null as a makefile. Apparently, on some platforms the modification
time of /dev/null changes sporadically, and so the .pc files would end
up being rebuilt every so often. Fix that by changing the makefile code
to do without using /dev/null.
Tom Lane [Fri, 12 Apr 2013 23:25:20 +0000 (19:25 -0400)]
Clean up the mess around EXPLAIN and materialized views.
Revert the matview-related changes in explain.c's API, as per recent
complaint from Robert Haas. The reason for these appears to have been
principally some ill-considered choices around having intorel_startup do
what ought to be parse-time checking, plus a poor arrangement for passing
it the view parsetree it needs to store into pg_rewrite when creating a
materialized view. Do the latter by having parse analysis stick a copy
into the IntoClause, instead of doing it at runtime. (On the whole,
I seriously question the choice to represent CREATE MATERIALIZED VIEW as a
variant of SELECT INTO/CREATE TABLE AS, because that means injecting even
more complexity into what was already a horrid legacy kluge. However,
I didn't go so far as to rethink that choice ... yet.)
I also moved several error checks into matview parse analysis, and
made the check for external Params in a matview more accurate.
In passing, clean things up a bit more around interpretOidsOption(),
and fix things so that we can use that to force no-oids for views,
sequences, etc, thereby eliminating the need to cons up "oids = false"
options when creating them.
catversion bump due to change in IntoClause. (I wonder though if we
really need readfuncs/outfuncs support for IntoClause anymore.)
Latch activity was not being detected by non-database-connected workers; the
SIGUSR1 signal handler which is normally in charge of that was set to SIG_IGN.
Create a simple handler to call latch_sigusr1_handler instead.
Add a SignalUnconnectedWorkers() call so that non-database-connected background
workers are also notified when postmaster is SIGHUPped. Previously, only
database-connected workers were.
Kevin Grittner [Tue, 9 Apr 2013 18:02:49 +0000 (13:02 -0500)]
Create a distinction between a populated matview and a scannable one.
The intent was that being populated would, long term, be just one
of the conditions which could affect whether a matview was
scannable; being populated should be necessary but not always
sufficient to scan the relation. Since only CREATE and REFRESH
currently determine the scannability, names and comments
accidentally conflated these concepts, leading to confusion.
Also add missing locking for the SQL function which allows a
test for scannability, and fix a modularity violatiion.
Per complaints from Tom Lane, although its not clear that these
will satisfy his concerns. Hopefully this will at least better
frame the discussion.
Robert Haas [Tue, 9 Apr 2013 14:13:38 +0000 (10:13 -0400)]
Adjust ExplainOneQuery_hook_type to take a DestReceiver argument.
The materialized views patch adjusted ExplainOneQuery to take an
additional DestReceiver argument, but failed to add a matching
argument to the definition of ExplainOneQuery_hook. This is a
problem for users of the hook that want to call ExplainOnePlan.
Fix by adding the missing argument.
Tom Lane [Tue, 9 Apr 2013 05:05:55 +0000 (01:05 -0400)]
Support indexing of regular-expression searches in contrib/pg_trgm.
This works by extracting trigrams from the given regular expression,
in generally the same spirit as the previously-existing support for
LIKE searches, though of course the details are far more complicated.
Currently, only GIN indexes are supported. We might be able to make
it work with GiST indexes later.
The implementation includes adding API functions to backend/regex/
to provide a view of the search NFA created from a regular expression.
These functions are meant to be generic enough to be supportable in
a standalone version of the regex library, should that ever happen.
Alexander Korotkov, reviewed by Heikki Linnakangas and Tom Lane
Simon Riggs [Mon, 8 Apr 2013 08:11:49 +0000 (09:11 +0100)]
Skip extraneous locking in XLogCheckBuffer().
Heikki reported comment was wrong, so fixed
code to match the comment: we only need to
take additional locking precautions when we
have a shared lock on the buffer.
Simon Riggs [Mon, 8 Apr 2013 07:52:39 +0000 (08:52 +0100)]
Avoid tricky race condition recording XLOG_HINT
We copy the buffer before inserting an XLOG_HINT to avoid WAL CRC errors
caused by concurrent hint writes to buffer while share locked. To make this work
we refactor RestoreBackupBlock() to allow an XLOG_HINT to avoid the normal
path for backup blocks, which assumes the underlying buffer is exclusive locked.
Resulting code completely changes layout of XLOG_HINT WAL records, but
this isn't even beta code, so this is a low impact change.
In passing, avoid taking WALInsertLock for full page writes on checksummed
hints, remove related cruft from XLogInsert() and improve xlog_desc record for
XLOG_HINT.
Andres Freund
Bug report by Fujii Masao, testing by Jeff Janes and Jaime Casanova,
review by Jeff Davis and Simon Riggs. Applied with changes from review
and some comment editing.
Simon Riggs [Sun, 7 Apr 2013 21:16:51 +0000 (22:16 +0100)]
Fix checksums for CLUSTER, VACUUM FULL etc.
In CLUSTER, VACUUM FULL and ALTER TABLE SET TABLESPACE
I erroneously set checksum before log_newpage, which
sets the LSN and invalidates the checksum. So set
checksum immediately *after* log_newpage.
Bug report Fujii Masao, Fix and patch by Jeff Davis
Tom Lane [Sun, 7 Apr 2013 18:45:33 +0000 (14:45 -0400)]
Get rid of USE_WIDE_UPPER_LOWER dependency in trigram construction.
contrib/pg_trgm's make_trigrams() was coded to ignore multibyte character
boundaries and just make trigrams from bytes if USE_WIDE_UPPER_LOWER wasn't
defined. This is a bit odd, since there's no obvious reason why trigram
compaction rules should depend on the presence of towlower() and friends.
What's more, there was an Assert() that would fail if that code path was
fed any multibyte characters.
We need to do something about this since the pending regex-indexing patch
has an assumption that you get just one "trgm" from any three characters.
The best solution seems to be to remove the USE_WIDE_UPPER_LOWER
dependency, which shouldn't really have been there in the first place.
The second loop in make_trigrams() is now just a fast path and not a
potentially incompatible algorithm.
If there is anybody still using Postgres on machines without wcstombs() or
towlower(), and they have non-ASCII data indexed by pg_trgm, they'll need
to REINDEX those indexes after pg_upgrade to 9.3, else searches may fail
incorrectly. It seems likely that there are no such installations, though.
In passing, rename cnt_trigram to compact_trigram, which seems to better
describe its functionality, and improve make_trigrams' test for whether it
has to use the slow path or not (per a suggestion from Alexander Korotkov).
Tom Lane [Fri, 5 Apr 2013 00:29:46 +0000 (20:29 -0400)]
Fix line count in slashUsage().
Counting newlines shows that quite a few recent patches have neglected
to update the output-lines count given to PageOutput(). Fortunately
it's not terribly critical that this be exact, since we long since
exceeded the height of most people's terminal windows. Still, maybe
we ought to think of a way to not have to maintain this manually anymore.