Tom Lane [Mon, 11 Apr 2016 23:37:04 +0000 (19:37 -0400)]
Fix two places that thought Windows64 is indicated by WIN64 macro.
Everyplace else thinks it's _WIN64, so make these places fall in line.
The pg_regress.c usage is not going to result in any change in behavior,
only suppressing (or not) a compiler warning about downcasting HANDLEs.
So there seems no need for back-patching there.
The libpq/win32.mak usage might represent an actual bug, if anyone were
using this script to build for Windows64, which perhaps nobody is.
Given the lack of field complaints, no back-patch here either.
pg_regress.c problem found by Christian Ullrich, the other by me.
Tom Lane [Mon, 11 Apr 2016 22:17:02 +0000 (18:17 -0400)]
Fix freshly-introduced PL/Python portability bug.
It turns out that those PyErr_Clear() calls I removed from plpy_elog.c
in
7e3bb080387f4143 et al were not quite as random as they appeared: they
mask a Python 2.3.x bug. (Specifically, it turns out that PyType_Ready()
can fail if the error indicator is set on entry, and PLy_traceback's fetch
of frame.f_code may be the first operation in a session that requires the
"frame" type to be readied. Ick.) Put back the clear call, but in a more
centralized place closer to what it's protecting, and this time with a
comment warning what it's really for.
Per buildfarm member prairiedog. Although prairiedog was only failing
on HEAD, it seems clearly possible for this to occur in older branches
as well, so back-patch to 9.2 the same as the previous patch.
Kevin Grittner [Mon, 11 Apr 2016 21:47:50 +0000 (16:47 -0500)]
Use static inline function for BufferGetPage()
I was initially concerned that the some of the hundreds of
references to BufferGetPage() where the literal
BGP_NO_SNAPSHOT_TEST were passed might not optimize as well as a
macro, leading to some hard-to-find performance regressions in
corner cases. Inspection of disassembled code has shown identical
code at all inspected locations, and the size difference doesn't
amount to even one byte per such call. So make it readable.
Per gripes from Álvaro Herrera and Tom Lane
Kevin Grittner [Mon, 11 Apr 2016 20:43:52 +0000 (15:43 -0500)]
Make oldSnapshotControl a pointer to a volatile structure
It was incorrectly declared as a volatile pointer to a non-volatile
structure. Eliminate the OldSnapshotControl struct definition; it
is really not needed. Pointed out by Tom Lane.
While at it, add OldSnapshotControlData to pgindent's list of
structures.
Peter Eisentraut [Mon, 11 Apr 2016 18:44:51 +0000 (14:44 -0400)]
Fix whitespace
Stephen Frost [Mon, 11 Apr 2016 18:12:33 +0000 (14:12 -0400)]
Prefix RLS regression test roles with 'regress_'
To avoid any possible overlap with existing roles on a system when
doing a 'make installcheck', use role names which start with
'regress_'.
Pointed out by Tom.
Peter Eisentraut [Mon, 11 Apr 2016 18:01:15 +0000 (14:01 -0400)]
Add directory created during build to gitignore
Tom Lane [Mon, 11 Apr 2016 15:49:48 +0000 (11:49 -0400)]
Fix missing "volatile" in PLy_output().
Commit
5c3c3cd0a3046339 plastered "volatile" on a bunch of variables
in PLy_output(), but removed the one that actually mattered, ie the
one on "oldcontext". This allows some versions of clang to generate
code in which "oldcontext" has been trashed when control reaches the
PG_CATCH block. Per buildfarm member tick.
Peter Eisentraut [Mon, 11 Apr 2016 15:16:16 +0000 (11:16 -0400)]
cpluspluscheck: Update include path
Some things in src/include/fe_utils require libpq headers, so add
libpq's include path to the command line used here.
Fujii Masao [Mon, 11 Apr 2016 12:28:17 +0000 (21:28 +0900)]
Fix documented return type of pg_logical_emit_message() in func.sgml.
Fujii Masao [Mon, 11 Apr 2016 06:52:27 +0000 (15:52 +0900)]
Use ereport(ERROR) instead of Assert() to emit syncrep_parser error.
The existing code would either Assert or generate an invalid
SyncRepConfig variable, neither of which is desirable. A regular
error should be thrown instead.
This commit silences compiler warning in non assertion-enabled builds.
Per report from Jeff Janes.
Suggested fix by Tom Lane.
Tom Lane [Mon, 11 Apr 2016 04:28:44 +0000 (00:28 -0400)]
Fix poorly thought-through code from commit
5c3c3cd0a3046339.
It's not entirely clear to me whether PyString_AsString can return
null (looks like the answer might vary between Python 2 and 3).
But in any case, this code's attempt to cope with the possibility
was quite broken, because pstrdup() neither allows a null argument
nor ever returns a null.
Moreover, the code below this point assumes that "message" is a
palloc'd string, which would not be the case for a dgettext result.
Fix both problems by doing the pstrdup step separately.
Tom Lane [Mon, 11 Apr 2016 04:00:08 +0000 (00:00 -0400)]
pg_dump: add missing "destroyPQExpBuffer(query)" in dumpForeignServer().
Coverity complained about this resource leak (why now, I don't know,
since it's been like that a long time). Our general policy in pg_dump
is that PQExpBuffers are worth cleaning up, so do it here too. But
don't bother with a back-patch, because it seems unlikely that very
many databases contain enough FOREIGN SERVER objects to notice.
Tom Lane [Mon, 11 Apr 2016 03:52:34 +0000 (23:52 -0400)]
Add comment about intentional fallthrough in switch.
Coverity complained about an apparent missing "break" in a switch
added by
bb140506df605fab. The human-readable comments are pretty
clear that this is intentional, but add a standard /* FALL THRU */
comment to make it clear to tools too.
Tom Lane [Mon, 11 Apr 2016 03:47:30 +0000 (23:47 -0400)]
Clean up foreign-key caching code in planner.
Coverity complained that the code added by
015e88942aa50f0d lacked an
error check for SearchSysCache1 failures, which it should have. But
the code was pretty duff in other ways too, including failure to think
about whether it could really cope with arrays of different lengths.
Tom Lane [Mon, 11 Apr 2016 03:15:55 +0000 (23:15 -0400)]
Fix access-to-already-freed-memory issue in plpython's error handling.
PLy_elog() could attempt to access strings that Python had already freed,
because the strings that PLy_get_spi_error_data() returns are simply
pointers into storage associated with the error "val" PyObject. That's
fine at the instant PLy_get_spi_error_data() returns them, but just after
that PLy_traceback() intentionally releases the only refcount on that
object, allowing it to be freed --- so that the strings we pass to
ereport() are dangling pointers.
In principle this could result in garbage output or a coredump. In
practice, I think the risk is pretty low, because there are no Python
operations between where we decrement that refcount and where we use the
strings (and copy them into PG storage), and thus no reason for Python
to recycle the storage. Still, it's clearly hazardous, and it leads to
Valgrind complaints when running under a Valgrind that hasn't been
lobotomized to ignore Python memory allocations.
The code was a mess anyway: we fetched the error data out of Python
(clearing Python's error indicator) with PyErr_Fetch, examined it, pushed
it back into Python with PyErr_Restore (re-setting the error indicator),
then immediately pulled it back out with another PyErr_Fetch. Just to
confuse matters even more, there were some gratuitous-and-yet-hazardous
PyErr_Clear calls in the "examine" step, and we didn't get around to doing
PyErr_NormalizeException until after the second PyErr_Fetch, making it even
less clear which object was being manipulated where and whether we still
had a refcount on it. (If PyErr_NormalizeException did substitute a
different "val" object, it's possible that the problem could manifest for
real, because then we'd be doing assorted Python stuff with no refcount
on the object we have string pointers into.)
So, rearrange all that into some semblance of sanity, and don't decrement
the refcount on the Python error objects until the end of PLy_elog().
In HEAD, I failed to resist the temptation to reformat some messy bits
from
5c3c3cd0a3046339 along the way.
Back-patch as far as 9.2, because the code is substantially the same
that far&nb