summaryrefslogtreecommitdiff
path: root/src/backend/commands
AgeCommit message (Collapse)Author
2012-02-23Require execute permission on the trigger function for CREATE TRIGGER.Tom Lane
This check was overlooked when we added function execute permissions to the system years ago. For an ordinary trigger function it's not a big deal, since trigger functions execute with the permissions of the table owner, so they couldn't do anything the user issuing the CREATE TRIGGER couldn't have done anyway. However, if a trigger function is SECURITY DEFINER, that is not the case. The lack of checking would allow another user to install it on his own table and then invoke it with, essentially, forged input data; which the trigger function is unlikely to realize, so it might do something undesirable, for instance insert false entries in an audit log table. Reported by Dinesh Kumar, patch by Robert Haas Security: CVE-2012-0866
2012-01-30Accept a non-existent value in "ALTER USER/DATABASE SET ..." command.Heikki Linnakangas
When default_text_search_config, default_tablespace, or temp_tablespaces setting is set per-user or per-database, with an "ALTER USER/DATABASE SET ..." statement, don't throw an error if the text search configuration or tablespace does not exist. In case of text search configuration, even if it doesn't exist in the current database, it might exist in another database, where the setting is intended to have its effect. This behavior is now the same as search_path's. Tablespaces are cluster-wide, so the same argument doesn't hold for tablespaces, but there's a problem with pg_dumpall: it dumps "ALTER USER SET ..." statements before the "CREATE TABLESPACE" statements. Arguably that's pg_dumpall's fault - it should dump the statements in such an order that the tablespace is created first and then the "ALTER USER SET default_tablespace ..." statements after that - but it seems better to be consistent with search_path and default_text_search_config anyway. Besides, you could still create a dump that throws an error, by creating the tablespace, running "ALTER USER SET default_tablespace", then dropping the tablespace and running pg_dumpall on that. Backpatch to all supported versions.
2011-10-26Change FK trigger creation order to better support self-referential FKs.Tom Lane
When a foreign-key constraint references another column of the same table, row updates will queue both the PK's ON UPDATE action and the FK's CHECK action in the same event. The ON UPDATE action must execute first, else the CHECK will check a non-final state of the row and possibly throw an inappropriate error, as seen in bug #6268 from Roman Lytovchenko. Now, the firing order of multiple triggers for the same event is determined by the sort order of their pg_trigger.tgnames, and the auto-generated names we use for FK triggers are "RI_ConstraintTrigger_NNNN" where NNNN is the trigger OID. So most of the time the firing order is the same as creation order, and so rearranging the creation order fixes it. This patch will fail to fix the problem if the OID counter wraps around or adds a decimal digit (eg, from 99999 to 100000) while we are creating the triggers for an FK constraint. Given the small odds of that, and the low usage of self-referential FKs, we'll live with that solution in the back branches. A better fix is to change the auto-generated names for FK triggers, but it seems unwise to do that in stable branches because there may be client code that depends on the naming convention. We'll fix it that way in HEAD in a separate patch. Back-patch to all supported branches, since this bug has existed for a long time.
2011-09-06Avoid possibly accessing off the end of memory in examine_attribute().Tom Lane
Since the last couple of columns of pg_type are often NULL, sizeof(FormData_pg_type) can be an overestimate of the actual size of the tuple data part. Therefore memcpy'ing that much out of the catalog cache, as analyze.c was doing, poses a small risk of copying past the end of memory and incurring SIGSEGV. No such crash has been identified in the field, but we've certainly seen the equivalent happen in other code paths, so patch this one all the way back. Per valgrind testing by Noah Misch, though this is not his proposed patch. I chose to use SearchSysCacheCopy1 rather than inventing special-purpose infrastructure for copying only the minimal part of a pg_type tuple.
2011-06-02Clean up after erroneous SELECT FOR UPDATE/SHARE on a sequence.Tom Lane
My previous commit disallowed this operation, but did nothing about cleaning up the damage if one had already been done. With the operation disallowed, it's okay to just forcibly clear xmax in a sequence's tuple, since any value seen there could not represent a live transaction's lock. So, any sequence-specific operation will repair the problem automatically, whether or not the user has already seen "could not access status of transaction" failures.
2011-05-01Make CLUSTER lock the old table's toast table before copying data.Tom Lane
We must lock out autovacuuming of the old toast table before computing the OldestXmin horizon we will use. Otherwise, autovacuum could start on the toast table later, compute a later OldestXmin horizon, and remove as DEAD toast tuples that we still need (because we think their parent tuples are only RECENTLY_DEAD). Per further thought about bug #5998.
2011-04-19Avoid changing an index's indcheckxmin horizon during REINDEX.Tom Lane
There can never be a need to push the indcheckxmin horizon forward, since any HOT chains that are actually broken with respect to the index must pre-date its original creation. So we can just avoid changing pg_index altogether during a REINDEX operation. This offers a cleaner solution than my previous patch for the problem found a few days ago that we mustn't try to update pg_index while we are reindexing it. System catalog indexes will always be created with indcheckxmin = false during initdb, and with this modified code we should never try to change their pg_index entries. This avoids special-casing system catalogs as the former patch did, and should provide a performance benefit for many cases where REINDEX formerly caused an index to be considered unusable for a short time. Back-patch to 8.3 to cover all versions containing HOT. Note that this patch changes the API for index_build(), but I believe it is unlikely that any add-on code is calling that directly.
2011-03-28Prevent a rowtype from being included in itself.Tom Lane
Eventually we might be able to allow that, but it's not clear how many places need to be fixed to prevent infinite recursion when there's a direct or indirect inclusion of a rowtype in itself. One such place is CheckAttributeType(), which will recurse to stack overflow in cases such as those exhibited in bug #5950 from Alex Perepelica. If we were sure it was the only such place, we could easily modify the code added by this patch to stop the recursion without a complaint ... but it probably isn't the only such place. Hence, throw error until such time as someone is excited enough about this type of usage to put work into making it safe. Back-patch as far as 8.3. 8.2 doesn't have the recursive call in CheckAttributeType in the first place, so I see no need to add code there in the absence of clear evidence of a problem elsewhere.
2011-02-21Fix dangling-pointer problem in before-row update trigger processing.Tom Lane
ExecUpdate checked for whether ExecBRUpdateTriggers had returned a new tuple value by seeing if the returned tuple was pointer-equal to the old one. But the "old one" was in estate->es_junkFilter's result slot, which would be scribbled on if we had done an EvalPlanQual update in response to a concurrent update of the target tuple; therefore we were comparing a dangling pointer to a live one. Given the right set of circumstances we could get a false match, resulting in not forcing the tuple to be stored in the slot we thought it was stored in. In the case reported by Maxim Boguk in bug #5798, this led to "cannot extract system attribute from virtual tuple" failures when trying to do "RETURNING ctid". I believe there is a very-low-probability chance of more serious errors, such as generating incorrect index entries based on the original rather than the trigger-modified version of the row. In HEAD, change all of ExecBRInsertTriggers, ExecIRInsertTriggers, ExecBRUpdateTriggers, and ExecIRUpdateTriggers so that they continue to have similar APIs. In the back branches I just changed ExecBRUpdateTriggers, since there is no bug in the ExecBRInsertTriggers case.
2010-11-09Repair memory leakage while ANALYZE-ing complex index expressions.Tom Lane
The general design of memory management in Postgres is that intermediate results computed by an expression are not freed until the end of the tuple cycle. For expression indexes, ANALYZE has to re-evaluate each expression for each of its sample rows, and it wasn't bothering to free intermediate results until the end of processing of that index. This could lead to very substantial leakage if the intermediate results were large, as in a recent example from Jakub Ouhrabka. Fix by doing ResetExprContext for each sample row. This necessitates adding a datumCopy step to ensure that the final expression value isn't recycled too. Some quick testing suggests that this change adds at worst about 10% to the time needed to analyze a table with an expression index; which is annoying, but seems a tolerable price to pay to avoid unexpected out-of-memory problems. Back-patch to all supported branches.
2010-09-23Prevent show_session_authorization from crashing when session_authorizationTom Lane
hasn't been set. The only known case where this can happen is when show_session_authorization is invoked in an autovacuum process, which is possible if an index function calls it, as for example in bug #5669 from Andrew Geery. We could perhaps try to return a sensible value, such as the name of the cluster-owning superuser; but that seems like much more trouble than the case is worth, and in any case it could create new possible failure modes. Simply returning an empty string seems like the most appropriate fix. Back-patch to all supported versions, even those before autovacuum, just in case there's another way to provoke this crash.
2010-07-29Fix another longstanding problem in copy_relation_data: it was blithelyTom Lane
assuming that a local char[] array would be aligned on at least a word boundary. There are architectures on which that is pretty much guaranteed to NOT be the case ... and those arches also don't like non-aligned memory accesses, meaning that log_newpage() would crash if it ever got invoked. Even on Intel-ish machines there's a potential for a large performance penalty from doing I/O to an inadequately aligned buffer. So palloc it instead. Backpatch to 8.0 --- 7.4 doesn't have this code.
2010-07-03Allow REASSIGNED OWNED to handle opclasses and opfamilies.Robert Haas
Backpatch to 8.3, which is as far back as we have opfamilies. The opclass portion could probably be backpatched to 8.2, when REASSIGN OWNED was added, but for now I have not done that. Asko Tiidumaa, with minor adjustments by me.
2010-07-01Allow ALTER TABLE .. SET TABLESPACE to be interrupted.Robert Haas
Backpatch to 8.0, where tablespaces were introduced. Guillaume Lelarge
2010-03-25Prevent ALTER USER f RESET ALL from removing the settings that were put thereAlvaro Herrera
by a superuser -- "ALTER USER f RESET setting" already disallows removing such a setting. Apply the same treatment to ALTER DATABASE d RESET ALL when run by a database owner that's not superuser.
2010-01-24Fix assorted core dumps and Assert failures that could occur duringTom Lane
AbortTransaction or AbortSubTransaction, when trying to clean up after an error that prevented (sub)transaction start from completing: * access to TopTransactionResourceOwner that might not exist * assert failure in AtEOXact_GUC, if AtStart_GUC not called yet * assert failure or core dump in AfterTriggerEndSubXact, if AfterTriggerBeginSubXact not called yet Per testing by injecting elog(ERROR) at successive steps in StartTransaction and StartSubTransaction. It's not clear whether all of these cases could really occur in the field, but at least one of them is easily exposed by simple stress testing, as per my accidental discovery yesterday.
2009-12-29Previous fix for temporary file management broke returning a set fromHeikki Linnakangas
PL/pgSQL function within an exception handler. Make sure we use the right resource owner when we create the tuplestore to hold returned tuples. Simplify tuplestore API so that the caller doesn't need to be in the right memory context when calling tuplestore_put* functions. tuplestore.c automatically switches to the memory context used when the tuplestore was created. Tuplesort was already modified like this earlier. This patch also removes the now useless MemoryContextSwitch calls from callers. Report by Aleksei on pgsql-bugs on Dec 22 2009. Backpatch to 8.1, like the previous patch that broke this.
2009-12-09Prevent indirect security attacks via changing session-local state withinTom Lane
an allegedly immutable index function. It was previously recognized that we had to prevent such a function from executing SET/RESET ROLE/SESSION AUTHORIZATION, or it could trivially obtain the privileges of the session user. However, since there is in general no privilege checking for changes of session-local state, it is also possible for such a function to change settings in a way that might subvert later operations in the same session. Examples include changing search_path to cause an unexpected function to be called, or replacing an existing prepared statement with another one that will execute a function of the attacker's choosing. The present patch secures VACUUM, ANALYZE, and CREATE INDEX/REINDEX against these threats, which are the same places previously deemed to need protection against the SET ROLE issue. GUC changes are still allowed, since there are many useful cases for that, but we prevent security problems by forcing a rollback of any GUC change after completing the operation. Other cases are handled by throwing an error if any change is attempted; these include temp table creation, closing a cursor, and creating or deleting a prepared statement. (In 7.4, the infrastructure to roll back GUC changes doesn't exist, so we settle for rejecting changes of "search_path" in these contexts.) Original report and patch by Gurjeet Singh, additional analysis by Tom Lane. Security: CVE-2009-4136
2009-11-10Fix longstanding problems in VACUUM caused by untimely interruptionsAlvaro Herrera
In VACUUM FULL, an interrupt after the initial transaction has been recorded as committed can cause postmaster to restart with the following error message: PANIC: cannot abort transaction NNNN, it was already committed This problem has been reported many times. In lazy VACUUM, an interrupt after the table has been truncated by lazy_truncate_heap causes other backends' relcache to still point to the removed pages; this can cause future INSERT and UPDATE queries to error out with the following error message: could not read block XX of relation 1663/NNN/MMMM: read only 0 of 8192 bytes The window to this race condition is extremely narrow, but it has been seen in the wild involving a cancelled autovacuum process. The solution for both problems is to inhibit interrupts in both operations until after the respective transactions have been committed. It's not a complete solution, because the transaction could theoretically be aborted by some other error, but at least fixes the most common causes of both problems.
2009-10-27Fix AfterTriggerSaveEvent to use a test and elog, not just Assert, to checkTom Lane
that it's called within an AfterTriggerBeginQuery/AfterTriggerEndQuery pair. The RI cascade triggers suppress that overhead on the assumption that they are always run non-deferred, so it's possible to violate the condition if someone mistakenly changes pg_trigger to mark such a trigger deferred. We don't really care about supporting that, but throwing an error instead of crashing seems desirable. Per report from Marcelo Costa.
2009-09-03Disallow RESET ROLE and RESET SESSION AUTHORIZATION inside security-definerTom Lane
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
2009-08-12Fix old bug in log_autovacuum_min_duration code: it was relying on being ableTom Lane
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.
2009-05-19Update relpages and reltuples estimates in stand-alone ANALYZE, even ifHeikki Linnakangas
there's no analyzable attributes or indexes. We also used to report 0 live and dead tuples for such tables, which messed with autovacuum threshold calculations. This fixes bug #4812 reported by George Su. Backpatch back to 8.1.
2009-02-27In CREATE CONVERSION, test that the given function is a valid conversionHeikki Linnakangas
function for the specified source and destination encodings. We do that by calling the function with an empty string. If it can't perform the requested conversion, it will throw an error. Backport to 7.4 - 8.3. Per bug report #4680 by Denis Afonin.
2009-02-24Repair a longstanding bug in CLUSTER and the rewriting variants of ALTERTom Lane
TABLE: if the command is executed by someone other than the table owner (eg, a superuser) and the table has a toast table, the toast table's pg_type row ends up with the wrong typowner, ie, the command issuer not the table owner. This is quite harmless for most purposes, since no interesting permissions checks consult the pg_type row. However, it could lead to unexpected failures if one later tries to drop the role that issued the command (in 8.1 or 8.2), or strange warnings from pg_dump afterwards (in 8.3 and up, which will allow the DROP ROLE because we don't create a "redundant" owner dependency for table rowtypes). Problem identified by Cott Lang. Back-patch to 8.1. The problem is actually far older --- the CLUSTER variant can be demonstrated in 7.0 --- but it's mostly cosmetic before 8.1 because we didn't track ownership dependencies before 8.1. Also, fixing it before 8.1 would require changing the call signature of heap_create_with_catalog(), which seems to carry a nontrivial risk of breaking add-on modules.
2009-02-13Fix UNLISTEN to fall out quickly if the current backend has never executedTom Lane
any LISTEN command. This is more important than it used to be because DISCARD ALL invokes UNLISTEN. Connection-pooled applications making heavy use of DISCARD ALL were seeing significant contention for pg_listener, as reported by Matteo Beccati. It seems unlikely that clients using LISTEN would use pooled connections, so this simple tweak seems sufficient, especially since the pg_listener implementation is slated to go away soon anyway. Back-patch to 8.3, where DISCARD ALL was introduced.
2009-01-06Fix logic in lazy vacuum to decide if it's worth trying to truncate the heap.Heikki Linnakangas
If the table was smaller than REL_TRUNCATE_FRACTION (= 16) pages, we always tried to acquire AccessExclusiveLock on it even if there was no empty pages at the end. Report by Simon Riggs. Back-patch all the way to 7.4.
2008-12-13Fix failure to ensure that a snapshot is available to datatype input functionsTom Lane
when they are invoked by the parser. We had been setting up a snapshot at plan time but really it needs to be done earlier, before parse analysis. Per report from Dmitry Koterov. Also fix two related problems discovered while poking at this one: exec_bind_message called datatype input functions without establishing a snapshot, and SET CONSTRAINTS IMMEDIATE could call trigger functions without establishing a snapshot. Backpatch to 8.2. The underlying problem goes much further back, but it is masked in 8.1 and before because we didn't attempt to invoke domain check constraints within datatype input. It would only be exposed if a C-language datatype input function used the snapshot; which evidently none do, or we'd have heard complaints sooner. Since this code has changed a lot over time, a back-patch is hardly risk-free, and so I'm disinclined to patch further than absolutely necessary.
2008-12-01Ensure that the contents of a holdable cursor don't depend on out-of-lineTom Lane
toasted values, since those could get dropped once the cursor's transaction is over. Per bug #4553 from Andrew Gierth. Back-patch as far as 8.1. The bug actually exists back to 7.4 when holdable cursors were introduced, but this patch won't work before 8.1 without significant adjustments. Given the lack of field complaints, it doesn't seem worth the work (and risk of introducing new bugs) to try to make a patch for the older branches.
2008-11-27Back-patch change to make DISCARD ALL release advisory locks.Tom Lane
Per discussion.
2008-10-25Fix an old bug in after-trigger handling: AfterTriggerEndQuery took theTom Lane
address of afterTriggers->query_stack[afterTriggers->query_depth] and hung onto it through all its firings of triggers. However, if a trigger causes sufficiently many nested query executions, query_stack will get repalloc'd bigger, leaving AfterTriggerEndQuery --- and hence afterTriggerInvokeEvents --- using a stale pointer. So far as I can find, the only consequence of this error is to stomp on a couple of words of already-freed memory; which would lead to a failure only if that chunk had already gotten re-allocated for something else. So it's hard to exhibit a simple failure case, but this is surely a bug. I noticed this while working on my recent patch to reduce pending-trigger space usage. The present patch is mighty ugly, because it requires making afterTriggerInvokeEvents know about all the possible event lists it might get called on. Fortunately, this is only needed in back branches because CVS HEAD avoids the problem in a different way: afterTriggerInvokeEvents only touches the passed AfterTriggerEventList pointer once at startup. Back branches are stable enough that wiring in knowledge of all possible call usages doesn't seem like a killer problem. Back-patch to 8.0. 7.4's trigger code is completely different and doesn't seem to have the problem (it doesn't even use repalloc).
2008-10-09Force a checkpoint in CREATE DATABASE before starting to copy the files,Heikki Linnakangas
to process any pending unlinks for the source database. Before, if you dropped a relation in the template database just before CREATE DATABASE, and a checkpoint happened during copydir(), the checkpoint might delete a file that we're just about to copy, causing lstat() in copydir() to fail with ENOENT. Backpatch to 8.3, where the pending unlinks were introduced. Per report by Matthew Wakeling and analysis by Tom Lane.
2008-10-07When a relation is moved to another tablespace, we can't assume that we canHeikki Linnakangas
use the old relfilenode in the new tablespace. There might be another relation in the new tablespace with the same relfilenode, so we must generate a fresh relfilenode in the new tablespace. The 8.3 patch to let deleted relation files linger as zero-length files until the next checkpoint made this more obvious: moving a relation from one table space another, and then back again, caused a collision with the lingering file. Back-patch to 8.1. The issue is present in 8.0 as well, but it doesn't seem worth fixing there, because we didn't have protection from OID collisions after OID wraparound before 8.1. Report by Guillaume Lelarge.
2008-09-11Initialize the minimum frozen Xid in vac_update_datfrozenxid usingAlvaro Herrera
GetOldestXmin() instead of RecentGlobalXmin; this is safer because we do not depend on the latter being correctly set elsewhere, and while it is more expensive, this code path is not performance-critical. This is a real risk for autovacuum, because it can execute whole cycles without doing a single vacuum, which would mean that RecentGlobalXmin would stay at its initialization value, FirstNormalTransactionId, causing a bogus value to be inserted in pg_database. This bug could explain some recent reports of failure to truncate pg_clog. At the same time, change the initialization of RecentGlobalXmin to InvalidTransactionId, and ensure that it's set to something else whenever it's going to be used. Using it as FirstNormalTransactionId in HOT page pruning could incur in data loss. InitPostgres takes care of setting it to a valid value, but the extra checks are there to prevent "special" backends from behaving in unusual ways. Per Tom Lane's detailed problem dissection in 29544.1221061979@sss.pgh.pa.us
2008-06-08ALTER AGGREGATE OWNER seems to have been missed by the last couple ofTom Lane
patches that dealt with object ownership. It wasn't updating pg_shdepend nor adjusting the aggregate's ACL. In 8.2 and up, fix this permanently by making it use AlterFunctionOwner_oid. In 8.1, the function code wasn't factored that way, so just copy and paste.
2008-05-19Coercion sanity check in ri_HashCompareOp failed to allow for enums, as perTom Lane
example from Rod Taylor. On reflection the correct test here is for any polymorphic type, not specifically ANYARRAY as in the original coding.
2008-05-09Fix an ancient oversight in change_varattnos_of_a_node: it neglected to updateTom Lane
varoattno along with varattno. This resulted in having Vars that were not seen as equal(), causing inheritance of the "same" constraint from different parent relations to fail. An example is create table pp1 (f1 int check (f1>0)); create table cc1 (f2 text, f3 int) inherits (pp1); create table cc2(f4 float) inherits(pp1,cc1); Backpatch as far as 7.4. (The test case still fails in 7.4, for reasons that I don't feel like investigating at the moment.) This is a backpatch commit only. The fix will be applied in HEAD as part of the upcoming pg_constraint patch.
2008-04-29Fix REASSIGN OWNED so that it works on procedural languages too.Alvaro Herrera
The capability for changing language owners is new in 8.3, so that's how far back this needs to be backpatched. Per bug #4132 by Kirill Simonov.
2008-04-24Fix ALTER TABLE ADD COLUMN ... PRIMARY KEY so that the new column is correctlyTom Lane
checked to see if it's been initialized to all non-nulls. The implicit NOT NULL constraint was not being checked during the ALTER (in fact, not even if there was an explicit NOT NULL too), because ATExecAddColumn neglected to set the flag needed to make the test happen. This has been broken since the capability was first added, in 8.0. Brendan Jurd, per a report from Kaloyan Iliev.
2008-04-18Fix rmtree() so that it keeps going after failure to remove any individualTom Lane
file; the idea is that we should clean up as much as we can, even if there's some problem removing one file. Make the error messages a bit less misleading, too. In passing, const-ify function arguments.
2008-04-18Fix two race conditions between the pending unlink mechanism that was put inHeikki Linnakangas
place to prevent reusing relation OIDs before next checkpoint, and DROP DATABASE. First, if a database was dropped, bgwriter would still try to unlink the files that the rmtree() call by the DROP DATABASE command has already deleted, or is just about to delete. Second, if a database is dropped, and another database is created with the same OID, bgwriter would in the worst case delete a relation in the new database that happened to get the same OID as a dropped relation in the old database. To fix these race conditions: - make rmtree() ignore ENOENT errors. This fixes the 1st race condition. - make ForgetDatabaseFsyncRequests forget unlink requests as well. - force checkpoint on in dropdb on all platforms Since ForgetDatabaseFsyncRequests() is asynchronous, the 2nd change isn't enough on its own to fix the problem of dropping and creating a database with same OID, but forcing a checkpoint on DROP DATABASE makes it sufficient. Per Tom Lane's bug report and proposal. Backpatch to 8.3.
2008-04-16Repair two places where SIGTERM exit could leave shared memory stateTom Lane
corrupted. (Neither is very important if SIGTERM is used to shut down the whole database cluster together, but there's a problem if someone tries to SIGTERM individual backends.) To do this, introduce new infrastructure macros PG_ENSURE_ERROR_CLEANUP/PG_END_ENSURE_ERROR_CLEANUP that take care of transiently pushing an on_shmem_exit cleanup hook. Also use this method for createdb cleanup --- that wasn't a shared-memory-corruption problem, but SIGTERM abort of createdb could leave orphaned files lying around. Backpatch as far as 8.2. The shmem corruption cases don't exist in 8.1, and the createdb usage doesn't seem important enough to risk backpatching further.
2008-04-03Teach ANALYZE to distinguish dead and in-doubt tuples, which it formerlyTom Lane
classed all as "dead"; also get it to count DEAD item pointers as dead rows, instead of ignoring them as before. Also improve matters so that tuples previously inserted or deleted by our own transaction are handled nicely: the stats collector's live-tuple and dead-tuple counts will end up correct after our transaction ends, regardless of whether we end in commit or abort. While there's more work that could be done to improve the counting of in-doubt tuples in both VACUUM and ANALYZE, this commit is enough to alleviate some known bad behaviors in 8.3; and the other stuff that's been discussed seems like research projects anyway. Pavan Deolasee and Tom Lane
2008-04-02Revert my bad decision of about a year ago to make PortalDefineQueryTom Lane
responsible for copying the query string into the new Portal. Such copying is unnecessary in the common code path through exec_simple_query, and in this case it can be enormously expensive because the string might contain a large number of individual commands; we were copying the entire, long string for each command, resulting in O(N^2) behavior for N commands. (This is the cause of bug #4079.) A second problem with it is that PortalDefineQuery really can't risk error, because if it elog's before having set up the Portal, we will leak the plancache refcount that the caller is trying to hand off to the portal. So go back to the design in which the caller is responsible for making sure everything is copied into the portal if necessary.
2008-03-24Fix various infelicities that have snuck into usage of errdetail() andTom Lane
friends. Avoid double translation of some messages, ensure other messages are exposed for translation (and make them follow the style guidelines), avoid unsafe passing of an unpredictable message text as a format string.
2008-03-14Fix vacuum so that autovacuum is really not cancelled when doing an emergencyAlvaro Herrera
job (i.e. to prevent Xid wraparound problems.) Bug reported by ITAGAKI Takahiro in 20080314103837.63D3.52131E4D@oss.ntt.co.jp, though I didn't use his patch.
2008-03-12Fix LISTEN/NOTIFY race condition reported by Laurent Birtz, by postponingTom Lane
pg_listener modifications commanded by LISTEN and UNLISTEN until the end of the current transaction. This allows us to hold the ExclusiveLock on pg_listener until after commit, with no greater risk of deadlock than there was before. Aside from fixing the race condition, this gets rid of a truly ugly kludge that was there before, namely having to ignore HeapTupleBeingUpdated failures during NOTIFY. There is a small potential incompatibility, which is that if a transaction issues LISTEN or UNLISTEN and then looks into pg_listener before committing, it won't see any resulting row insertion or deletion, where before it would have. It seems unlikely that anyone would be depending on that, though. This patch also disallows LISTEN and UNLISTEN inside a prepared transaction. That case had some pretty undesirable properties already, such as possibly allowing pg_listener entries to be made for PIDs no longer present, so disallowing it seems like a better idea than trying to maintain the behavior.
2008-02-11Repair VACUUM FULL bug introduced by HOT patch: the original way ofTom Lane
calculating a page's initial free space was fine, and should not have been "improved" by letting PageGetHeapFreeSpace do it. VACUUM FULL is going to reclaim LP_DEAD line pointers later, so there is no need for a guard against the page being too full of line pointers, and having one risks rejecting pages that are perfectly good move destinations. This also exposed a second bug, which is that the empty_end_pages logic assumed that any page with no live tuples would get entered into the fraged_pages list automatically (by virtue of having more free space than the threshold in the do_frag calculation). This assumption certainly seems risky when a low fillfactor has been chosen, and even without tunable fillfactor I think it could conceivably fail on a page with many unused line pointers. So fix the code to force do_frag true when notup is true, and patch this part of the fix all the way back. Per report from Tomas Szepe.
2008-02-07Some variants of ALTER OWNER tried to make the "object" field of theTom Lane
statement be a list of bare C strings, rather than String nodes, which is what they need to be for copyfuncs/equalfuncs to work. Fortunately these node types never go out to disk (if they did, we'd likely have noticed the problem sooner), so we can just fix it without creating a need for initdb. This bug has been there since 8.0, but 8.3 exposes it in a more common code path (Parse messages) than prior releases did. Per bug #3940 from Vladimir Kokovic.
2008-02-07Fix CREATE TABLE ... LIKE ... INCLUDING INDEXES to not cause unwantedTom Lane
tablespace permissions failures when copying an index that is in the database's default tablespace. A side-effect of the change is that explicitly specifying the default tablespace no longer triggers a permissions check; this is not how it was done in pre-8.3 releases but is argued to be more consistent. Per bug #3921 from Andrew Gilligan. (Note: I argued in the subsequent discussion that maybe LIKE shouldn't copy index tablespaces at all, but since no one indicated agreement with that idea, I've refrained from doing it.)