the alreadyDeleted list has to be passed down through
deleteDependentObjects(), else objects that are deleted via auto/internal
dependencies don't get reported back up to performMultipleDeletions().
Depending on the visitation order, this could cause the code to try to delete
an already-deleted object, leading to strange errors in DROP OWNED (typically
"cache lookup failed for relation NNNNN" or similar). Per bug #4289.
Patch for back branches only. This code has recently been rewritten in HEAD,
and doesn't have this particular bug anymore.
bug #4290. The fundamental bug is that masking extParam by outer_params,
as finalize_plan had been doing, caused us to lose the information that
an initPlan depended on the output of a sibling initPlan. On reflection
the best thing to do seemed to be not to try to adjust outer_params for
this case but get rid of it entirely. The only thing it was really doing
for us was to filter out param IDs associated with SubPlan nodes, and that
can be done (with greater accuracy) while processing individual SubPlan
nodes in finalize_primnode. This approach was vindicated by the discovery
that the masking method was hiding a second bug: SS_finalize_plan failed to
remove extParam bits for initPlan output params that were referenced in the
main plan tree (it only got rid of those referenced by other initPlans).
It's not clear that this caused any real problems, given the limited use
of extParam by the executor, but it's certainly not what was intended.
I originally thought that there was also a problem with needing to include
indirect dependencies on external params in initPlans' param sets, but it
turns out that the executor handles this correctly so long as the depended-on
initPlan is earlier in the initPlans list than the one using its output.
That seems a bit of a fragile assumption, but it is true at the moment,
so I just documented it in some code comments rather than making what would
be rather invasive changes to remove the assumption.
Back-patch to 8.1. Previous versions don't have the case of initPlans
referring to other initPlans' outputs, so while the existing logic is still
questionable for them, there are not any known bugs to be fixed. So I'll
refrain from changing them for now.
log message at newlines cost O(N^2) for very long messages with few or no
newlines. For messages in the megabyte range this became the dominant cost.
Per gripe from Achilleas Mantzios.
Patch all the way back, since this is a safe change with no portability
risks. I am also thinking of increasing PG_SYSLOG_LIMIT, but that should
be done separately.
results always contribute two groups, regardless of the expression contents.
This is very substantially more accurate than the regular heuristic for
certain boolean tests like "col IS NULL". Per gripe from Sam Mason.
Back-patch to all supported releases, since the behavior of
estimate_num_groups() hasn't changed all that much since 7.4.
the timezone argument as a timezone abbreviation, and only try it as a full
timezone name if that fails. The zic database has four zones (CET, EET, MET,
WET) that are full daylight-savings zones and yet have names that are the
same as their abbreviations for standard time, resulting in ambiguity.
In the timestamp input functions we resolve the ambiguity by preferring the
abbreviation, and AT TIME ZONE should work the same way. (No functionality
is lost because the zic database also has other names for these zones, eg
Europe/Zurich.) Per gripe from Jaromir Talir.
Backpatch to 8.1. Older releases did not have the issue because AT TIME ZONE
only accepted abbreviations not zone names. (Thus, this patch also arguably
fixes a compatibility botch introduced at 8.1: in ambiguous cases we now
behave the same as 8.0 did.)
variable that has units. Per report from Stefan Kaltenbrunner.
Backport to 8.2. I also backported my patch of 2007-06-21 that prevented
comparable overflows on the input side, since that now seems to have enough
field track record to be back-patched safely. That patch included addition
of hints listing the available unit names, which I did not bother to strip
out of it --- this will make a little more work for the translators, but
they can copy the translation from 8.3, and anyway an untranslated hint
is better than no hint.
timezone setting in the current year and for 100 years back, rather than
always examining years 1904-2004. The original coding would have problems
distinguishing zones whose behavior diverged only after 2004; which is a
situation we will surely face sometime, if it's not out there already.
In passing, also prevent selection of the dummy "Factory" timezone, even
if that's exactly what the system is using. Reporting time as GMT seems
better than that.
of any lower outer join, even if it also references the non-nullable side and
so could not get pushed below the outer join anyway. We need this in case
the clause is an OR clause: if it doesn't get marked outerjoin_delayed,
create_or_index_quals() could pull an indexable restriction for the nullable
side out of it, leading to wrong results as demonstrated by today's bug
report from toruvinn. (See added regression test case for an example.)
In principle this has been wrong for quite a while. In practice I don't
think any branch before 8.3 can really show the failure, because
create_or_index_quals() will only pull out indexable conditions, and before
8.3 those were always strict. So though we might have improperly generated
null-extended rows in the outer join, they'd get discarded from the result
anyway. The gating factor that makes the failure visible is that 8.3
considers "col IS NULL" to be indexable. Hence I'm not going to risk
back-patching further than 8.3.
by installing an error context subroutine that will provide the file name
and line number for all errors detected while reading a config file.
Some of the reader routines were already doing that in an ad-hoc way for
errors detected directly in the reader, but it didn't help for problems
detected in subroutines, such as encoding violations.
Back-patch to 8.3 because 8.3 is where people will be trying to debug
configuration files.
that it depends on for replan-forcing purposes. We need to consider plain OID
constants too, because eval_const_expressions folds a RelabelType atop a Const
to just a Const. This change could result in OID values that aren't really
for tables getting added to the dependency list, but the worst-case
consequence would be occasional useless replans. Per report from Gabriele
Messineo.
1. Directly reading interp->result is deprecated in Tcl 8.0 and later;
you're supposed to use Tcl_GetStringResult. This code finally broke with
Tcl 8.5, because Tcl_GetVar can now have side-effects on interp->result even
though it preserves the logical state of the result. (There's arguably a
Tcl issue here, because Tcl_GetVar could invalidate the pointer result of a
just-preceding Tcl_GetStringResult, but I doubt the Tcl guys will see it as
a bug.)
2. We were being sloppy about the encoding of the result: some places would
push database-encoding data into the Tcl result, which should not happen,
and we were assuming that any error result coming back from Tcl was in the
database encoding, which is not a good assumption.
3. There were a lot of calls of Tcl_SetResult that uselessly specified
TCL_VOLATILE for constant strings. This is only a minor performance issue,
but I fixed it in passing since I had to look at all the calls anyway.
#2 is a live bug regardless of which Tcl version you are interested in,
so back-patch even to branches that are unlikely to be used with Tcl 8.5.
I went back as far as 8.0, which is as far as the patch applied easily;
7.4 was using a different error processing scheme that has got its own
problems :-(
the problem happened in. These are all supposedly can't-happen cases, but
when they do happen it's useful to know where.
Back-patch to 8.3, but not further because the patch doesn't apply cleanly
further back. Given the lack of response to my proposal of this, there
doesn't seem to be enough interest to justify much back-porting effort.
CacheInvalidateRelcache() crashes if called in WAL recovery, because the
invalidation infrastructure hasn't been initialized yet.
Back-patch to 8.2, where the bug was introduced.
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.
This is needed because :: casting binds more tightly than minus, so for
example -1::integer is not the same as (-1)::integer, and there are cases
where the difference is important. In particular this caused a failure
in SELECT DISTINCT ... ORDER BY ... where expressions that should have
matched were seen as different by the parser; but I suspect that there
could be other cases where failure to parenthesize leads to subtler
semantic differences in reloaded rules. Per report from Alexandr Popov.
doesn't work, and the real reason why not is it's unclear where the path
is relative to (initdb's CWD, or the data directory?). We could make an
arbitrary decision, but it seems best to make the user be unambiguous.
Per gripe from Devrim.
rather than "\x09". Before 8.3 we just printed tabs as-is, leading to poor
formatting of subsequent columns, but consensus is that "\x09" is not an
improvement over that. Back-patch of fix that's already in HEAD.
memory if the compressed data is corrupt.
Backpatch as far as 8.2. The issue exists in older branches too, but given
the lack of field reports, it's not clear it's worth any additional effort
to adapt the patch to the slightly different code in older branches.
This is required on Windows due to the special locale
handling for UTF8 that doesn't change the full environment.
Fixes crash with translated error messages per bugs 4180
and 4196.
Tom Lane
called before, not after, calling the assign_hook if any. This is because
push_old_value might fail (due to palloc out-of-memory), and in that case
there would be no stack entry to tell transaction abort to undo the GUC
assignment. Of course the actual assignment to the GUC variable hasn't
happened yet --- but the assign_hook might have altered subsidiary state.
Without a stack entry we won't call it again to make it undo such actions.
So this is necessary to make the world safe for assign_hooks with side
effects. Per a discussion a couple weeks ago with Magnus.
Back-patch to 8.0. 7.x did not have the problem because it did not have
allocatable stacks of GUC values.
cases. Recent buildfarm experience shows that it is sometimes possible
to execute several SQL commands in less time than the granularity of
Windows' not-very-high-resolution gettimeofday(), leading to a failure
because the tests expect the value of now() to change and it doesn't.
Also, it was recognized some time ago that the same area of the tests
could fail if local midnight passes between the insertion and the checking
of the values for 'yesterday', 'tomorrow', etc. Clean all this up per
ideas from myself and Greg Stark.
There remains a window for failure if the transaction block is entered
exactly at local midnight (so that 'now' and 'today' have the same value),
but that seems low-probability enough to live with.
Since the point of this change is mostly to eliminate buildfarm noise,
back-patch to all versions we are still actively testing.
to go beoynd 10MB, as demonstrated by Gavin Sharry's example of dropping a
schema with ~25000 objects. The really bogus thing about the limit was that
it was enforced when a state file file was read in, not when it was written,
so you would end up with a prepared transaction that you can't commit or
abort, and the only recourse was to shut down the server and remove the file
by hand.
Raise the limit to MaxAllocSize, and enforce it also when a state file is
written. We could've removed the limit altogether, but reading in a file
larger than MaxAllocSize would fail anyway because we read it into a
palloc'd buffer.
Backpatch down to 8.1, where 2PC and this issue was introduced.
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.
parameter. This fixes bug 4137 reported by Wojciech Strzalka, where a WAL
file is deleted too early when starting the recovery of a warm standby server.
Also add a sanity check in pg_standby so that it will refuse to delete anything
earlier than the file being restored, and improve the debug message in case
nothing is deleted.
Simon Riggs. Backpatch to 8.3, which is where %r was introduced.
a user-supplied TID is out of range for the relation. This is needed to
preserve compatibility with our pre-8.3 behavior, and it is sensible anyway
since if the query were implemented by brute force rather than optimized
into a TidScan, the behavior for a non-existent TID would be zero rows out,
never an error. Per gripe from Gurjeet Singh.
binary search instead of linear search when checking child-transaction XIDs.
Per example from Robert Treat, the speed of TransactionIdIsCurrentTransactionId
is significantly more important in 8.3 than it was in prior releases, so
this seems worth taking back-patching risk for.
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.
<craig@postnewspapers.com.au>.
It was my mistake, I missed limitation of number of held locks, now GIN doesn't
use continiuous locks, but still hold buffers pinned to prevent interference
with vacuum's deletion algorithm.
output is not of the same type that's needed for the IN comparison (ie,
where the parser inserted an implicit coercion above the subselect result).
We should record the coerced expression, not just a raw Var referencing
the subselect output, as the quantity that needs to be unique-ified if
we choose to implement the IN as Unique followed by a plain join.
As of 8.3 this error was causing crashes, as seen in bug #4113 from Javier
Hernandez, because the executor was being told to hash or sort the raw
subselect output column using operators appropriate to the coerced type.
In prior versions there was no crash because the executor chose the
hash or sort operators for itself based on the column type it saw.
However, that's still not really right, because what's unique for one data
type might not be unique for another. In corner cases we could get multiple
outputs of a row that should appear only once, as demonstrated by the
regression test case included in this commit.
However, this patch doesn't apply cleanly to 8.2 or before, and the code
involved has shifted enough over time that I'm hesitant to try to back-patch.
Given the lack of complaints from the field about such corner cases, I think
the bug may not be important enough to risk breaking other things with a
back-patch.
UPDATE/SHARE couldn't occur as a subquery in a query with a non-SELECT
top-level operation. Symptoms included outright failure (as in report from
Mark Mielke) and silently neglecting to take the requested row locks.
Back-patch to 8.3, because the visible failure in the INSERT ... SELECT case
is a regression from 8.2. I'm a bit hesitant to back-patch further given the
lack of field complaints.
I never understood why initial authors GiST in pgsql choose so
stgrange signature for 'same' method:
bool *sameFn(Datum a, Datum b, bool* result)
instead of simple, logical
bool sameFn(Datum a, Datum b)
This change will break any existing GiST extension, so we still live with
it and will live.
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.
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.
we had several code paths where a physical tlist could be used for the input
to a Sort node, which is a dumb idea because any unneeded table columns will
increase the volume of data the sort has to push around.
(Unfortunately the easy-looking fix of calling disuse_physical_tlist during
make_sort_xxx doesn't work because in most cases we're already committed to
the current input tlist --- it's been marked with sort column numbers, or
we've built grouping column numbers using it, etc. The tlist has to be
selected properly at the calling level before we start constructing sort-col
information. This is easy enough to do, we were just failing to take the
point into consideration.)
Back-patch to 8.3. I believe the problem probably exists clear back to 7.4
when the physical tlist optimization was added, but I'm afraid to back-patch
further than 8.3 without a great deal more study than I want to put into it.
The code in this area has drifted a lot over time. The real-world importance
of these code paths is uncertain anyway --- I think in many cases we'd
probably prefer hash-based methods.
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.
it is trying to build a relcache entry for. This is an oversight in my 8.2
patch that tried to ensure we always took a lock on a relation before trying
to build its relcache entry. The implication is that if someone committed a
reindex of a critical system index at about the same time that some other
backend were starting up without a valid pg_internal.init file, the second one
might PANIC due to not seeing any valid version of the index's pg_class row.
Improbable case, but definitely not impossible.
results to contain uninitialized, unpredictable values. While this was okay
as far as the datatypes themselves were concerned, it's a problem for the
parser because occurrences of the "same" literal might not be recognized as
equal by datumIsEqual (and hence not by equal()). It seems sufficient to fix
this in the input functions since the only critical use of equal() is in the
parser's comparisons of ORDER BY and DISTINCT expressions.
Per a trouble report from Marc Cousin.
Patch all the way back. Interestingly, array_in did not have the bug before
8.2, which may explain why the issue went unnoticed for so long.
on win32, because the stat() function in the runtime cannot
be trusted to always update the st_size field.
Per report and research by Sergey Zubkovsky.
the columns it works with to be domains over the expected type, not just
exactly the expected type. In passing, fix ts_stat() the same way.
Per report from Markus Wollny.
currently support this because we must be able to build Vars referencing
join columns, and varattno is only 16 bits wide. Perhaps this should be
improved in future, but considering that it never came up before, I'm not
sure the problem is worth much effort. Per bug #4070 from Marcello
Ceschia.
The problem seems largely academic in 8.0 and 7.4, because they have
(different) O(N^2) performance issues with such wide joins, but
back-patch all the way anyway.
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
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.
eval_const_expressions needs to be passed the PlannerInfo ("root") structure,
because in some cases we want it to substitute values for Param nodes.
(So "constant" is not so constant as all that ...) This mistake partially
disabled optimization of unnamed extended-Query statements in 8.3: in
particular the LIKE-to-indexscan optimization would never be applied if the
LIKE pattern was passed as a parameter, and constraint exclusion depending
on a parameter value didn't work either.
The places that did, eg,
(statbuf.st_mode & S_IFMT) == S_IFDIR
were correct, but there is no good reason not to use S_ISDIR() instead,
especially when that's what the other 90% of our code does. The places
that did, eg,
(statbuf.st_mode & S_IFDIR)
were flat out *wrong* and would fail in various platform-specific ways,
eg a symlink could be mistaken for a regular file on most Unixen.
The actual impact of this is probably small, since the problem cases
seem to always involve symlinks or sockets, which are unlikely to be
found in the directories that PG code might be scanning. But it's
clearly trouble waiting to happen, so patch all the way back anyway.
(There seem to be no occurrences of the mistake in 7.4.)
the query result must be exactly one row (since we don't do this when there's
any GROUP BY). Therefore any ORDER BY or DISTINCT attached to the query is
useless and can be dropped. Aside from saving useless cycles, this protects
us against problems with matching the hacked-up tlist entries to sort clauses,
as seen in a bug report from Taiki Yamaguchi. We might need to work harder
if we ever try to optimize grouped queries with this approach, but this
solution will do for now.
knowledge up through any joins it participates in. We were doing that already
in some special cases but not in the general case. Also, defend against zero
row estimates for the input relations in cost_mergejoin --- this fix may have
eliminated the only scenario in which that can happen, but be safe. Per
report from Alex Solovey.
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.
ISO_8859-5 <-> MULE_INTERNAL conversion tables.
This was discovered when trying to convert a string containing those characters
from ISO_8859-5 to Windows-1251, because we use MULE_INTERNAL/KOI8R as an
intermediate encoding between those two.
While the missing "Yo" was just an omission in the conversion tables, there are
a few other characters like the "Numero" sign ("No" as a single character) that
exists in all the other cyrillic encodings (win1251, ISO_8859-5 and cp866), but
not in KOI8R. Added comments about that.
Patch by Sergey Burladyan. Back-patch to 7.4.
case where there is a match to the pattern overall but the user has specified
a parenthesized subexpression and that subexpression hasn't got a match.
An example is substring('foo' from 'foo(bar)?'). This should return NULL,
since (bar) isn't matched, but it was mistakenly returning the whole-pattern
match instead (ie, 'foo'). Per bug #4044 from Rui Martins.
This has been broken since the beginning; patch in all supported versions.
The old behavior was sufficiently inconsistent that it's impossible to believe
anyone is depending on it.
This accidentally failed to fail before 8.3, because the context we were
switching back to was long-lived anyway; but it sure looks risky as can be
now. Well spotted by Pavan Deolasee.
that are reported as "equal" by wcscoll() are checked to see if they really
are bitwise equal, and are sorted per strcmp() if not. We made this happen
a couple of years ago in the regular code path, but it unaccountably got
left out of the Windows/UTF8 case (probably brain fade on my part at the
time). As in the prior set of changes, affected users may need to reindex
indexes on textual columns.
Backpatch as far as 8.2, which is the oldest release we are still supporting
on Windows.
messages if the calling transaction aborts later on. Collapsing out line
pointer redirects is a done deal as soon as we complete the page update,
so syscache *must* be notified even if the VACUUM FULL as a whole doesn't
complete. To fix, add some functionality to inval.c to allow the pending
inval messages to be sent immediately while heap_page_prune is still
running. The implementation is a bit chintzy: it will only work in the
context of VACUUM FULL. But that's all we need now, and it can always be
extended later if needed. Per my trouble report of a week ago.
(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.
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.
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.
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.
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.
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).
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.
"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.
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.
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.
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).
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.
Dave Page with some modifications from me
"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.
Backpatch to 8.3 and 8.2 stable branches.
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.
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.
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.
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.
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.
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.
Tom Lane and Stephan Szabo
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.
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.
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.
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.
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.
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.
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.)
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.
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.
Per report from Gurjeet Singh.
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.
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.
Per report from Stephen Denne.
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.
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.
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.
whether to execute an immediate interrupt, rather than testing whether
LockWaitCancel() cancelled a lock wait. The old way misclassified the case
where we were blocked in ProcWaitForSignal(), and arguably would misclassify
any other future additions of new ImmediateInterruptOK states too. This
allows reverting the old kluge that gave LockWaitCancel() a return value,
since no callers care anymore. Improve comments in the various
implementations of PGSemaphoreLock() to explain that on some platforms, the
assumption that semop() exits after a signal is wrong, and so we must ensure
that the signal handler itself throws elog if we want cancel or die interrupts
to be effective. Per testing related to bug #3883, though this patch doesn't
solve those problems fully.
Perhaps this change should be back-patched, but since pre-8.3 branches aren't
really relying on autovacuum to respond to SIGINT, it doesn't seem critical
for them.
a double-pfree crash and another that effectively disabled size-based rotation
for CSV logs. Also suppress a memory leak and make some trivial cosmetic
improvements. Per bug #3901 from Chris Hoover and additional code-reading.
ri_FetchConstraintInfo, to avoid a query-duration memory leak when that
routine is called by RI_FKey_keyequal_upd_fk (which isn't executed in a
short-lived context). This problem was latent when the routine was added
in February, but it didn't become serious until the varvarlena patch made
it quite likely that the fields being examined would be "toasted" (ie, have
short headers). Per report from Stephen Denne.
TimestampTzPlusMilliseconds. An integer argument of more than INT_MAX/1000
milliseconds (ie, about 35 minutes) would provoke a wrong result, resulting
in incorrect enforcement of statement_timestamp values larger than that.
Bug was introduced in my rewrite of 2006-06-20, which fixed some other
overflow risks, but missed this one :-( Per report from Elein.
* The temporary enabling of the caller opcode here is to work around a
* bug in perl 5.10, which unkindly changed the way its Safe.pm works, without
* notice. It is quite safe, as caller is informational only, and in any case
* we only enable it while we load the 'strict' module.
regis. Correct the latter's oversight that a bracket-expression needs to be
terminated. Reduce the ereports to elogs, since they are now not expected to
ever be hit (thus addressing Alvaro's original complaint).
In passing, const-ify the string argument to RS_compile.
to format properly for the actually needed column width, instead of having
a hard-wired assumption about the longest command name length. Also make it
respond to the current screen width. In passing, const-ify the constant
table.
On other platforms it's better to let the Makefile handle it, but we want
the regression tests to be invokable without make on Windows. A batch
file would be a better solution, but no time for that before 8.3.
Per my discovery that this breaks testing under SELinux, and subsequent
discussion.
main code path for enlarging libpq's input buffer in one swoop when needing to
read a long data message. Without this, the code will double the buffer size,
read more data, notice it still hasn't got the whole message, and repeat till
it finally has a large enough buffer. Which wastes a lot of data-moving
effort and also memory (since malloc probably can't do anything very useful
with the freed-up smaller buffers). Not sure why this wasn't there already;
certainly the COPY data path is a place where we're quite likely to see long
data messages. I'm not backpatching though, since this is just a marginal
performance issue rather than a real bug.
subquery output column exactly once left-to-right. Although this is the case
in the original parser output, it might not be so after rewriting and
constant-folding, as illustrated by bug #3882 from Jan Mate. Instead
scan the subquery's target list to obtain needed per-column information;
this is duplicative of what the parser did, but only a couple dozen lines
need be copied, and we can clean up a couple of notational uglinesses.
Bug was introduced in 8.2 as part of revision of SubLink representation.
constraint, the constraint is renamed as well. This avoids inconsistent
situations that could confuse pg_dump (not to mention humans). We might at
some point provide ALTER TABLE RENAME CONSTRAINT as a more general solution,
but there seems no reason not to allow doing it this way too. Per bug #3854
and related discussions.
ParameterStatus message can be sent during COPY OUT: it's definitely
possible, since COPY from a SELECT subquery can trigger any user-defined
function.
in whichever context happens to be current during a call of an xml.c function,
use a dedicated context that will not go away until we explicitly delete it
(which we do at transaction end or subtransaction abort). This makes recovery
after an error much simpler --- we don't have to individually delete the data
structures created by libxml. Also, we need to initialize and cleanup libxml
only once per transaction (if there's no error) instead of once per function
call, so it should be a bit faster. We'll need to keep an eye out for
intra-transaction memory leaks, though. Alvaro and Tom.
This is to avoid uselessly requiring superuser permissions to restore
the dump without errors. Pretty grotty, but no better alternative seems
available, at least not in the near term.
we need to be able to swallow NOTICE messages, and potentially also
ParameterStatus messages (although the latter would be a bit weird),
without exiting COPY OUT state. Fix it, and adjust the protocol documentation
to emphasize the need for this. Per off-list report from Alexander Galler.
its second pass over the table. It has to start at block zero, else the
"merge join" logic for detecting which TIDs are already in the index
doesn't work. Hence, extend heapam.c's API so that callers can enable or
disable syncscan. (I put in an option to disable buffer access strategy,
too, just in case somebody needs it.) Per report from Hannes Dorbath.
Therefore we must xmlCleanupParser(), or we risk leaving behind
dangling pointers to whatever memory context is current when xml_init()
is called. This seems to fix bug #3860, though we might still want
the more invasive solution being worked on by Alvaro.
constraint yields TRUE for every row of its table, only that it does not
yield FALSE (a NULL result isn't disallowed). This breaks a couple of
implications that would be true in two-valued logic. I had put in one such
mistake in an 8.2.5 patch: foo IS NULL doesn't refute a strict operator
on foo. But there was another in the original 8.2 release: NOT foo doesn't
refute an expression whose truth would imply the truth of foo.
Per report from Rajesh Kumar Mallah.
To preserve the ability to do constraint exclusion with one partition
holding NULL values, extend relation_excluded_by_constraints() to check
for attnotnull flags, and add col IS NOT NULL expressions to the set of
constraints we hope to refute.
checking of argument compatibility right; although the problem is only exposed
with multiple-input aggregates in which some arguments are polymorphic and
some are not. Per bug #3852 from Sokolov Yura.
for unhandled clause types ought to be 0.5, not 1.0. I fear I introduced
this silliness due to misreading the intent of the very-poorly-structured
code that was there when we inherited the file from Berkeley. The lack
of sanity in this behavior was exposed by an example from Sim Zacks.
(Arguably this is a bug fix and should be back-patched, but I'm a bit
hesitant to introduce a possible planner behavior change in the back
branches; it might detune queries that worked acceptably in the past.)
While at it, make estimation for DistinctExpr do something marginally
realistic, rather than just defaulting.
clauseless joins of relations that have unexploited join clauses. Rather
than looking at every other base relation in the query, the correct thing is
to examine the other relations in the "initial_rels" list of the current
make_rel_from_joinlist() invocation, because those are what we actually have
the ability to join against. This might be a subset of the whole query in
cases where join_collapse_limit or from_collapse_limit or full joins have
prevented merging the whole query into a single join problem. This is a bit
untidy because we have to pass those rels down through a new PlannerInfo
field, but it's necessary. Per bug #3865 from Oleg Kharin.
finish archiving everything (when there's no error), and to eliminate various
hazards as best we can. This fixes a previous 8.3 patch that caused the
postmaster to kill and then restart the archiver during shutdown (!?).
The new behavior is that the archiver is allowed to run unmolested until
the bgwriter has exited; then it is sent SIGUSR2 to tell it to do a final
archiving cycle and quit. We only SIGQUIT the archiver if we want a panic
stop; this is important since SIGQUIT will also be sent to any active
archive_command. The postmaster also now doesn't SIGQUIT the stats collector
until the bgwriter is done, since the bgwriter can send stats messages in 8.3.
The postmaster will not exit until both the archiver and stats collector are
gone; this provides some defense (not too bulletproof) against conflicting
archiver or stats collector processes being started by a new postmaster
instance. We continue the prior practice that the archiver will check
for postmaster death immediately before issuing any archive_command; that
gives some additional protection against conflicting archivers.
Also, modify the archiver process to notice SIGTERM and refuse to issue any
more archive commands if it gets it. The postmaster doesn't ever send it
SIGTERM; we assume that any such signal came from init and is a notice of
impending whole-system shutdown. In this situation it seems imprudent to try
to start new archive commands --- if they aren't extremely quick they're
likely to get SIGKILL'd by init.
All per discussion.
of poorer planning in 8.3 than 8.2:
1. After pushing a constant across an outer join --- ie, given
"a LEFT JOIN b ON (a.x = b.y) WHERE a.x = 42", we can deduce that b.y is
sort of equal to 42, in the sense that we needn't fetch any b rows where
it isn't 42 --- loop to see if any additional deductions can be made.
Previous releases did that by recursing, but I had mistakenly thought that
this was no longer necessary given the EquivalenceClass machinery.
2. Allow pushing constants across outer join conditions even if the
condition is outerjoin_delayed due to a lower outer join. This is safe
as long as the condition is strict and we re-test it at the upper join.
3. Keep the outer-join clause even if we successfully push a constant
across it. This is *necessary* in the outerjoin_delayed case, but
even in the simple case, it seems better to do this to ensure that the
join search order heuristics will consider the join as reasonable to
make. Mark such a clause as having selectivity 1.0, though, since it's
not going to eliminate very many rows after application of the constant
condition.
4. Tweak have_relevant_eclass_joinclause to report that two relations
are joinable when they have vars that are equated to the same constant.
We won't actually generate any joinclause from such an EquivalenceClass,
but again it seems that in such a case it's a good idea to consider
the join as worth costing out.
5. Fix a bug in select_mergejoin_clauses that was exposed by these
changes: we have to reject candidate mergejoin clauses if either side was
equated to a constant, because we can't construct a canonical pathkey list
for such a clause. This is an implementation restriction that might be
worth fixing someday, but it doesn't seem critical to get it done for 8.3.
constant ORDER/GROUP BY entries properly:
http://archives.postgresql.org/pgsql-hackers/2001-04/msg00457.php
The original solution to that was in fact no good, as demonstrated by
today's report from Martin Pitt:
http://archives.postgresql.org/pgsql-bugs/2008-01/msg00027.php
We can't use the column-number-reference format for a constant that is
a resjunk targetlist entry, a case that was unfortunately not thought of
in the original discussion. What we can do instead (which did not work
at the time, but does work in 7.3 and up) is to emit the constant with
explicit ::typename decoration, even if it otherwise wouldn't need it.
This is sufficient to keep the parser from thinking it's a column number
reference, and indeed is probably what the user must have done to get
such a thing into the querytree in the first place.
and CLUSTER) execute as the table owner rather than the calling user, using
the same privilege-switching mechanism already used for SECURITY DEFINER
functions. The purpose of this change is to ensure that user-defined
functions used in index definitions cannot acquire the privileges of a
superuser account that is performing routine maintenance. While a function
used in an index is supposed to be IMMUTABLE and thus not able to do anything
very interesting, there are several easy ways around that restriction; and
even if we could plug them all, there would remain a risk of reading sensitive
information and broadcasting it through a covert channel such as CPU usage.
To prevent bypassing this security measure, execution of SET SESSION
AUTHORIZATION and SET ROLE is now forbidden within a SECURITY DEFINER context.
Thanks to Itagaki Takahiro for reporting this vulnerability.
Security: CVE-2007-6600
are shared with Tcl, since it's their code to begin with, and the patches
have been copied from Tcl 8.5.0. Problems:
CVE-2007-4769: Inadequate check on the range of backref numbers allows
crash due to out-of-bounds read.
CVE-2007-4772: Infinite loop in regex optimizer for pattern '($|^)*'.
CVE-2007-6067: Very slow optimizer cleanup for regex with a large NFA
representation, as well as crash if we encounter an out-of-memory condition
during NFA construction.
Part of the response to CVE-2007-6067 is to put a limit on the number of
states in the NFA representation of a regex. This seems needed even though
the within-the-code problems have been corrected, since otherwise the code
could try to use very large amounts of memory for a suitably-crafted regex,
leading to potential DOS by driving the system into swap, activating a kernel
OOM killer, etc.
Although there are certainly plenty of ways to drive the system into effective
DOS with poorly-written SQL queries, these problems seem worth treating as
security issues because many applications might accept regex search patterns
from untrustworthy sources.
Thanks to Will Drewry of Google for reporting these problems. Patches by Will
Drewry and Tom Lane.
Security: CVE-2007-4769, CVE-2007-4772, CVE-2007-6067
in the current backend for the target table. These operations move tuples
around and would thus invalidate the TIDs stored in the trigger event records.
(We need not worry about events in other backends, since acquiring exclusive
lock should be enough to ensure there aren't any.) It might be sufficient
to forbid only the table-rewriting variants of ALTER TABLE, but in the absence
of any compelling use-case, let's just be safe and simple. Per follow-on
investigation of bug #3847, though this is not actually the same problem
reported therein.
Possibly this should be back-patched, but since the case has never been
reported from the field, I didn't bother.
a trigger's target table. The rowtype could change from one call to the
next, so cope in such cases, while avoiding doing repetitive catalog lookups.
Per bug #3847 from Mark Reid.
Backpatch to 8.2.x. Likely this fix should go further back, but I can't test
it because I no longer have a machine with a pre-2.5 Python installation.
(Maybe we should rethink that idea about not supporting Python 2.5 in the
older branches.)
since these seem to happen after all in corrupted indexes. Make sure we
supply the index name in all cases, and provide relevant block numbers where
available. Also consistently identify the index name as such.
Back-patch to 8.2, in hopes that this might help Mason Hale figure out his
problem.
Applied patch send by ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> to fix bug in connect statement if user name is a variable.
Also fixed test case that didn't detect this.
were reporting ERROR for interactive assignments and LOG for other cases,
some were saying nothing for non-interactive cases, and a few did yet other
things. Make them use a new function GUC_complaint_elevel() to establish
a reasonably uniform policy about how to report. There are still a few
edge cases such as assign_search_path(), but it's much better than before.
Per gripe from Devrim Gunduz and subsequent discussion.
As noted by Alvaro, it'd be better to fold these custom messages into the
standard "invalid parameter value" complaint from guc.c, perhaps as the DETAIL
field. However that will require more redesign than seems prudent for 8.3.
This is a relatively safe, low-impact change that we can afford to risk now.
COPY. We need a restriction here because when the delimiter occurs as a
data character, it is emitted with a backslash, and that will only work
as desired if CopyReadAttributesText() will interpret the backslash sequence
as representing the second character literally. This is currently untrue
for 'b', 'f', 'n', 'r', 't', 'v', 'x', and octal digits. For future-proofing
and simplicity of explanation, it seems best to disallow a-z and 0-9.
We must also disallow dot, since "\." by itself would look like copy EOF.
Note: "\N" is by default the null print string, so N would also cause a
problem, but that is already tested for.
CopyAttributeOutText(), so that control characters are converted to the
C-style escape sequences even if they happen to be equal to the column
delimiter (as is true by default for tab, for example). Oversight in my
previous patch to restore pre-8.3 behavior of COPY OUT escaping. Per report
from Tomas Szepe.
print the index key variable or expression for that column. It was mistakenly
printing ASC/DESC/NULLS FIRST/NULLS LAST decoration too --- and not only for
the target column, but all columns. Someday we should have an option to
extract that info (and the opclass decoration as well) for a single index
column ... but today is not that day. Per bug #3829 and subsequent
discussion.
The zero-point case is sensible so far as the data structure is concerned,
so maybe we ought to allow it sometime; but right now the textual input
routines for these types don't allow it, and it seems that not all the
functions for the types are prepared to cope.
Report and patch by Merlin Moncure.
psql's \d commands and other uses of printQuery(). Previously we would pass
these strings through gettext() and then send them to the server as literals
in the SQL query. But the code was not set up to handle doubling of quotes in
the strings, causing failure if a translation attempted to use the wrong kind
of quote marks, as indeed is now the case for (at least) the French
translation of \dFp. Another hazard was that gettext() would translate to
whatever encoding was implied by the client's LC_CTYPE setting, which might be
different from the client_encoding setting, which would probably cause the
server to reject the query as mis-encoded. The new arrangement is to send the
untranslated ASCII strings to the server, and do the translations inside
printQuery() after the query results come back. Per report from Guillaume
Lelarge and subsequent discussion.
useful and confuses people who think it is the same as -U. (Eventually
we might want to re-introduce it as being an alias for -U, but that should
not happen until the switch has actually not been there for a few releases.)
Likewise in pg_dump and pg_restore. Per gripe from Robert Treat and
subsequent discussion.
with the logged event. CSV logs are now a first-class citizen along plain
text logs in that they carry much of the same information.
Per complaint from depesz on bug #3799.
PQconnectionNeedsPassword function that tells the right thing for whether to
prompt for a password, and improve PQconnectionUsedPassword so that it checks
whether the password used by the connection was actually supplied as a
connection argument, instead of coming from environment or a password file.
Per bug report from Mark Cave-Ayland and subsequent discussion.
the two join variables at both ends: not only trailing rows that need not be
scanned because there cannot be a match on the other side, but initial rows
that will be scanned without possibly having a match. This allows a more
realistic estimate of startup cost to be made, per recent pgsql-performance
discussion. In passing, fix a couple of bugs that had crept into
mergejoinscansel: it was not quite up to speed for the task of estimating
descending-order scans, which is a new requirement in 8.3.
indexable-clauses list for a btree index. Formerly it just Asserted that
all such clauses were opclauses, but that's no longer true in 8.3.
Per bug #3796 from Matthias Schoeneich.
namely that \r, \n, \t, \b, \f, \v are dumped as those two-character
representations rather than a backslash and the literal control character.
I had made it do the other to save some code, but this was ill-advised,
because dump files in which these characters appear literally are prone to
newline mangling. Fortunately, doing it the old way should only cost a few
more lines of code, and not slow down the copy loop materially.
Per bug #3795 from Lou Duchez.
constraint status of copied indexes (bug #3774), as well as various other
small bugs such as failure to pstrdup when needed. Allow INCLUDING INDEXES
indexes to be merged with identical declared indexes (perhaps not real useful,
but the code is there and having it not apply to LIKE indexes seems pretty
unorthogonal). Avoid useless work in generateClonedIndexStmt(). Undo some
poorly chosen API changes, and put a couple of routines in modules that seem
to be better places for them.
implicit loading of modules, thereby breaking Safe rules.
We compile and call a tiny perl function on trusted interpreter init, after which
the problem does not occur.
but no database changes have been made since the last CommandCounterIncrement.
This should result in a significant improvement in the number of "commands"
that can typically be performed within a transaction before hitting the 2^32
CommandId size limit. In particular this buys back (and more) the possible
adverse consequences of my previous patch to fix plan caching behavior.
The implementation requires tracking whether the current CommandCounter
value has been "used" to mark any tuples. CommandCounter values stored into
snapshots are presumed not to be used for this purpose. This requires some
small executor changes, since the executor used to conflate the curcid of
the snapshot it was using with the command ID to mark output tuples with.
Separating these concepts allows some small simplifications in executor APIs.
Something for the TODO list: look into having CommandCounterIncrement not do
AcceptInvalidationMessages. It seems fairly bogus to be doing it there,
but exactly where to do it instead isn't clear, and I'm disinclined to mess
with asynchronous behavior during late beta.
plan before the effects of DDL executed in an immediately prior SPI operation
had been absorbed. Per report from Chris Wood.
This patch has an unpleasant side effect of causing the number of
CommandCounterIncrement()s done by a typical plpgsql function to
approximately double. Amelioration of the consequences of that
will be undertaken in a separate patch.
reloading of operator class information on each use of LookupOpclassInfo.
Had this been in place a year ago, it would have helped me find a bug
in the then-new 'operator family' code. Now that we have a build farm
member testing CLOBBER_CACHE_ALWAYS on a regular basis, it seems worth
expending a little bit of effort here.
inappropriately generic-sounding names. This is more or less free since
we already forced initdb for the next beta, and it may prevent confusion or
name conflicts (particularly at the C-global-symbol level) down the road.
Per my proposal yesterday.
by short-circuiting schema search path and ambiguous-operator resolution
computations. Remarkably, this buys as much as 45% speedup of repetitive
simple queries that involve operators that are not an exact match to the
input datatypes. It should be marginally faster even for exact-match
cases, though I've not had success in proving an improvement in benchmark
tests. Per report from Guillame Smet and subsequent discussion.
is sane (eg, EXIT argument or first part of a qualified identifier), and cause
more-closely-nested record variables to take precedence over outer block
labels. This cures the breakage exhibited by Marko Kreen that 8.3 no longer
allowed a function's name to be used for a variable within the function, and
makes plpgsql's handling of block labels more like Oracle's. It's important
to fix this now because we are documenting the use of block labels as variable
qualifiers for the first time in 8.3.
useful consequence of the former liberal implicit casting to text;
namely that you can feed non-string values to quote_literal() and get
unsurprising results. Per discussion.
to a UNION, CASE, or related construct are of the same domain type. The
main part of this routine smashes domains to their base types, which seems
necessary because the logic involves TypeCategory() and IsPreferredType(),
neither of which work usefully on domains. However, we can add a first
pass that just detects whether all the inputs are exactly the same type,
and if so accept that without question (so long as it's not UNKNOWN).
Per recent gripe from Dean Rasheed.
In passing, remove some tests for InvalidOid, which have clearly been dead
code for quite some time now, because getBaseType() would fail on that input.
Also, clarify the manual's not-very-precise description of the existing
algorithm's behavior.
subtlety that this function only returns a null terminator if it's
fed input that includes one; which, in the usage here, it's not.
This probably fixes bugs reported by Thomas Haegi.
clauselist_selectivity skip some analysis that's useless when there's only
one clause in the given list. Actually this can win even for not-so-simple
queries, because we also apply clauselist_selectivity to sublists such as the
quals matching an index; which are likely to have only a single entry even
when the total query is quite complicated.
if the locale has the thousands separator as "". This now matches the
to_char and psql numericlocale behavior. (Previously this data type was
basically useless for such setups.)
where rtoffset == 0. In that case there is no need to change Var nodes,
and since filling in unset opfuncid fields is always safe, scribbling on the
input tree to that extent is not objectionable. This brings the cost of this
operation back down to what it was in 8.2 for simple queries. Per
investigation of performance gripe from Guillaume Smet.
where the EquivalenceClass machinery is unable to deduce anything more from a
simple "var = const" qual clause. There are probably some more cases where
this could be done, but this seems to take care of most of the added overhead
for simple queries. Per gripe from Guillaume Smet.
In passing, fix a problem that was exposed by this change:
reconsider_outer_join_clause and friends were passing the wrong relids to
build_implied_join_equality, resulting in RestrictInfos with the wrong
required_relids. This mistake was masked in typical cases since the bogus
RestrictInfos would never have escaped from the EquivalenceClass machinery,
but I think there might be corner cases involving "broken" ECs where there
would have been a visible failure even without the new optimization. In any
case the code was certainly not operating as intended.
opfuncid of an OpExpr initially, considering that it has the information
at hand already. We'll still treat opfuncid as a cache rather than a
guaranteed-valid value, but this change saves one more syscache lookup
in the normal code path.
OpExpr and related nodes. We're going to have to set the opfuncid of
such nodes eventually (if we haven't already), so we might as well
exploit the opportunity to cache the function OID. Buys back some
of the extra planner overhead noted by Guillaume Smet, though I still
need to fool with equivclass.c to really respond to that.
"bool" into plperl.c. This has always been a hazard since Perl allows a
platform-specific choice to define bool as int rather than char, but
evidently this didn't happen on any platform we support ... until OS X 10.5.
Per report from Brandon Maust.
Back-patch as far as 8.0 --- a bit arbitrary, but it seems unlikely anyone
will be trying to port 7.x onto new platforms.
happened to be right up against the end of memory, per report from
Matt Magoffin. While at it, avoid useless multiple copying of string
by not depending on xmlStrncatNew.
Allow tag and entity names that follow XML rules. Provide for hexadecimal
as well as decimal numeric entities. Adjust code names to coincide with
new descriptions.
GetMemoryChunkSpace, not just the palloc request size. This brings the
allocatedMemory counter close enough to reality (as measured by
MemoryContextStats printouts) that I think we can get rid of the arbitrary
factor-of-2 adjustment that was put into the code initially. Given the
sensitivity of GIN build to work memory size, not using as much of work
memory as we're allowed to seems a pretty bad idea.
Else, in a 64-bit machine with maintenance_work_mem set to above 4Gb,
the counter overflows and we never recognize having reached the
maintenance_work_mem limit. I believe this explains out-of-memory
failure recently reported by Sean Davis.
This is a bug, so backpatch to 8.2.
it failed for splits of non-leaf pages because in such pages the first
data key on a page is suppressed, and so we can't just copy the first
key from the right page to reconstitute the left page's high key.
Problem found by Koichi Suzuki, patch by Heikki.
