> I've now tested this patch at home w/ 8.2HEAD and it seems to fix the
> bug. I plan on testing it under 8.1.2 at work tommorow with
> mod_auth_krb5, etc, and expect it'll work there. Assuming all goes
> well and unless someone objects I'll forward the patch to -patches.
> It'd be great to have this fixed as it'll allow us to use Kerberos to
> authenticate to phppgadmin and other web-based tools which use
> Postgres.
While playing with this patch under 8.1.2 at home I discovered a
mistake in how I manually applied one of the hunks to fe-auth.c.
Basically, the base code had changed and so the patch needed to be
modified slightly. This is because the code no longer either has a
freeable pointer under 'name' or has 'name' as NULL.
The attached patch correctly frees the string from pg_krb5_authname
(where it had been strdup'd) if and only if pg_krb5_authname returned
a string (as opposed to falling through and having name be set using
name = pw->name;). Also added a comment to this effect.
Backpatch to 8.1.X.
Stephen Frost
/dev/tty, but it isn't a device file and doesn't work as expected.
This fixes a known bug where psql does not prompt for a password on some
Win32 systems.
Backpatch to 8.1.X.
Robert Kinberg
---------------------------------------------------------------------------
> True, but they're not being used where you'd expect. This seems to be
> something to do with the fact that it's not pg_authid which is being
> accessed, but rather the view pg_roles.
I looked into this and it seems the problem is that the view doesn't
get flattened into the main query because of the has_nullable_targetlist
limitation in prepjointree.c. That's triggered because pg_roles has
'********'::text AS rolpassword
which isn't nullable, meaning it would produce wrong behavior if
referenced above the outer join.
Ultimately, the reason this is a problem is that the planner deals only
in simple Vars while processing joins; it doesn't want to think about
expressions. I'm starting to think that it may be time to fix this,
because I've run into several related restrictions lately, but it seems
like a nontrivial project.
In the meantime, reducing the LEFT JOIN to pg_roles to a JOIN as per
Peter's suggestion seems like the best short-term workaround.
then modified within the same transaction. The code was using a linked list
of active PLpgSQL_expr structs, which was OK when it was written because
plpgsql never released any parse data structures for the life of the backend.
But since Neil fixed plpgsql's memory management, elements of the linked list
could be freed, leading to crash when the list is chased. Per report and test
case from Kris Jurka.
by decompiling the typdefaultbin expression, not just printing the typdefault
text which may be out-of-date or assume the wrong schema search path. (It's
the same hazard as for adbin vs adsrc in column defaults.) The catalogs.sgml
spec for pg_type implies that the correct procedure is to look to
typdefaultbin first and consider typdefault only if typdefaultbin is NULL.
I made dumping of both domains and base types do that, even though in the
current backend code typdefaultbin is always correct for domains and
typdefault for base types --- might as well try to future-proof it a little.
Per bug report from Alexander Galler.
in leaking memory when invoking a PL/Python procedure that raises an
exception. Unfortunately this still leaks memory, but at least the
largest leak has been plugged.
This patch also fixes a reference counting mistake in PLy_modify_tuple()
for 8.0, 8.1 and HEAD: we don't actually own a reference to `platt', so
we shouldn't Py_DECREF() it.
we are not holding a buffer content lock; where it was, InterruptHoldoffCount
is positive and so we'd not respond to cancel signals as intended. Also
add missing vacuum_delay_point() call in btvacuumcleanup. This should fix
complaint from Evgeny Gridasov about failure to respond to SIGINT/SIGTERM
in a timely fashion (bug #2257).
Var referencing the subselect output. While this case could possibly be made
to work, it seems not worth expending effort on. Per report from Magnus
Naeslund(f).
id (CVE-2006-0553). Also fix related bug in SET SESSION AUTHORIZATION that
allows unprivileged users to crash the server, if it has been compiled with
Asserts enabled. The escalation-of-privilege risk exists only in 8.1.0-8.1.2.
However, the Assert-crash risk exists in all releases back to 7.3.
Thanks to Akio Ishida for reporting this problem.
regardless of the current schema search path. Since CREATE OPERATOR CLASS
only allows one default opclass per datatype regardless of schemas, this
should have minimal impact, and it fixes problems with failure to find a
desired opclass while restoring dump files. Per discussion at
http://archives.postgresql.org/pgsql-hackers/2006-02/msg00284.php.
Remove now-redundant-or-unused code in typcache.c and namespace.c,
and backpatch as far as 8.0.
it later. This fixes a problem where EXEC_BACKEND didn't have progname
set, causing a segfault if log_min_messages was set below debug2 and our
own snprintf.c was being used.
Also alway strdup() progname.
Backpatch to 8.1.X and 8.0.X.
to avoid sharing substructure with the lower-level indexquals. This is
currently only an issue if there are SubPlans in the indexquals, which is
uncommon but not impossible --- see bug #2218 reported by Nicholas Vinen.
We use the same kluge for indexqual vs indexqualorig in the index scans
themselves ... would be nice to clean this up someday.
requested sort order. It was assuming that build_index_pathkeys always
generates a pathkey per index column, which was not true if implied equality
deduction had determined that two index columns were effectively equated to
each other. Simplest fix seems to be to install an option that causes
build_index_pathkeys to support this behavior as well as the original one.
Per report from Brian Hirt.
memory in the executor's per-query memory context. It also inefficient:
it invokes get_call_result_type() and TupleDescGetAttInMetadata() for
every call to return_next, rather than invoking them once (per PL/Perl
function call) and memoizing the result.
This patch makes the following changes:
- refactor the code to include all the "per PL/Perl function call" data
inside a single struct, "current_call_data". This means we don't need to
save and restore N pointers for every recursive call into PL/Perl, we
can just save and restore one.
- lookup the return type metadata needed by plperl_return_next() once,
and then stash it in "current_call_data", so as to avoid doing the
lookup for every call to return_next.
- create a temporary memory context in which to evaluate the return
type's input functions. This memory context is reset for each call to
return_next.
The patch appears to fix the memory leak, and substantially reduces
the overhead imposed by return_next.
While we normally prefer the notation "foo.*" for a whole-row Var, that does
not work at SELECT top level, because in that context the parser will assume
that what is wanted is to expand the "*" into a list of separate target
columns, yielding behavior different from a whole-row Var. We have to emit
just "foo" instead in that context. Per report from Sokolov Yura.
because pqSendSome will absorb input data anytime it'd be forced to block.
Avoiding a kernel call per PQputCopyData call helps COPY speed materially.
Alon Goldshuv
to try to create a log segment file concurrently, but the code erroneously
specified O_EXCL to open(), resulting in a needless failure. Before 7.4,
it was even a PANIC condition :-(. Correct code is actually simpler than
what we had, because we can just say O_CREAT to start with and not need a
second open() call. I believe this accounts for several recent reports of
hard-to-reproduce "could not create file ...: File exists" errors in both
pg_clog and pg_subtrans.
temp table not only our own process' tables. It's not real important
since vacuum.c will skip temp tables anyway, but might as well make the
code do what it claims to do.
index's support-function cache (in index_getprocinfo). Since none of that
data can change for an index that's in active use, it seems sufficient to
treat all open indexes the same way we were treating "nailed" system indexes
--- that is, just re-read the pg_class row and leave the rest of the relcache
entry strictly alone. The pg_class re-read might not be strictly necessary
either, but since the reltablespace and relfilenode can change in normal
operation it seems safest to do it. (We don't support changing any of the
other info about an index at all, at the moment.)
Back-patch as far as 8.0. It might be possible to adapt the patch to 7.4,
but it would take more work than I care to expend for such a low-probability
problem. 7.3 is out of luck for sure.
occurs when it tries to heap_open pg_tablespace. When control returns to
smgrcreate, that routine will be holding a dangling pointer to a closed
SMgrRelation, resulting in mayhem. This is of course a consequence of
the violation of proper module layering inherent in having smgr.c call
a tablespace command routine, but the simplest fix seems to be to change
the locking mechanism. There's no real need for TablespaceCreateDbspace
to touch pg_tablespace at all --- it's only opening it as a way of locking
against a parallel DROP TABLESPACE command. A much better answer is to
create a special-purpose LWLock to interlock these two operations.
This drops TablespaceCreateDbspace quite a few layers down the food chain
and makes it something reasonably safe for smgr to call.
This is utterly insignificant in normal operation, but it becomes a
problem during cache inval stress testing. The original coding in fact
had no leak --- the 8.0 List rewrite created the issue. I wonder whether
list_concat should pfree the discarded header?
files: avoid creating stats hashtable entries for tables that aren't being
touched except by vacuum/analyze, ensure that entries for dropped tables are
removed promptly, and tweak the data layout to avoid storing useless struct
padding. Also improve the performance of pgstat_vacuum_tabstat(), and make
sure that autovacuum invokes it exactly once per autovac cycle rather than
multiple times or not at all. This should cure recent complaints about 8.1
showing much higher stats I/O volume than was seen in 8.0. It'd still be a
good idea to revisit the design with an eye to not re-writing the entire
stats dataset every half second ... but that would be too much to backpatch,
I fear.
discarded by cache flush while still in use. This is a minimal patch that
just copies the tupdesc anywhere it could be needed across a flush. Applied
to back branches only; Neil Conway is working on a better long-term solution
for HEAD.
a va_list. Christof Petig's previous patch made this change, but neglected
to update ecpglib/descriptor.c, resulting in a compiler warning (and a
likely runtime crash) on AMD64 and PPC.
our own command (or more generally, xmin = our xact and cmin >= current
command ID) should not be seen as good. Else we may try to update rows
we already updated. This error was inserted last August while fixing the
even bigger problem that the old coding wouldn't see *any* tuples inserted
by our own transaction as good. Per report from Euler Taveira de Oliveira.
one argument at a time and then inserting the argument into a Python
list via PyList_SetItem(). This "steals" the reference to the argument:
that is, the reference to the new list member is now held by the Python
list itself. This works fine, except if an elog occurs. This causes the
function's PG_CATCH() block to be invoked, which decrements the
reference counts on both the current argument and the list of arguments.
If the elog happens to occur during the second or subsequent iteration
of the loop, the reference count on the current argument will be
decremented twice.
The fix is simple: set the local pointer to the current argument to NULL
immediately after adding it to the argument list. This ensures that the
Py_XDECREF() in the PG_CATCH() block doesn't double-decrement.
operator names. This is needed when dumping operator definitions that have
COMMUTATOR (or similar) links to operators in other schemas.
Apparently Daniel Whitter is the first person ever to try this :-(
use it. While it normally has been opened earlier during btree index
build, testing shows that it's possible for the link to be closed again
if an sinval reset occurs while the index is being built.
dead and have become unreferenced. Before 8.1, such members were left
for AtEOXact_CatCache() to clean up, but now AtEOXact_CatCache isn't
supposed to have anything to do. In an assert-enabled build this bug
leads to an assertion failure at transaction end, but in a non-assert
build the dead member is effectively just a small memory leak.
Per report from Jeremy Drake.
rather than elog(FATAL), when there is no more room in ShmemBackendArray.
This is a security issue since too many connection requests arriving close
together could cause the postmaster to shut down, resulting in denial of
service. Reported by Yoshiyuki Asaba, fixed by Magnus Hagander.
the relation but it finds a pre-existing valid buffer. The buffer does not
correspond to any page known to the kernel, so we *must* do smgrextend to
ensure that the space becomes allocated. The 7.x branches all do this
correctly, but the corner case got lost somewhere during 8.0 bufmgr rewrites.
(My fault no doubt :-( ... I think I assumed that such a buffer must be
not-BM_VALID, which is not so.)
from Andrus Moor. The former state-machine-style coding wasn't actually
doing much except obscuring the control flow, and it didn't extend
readily to fix this case, so I just took it out. Also, add a
YY_FLUSH_BUFFER call to ensure the lexer is reset correctly if the
previous scan failed partway through the file.
selection of a field from the result of a function returning RECORD.
I believe this case is new in 8.1; it's due to the addition of OUT parameters.
Per example from Michael Fuhr.
if (c == '\\' && cstate->line_buf.len == 0)
The problem with that is the because of the input and _output_
buffering, cstate->line_buf.len could be zero even if we are not on the
first character of a line. In fact, for a typical line, it is zero for
all characters on the line. The proper solution is to introduce a
boolean, first_char_in_line, that we set as we enter the loop and clear
once we process a character.
I have restructured the line-reading code in copy.c by:
o merging the CSV/non-CSV functions into a single function
o used macros to centralize and clarify the buffering code
o updated comments
o renamed client_encoding_only to encoding_embeds_ascii
o added a high-bit test to the encoding_embeds_ascii test for
performance
o in CSV mode, allow a backslash followed by a non-period to
continue being processed as a data value
There should be no performance impact from this patch because it is
functionally equivalent. If you apply the patch you will see copy.c is
much clearer in this area now and might suggest additional
optimizations.
I have also attached a 8.1-only patch to fix the CSV \. handling bug
with no code restructuring.
See:
Subject: [HACKERS] bugs with certain Asian multibyte charsets
From: Tatsuo Ishii <ishii@sraoss.co.jp>
To: pgsql-hackers@postgresql.org
Date: Sat, 24 Dec 2005 18:25:33 +0900 (JST)
for more details.
Also make the code more robust by searching for target encoding
in the internal charset map.
Problem reported by Sagi Bashari on 2005/12/21.
See "[BUGS] BUG #2120: Crash when doing UTF8<->ISO_8859_8 encoding conversion"
on pgsql-bugs list for more details.
Back patch for 8.1_STABLE.
differ by more than the last directory component. Instead of insisting
that they match up to the last component, accept whatever common prefix
they have, and try to replace the non-matching part of bin_path with
the non-matching part of target_path in the actual executable's path.
In one way this is tighter than the old code, because it insists on
a match to the part of bin_path we want to substitute for, rather than
blindly stripping one directory component from the executable's path.
Per gripe from Martin Pitt and subsequent discussion.
equal: if strcoll claims two strings are equal, check it with strcmp, and
sort according to strcmp if not identical. This fixes inconsistent
behavior under glibc's hu_HU locale, and probably under some other locales
as well. Also, take advantage of the now-well-defined behavior to speed up
texteq, textne, bpchareq, bpcharne: they may as well just do a bitwise
comparison and not bother with strcoll at all.
NOTE: affected databases may need to REINDEX indexes on text columns to be
sure they are self-consistent.
messages, when client attempts to execute these outside a transaction (start
one) or in a failed transaction (reject message, except for COMMIT/ROLLBACK
statements which we can handle). Per report from Francisco Figueiredo Jr.
that simplify_boolean_equality() may leave behind. This is only relevant
if the user writes something a bit silly, like CASE x=y WHEN TRUE THEN.
Per example from Michael Fuhr; may or may not explain bug #2106.
I have the problem, when building by MS-VC6.
An error occurs in the 8.1.0 present source codes.
nmake -f win32.mak
..\..\port\getaddrinfo.c(244) : error C2065: 'WSA_NOT_ENOUGH_MEMORY'
..\..\port\getaddrinfo.c(342) : error C2065: 'WSATYPE_NOT_FOUND'
This is used by winsock2.h. However, Construction of a windows base is
winsock.h.
Then, Since MinGW has special environment, this is right. but, it is not
found in VC6.
Furthermore, in getaddrinfo.c, IPV6-API is used by
LoadLibraryA("ws2_32");
Referring to of dll the external memory generates this violation by VC6
specification.
I considered whether the whole should have been converted into winsock2.
However, Now, DLL of MinGW creation operates wonderfully as it is.
That's right, it has pliability by replacement of simple DLL.
Then, I propose the system using winsock(non IPV6) in construction of
VC6.
Hiroshi Saito
clauses even if it's an outer join. This is a corner case since such
clauses could only arise from weird OUTER JOIN ON conditions, but worth
fixing. Per example from Ron at cheapcomplexdevices.com.
incorrect implementation of argument reordering, arbitrary limit of output
size for sprintf and fprintf, willingness to access more bytes than "%.Ns"
specification allows, wrong formatting of LONGLONG_MIN, various field-padding
bugs and omissions. I believe it now accurately implements a subset of
the Single Unix Spec requirements (remaining unimplemented features are
documented, too). Bruce Momjian and Tom Lane.
Map them to a single day, so '30 hours' is 'AM'.
Have to_char(interval) and to_char(time) use "HH", "HH12" as 12-hour
intervals, rather than bypass and print the full interval hours. This
is neeeded because to_char(time) is mapped to interval in this function.
Intervals should use "HH24", and document suggestion.
Allow "D" format specifiers for interval/time.
child plan nodes until we have acquired lock on the relation to scan.
The relative order of initialization of plan nodes isn't real important in
other cases, but it's critical here because one is supposed to lock a
relation before its indexes, not vice versa. The original coding was at
least vulnerable to deadlock against DROP INDEX, and perhaps worse things.
Also add a retry for Unixen returning EINTR, which hasn't been reported
as an issue but at least theoretically could be. Patch by Qingqing Zhou,
some minor adjustments by me.
#2075: consider an index redundant if any of its index conditions were already
used, rather than if all of them were. Also, make the selectivity comparison
a bit fuzzy, so that very small differences in estimated selectivities don't
skew the results.
it's worth probing the outer relation for emptiness before building the
hash table. To wit, if we're rescanning a join previously performed,
remember whether we found it nonempty the previous time, and don't bother
with the probe if it was nonempty. This buys back the performance lost
in examples like Mario Weilguni's.
one child or the other had a problem: they did not leave the node in a
state that ExecReScanHashJoin would understand. In particular it would
tend to fail to reset the child plans when needed. Per report from
Mario Weilguni.
generate their output tuple descriptors from their target lists (ie, using
ExecAssignResultTypeFromTL()). We long ago fixed things so that all node
types have minimally valid tlists, so there's no longer any good reason to
have two different ways of doing it. This change is needed to fix bug
reported by Hayden James: the fix of 2005-11-03 to emit the correct column
names after optimizing away a SubqueryScan node didn't work if the new
top-level plan node used ExecAssignResultTypeFromOuterPlan to generate its
tupdesc, since the next plan node down won't have the correct column labels.
a SubLink expression into a rule query. Pre-8.1 we essentially did this
unconditionally; 8.1 tries to do it only when needed, but was missing a
couple of cases. Per report from Kyle Bateman. Add some regression test
cases covering this area.
comment line where output as too long, and update typedefs for /lib
directory. Also fix case where identifiers were used as variable names
in the backend, but as typedefs in ecpg (favor the backend for
indenting).
Backpatch to 8.1.X.
tuple in-place, but instead passes back an all-new tuple structure if
any changes are needed. This is a much cleaner and more robust solution
for the bug discovered by Alexey Beschiokov; accordingly, revert the
quick hack I installed yesterday.
With this change, HeapTupleData.t_datamcxt is no longer needed; will
remove it in a separate commit in HEAD only.
doing heap_insert or heap_update, wipe out any extracted fields in
the TupleTableSlot containing the tuple, because they might not be valid
anymore if tuptoaster.c changed the tuple. Safe because slot must be
in the materialized state, but mighty ugly --- find a better answer!
recursed twice on its first argument, leading to exponential time spent
on a deep nest of COALESCEs ... such as a deeply nested FULL JOIN would
produce. Per report from Matt Carter.
that was added to localbuf.c in 8.1; therefore, applying it to a temp table
left corrupt lookup state in memory. The only case where this had a
significant chance of causing problems was an ON COMMIT DELETE ROWS temp
table; the other possible paths left bogus state that was unlikely to
be used again. Per report from Csaba Nagy.
names from being added to pgindent's typedef list. The existance of
them caused weird formatting in the date/type files, and in keywords.c.
Backpatch to 8.1.X.
columns, shifting comment to the right when more than 150 'else if'
clauses were used, and update typedefs for 8.1.X.
NetBSD patched updated, with documentation.
sense and rename to "outerjoin_delayed" to more clearly reflect what it
means). I had decided that it was redundant in 8.1, but the folly of this
is exposed by a bug report from Sebastian Böck. The place where it's
needed is to prevent orindxpath.c from cherry-picking arms of an outer-join
OR clause to form a relation restriction that isn't actually legal to push
down to the relation scan level. There may be some legal cases that this
forbids optimizing, but we'd need much closer analysis to determine it.
slot of the topmost plan node when a trigger returns a modified tuple.
These appear to be the only places where a plan node's caller did not
treat the result slot as read-only, which is an assumption that nodeUnique
makes as of 8.1. Fixes trigger-vs-DISTINCT bug reported by Frank van Vugt.
surprising results when it's some other numeric type. This doesn't solve
the generic problem of surprising implicit casts to text, but it's a
low-impact way of making sure this particular case behaves sanely.
Per gripe from Harald Fuchs and subsequent discussion.
anything but transaction-exiting commands (ROLLBACK etc). We already rejected
Parse and Execute in such cases, so there seems little point in allowing Bind.
This prevents at least an Assert failure, and probably worse things, since
there's a lot of infrastructure that doesn't work when not in a live
transaction. We can also simplify the Bind logic a bit by rejecting messages
with a nonzero number of parameters, instead of the former kluge to silently
substitute NULL for each parameter. Per bug #2033 from Joel Stevenson.
to assume that the string pointer passed to set_ps_display is good forever.
There's no need to anyway since ps_status.c itself saves the string, and
we already had an API (get_ps_display) to return it.
I believe this explains Jim Nasby's report of intermittent crashes in
elog.c when %i format code is in use in log_line_prefix.
While at it, repair a previously unnoticed problem: on some platforms such as
Darwin, the string returned by get_ps_display was blank-padded to the maximum
length, meaning that lock.c's attempt to append " waiting" to it never worked.
create circularity of role memberships. This is a minimum-impact fix
for the problem reported by Florian Pflug. I thought about removing
the superuser_arg test from is_member_of_role() altogether, as it seems
redundant for many of the callers --- but not all, and it's way too late
in the 8.1 cycle to be making large changes. Perhaps reconsider this
later.
