Tom Lane [Wed, 12 Mar 2008 23:58:35 +0000 (23:58 +0000)]
Fix pg_plan_queries() to restore the previous setting of ActiveSnapshot
(probably NULL) before exiting. Up to now it's just left the variable as it
set it, which means that after we're done processing the current client
message, ActiveSnapshot is probably pointing at garbage (because this function
is typically run in MessageContext which will get reset). There doesn't seem
to have been any code path in which that mattered before 8.3, but now the
plancache module might try to use the stale value if the next client message
is a Bind for a prepared statement that is in need of replanning. Per report
from Alex Hunsaker.
Tom Lane [Wed, 12 Mar 2008 20:11:54 +0000 (20:11 +0000)]
Fix LISTEN/NOTIFY race condition reported by Laurent Birtz, by postponing
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.
Tom Lane [Mon, 10 Mar 2008 21:50:23 +0000 (21:50 +0000)]
Use -fwrapv in CFLAGS if we are using a version of gcc that accepts this flag.
This prevents compiler optimizations that assume overflow won't occur, which
breaks numerous overflow tests that we need to have working. It is known
that gcc 4.3 causes problems and possible that 4.1 does. Per my proposal
of some time ago and a recent report from Kris Jurka.
Backpatch as far as 8.0, which is as far as the patch conveniently goes.
7.x was pretty short of overflow tests anyway, so it may not matter there,
even assuming that anyone cares whether 7.x builds on recent gcc.
Tom Lane [Sat, 8 Mar 2008 21:58:07 +0000 (21:58 +0000)]
Refactor heap_page_prune so that instead of changing item states on-the-fly,
it accumulates the set of changes to be made and then applies them. It had
to accumulate the set of changes anyway to prepare a WAL record for the
pruning action, so this isn't an enormous change; the only new complexity is
to not doubly mark tuples that are visited twice in the scan. The main
advantage is that we can substantially reduce the scope of the critical
section in which the changes are applied, thus avoiding PANIC in foreseeable
cases like running out of memory in inval.c. A nice secondary advantage is
that it is now far clearer that WAL replay will actually do the same thing
that the original pruning did.
This commit doesn't do anything about the open problem that
CacheInvalidateHeapTuple doesn't have the right semantics for a CTID change
caused by collapsing out a redirect pointer. But whatever we do about that,
it'll be a good idea to not do it inside a critical section.
Tom Lane [Fri, 7 Mar 2008 15:59:09 +0000 (15:59 +0000)]
Change hashscan.c to keep its list of active hash index scans in
TopMemoryContext, rather than scattered through executor per-query contexts.
This poses no danger of memory leak since the ResourceOwner mechanism
guarantees release of no-longer-needed items. It is needed because the
per-query context might already be released by the time we try to clean up
the hash scan list. Report by ykhuang, diagnosis by Heikki.
Back-patch to 8.0, where the ResourceOwner-based cleanup was introduced.
The given test case does not fail before 8.2, probably because we rearranged
transaction abort processing somehow; but this coding is undoubtedly risky
so I'll patch 8.0 and 8.1 anyway.
Teodor Sigaev [Fri, 7 Mar 2008 15:29:27 +0000 (15:29 +0000)]
Fix memory arrangement of tsquery after removing stop words. It causes
a unused memory holes in tsquery.
Per report by Richard Huxton <dev@archonet.com>.
It was working well because in fact tsquery->size is not used for any
kind of operation except comparing tsqueries. To prevent requirement
of renew all stored tsquery optimization in CompareTSQ is removed.
Tom Lane [Wed, 5 Mar 2008 17:01:33 +0000 (17:01 +0000)]
In PrepareToInvalidateCacheTuple, don't force initialization of catalog
caches that we don't actually need to touch. This saves some trivial
number of cycles and avoids certain cases of deadlock when doing concurrent
VACUUM FULL on system catalogs. Per report from Gavin Roy.
Backpatch to 8.2. In earlier versions, CatalogCacheInitializeCache didn't
lock the relation so there's no deadlock risk (though that certainly had
plenty of risks of its own).
Bruce Momjian [Wed, 5 Mar 2008 17:00:40 +0000 (17:00 +0000)]
Document that increasing the number of checkpoints segments or
checkpoint timeout can incrase the time needed for crash recovery, per
suggestion from Simon.
Tom Lane [Tue, 4 Mar 2008 19:54:13 +0000 (19:54 +0000)]
Fix PREPARE TRANSACTION to reject the case where the transaction has dropped a
temporary table; we can't support that because there's no way to clean up the
source backend's internal state if the eventual COMMIT PREPARED is done by
another backend. This was checked correctly in 8.1 but I broke it in 8.2 :-(.
Patch by Heikki Linnakangas, original trouble report by John Smith.
Tom Lane [Sat, 1 Mar 2008 19:26:28 +0000 (19:26 +0000)]
Fix another place that was assuming that a local variable declared as
"struct varlena" would be at least word-aligned. Per buildfarm results
from gypsy_moth. I did a little bit of trawling for other instances of
this coding pattern, and didn't find any; but if we turn up any more
of them I think we'd better revert the "char [4]" patch and find another
way of making tuptoaster.c alignment-safe.
Tom Lane [Sat, 1 Mar 2008 03:26:44 +0000 (03:26 +0000)]
Fix unportable usages of tolower(). On signed-char machines, it is necessary
to explicitly cast the output back to char before comparing it to a char
value, else we get the wrong result for high-bit-set characters. Found by
Rolf Jentsch. Also, fix several places where <ctype.h> functions were being
called without casting the argument to unsigned char; this is likewise
unportable, but we keep making that mistake :-(. These found by buildfarm
member salamander, which I will desperately miss if it ever goes belly-up.
Tom Lane [Sat, 1 Mar 2008 02:46:55 +0000 (02:46 +0000)]
Disable the undocumented xmlvalidate() function, which was unintentionally
left in the code though it was not meant to be provided. It represents a
security hole because unprivileged users could use it to look at (at least the
first line of) any file readable by the backend. Fortunately, this is only
possible if the backend was built with XML support, so the damage is at least
mitigated; and 8.3 probably hasn't propagated into any security-critical uses
yet anyway. Per report from Sergey Burladyan.
Tom Lane [Fri, 29 Feb 2008 17:47:47 +0000 (17:47 +0000)]
Reducing the assumed alignment of struct varlena means that the compiler
is also licensed to put a local variable declared that way at an unaligned
address. Which will not work if the variable is then manipulated with
SET_VARSIZE or other macros that assume alignment. So the previous patch
is not an unalloyed good, but on balance I think it's still a win, since
we have very few places that do that sort of thing. Fix the one place in
tuptoaster.c that does it. Per buildfarm results from gypsy_moth
(I'm a bit surprised that only one machine showed a failure).
Magnus Hagander [Fri, 29 Feb 2008 15:31:41 +0000 (15:31 +0000)]
Fix handling of restricted processes for Windows Vista (mainly),
by explicitly adding back the user to the DACL of the new process.
This fixes the failure case when executing as the Administrator
user, which had no permissions left at all after we dropped the
Administrators group.
Neil Conway [Fri, 29 Feb 2008 02:49:43 +0000 (02:49 +0000)]
Fix several memory leaks when rescanning SRFs. Arrange for an SRF's
"multi_call_ctx" to be a distinct sub-context of the EState's per-query
context, and delete the multi_call_ctx as soon as the SRF finishes
execution. This avoids leaking SRF memory until the end of the current
query, which is particularly egregious when the SRF is scanned
multiple times. This change also fixes a leak of the fields of the
AttInMetadata struct in shutdown_MultiFuncCall().
Also fix a leak of the SRF result TupleDesc when rescanning a
FunctionScan node. The TupleDesc is allocated in the per-query context
for every call to ExecMakeTableFunctionResult(), so we should free it
after calling that function. Since the SRF might choose to return
a non-expendable TupleDesc, we only free the TupleDesc if it is
not being reference-counted.
Tom Lane [Wed, 27 Feb 2008 17:44:27 +0000 (17:44 +0000)]
If RelationBuildDesc() fails to open a critical system index, PANIC with
a relevant error message instead of just dumping core. Odd that nobody
reported this before Darren Reed.
Tom Lane [Tue, 26 Feb 2008 02:54:14 +0000 (02:54 +0000)]
Fix encode(...bytea..., 'escape') so that it converts all high-bit-set byte
values into \nnn octal escape sequences. When the database encoding is
multibyte this is *necessary* to avoid generating invalidly encoded text.
Even in a single-byte encoding, the old behavior seems very hazardous ---
consider for example what happens if the text is transferred to another
database with a different encoding. Decoding would then yield some other
bytea value than what was encoded, which is surely undesirable. Per gripe
from Hernan Gonzalez.
Backpatch to 8.3, but not further. This is a bit of a judgment call, but I
make it on these grounds: pre-8.3 we don't really have much encoding safety
anyway because of the convert() function family, and we would also have much
higher risk of breaking existing apps that may not be expecting this behavior.
8.3 is still new enough that we can probably get away with making this change
in the function's behavior.
Tom Lane [Mon, 25 Feb 2008 23:21:08 +0000 (23:21 +0000)]
Fix datetime input to behave correctly for Feb 29 in years BC.
Formerly, DecodeDate attempted to verify the day-of-the-month exactly, but
it was under the misapprehension that it would know whether we were looking
at a BC year or not. In reality this check can't be made until the calling
function (eg DecodeDateTime) has processed all the fields. So, split the
BC adjustment and validity checks out into a new function ValidateDate that
is called only after processing all the fields. In passing, this patch
makes DecodeTimeOnly work for BC inputs, which it never did before.
(The historical veracity of all this is nonexistent, of course, but if
we're going to say we support proleptic Gregorian calendar then we should
do it correctly. In any case the unpatched code is broken because it could
emit dates that it would then reject on re-inputting.)
Per report from Bernd Helmle. Back-patch as far as 8.0; in 7.x we were
not using our own calendar support and so this seems a bit too risky
to put into 7.4.
Tom Lane [Sat, 23 Feb 2008 19:11:55 +0000 (19:11 +0000)]
Change the declaration of struct varlena so that the length word is
represented as "char ...[4]" not "int32". Since the length word is never
supposed to be accessed via this struct member anyway, this won't break
any existing code that is following the rules. The advantage is that C
compilers will no longer assume that a pointer to struct varlena is
word-aligned, which prevents incorrect optimizations in TOAST-pointer
access and perhaps other places. gcc doesn't seem to do this (at least
not at -O2), but the problem is demonstrable on some other compilers.
I changed struct inet as well, but didn't bother to touch a lot of other
struct definitions in which it wouldn't make any difference because there
were other fields forcing int alignment anyway. Hopefully none of those
struct definitions are used for accessing unaligned Datums.
Tom Lane [Wed, 20 Feb 2008 22:18:20 +0000 (22:18 +0000)]
Fix mistakes in pg_ctl's code for "start -w" that tries to cope with
non-default settings for the postmaster's port number. The code to parse
command line options and postgresql.conf entries wasn't quite right about
whitespace or quotes, and it was coded in a not-very-readable way too.
Per bug #3969 from Itagaki Takahiro, though this is more extensive than his
proposed patch (which fixed only the whitespace problem).
This code has been broken since it was put in in 8.0, so patch all the way
back.
Tom Lane [Wed, 20 Feb 2008 17:44:14 +0000 (17:44 +0000)]
Put a CHECK_FOR_INTERRUPTS call into the loops that try to find a unique new
OID or new relfilenode. If the existing OIDs are sufficiently densely
populated, this could take a long time (perhaps even be an infinite loop),
so it seems wise to allow the system to respond to a cancel interrupt here.
Per a gripe from Jacky Leng.
Backpatch as far as 8.1. Older versions just fail on OID collision,
instead of looping.
Tom Lane [Mon, 18 Feb 2008 23:00:38 +0000 (23:00 +0000)]
Remove unnecessary opening of other relation in RI_FKey_keyequal_upd_pk
and RI_FKey_keyequal_upd_fk, as well as no-longer-needed calls of
ri_BuildQueryKeyFull. Aside from saving a few cycles, this avoids needless
deadlock risks when an update is not changing the columns that participate
in an RI constraint. Per a gripe from Alexey Nalbat.
Back-patch to 8.3. Earlier releases did have a need to open the other
relation due to the way in which they retrieved information about the RI
constraint, so this problem unfortunately can't easily be improved pre-8.3.
Michael Meskes [Fri, 15 Feb 2008 12:11:02 +0000 (12:11 +0000)]
Changed the way symbols are defined in C in INFORMIX mode.
Added SQLSTATE macro closing bug #3961.
EXECUTE can return NOT FOUND so it should be checked here too.
Tom Lane [Tue, 12 Feb 2008 04:09:44 +0000 (04:09 +0000)]
Fix SPI_cursor_open() and SPI_is_cursor_plan() to push the SPI stack before
doing anything interesting, such as calling RevalidateCachedPlan(). The
necessity of this is demonstrated by an example from Willem Buitendyk:
during a replan, the planner might try to evaluate SPI-using functions,
and so we'd better be in a clean SPI context.
A small downside of this fix is that these two functions will now fail
outright if called when not inside a SPI-using procedure (ie, a
SPI_connect/SPI_finish pair). The documentation never promised or suggested
that that would work, though; and they are normally used in concert with
other functions, mainly SPI_prepare, that always have failed in such a case.
So the odds of breaking something seem pretty low.
In passing, make SPI_is_cursor_plan's error handling convention clearer,
and fix documentation's erroneous claim that SPI_cursor_open would
return NULL on error.
Before 8.3 these functions could not invoke replanning, so there is probably
no need for back-patching.
Tom Lane [Mon, 11 Feb 2008 19:14:30 +0000 (19:14 +0000)]
Repair VACUUM FULL bug introduced by HOT patch: the original way of
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.
Tom Lane [Sun, 10 Feb 2008 20:39:08 +0000 (20:39 +0000)]
Fix PageGetExactFreeSpace() so that it actually behaves sensibly
if pd_lower > pd_upper, rather than merely claiming to. This would
only matter if the page header were corrupt, which shouldn't occur,
but ...
Tom Lane [Fri, 8 Feb 2008 17:58:46 +0000 (17:58 +0000)]
Since GSSAPI and SSPI authentication don't work in protocol version 2,
issue a helpful error message instead of sending unparsable garbage.
(It is clearly a design error that this doesn't work, but fixing it
is not worth the trouble at this point.) Per discussion.
Tom Lane [Thu, 7 Feb 2008 22:58:35 +0000 (22:58 +0000)]
Avoid misbehavior in foreign key checks when casting to a datatype for which
the parser supplies a default typmod that can result in data loss (ie,
truncation). Currently that appears to be only CHARACTER and BIT.
We can avoid the problem by specifying the type's internal name instead
of using SQL-spec syntax. Since the queries generated here are only used
internally, there's no need to worry about portability. This problem is
new in 8.3; before we just let the parser do whatever it wanted to resolve
the operator, but 8.3 is trying to be sure that the semantics of FK checks
are consistent. Per report from Harald Fuchs.
Tom Lane [Thu, 7 Feb 2008 21:07:55 +0000 (21:07 +0000)]
Some variants of ALTER OWNER tried to make the "object" field of the
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.
Tom Lane [Thu, 7 Feb 2008 20:19:47 +0000 (20:19 +0000)]
Add missing copyfuncs/equalfuncs support for AlterTSDictionaryStmt and
AlterTSConfigurationStmt. All utility statement node types are expected
to be supported here, though they do not have to have outfuncs/readfuncs
support. Found by running regression tests with COPY_PARSE_PLAN_TREES
enabled.
Tom Lane [Thu, 7 Feb 2008 17:53:53 +0000 (17:53 +0000)]
Fix silly mistake in expand_indexqual_rowcompare --- in converting a forboth()
into an iteration over three parallel lists, I had accidentally put the lnext
steps outside the loop. Sigh. Per bug #3938.
Tom Lane [Thu, 7 Feb 2008 17:09:51 +0000 (17:09 +0000)]
Fix CREATE TABLE ... LIKE ... INCLUDING INDEXES to not cause unwanted
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.)
Magnus Hagander [Wed, 6 Feb 2008 15:13:25 +0000 (15:13 +0000)]
Fix very broken clean.bat for msvc install. The way we used subroutines
in .bat simply did not work, and it called them in the wrong order,
some several times, and some not at all. So this unrolls all subroutine
calls.
This should fix the issues with clean deleting the wrong files reported
by Dave Page.
While at it, add the "clean dist" option to act like "make distclean",
and no longer remove the flex/bison output files by default. This shuold
fix the problem reported by Pavel Golub in bug #3909.
Tom Lane [Sat, 2 Feb 2008 22:26:17 +0000 (22:26 +0000)]
Fix WaitOnLock() to ensure that the process's "waiting" flag is reset after
erroring out of a wait. We can use a PG_TRY block for this, but add a comment
explaining why it'd be a bad idea to use it for any other state cleanup.
Back-patch to 8.2. Prior releases had the same issue, but only with respect
to the process title, which is likely to get reset almost immediately anyway
after the transaction aborts, so it seems not worth changing them. In 8.2
and HEAD, the pg_stat_activity "waiting" flag could remain set incorrectly
for a long time.
Bruce Momjian [Fri, 1 Feb 2008 02:41:10 +0000 (02:41 +0000)]
Update item:
< * Improve deadlock detection when deleting items from shared buffers
> * Improve deadlock detection when a page cleaning lock conflicts
> with a shared buffer that is pinned
Tom Lane [Thu, 31 Jan 2008 20:29:30 +0000 (20:29 +0000)]
Remove the old table of "supported platforms" in favor of a link to the
buildfarm plus a narrative description of the CPU types and operating systems
on which Postgres is likely to work. Now that we've almost completely
decoupled CPU and OS considerations, the former tabular style isn't all that
enlightening anyway. Perhaps more importantly, no one seems particularly
interested in maintaining the table by hand when we have the buildfarm.
Tom Lane [Thu, 31 Jan 2008 18:58:30 +0000 (18:58 +0000)]
Fix pg_GSS_error to use conn->errorMessage more sanely, ie, actually
work with the PQExpBuffer code instead of fighting it. This avoids an
unnecessary limit on message length and fixes the latent bug that
errorMessage.len wasn't getting set.
Tom Lane [Thu, 31 Jan 2008 18:40:02 +0000 (18:40 +0000)]
Improve pg_autovacuum documentation to clarify that the enabled field cannot
prevent anti-wraparound vacuuming, and to caution against setting unreasonably
small values of freeze_max_age. Also put in a notice that this catalog is
likely to disappear entirely in some future release. Per discussion of
bug #3898 from Steven Flatt.
Magnus Hagander [Thu, 31 Jan 2008 09:21:17 +0000 (09:21 +0000)]
Add pid to the pgident event name on win32.
Should fix a problem where two clusters are running under
two different service accounts and get colliding names,
causing only the first cluster to contain the pgident
event description.
Tom Lane [Wed, 30 Jan 2008 19:46:48 +0000 (19:46 +0000)]
Add checks to TRUNCATE, CLUSTER, and REINDEX to prevent performing these
operations when the current transaction has any open references to the
target relation or index (implying it has an active query using the relation).
The need for this was previously recognized in connection with ALTER TABLE,
but anything that summarily eliminates tuples or moves them around would
confuse an active scan.
While this patch does not in itself fix bug #3883 (the deadlock would happen
before the new check fires), it will discourage people from attempting the
sequence of operations that creates a deadlock risk, so it's at least a
partial response to that problem.
In passing, add a previously-missing check to REINDEX to prevent trying to
reindex another backend's temp table. This isn't a security problem since
only a superuser would get past the schema permission checks, but if we are
testing for this in other utility commands then surely REINDEX should too.
Tom Lane [Wed, 30 Jan 2008 18:35:55 +0000 (18:35 +0000)]
Add a GUC variable "synchronize_seqscans" to allow clients to disable the new
synchronized-scanning behavior, and make pg_dump disable sync scans so that
it will reliably preserve row ordering. Per recent discussions.
Tom Lane [Wed, 30 Jan 2008 04:11:19 +0000 (04:11 +0000)]
Don't putenv() a string that is allocated in a context that will go away
soon. I suspect this explains bug #3902, though I'm still not able to
reproduce that.
Tom Lane [Tue, 29 Jan 2008 02:06:30 +0000 (02:06 +0000)]
Fix up closePGconn() so that PQreset() will work on GSSAPI/SSPI connections;
the patch for those features put its cleanup code into freePGconn() which is
really the wrong place. Remove redundant code from freePGconn() and add
comments in hopes of preventing similar mistakes in future.
Noticed while trying (futilely) to reproduce bug #3902.
Tom Lane [Tue, 29 Jan 2008 02:03:39 +0000 (02:03 +0000)]
Arrange to ignore SIGPIPE during SSL_read() and SSL_shutdown(), as these
are known to write on the socket sometimes and thus we are vulnerable to
being killed by the signal if the server happens to go away unexpectedly.
Noticed while trying (futilely) to reproduce bug #3902.
This bug has been there all along, but since the situation is usually
only of interest to developers, I chose not to back-patch the changes.