aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Translation updatesPeter Eisentraut2012-09-06
|
* Fix WAL file replacement during cascading replication on Windows.Heikki Linnakangas2012-09-05
| | | | | | | | | | | When the startup process restores a WAL file from the archive, it deletes any old file with the same name and renames the new file in its place. On Windows, however, when a file is deleted, it still lingers as long as a process holds a file handle open on it. With cascading replication, a walsender process can hold the old file open, so the rename() in the startup process would fail. To fix that, rename the old file to a temporary name, to make the original file name available for reuse, before deleting the old file.
* Fix inappropriate error messages for Hot Standby misconfiguration errors.Tom Lane2012-09-05
| | | | | | | | Give the correct name of the GUC parameter being complained of. Also, emit a more suitable SQLSTATE (INVALID_PARAMETER_VALUE, not the default INTERNAL_ERROR). Gurjeet Singh, errcode adjustment by me
* Fix compiler warnings about unused variables, caused by my previous commit.Heikki Linnakangas2012-09-04
| | | | Reported by Peter Eisentraut.
* Fix bugs in cascading replication with recovery_target_timeline='latest'Heikki Linnakangas2012-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The cascading replication code assumed that the current RecoveryTargetTLI never changes, but that's not true with recovery_target_timeline='latest'. The obvious upshot of that is that RecoveryTargetTLI in shared memory needs to be protected by a lock. A less obvious consequence is that when a cascading standby is connected, and the standby switches to a new target timeline after scanning the archive, it will continue to stream WAL to the cascading standby, but from a wrong file, ie. the file of the previous timeline. For example, if the standby is currently streaming from the middle of file 000000010000000000000005, and the timeline changes, the standby will continue to stream from that file. However, the WAL on the new timeline is in file 000000020000000000000005, so the standby sends garbage from 000000010000000000000005 to the cascading standby, instead of the correct WAL from file 000000020000000000000005. This also fixes a related bug where a partial WAL segment is restored from the archive and streamed to a cascading standby. The code assumed that when a WAL segment is copied from the archive, it can immediately be fully streamed to a cascading standby. However, if the segment is only partially filled, ie. has the right size, but only N first bytes contain valid WAL, that's not safe. That can happen if a partial WAL segment is manually copied to the archive, or if a partial WAL segment is archived because a server is started up on a new timeline within that segment. The cascading standby will get confused if the WAL it received is not valid, and will get stuck until it's restarted. This patch fixes that problem by not allowing WAL restored from the archive to be streamed to a cascading standby until it's been replayed, and thus validated.
* Fix serializable mode with index-only scans.Kevin Grittner2012-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | Serializable Snapshot Isolation used for serializable transactions depends on acquiring SIRead locks on all heap relation tuples which are used to generate the query result, so that a later delete or update of any of the tuples can flag a read-write conflict between transactions. This is normally handled in heapam.c, with tuple level locking. Since an index-only scan avoids heap access in many cases, building the result from the index tuple, the necessary predicate locks were not being acquired for all tuples in an index-only scan. To prevent problems with tuple IDs which are vacuumed and re-used while the transaction still matters, the xmin of the tuple is part of the tag for the tuple lock. Since xmin is not available to the index-only scan for result rows generated from the index tuples, it is not possible to acquire a tuple-level predicate lock in such cases, in spite of having the tid. If we went to the heap to get the xmin value, it would no longer be an index-only scan. Rather than prohibit index-only scans under serializable transaction isolation, we acquire an SIRead lock on the page containing the tuple, when it was not necessary to visit the heap for other reasons. Backpatch to 9.2. Kevin Grittner and Tom Lane
* Make configure probe for mbstowcs_l as well as wcstombs_l.Tom Lane2012-08-31
| | | | | | | | | | | We previously supposed that any given platform would supply both or neither of these functions, so that one configure test would be sufficient. It now appears that at least on AIX this is not the case ... which is likely an AIX bug, but nonetheless we need to cope with it. So use separate tests. Per bug #6758; thanks to Andrew Hastie for doing the followup testing needed to confirm what was happening. Backpatch to 9.1, where we began using these functions.
* Back-patch recent fixes for gistchoose and gistRelocateBuildBuffersOnSplit.Tom Lane2012-08-30
| | | | | | | | | | | | | | | | This back-ports commits c8ba697a4bdb934f0c51424c654e8db6133ea255 and e5db11c5582b469c04a11f217a0f32c827da5dd7, which fix one definite and one speculative bug in gistchoose, and make the code a lot more intelligible as well. In 9.2 only, this also affects the largely-copied-and-pasted logic in gistRelocateBuildBuffersOnSplit. The impact of the bugs was that the functions might make poor decisions as to which index tree branch to push a new entry down into, resulting in GiST index bloat and poor performance. The fixes rectify these decisions for future insertions, but a REINDEX would be needed to clean up any existing index bloat. Alexander Korotkov, Robert Haas, Tom Lane
* Add missing period to detail message.Robert Haas2012-08-30
| | | | Per note from Peter Eisentraut.
* Fix DROP INDEX CONCURRENTLY IF EXISTS.Tom Lane2012-08-27
| | | | | | | | This threw ERROR, not the expected NOTICE, if the index didn't exist. The bug was actually visible in not-as-expected regression test output, so somebody wasn't paying too close attention in commit 8cb53654dbdb4c386369eb988062d0bbb6de725e. Per report from Brendan Byrd.
* Fix issues with checks for unsupported transaction states in Hot Standby.Tom Lane2012-08-24
| | | | | | | | | | | | | | | | | | | | | | | | | The GUC check hooks for transaction_read_only and transaction_isolation tried to check RecoveryInProgress(), so as to disallow setting read/write mode or serializable isolation level (respectively) in hot standby sessions. However, GUC check hooks can be called in many situations where we're not connected to shared memory at all, resulting in a crash in RecoveryInProgress(). Among other cases, this results in EXEC_BACKEND builds crashing during child process start if default_transaction_isolation is serializable, as reported by Heikki Linnakangas. Protect those calls by silently allowing any setting when not inside a transaction; which is okay anyway since these GUCs are always reset at start of transaction. Also, add a check to GetSerializableTransactionSnapshot() to complain if we are in hot standby. We need that check despite the one in check_XactIsoLevel() because default_transaction_isolation could be serializable. We don't want to complain any sooner than this in such cases, since that would prevent running transactions at all in such a state; but a transaction can be run, if SET TRANSACTION ISOLATION is done before setting a snapshot. Per report some months ago from Robert Haas. Back-patch to 9.1, since these problems were introduced by the SSI patch. Kevin Grittner and Tom Lane, with ideas from Heikki Linnakangas
* Translation updatesPeter Eisentraut2012-08-23
|
* Fix cascading privilege revoke to notice when privileges are still held.Tom Lane2012-08-23
| | | | | | | | | | | | If we revoke a grant option from some role X, but X still holds the option via another grant, we should not recursively revoke the privilege from role(s) Y that X had granted it to. This was supposedly fixed as one aspect of commit 4b2dafcc0b1a579ef5daaa2728223006d1ff98e9, but I must not have tested it, because in fact that code never worked: it forgot to shift the grant-option bits back over when masking the bits being revoked. Per bug #6728 from Daniel German. Back-patch to all active branches, since this has been wrong since 8.0.
* Check LIBXML_VERSION instead of testing in configure script.Tom Lane2012-08-17
| | | | | | | | | | We had put a test for libxml2's xmlStructuredErrorContext variable in configure, but of course that doesn't work on Windows builds. The next best alternative seems to be to test the LIBXML_VERSION symbol provided by xmlversion.h. Per report from Talha Bin Rizwan, though this fixes it in a different way than his proposed patch.
* Allow create_index_paths() to consider multiple join bitmapscan paths.Tom Lane2012-08-16
| | | | | | | | | | | | | In the initial cut at the "parameterized paths" feature, I'd simplified create_index_paths() to the point where it would only generate a single parameterized bitmap path per relation. Experimentation with an example supplied by Josh Berkus convinces me that that's not good enough: we really need to consider a bitmap path for each possible outer relation. Otherwise we have regressions relative to pre-9.2 versions, in which the planner picks a plain indexscan where it should have used a bitmap scan in queries involving three or more tables. Indeed, after fixing this, several queries in the regression tests show improved plans as a result of using bitmap not plain indexscans.
* Fix GiST buffering build bug, which caused "failed to re-find parent" errors.Heikki Linnakangas2012-08-16
| | | | | | | | | | | | | | | We use a hash table to track the parents of inner pages, but when inserting to a leaf page, the caller of gistbufferinginserttuples() must pass a correct block number of the leaf's parent page. Before gistProcessItup() descends to a child page, it checks if the downlink needs to be adjusted to accommodate the new tuple, and updates the downlink if necessary. However, updating the downlink might require splitting the page, which might move the downlink to a page to the right. gistProcessItup() doesn't realize that, so when it descends to the leaf page, it might pass an out-of-date parent block number as a result. Fix that by returning the block a tuple was inserted to from gistbufferinginserttuples(). This fixes the bug reported by Zdeněk Jílovec.
* Fix rescan logic in nodeCtescan.Tom Lane2012-08-15
| | | | | | | | | | | | | | | | | | | | | | The previous coding essentially assumed that nodes would be rescanned in the same order they were initialized in; or at least that the "leader" of a group of CTEscans would be rescanned before any others were required to execute. Unfortunately, that isn't even a little bit true. It's possible to devise queries in which the leader isn't rescanned until other CTEscans on the same CTE have run to completion, or even in which the leader never gets a rescan call at all. The fix makes the leader specially responsible only for initial creation and final destruction of the tuplestore; rescan resets are now a symmetrically shared responsibility. This means that we might reset the tuplestore multiple times when restarting a plan subtree containing multiple CTEscans; but resetting an already-empty tuplestore is cheap enough that that doesn't seem like a problem. Per report from Adam Mackler; the new regression test cases are based on his example query. Back-patch to 8.4 where CTE scans were introduced.
* Disallow extensions from owning the schema they are assigned to.Tom Lane2012-08-15
| | | | | | | | | | | This situation creates a dependency loop that confuses pg_dump and probably other things. Moreover, since the mental model is that the extension "contains" schemas it owns, but "is contained in" its extschema (even though neither is strictly true), having both true at once is confusing for people too. So prevent the situation from being set up. Reported and patched by Thom Brown. Back-patch to 9.1 where extensions were added.
* Resurrect the "last ditch" code path in join_search_one_level().Tom Lane2012-08-15
| | | | | | | | | | | | | | | | | | | | This essentially reverts commit e54b10a62db2991235fe800c629baef4531a6d67, in which I'd decided that the "last ditch" join logic was useless. The folly of that is now exposed by a report from Pavel Stehule: although the function should always find at least one join in a self-contained join problem, it can still fail to do so in a sub-problem created by artificial from_collapse_limit or join_collapse_limit constraints. Adjust the comments to describe this, and simplify the code a bit to match the new coding of the earlier loop in the function. I'm not terribly happy about this: I still subscribe to the opinion stated in the previous commit message that the "last ditch" code can obscure logic bugs elsewhere. But the alternative seems to be to complicate the earlier tests for does-this-relation-have-a-join-clause to the point where they can tell whether the join clauses link outside the current join sub-problem. And that looks messy, slow, and possibly a source of bugs in itself. In any case, now is not the time to be inserting experimental code into 9.2, so let's just go back to the time-tested solution.
* Prevent access to external files/URLs via XML entity references.Tom Lane2012-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | xml_parse() would attempt to fetch external files or URLs as needed to resolve DTD and entity references in an XML value, thus allowing unprivileged database users to attempt to fetch data with the privileges of the database server. While the external data wouldn't get returned directly to the user, portions of it could be exposed in error messages if the data didn't parse as valid XML; and in any case the mere ability to check existence of a file might be useful to an attacker. The ideal solution to this would still allow fetching of references that are listed in the host system's XML catalogs, so that documents can be validated according to installed DTDs. However, doing that with the available libxml2 APIs appears complex and error-prone, so we're not going to risk it in a security patch that necessarily hasn't gotten wide review. So this patch merely shuts off all access, causing any external fetch to silently expand to an empty string. A future patch may improve this. In HEAD and 9.2, also suppress warnings about undefined entities, which would otherwise occur as a result of not loading referenced DTDs. Previous branches don't show such warnings anyway, due to different error handling arrangements. Credit to Noah Misch for first reporting the problem, and for much work towards a solution, though this simplistic approach was not his preference. Also thanks to Daniel Veillard for consultation. Security: CVE-2012-3489
* Translation updatesPeter Eisentraut2012-08-14
|
* Fix dependencies generated during ALTER TABLE ADD CONSTRAINT USING INDEX.Tom Lane2012-08-11
| | | | | | | | | | | | | | | | | | This command generated new pg_depend entries linking the index to the constraint and the constraint to the table, which match the entries made when a unique or primary key constraint is built de novo. However, it did not bother to get rid of the entries linking the index directly to the table. We had considered the issue when the ADD CONSTRAINT USING INDEX patch was written, and concluded that we didn't need to get rid of the extra entries. But this is wrong: ALTER COLUMN TYPE wasn't expecting such redundant dependencies to exist, as reported by Hubert Depesz Lubaczewski. On reflection it seems rather likely to break other things as well, since there are many bits of code that crawl pg_depend for one purpose or another, and most of them are pretty naive about what relationships they're expecting to find. Fortunately it's not that hard to get rid of the extra dependency entries, so let's do that. Back-patch to 9.1, where ALTER TABLE ADD CONSTRAINT USING INDEX was added.
* Fix upper limit of superuser_reserved_connections, add limit for wal_sendersMagnus Hagander2012-08-10
| | | | | | | | Should be limited to the maximum number of connections excluding autovacuum workers, not including. Add similar check for max_wal_senders, which should never be higher than max_connections.
* Turn off WalSender keepalives by default, users can enable if desiredSimon Riggs2012-08-09
|
* Ensure all replication message info is available and correct via WalRcvSimon Riggs2012-08-09
|
* Force archive_status of .done for xlogs created by dearchival/replication.Simon Riggs2012-08-08
| | | | | | | This prevents spurious attempts to archive xlog files after promotion of standby, a bug introduced by cascading replication patch in 9.2. Fujii Masao, simplified and extended to cover streaming by Simon Riggs
* Fix typo in commentAlvaro Herrera2012-08-08
|
* Fix minor bug in XLogFileRead() that accidentally worked.Simon Riggs2012-08-08
| | | | | | | | | Cascading replication copied the incoming file into pg_xlog but didn't set path correctly, so the first attempt to open file failed causing it to loop around and look for file in pg_xlog. So the earlier coding worked, but accidentally rather than by design. Spotted by Fujii Masao, fix by Fujii Masao and Simon Riggs
* Fix TwoPhaseGetDummyBackendId().Tom Lane2012-08-08
| | | | | | | | | | | | | This was broken in commit ed0b409d22346b1b027a4c2099ca66984d94b6dd, which revised the GlobalTransactionData struct to not include the associated PGPROC as its first member, but overlooked one place where a cast was used in reliance on that equivalence. The most effective way of fixing this seems to be to create a new function that looks up the GlobalTransactionData struct given the XID, and make both TwoPhaseGetDummyBackendId and TwoPhaseGetDummyProc rely on that. Per report from Robert Ross.
* Fix redundant wordingAlvaro Herrera2012-08-07
|
* fsync backup_label after pg_start_backup()Simon Riggs2012-08-07
| | | | Dave Kerr
* Make strings identicalAlvaro Herrera2012-08-06
|
* Fix bugs with parsing signed hh:mm and hh:mm:ss fields in interval input.Tom Lane2012-08-03
| | | | | | | | | | | | | | | | | | | | | | | DecodeInterval() failed to honor the "range" parameter (the special SQL syntax for indicating which fields appear in the literal string) if the time was signed. This seems inappropriate, so make it work like the not-signed case. The inconsistency was introduced in my commit f867339c0148381eb1d01f93ab5c79f9d10211de, which as noted in its log message was only really focused on making SQL-compliant literals work per spec. Including a sign here is not per spec, but if we're going to allow it then it's reasonable to expect it to work like the not-signed case. Also, remove bogus setting of tmask, which caused subsequent processing to think that what had been given was a timezone and not an hh:mm(:ss) field, thus confusing checks for redundant fields. This seems to be an aboriginal mistake in Lockhart's commit 2cf1642461536d0d8f3a1cf124ead0eac04eb760. Add regression test cases to illustrate the changed behaviors. Back-patch as far as 8.4, where support for spec-compliant interval literals was added. Range problem reported and diagnosed by Amit Kapila, tmask problem by me.
* Improve underdocumented btree_xlog_delete_get_latestRemovedXid() code.Tom Lane2012-08-03
| | | | | | | | | | | | | | | | | As noted by Noah Misch, btree_xlog_delete_get_latestRemovedXid is critically dependent on the assumption that it's examining a consistent state of the database. This was undocumented though, so the seemingly-unrelated check for no active HS sessions might be thought to be merely an optional optimization. Improve comments, and add an explicit check of reachedConsistency just to be sure. This function returns InvalidTransactionId (thereby killing all HS transactions) in several cases that are not nearly unlikely enough for my taste. This commit doesn't attempt to fix those deficiencies, just document them. Back-patch to 9.2, not from any real functional need but just to keep the branches more closely synced to simplify possible future back-patching.
* In SPGiST replay, do conflict resolution before modifying the page.Tom Lane2012-08-03
| | | | | | | | | In yesterday's commit 962e0cc71e839c58fb9125fa85511b8bbb8bdbee, I added the ResolveRecoveryConflictWithSnapshot call in the wrong place. I correctly put it before spgRedoVacuumRedirect itself would modify the index page --- but not before RestoreBkpBlocks, so replay of a record with a full-page image would modify the page before kicking off any conflicting HS transactions. Oops.
* Translation updatesPeter Eisentraut2012-08-02
|
* Fix race conditions associated with SPGiST redirection tuples.Tom Lane2012-08-02
| | | | | | | | | | | | | | | The correct test for whether a redirection tuple is removable is whether tuple's xid < RecentGlobalXmin, not OldestXmin; the previous coding failed to protect index searches being done in concurrent transactions that have no XID. This mirrors the recent fix in btree's page recycling logic made in commit d3abbbebe52eb1e59e621c880ad57df9d40d13f2. Also, WAL-log the newest XID of any removed redirection tuple on an index page, and apply ResolveRecoveryConflictWithSnapshot during InHotStandby WAL replay. This protects against concurrent Hot Standby transactions possibly needing to see the redirection tuple(s). Per my query of 2012-03-12 and subsequent discussion.
* Fix WITH attached to a nested set operation (UNION/INTERSECT/EXCEPT).Tom Lane2012-07-31
| | | | | | | | | | | | | Parse analysis neglected to cover the case of a WITH clause attached to an intermediate-level set operation; it only handled WITH at the top level or WITH attached to a leaf-level SELECT. Per report from Adam Mackler. In HEAD, I rearranged the order of SelectStmt's fields to put withClause with the other fields that can appear on non-leaf SelectStmts. In back branches, leave it alone to avoid a possible ABI break for third-party code. Back-patch to 8.4 where WITH support was added.
* Fix syslogger so that log_truncate_on_rotation works in the first rotation.Tom Lane2012-07-31
| | | | | | | | | | | | | | | In the original coding of the log rotation stuff, we did not bother to make the truncation logic work for the very first rotation after postmaster start (or after a syslogger crash and restart). It just always appended in that case. It did not seem terribly important at the time, but we've recently had two separate complaints from people who expected it to work unsurprisingly. (Both users tend to restart the postmaster about as often as a log rotation is configured to happen, which is maybe not typical use, but still...) Since the initial log file is opened in the postmaster, fixing this requires passing down some more state to the syslogger child process. It's always been like this, so back-patch to all supported branches.
* Only allow autovacuum to be auto-canceled by a directly blocked process.Tom Lane2012-07-26
| | | | | | | | | | | | | | | | | | | In the original coding of the autovacuum cancel feature, commit acac68b2bcae818bc8803b8cb8cbb17eee8d5e2b, an autovacuum process was considered a target for cancellation if it was found to hard-block any process examined in the deadlock search. This patch tightens the test so that the autovacuum must directly hard-block the current process. This should make the behavior more predictable in general, and in particular it ensures that an autovacuum will not be canceled with less than deadlock_timeout grace period. In the old coding, it was possible for an autovacuum to be canceled almost instantly, given unfortunate timing of two or more other processes' lock attempts. This also justifies the logging methodology in the recent commit d7318d43d891bd63e82dcfc27948113ed7b1db80; without this restriction, that patch isn't providing enough information to see the connection of the canceling process to the autovacuum. Like that one, patch all the way back.
* Log a better message when canceling autovacuum.Robert Haas2012-07-26
| | | | | | | | | | The old message was at DEBUG2, so typically it didn't show up in the log at all. As a result, in most cases where autovacuum was canceled, the only information that was logged was the table being vacuumed, with no indication as to what problem caused the cancel. Crank up the level to LOG and add some more details to assist with debugging. Back-patch all the way, per discussion on pgsql-hackers.
* Fix longstanding crash-safety bug with newly-created-or-reset sequences.Tom Lane2012-07-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a crash occurred immediately after the first nextval() call for a serial column, WAL replay would restore the sequence to a state in which it appeared that no nextval() had been done, thus allowing the first sequence value to be returned again by the next nextval() call; as reported in bug #6748 from Xiangming Mei. More generally, the problem would occur if an ALTER SEQUENCE was executed on a freshly created or reset sequence. (The manifestation with serial columns was introduced in 8.2 when we added an ALTER SEQUENCE OWNED BY step to serial column creation.) The cause is that sequence creation attempted to save one WAL entry by writing out a WAL record that made it appear that the first nextval() had already happened (viz, with is_called = true), while marking the sequence's in-database state with log_cnt = 1 to show that the first nextval() need not emit a WAL record. However, ALTER SEQUENCE would emit a new WAL entry reflecting the actual in-database state (with is_called = false). Then, nextval would allocate the first sequence value and set is_called = true, but it would trust the log_cnt value and not emit any WAL record. A crash at this point would thus restore the sequence to its post-ALTER state, causing the next nextval() call to return the first sequence value again. To fix, get rid of the idea of logging an is_called status different from reality. This means that the first nextval-driven WAL record will happen at the first nextval call not the second, but the marginal cost of that is pretty negligible. In addition, make sure that ALTER SEQUENCE resets log_cnt to zero in any case where it touches sequence parameters that affect future nextval results. This will result in some user-visible changes in the contents of a sequence's log_cnt column, as reflected in the patch's regression test changes; but no application should be depending on that anyway, since it was already true that log_cnt changes rather unpredictably depending on checkpoint timing. In addition, make some basically-cosmetic improvements to get rid of sequence.c's undesirable intimacy with page layout details. It was always really trying to WAL-log the contents of the sequence tuple, so we should have it do that directly using a HeapTuple's t_data and t_len, rather than backing into it with some magic assumptions about where the tuple would be on the sequence's page. Back-patch to all supported branches.
* Change syntax of new CHECK NO INHERIT constraintsAlvaro Herrera2012-07-24
| | | | | | | | | | The initially implemented syntax, "CHECK NO INHERIT (expr)" was not deemed very good, so switch to "CHECK (expr) NO INHERIT" instead. This way it looks similar to SQL-standards compliant constraint attribute. Backport to 9.2 where the new syntax and feature was introduced. Per discussion.
* Account for SRFs in targetlists in planner rowcount estimates.Tom Lane2012-07-21
| | | | | | | | | | | | | We made use of the ROWS estimate for set-returning functions used in FROM, but not for those used in SELECT targetlists; which is a bit of an oversight considering there are common usages that require the latter approach. Improve that. (I had initially thought it might be worth folding this into cost_qual_eval, but after investigation concluded that that wouldn't be very helpful, so just do it separately.) Per complaint from David Johnston. Back-patch to 9.2, but not further, for fear of destabilizing plan choices in existing releases.
* connoinherit may be true only for CHECK constraintsAlvaro Herrera2012-07-20
| | | | | | | | | | | | | | | | | The code was setting it true for other constraints, which is bogus. Doing so caused bogus catalog entries for such constraints, and in particular caused an error to be raised when trying to drop a constraint of types other than CHECK from a table that has children, such as reported in bug #6712. In 9.2, additionally ignore connoinherit=true for other constraint types, to avoid having to force initdb; existing databases might already contain bogus catalog entries. Includes a catversion bump (in HEAD only). Bug report from Miroslav Šulc Analysis from Amit Kapila and Noah Misch; Amit also contributed the patch.
* Fix whole-row Var evaluation to cope with resjunk columns (again).Tom Lane2012-07-20
| | | | | | | | | | | | | | | | | | When a whole-row Var is reading the result of a subquery, we need it to ignore any "resjunk" columns that the subquery might have evaluated for GROUP BY or ORDER BY purposes. We've hacked this area before, in commit 68e40998d058c1f6662800a648ff1e1ce5d99cba, but that fix only covered whole-row Vars of named composite types, not those of RECORD type; and it was mighty klugy anyway, since it just assumed without checking that any extra columns in the result must be resjunk. A proper fix requires getting hold of the subquery's targetlist so we can actually see which columns are resjunk (whereupon we can use a JunkFilter to get rid of them). So bite the bullet and add some infrastructure to make that possible. Per report from Andrew Dunstan and additional testing by Merlin Moncure. Back-patch to all supported branches. In 8.3, also back-patch commit 292176a118da6979e5d368a4baf27f26896c99a5, which for some reason I had not done at the time, but it's a prerequisite for this change.
* Rethink checkpointer's fsync-request table representation.Tom Lane2012-07-19
| | | | | | | | | | | | | | | | | | | Instead of having one hash table entry per relation/fork/segment, just have one per relation, and use bitmapsets to represent which specific segments need to be fsync'd. This eliminates the need to scan the whole hash table to implement FORGET_RELATION_FSYNC, which fixes the O(N^2) behavior recently demonstrated by Jeff Janes for cases involving lots of TRUNCATE or DROP TABLE operations during a single checkpoint cycle. Per an idea from Robert Haas. (FORGET_DATABASE_FSYNC still sucks, but since dropping a database is a pretty expensive operation anyway, we'll live with that.) In passing, improve the delayed-unlink code: remove the pass over the list in mdpreckpt, since it wasn't doing anything for us except supporting a useless Assert in mdpostckpt, and fix mdpostckpt so that it will absorb fsync requests every so often when clearing a large backlog of deletion requests.
* Send only one FORGET_RELATION_FSYNC request when dropping a relation.Tom Lane2012-07-19
| | | | | | | We were sending one per fork, but a little bit of refactoring allows us to send just one request with forknum == InvalidForkNumber. This not only reduces pressure on the shared-memory request queue, but saves repeated traversals of the checkpointer's hash table.
* Refactor the way code is shared between some range type functions.Heikki Linnakangas2012-07-18
| | | | | | | | | | | | | | | | | Functions like range_eq, range_before etc. are exposed at the SQL-level, but they're also used internally by the GiST consistent support function. The code sharing was done by a hack, TrickFunctionCall2, which relied on the knowledge that all the functions used fn_extra the same way. This commit splits the functions into internal versions that take a TypeCacheEntry as argument, and thin wrappers to expose the functions at the SQL-level. The internal versions can then be called directly and in a less hacky way from the GiST consistent function. This is just cosmetic, but backpatch to 9.2 anyway, to avoid having a different version of this code in the 9.2 branch. That would make backpatching fixes in this area more difficult. Alexander Korotkov
* Fix statistics breakage from bgwriter/checkpointer process split.Tom Lane2012-07-18
| | | | | | | | | | | | ForwardFsyncRequest() supposed that it could only be called in regular backends, which used to be true; but since the splitup of bgwriter and checkpointer, it is also called in the bgwriter. We do not want to count such calls in pg_stat_bgwriter.buffers_backend statistics, so fix things so that they aren't. (It's worth noting here that this implies an alarmingly large increase in the expected amount of cross-process fsync request traffic, which may well mean that the process splitup was not such a hot idea.)