]> granicus.if.org Git - postgresql/log
postgresql
9 years agoRationalize the APIs of array element/slice access functions.
Tom Lane [Mon, 16 Feb 2015 17:23:58 +0000 (12:23 -0500)]
Rationalize the APIs of array element/slice access functions.

The four functions array_ref, array_set, array_get_slice, array_set_slice
have traditionally declared their array inputs and results as being of type
"ArrayType *".  This is a lie, and has been since Berkeley days, because
they actually also support "fixed-length array" types such as "name" and
"point"; not to mention that the inputs could be toasted.  These values
should be declared Datum instead to avoid confusion.  The current coding
already risks possible misoptimization by compilers, and it'll get worse
when "expanded" array representations become a valid alternative.

However, there's a fair amount of code using array_ref and array_set with
arrays that *are* known to be ArrayType structures, and there might be more
such places in third-party code.  Rather than cluttering those call sites
with PointerGetDatum/DatumGetArrayTypeP cruft, what I did was to rename the
existing functions to array_get_element/array_set_element, fix their
signatures, then reincarnate array_ref/array_set as backwards compatibility
wrappers.

array_get_slice/array_set_slice have no such constituency in the core code,
and probably not in third-party code either, so I just changed their APIs.

9 years agoCorrect the path of pg_lzcompress.c in doc.
Fujii Masao [Mon, 16 Feb 2015 05:50:13 +0000 (14:50 +0900)]
Correct the path of pg_lzcompress.c in doc.

Commit 40bede5 moved pg_lzcompress.c to src/common, but forgot to
update its path in doc. This commit fixes that oversight.

9 years agoFix null-pointer-deref crash while doing COPY IN with check constraints.
Tom Lane [Mon, 16 Feb 2015 04:26:45 +0000 (23:26 -0500)]
Fix null-pointer-deref crash while doing COPY IN with check constraints.

In commit bf7ca15875988a88e97302e012d7c4808bef3ea9 I introduced an
assumption that an RTE referenced by a whole-row Var must have a valid eref
field.  This is false for RTEs constructed by DoCopy, and there are other
places taking similar shortcuts.  Perhaps we should make all those places
go through addRangeTableEntryForRelation or its siblings instead of having
ad-hoc logic, but the most reliable fix seems to be to make the new code in
ExecEvalWholeRowVar cope if there's no eref.  We can reasonably assume that
there's no need to insert column aliases if no aliases were provided.

Add a regression test case covering this, and also verifying that a sane
column name is in fact available in this situation.

Although the known case only crashes in 9.4 and HEAD, it seems prudent to
back-patch the code change to 9.2, since all the ingredients for a similar
failure exist in the variant patch applied to 9.3 and 9.2.

Per report from Jean-Pierre Pelletier.

9 years agoMinor housekeeping on JSON docs.
Andrew Dunstan [Sun, 15 Feb 2015 22:08:37 +0000 (17:08 -0500)]
Minor housekeeping on JSON docs.

9 years agopg_regress: Write processed input/*.source into output dir
Peter Eisentraut [Sun, 15 Feb 2015 02:33:41 +0000 (21:33 -0500)]
pg_regress: Write processed input/*.source into output dir

Before, it was writing the processed files into the input directory,
which is incorrect in a vpath build.

9 years agoAvoid returning undefined bytes in chkpass_in().
Tom Lane [Sat, 14 Feb 2015 17:20:56 +0000 (12:20 -0500)]
Avoid returning undefined bytes in chkpass_in().

We can't really fix the problem that the result is defined to depend on
random(), so it is still going to fail the "unstable input conversion"
test in parse_type.c.  However, we can at least satify valgrind.  (It
looks like this code used to be valgrind-clean, actually, until somebody
did a careless s/strncpy/strlcpy/g on it.)

In passing, let's just make real sure that chkpass_out doesn't overrun
its output buffer.

No need for backpatch, I think, since this is just to satisfy debugging
tools.

Asif Naeem

9 years agoFix broken #ifdef for __sparcv8
Heikki Linnakangas [Fri, 13 Feb 2015 21:51:23 +0000 (23:51 +0200)]
Fix broken #ifdef for __sparcv8

Rob Rowan. Backpatch to all supported versions, like the patch that added
the broken #ifdef.

9 years agoSimplify waiting logic in reading from / writing to client.
Heikki Linnakangas [Fri, 13 Feb 2015 19:46:14 +0000 (21:46 +0200)]
Simplify waiting logic in reading from / writing to client.

The client socket is always in non-blocking mode, and if we actually want
blocking behaviour, we emulate it by sleeping and retrying. But we have
retry loops at different layers for reads and writes, which was confusing.
To simplify, remove all the sleeping and retrying code from the lower
levels, from be_tls_read and secure_raw_read and secure_raw_write, and put
all the logic in secure_read() and secure_write().

9 years agoSimplify the way OpenSSL renegotiation is initiated in server.
Heikki Linnakangas [Fri, 13 Feb 2015 19:46:08 +0000 (21:46 +0200)]
Simplify the way OpenSSL renegotiation is initiated in server.

At least in all modern versions of OpenSSL, it is enough to call
SSL_renegotiate() once, and then forget about it. Subsequent SSL_write()
and SSL_read() calls will finish the handshake.

The SSL_set_session_id_context() call is unnecessary too. We only have
one SSL context, and the SSL session was created with that to begin with.

9 years agopg_upgrade: improve checksum mismatch error message
Bruce Momjian [Thu, 12 Feb 2015 03:22:26 +0000 (22:22 -0500)]
pg_upgrade:  improve checksum mismatch error message

Patch by Greg Sabino Mullane, slight adjustments by me

9 years agopg_upgrade: quote directory names in delete_old_cluster script
Bruce Momjian [Thu, 12 Feb 2015 03:06:04 +0000 (22:06 -0500)]
pg_upgrade:  quote directory names in delete_old_cluster script

This allows the delete script to properly function when special
characters appear in directory paths, e.g. spaces.

Backpatch through 9.0

9 years agopg_upgrade: preserve freeze info for postgres/template1 dbs
Bruce Momjian [Thu, 12 Feb 2015 02:02:07 +0000 (21:02 -0500)]
pg_upgrade:  preserve freeze info for postgres/template1 dbs

pg_database.datfrozenxid and pg_database.datminmxid were not preserved
for the 'postgres' and 'template1' databases.  This could cause missing
clog file errors on access to user tables and indexes after upgrades in
these databases.

Backpatch through 9.0

9 years agoFix typo in logicaldecoding.sgml.
Andres Freund [Thu, 12 Feb 2015 00:16:32 +0000 (01:16 +0100)]
Fix typo in logicaldecoding.sgml.

Author: Tatsuo Ishii

Backpatch to 9.4, where logicaldecoding was introduced.

9 years agoFix missing PQclear() in libpqrcv_endstreaming().
Tom Lane [Thu, 12 Feb 2015 00:20:49 +0000 (19:20 -0500)]
Fix missing PQclear() in libpqrcv_endstreaming().

This omission leaked one PGresult per WAL streaming cycle, which possibly
would never be enough to notice in the real world, but it's still a leak.

Per Coverity.  Back-patch to 9.3 where the error was introduced.

9 years agoFix minor memory leak in ident_inet().
Tom Lane [Thu, 12 Feb 2015 00:09:54 +0000 (19:09 -0500)]
Fix minor memory leak in ident_inet().

We'd leak the ident_serv data structure if the second pg_getaddrinfo_all
(the one for the local address) failed.  This is not of great consequence
because a failure return here just leads directly to backend exit(), but
if this function is going to try to clean up after itself at all, it should
not have such holes in the logic.  Try to fix it in a future-proof way by
having all the failure exits go through the same cleanup path, rather than
"optimizing" some of them.

Per Coverity.  Back-patch to 9.2, which is as far back as this patch
applies cleanly.

9 years agoFix more memory leaks in failure path in buildACLCommands.
Tom Lane [Wed, 11 Feb 2015 23:35:23 +0000 (18:35 -0500)]
Fix more memory leaks in failure path in buildACLCommands.

We already had one go at this issue in commit d73b7f973db5ec7e, but we
failed to notice that buildACLCommands also leaked several PQExpBuffers
along with a simply malloc'd string.  This time let's try to make the
fix a bit more future-proof by eliminating the separate exit path.

It's still not exactly critical because pg_dump will curl up and die on
failure; but since the amount of the potential leak is now several KB,
it seems worth back-patching as far as 9.2 where the previous fix landed.

Per Coverity, which evidently is smarter than clang's static analyzer.

9 years agoFix pg_dump's heuristic for deciding which casts to dump.
Tom Lane [Wed, 11 Feb 2015 03:38:15 +0000 (22:38 -0500)]
Fix pg_dump's heuristic for deciding which casts to dump.

Back in 2003 we had a discussion about how to decide which casts to dump.
At the time pg_dump really only considered an object's containing schema
to decide what to dump (ie, dump whatever's not in pg_catalog), and so
we chose a complicated idea involving whether the underlying types were to
be dumped (cf commit a6790ce85752b67ad994f55fdf1a450262ccc32e).  But users
are allowed to create casts between built-in types, and we failed to dump
such casts.  Let's get rid of that heuristic, which has accreted even more
ugliness since then, in favor of just looking at the cast's OID to decide
if it's a built-in cast or not.

In passing, also fix some really ancient code that supposed that it had to
manufacture a dependency for the cast on its cast function; that's only
true when dumping from a pre-7.3 server.  This just resulted in some wasted
cycles and duplicate dependency-list entries with newer servers, but we
might as well improve it.

Per gripes from a number of people, most recently Greg Sabino Mullane.
Back-patch to all supported branches.

9 years agoFix GEQO to not assume its join order heuristic always works.
Tom Lane [Wed, 11 Feb 2015 01:37:19 +0000 (20:37 -0500)]
Fix GEQO to not assume its join order heuristic always works.

Back in commit 400e2c934457bef4bc3cc9a3e49b6289bd761bc0 I rewrote GEQO's
gimme_tree function to improve its heuristic for modifying the given tour
into a legal join order.  In what can only be called a fit of hubris,
I supposed that this new heuristic would *always* find a legal join order,
and ripped out the old logic that allowed gimme_tree to sometimes fail.

The folly of this is exposed by bug #12760, in which the "greedy" clumping
behavior of merge_clump() can lead it into a dead end which could only be
recovered from by un-clumping.  We have no code for that and wouldn't know
exactly what to do with it if we did.  Rather than try to improve the
heuristic rules still further, let's just recognize that it *is* a
heuristic and probably must always have failure cases.  So, put back the
code removed in the previous commit to allow for failure (but comment it
a bit better this time).

It's possible that this code was actually fully correct at the time and
has only been broken by the introduction of LATERAL.  But having seen this
example I no longer have much faith in that proposition, so back-patch to
all supported branches.

9 years agoFixed array handling in ecpg.
Michael Meskes [Tue, 10 Feb 2015 11:00:13 +0000 (12:00 +0100)]
Fixed array handling in ecpg.

When ecpg was rewritten to the new protocol version not all variable types
were corrected. This patch rewrites the code for these types to fix that. It
also fixes the documentation to correctly tell the status of array handling.

9 years agoSpeed up CRC calculation using slicing-by-8 algorithm.
Heikki Linnakangas [Tue, 10 Feb 2015 08:54:40 +0000 (10:54 +0200)]
Speed up CRC calculation using slicing-by-8 algorithm.

This speeds up WAL generation and replay. The new algorithm is
significantly faster with large inputs, like full-page images or when
inserting wide rows. It is slower with tiny inputs, i.e. less than 10 bytes
or so, but the speedup with longer inputs more than make up for that. Even
small WAL records at least have 24 byte header in the front.

The output is identical to the current byte-at-a-time computation, so this
does not affect compatibility. The new algorithm is only used for the
CRC-32C variant, not the legacy version used in tsquery or the
"traditional" CRC-32 used in hstore and ltree. Those are not as performance
critical, and are usually only applied over small inputs, so it seems
better to not carry around the extra lookup tables to speed up those rare
cases.

Abhijit Menon-Sen

9 years agoFix MSVC build.
Heikki Linnakangas [Mon, 9 Feb 2015 20:13:50 +0000 (22:13 +0200)]
Fix MSVC build.

When I moved pg_crc.c from src/port to src/common, I forgot to modify MSVC
build script accordingly.

9 years agoMinor cleanup/code review for "indirect toast" stuff.
Tom Lane [Mon, 9 Feb 2015 17:30:52 +0000 (12:30 -0500)]
Minor cleanup/code review for "indirect toast" stuff.

Fix some issues I noticed while fooling with an extension to allow an
additional kind of toast pointer.  Much of this is just comment
improvement, but there are a couple of actual bugs, which might or might
not be reachable today depending on what can happen during logical
decoding.  An example is that toast_flatten_tuple() failed to cover the
possibility of an indirection pointer in its input.  Back-patch to 9.4
just in case that is reachable now.

In HEAD, also correct some really minor issues with recent compression
reorganization, such as dangerously underparenthesized macros.

9 years agoMove pg_crc.c to src/common, and remove pg_crc_tables.h
Heikki Linnakangas [Mon, 9 Feb 2015 09:17:56 +0000 (11:17 +0200)]
Move pg_crc.c to src/common, and remove pg_crc_tables.h

To get CRC functionality in a client program, you now need to link with
libpgcommon instead of libpgport. The CRC code has nothing to do with
portability, so libpgcommon is a better home. (libpgcommon didn't exist
when pg_crc.c was originally moved to src/port.)

Remove the possibility to get CRC functionality by just #including
pg_crc_tables.h. I'm not aware of any extensions that actually did that and
couldn't simply link with libpgcommon.

This also moves the pg_crc.h header file from src/include/utils to
src/include/common, which will require changes to any external programs
that currently does #include "utils/pg_crc.h". That seems acceptable, as
include/common is clearly the right home for it now, and the change needed
to any such programs is trivial.

9 years agoMove pg_lzcompress.c to src/common.
Fujii Masao [Mon, 9 Feb 2015 06:15:24 +0000 (15:15 +0900)]
Move pg_lzcompress.c to src/common.

The meta data of PGLZ symbolized by PGLZ_Header is removed, to make
the compression and decompression code independent on the backend-only
varlena facility. PGLZ_Header is being used to store some meta data
related to the data being compressed like the raw length of the uncompressed
record or some varlena-related data, making it unpluggable once PGLZ is
stored in src/common as it contains some backend-only code paths with
the management of varlena structures. The APIs of PGLZ are reworked
at the same time to do only compression and decompression of buffers
without the meta-data layer, simplifying its use for a more general usage.

On-disk format is preserved as well, so there is no incompatibility with
previous major versions of PostgreSQL for TOAST entries.

Exposing compression and decompression APIs of pglz makes possible its
use by extensions and contrib modules. Especially this commit is required
for upcoming WAL compression feature so that the WAL reader facility can
decompress the WAL data by using pglz_decompress.

Michael Paquier, reviewed by me.

9 years agoCheck DCH_MAX_ITEM_SIZ limits with <=, not <.
Noah Misch [Sat, 7 Feb 2015 04:39:52 +0000 (23:39 -0500)]
Check DCH_MAX_ITEM_SIZ limits with <=, not <.

We reserve space for the full amount, not one less.  The affected checks
deal with localized month and day names.  Today's DCH_MAX_ITEM_SIZ value
would suffice for a 60-byte day name, while the longest known is the
49-byte mn_CN.utf-8 word for "Saturday."  Thus, the upshot of this
change is merely to avoid misdirecting future readers of the code; users
are not expected to see errors either way.

9 years agoAssert(PqCommReadingMsg) in pq_peekbyte().
Noah Misch [Sat, 7 Feb 2015 04:14:27 +0000 (23:14 -0500)]
Assert(PqCommReadingMsg) in pq_peekbyte().

Interrupting pq_recvbuf() can break protocol sync, so its callers all
deserve this assertion.  The one pq_peekbyte() caller suffices already.

9 years agoReport WAL flush, not insert, position in replication IDENTIFY_SYSTEM
Heikki Linnakangas [Fri, 6 Feb 2015 09:18:14 +0000 (11:18 +0200)]
Report WAL flush, not insert, position in replication IDENTIFY_SYSTEM

When beginning streaming replication, the client usually issues the
IDENTIFY_SYSTEM command, which used to return the current WAL insert
position. That's not suitable for the intended purpose of that field,
however. pg_receivexlog uses it to start replication from the reported
point, but if it hasn't been flushed to disk yet, it will fail. Change
IDENTIFY_SYSTEM to report the flush position instead.

Backpatch to 9.1 and above. 9.0 doesn't report any WAL position.

9 years agoThis routine was calling ecpg_alloc to allocate to memory but did not
Michael Meskes [Thu, 5 Feb 2015 14:12:34 +0000 (15:12 +0100)]
This routine was calling ecpg_alloc to allocate to memory but did not
actually check the returned pointer allocated, potentially NULL which
could be the result of a malloc call.

Issue noted by Coverity, fixed by Michael Paquier <michael@otacoo.com>

9 years agoUse a separate memory context for GIN scan keys.
Heikki Linnakangas [Wed, 4 Feb 2015 15:40:25 +0000 (17:40 +0200)]
Use a separate memory context for GIN scan keys.

It was getting tedious to track and release all the different things that
form a scan key. We were leaking at least the queryCategories array, and
possibly more, on a rescan. That was visible if a GIN index was used in a
nested loop join. This also protects from leaks in extractQuery method.

No backpatching, given the lack of complaints from the field. Maybe later,
after this has received more field testing.

9 years agoFix reference-after-free when waiting for another xact due to constraint.
Heikki Linnakangas [Wed, 4 Feb 2015 14:00:34 +0000 (16:00 +0200)]
Fix reference-after-free when waiting for another xact due to constraint.

If an insertion or update had to wait for another transaction to finish,
because there was another insertion with conflicting key in progress,
we would pass a just-free'd item pointer to XactLockTableWait().

All calls to XactLockTableWait() and MultiXactIdWait() had similar issues.
Some passed a pointer to a buffer in the buffer cache, after already
releasing the lock. The call in EvalPlanQualFetch had already released the
pin too. All but the call in execUtils.c would merely lead to reporting a
bogus ctid, however (or an assertion failure, if enabled).

All the callers that passed HeapTuple->t_data->t_ctid were slightly bogus
anyway: if the tuple was updated (again) in the same transaction, its ctid
field would point to the next tuple in the chain, not the tuple itself.

Backpatch to 9.4, where the 'ctid' argument to XactLockTableWait was added
(in commit f88d4cfc)

9 years agopgcrypto: Code cleanup for decrypt_internal.
Robert Haas [Wed, 4 Feb 2015 13:41:35 +0000 (08:41 -0500)]
pgcrypto: Code cleanup for decrypt_internal.

Remove some unnecessary null-tests, and replace a goto-label construct
with an "if" block.

Michael Paquier, reviewed by me.

9 years agoFix memory leaks on OOM in ecpg.
Heikki Linnakangas [Wed, 4 Feb 2015 12:53:29 +0000 (14:53 +0200)]
Fix memory leaks on OOM in ecpg.

These are fairly obscure cases, but let's keep Coverity happy.

Michael Paquier with some further fixes by me.

9 years agoAdd missing float.h include to snprintf.c.
Andres Freund [Wed, 4 Feb 2015 12:27:31 +0000 (13:27 +0100)]
Add missing float.h include to snprintf.c.

On windows _isnan() (which isnan() is redirected to in port/win32.h)
is declared in float.h, not math.h.

Per buildfarm animal currawong.

Backpatch to all supported branches.

9 years agodoc: Fix markup
Fujii Masao [Wed, 4 Feb 2015 10:00:09 +0000 (19:00 +0900)]
doc: Fix markup

Ian Barwick

9 years agoAdd dummy PQsslAttributes function for non-SSL builds.
Heikki Linnakangas [Wed, 4 Feb 2015 07:13:15 +0000 (09:13 +0200)]
Add dummy PQsslAttributes function for non-SSL builds.

All the other new SSL information functions had dummy versions in
be-secure.c, but I missed PQsslAttributes(). Oops. Surprisingly, the linker
did not complain about the missing function on most platforms represented in
the buildfarm, even though it is exported, except for a few Windows systems.

9 years agoRemove ill-conceived Assertion in ProcessClientWriteInterrupt().
Andres Freund [Tue, 3 Feb 2015 22:52:15 +0000 (23:52 +0100)]
Remove ill-conceived Assertion in ProcessClientWriteInterrupt().

It's perfectly fine to have blocked interrupts when
ProcessClientWriteInterrupt() is called. In fact it's commonly the
case when emitting error reports. And we deal with that correctly.

Even if that'd not be the case, it'd be a bad location for such a
assertion. Because ProcessClientWriteInterrupt() is only called when
the socket is blocked it's hard to hit.

Per Heikki and buildfarm animals nightjar and dunlin.

9 years agoRemove remnants of ImmediateInterruptOK handling.
Andres Freund [Tue, 3 Feb 2015 22:25:47 +0000 (23:25 +0100)]
Remove remnants of ImmediateInterruptOK handling.

Now that nothing sets ImmediateInterruptOK to true anymore, we can
remove all the supporting code.

Reviewed-By: Heikki Linnakangas
9 years agoRemove the option to service interrupts during PGSemaphoreLock().
Andres Freund [Tue, 3 Feb 2015 22:25:00 +0000 (23:25 +0100)]
Remove the option to service interrupts during PGSemaphoreLock().

The remaining caller (lwlocks) doesn't need that facility, and we plan
to remove ImmedidateInterruptOK entirely. That means that interrupts
can't be serviced race-free and portably anyway, so there's little
reason for keeping the feature.

Reviewed-By: Heikki Linnakangas
9 years agoMove deadlock and other interrupt handling in proc.c out of signal handlers.
Andres Freund [Tue, 3 Feb 2015 22:24:38 +0000 (23:24 +0100)]
Move deadlock and other interrupt handling in proc.c out of signal handlers.

Deadlock checking was performed inside signal handlers up to
now. While it's a remarkable feat to have made this work reliably,
it's quite complex to understand why that is the case. Partially it
worked due to the assumption that semaphores are signal safe - which
is not actually documented to be the case for sysv semaphores.

The reason we had to rely on performing this work inside signal
handlers is that semaphores aren't guaranteed to be interruptable by
signals on all platforms. But now that latches provide a somewhat
similar API, which actually has the guarantee of being interruptible,
we can avoid doing so.

Signalling between ProcSleep, ProcWakeup, ProcWaitForSignal and
ProcSendSignal is now done using latches. This increases the
likelihood of spurious wakeups. As spurious wakeup already were
possible and aren't likely to be frequent enough to be an actual
problem, this seems acceptable.

This change would allow for further simplification of the deadlock
checking, now that it doesn't have to run in a signal handler. But
even if I were motivated to do so right now, it would still be better
to do that separately. Such a cleanup shouldn't have to be reviewed a
the same time as the more fundamental changes in this commit.

There is one possible usability regression due to this commit. Namely
it is more likely than before that log_lock_waits messages are output
more than once.

Reviewed-By: Heikki Linnakangas
9 years agoDon't allow immediate interrupts during authentication anymore.
Andres Freund [Tue, 3 Feb 2015 21:54:48 +0000 (22:54 +0100)]
Don't allow immediate interrupts during authentication anymore.

We used to handle authentication_timeout by setting
ImmediateInterruptOK to true during large parts of the authentication
phase of a new connection.  While that happens to work acceptably in
practice, it's not particularly nice and has ugly corner cases.

Previous commits converted the FE/BE communication to use latches and
implemented support for interrupt handling during both
send/recv. Building on top of that work we can get rid of
ImmediateInterruptOK during authentication, by immediately treating
timeouts during authentication as a reason to die. As die interrupts
are handled immediately during client communication that provides a
sensibly quick reaction time to authentication timeout.

Additionally add a few CHECK_FOR_INTERRUPTS() to some more complex
authentication methods. More could be added, but this already should
provides a reasonable coverage.

While it this overall increases the maximum time till a timeout is
reacted to, it greatly reduces complexity and increases
reliability. That seems like a overall win. If the increase proves to
be noticeable we can deal with those cases by moving to nonblocking
network code and add interrupt checking there.

Reviewed-By: Heikki Linnakangas
9 years agoRemove unused "m" field in LSEG.
Tom Lane [Tue, 3 Feb 2015 21:50:50 +0000 (16:50 -0500)]
Remove unused "m" field in LSEG.

This field has been unreferenced since 1998, and does not appear in lseg
values stored on disk (since sizeof(lseg) is only 32 bytes according to
pg_type).  There was apparently some idea of maintaining it just in values
appearing in memory, but the bookkeeping required to make that work would
surely far outweigh the cost of recalculating the line's slope when needed.
Remove it to (a) simplify matters and (b) suppress some uninitialized-field
whining from Coverity.

9 years agoProcess 'die' interrupts while reading/writing from the client socket.
Andres Freund [Tue, 3 Feb 2015 21:45:45 +0000 (22:45 +0100)]
Process 'die' interrupts while reading/writing from the client socket.

Up to now it was impossible to terminate a backend that was trying to
send/recv data to/from the client when the socket's buffer was already
full/empty. While the send/recv calls itself might have gotten
interrupted by signals on some platforms, we just immediately retried.

That could lead to situations where a backend couldn't be terminated ,
after a client died without the connection being closed, because it
was blocked in send/recv.

The problem was far more likely to be hit when sending data than when
reading. That's because while reading a command from the client, and
during authentication, we processed interrupts immediately . That
primarily left COPY FROM STDIN as being problematic for recv.

Change things so that that we process 'die' events immediately when
the appropriate signal arrives. We can't sensibly react to query
cancels at that point, because we might loose sync with the client as
we could be in the middle of writing a message.

We don't interrupt writes if the write buffer isn't full, as indicated
by write() returning EWOULDBLOCK, as that would lead to fewer error
messages reaching clients.

Per discussion with Kyotaro HORIGUCHI and Heikki Linnakangas

Discussion: 20140927191243.GD5423@alap3.anarazel.de

9 years agoIntroduce and use infrastructure for interrupt processing during client reads.
Andres Freund [Tue, 3 Feb 2015 21:25:20 +0000 (22:25 +0100)]
Introduce and use infrastructure for interrupt processing during client reads.

Up to now large swathes of backend code ran inside signal handlers
while reading commands from the client, to allow for speedy reaction to
asynchronous events. Most prominently shared invalidation and NOTIFY
handling. That means that complex code like the starting/stopping of
transactions is run in signal handlers...  The required code was
fragile and verbose, and is likely to contain bugs.

That approach also severely limited what could be done while
communicating with the client. As the read might be from within
openssl it wasn't safely possible to trigger an error, e.g. to cancel
a backend in idle-in-transaction state. We did that in some cases,
namely fatal errors, nonetheless.

Now that FE/BE communication in the backend employs non-blocking
sockets and latches to block, we can quite simply interrupt reads from
signal handlers by setting the latch. That allows us to signal an
interrupted read, which is supposed to be retried after returning from
within the ssl library.

As signal handlers now only need to set the latch to guarantee timely
interrupt processing, remove a fair amount of complicated & fragile
code from async.c and sinval.c.

We could now actually start to process some kinds of interrupts, like
sinval ones, more often that before, but that seems better done
separately.

This work will hopefully allow to handle cases like being blocked by
sending data, interrupting idle transactions and similar to be
implemented without too much effort.  In addition to allowing getting
rid of ImmediateInterruptOK, that is.

Author: Andres Freund
Reviewed-By: Heikki Linnakangas
9 years agoUse a nonblocking socket for FE/BE communication and block using latches.
Andres Freund [Tue, 3 Feb 2015 21:03:48 +0000 (22:03 +0100)]
Use a nonblocking socket for FE/BE communication and block using latches.

This allows to introduce more elaborate handling of interrupts while
reading from a socket.  Currently some interrupt handlers have to do
significant work from inside signal handlers, and it's very hard to
correctly write code to do so.  Generic signal handler limitations,
combined with the fact that we can't safely jump out of a signal
handler while reading from the client have prohibited implementation
of features like timeouts for idle-in-transaction.

Additionally we use the latch code to wait in a couple places where we
previously only had waiting code on windows as other platforms just
busy looped.

This can increase the number of systemcalls happening during FE/BE
communication. Benchmarks so far indicate that the impact isn't very
high, and there's room for optimization in the latch code. The chance
of cleaning up the usage of latches gives us, seem to outweigh the
risk of small performance regressions.

This commit theoretically can't used without the next patch in the
series, as WaitLatchOrSocket is not defined to be fully signal
safe. As we already do that in some cases though, it seems better to
keep the commits separate, so they're easier to understand.

Author: Andres Freund
Reviewed-By: Heikki Linnakangas
9 years agoFix breakage in GEODEBUG debug code.
Tom Lane [Tue, 3 Feb 2015 20:20:45 +0000 (15:20 -0500)]
Fix breakage in GEODEBUG debug code.

LINE doesn't have an "m" field (anymore anyway).  Also fix unportable
assumption that %x can print the result of pointer subtraction.

In passing, improve single_decode() in minor ways:
* Remove unnecessary leading-whitespace skip (strtod does that already).
* Make GEODEBUG message more intelligible.
* Remove entirely-useless test to see if strtod returned a silly pointer.
* Don't bother computing trailing-whitespace skip unless caller wants
  an ending pointer.

This has been broken since 261c7d4b653bc3e44c31fd456d94f292caa50d8f.
Although it's only debug code, might as well fix the 9.4 branch too.

9 years agoAdd API functions to libpq to interrogate SSL related stuff.
Heikki Linnakangas [Tue, 3 Feb 2015 17:57:52 +0000 (19:57 +0200)]
Add API functions to libpq to interrogate SSL related stuff.

This makes it possible to query for things like the SSL version and cipher
used, without depending on OpenSSL functions or macros. That is a good
thing if we ever get another SSL implementation.

PQgetssl() still works, but it should be considered as deprecated as it
only works with OpenSSL. In particular, PQgetSslInUse() should be used to
check if a connection uses SSL, because as soon as we have another
implementation, PQgetssl() will return NULL even if SSL is in use.

9 years agoRefactor page compactifying code.
Heikki Linnakangas [Tue, 3 Feb 2015 12:09:29 +0000 (14:09 +0200)]
Refactor page compactifying code.

The logic to compact away removed tuples from page was duplicated with
small differences in PageRepairFragmentation, PageIndexMultiDelete, and
PageIndexDeleteNoCompact. Put it into a common function.

Reviewed by Peter Geoghegan.

9 years agoRephrase the documentation on pg_receivexlog --synchronous option.
Heikki Linnakangas [Tue, 3 Feb 2015 08:35:46 +0000 (10:35 +0200)]
Rephrase the documentation on pg_receivexlog --synchronous option.

The old wording talked about a "sync command", meaining fsync(), but it
was not very clear.

9 years agoFix typo in comment.
Heikki Linnakangas [Tue, 3 Feb 2015 07:48:45 +0000 (09:48 +0200)]
Fix typo in comment.

Amit Langote

9 years agoRemove dead code.
Heikki Linnakangas [Tue, 3 Feb 2015 07:43:44 +0000 (09:43 +0200)]
Remove dead code.

Commit 13629df changed metaphone() function to return an empty string on
empty input, but it left the old error message in place. It's now dead code.

Michael Paquier, per Coverity warning.

9 years agoAdd new function BackgroundWorkerInitializeConnectionByOid.
Robert Haas [Mon, 2 Feb 2015 21:23:59 +0000 (16:23 -0500)]
Add new function BackgroundWorkerInitializeConnectionByOid.

Sometimes it's useful for a background worker to be able to initialize
its database connection by OID rather than by name, so provide a way
to do that.

9 years agoLast-minute updates for release notes.
Tom Lane [Mon, 2 Feb 2015 16:23:59 +0000 (11:23 -0500)]
Last-minute updates for release notes.

Add entries for security issues.

Security: CVE-2015-0241 through CVE-2015-0244

9 years agoBe more careful to not lose sync in the FE/BE protocol.
Heikki Linnakangas [Mon, 2 Feb 2015 15:08:45 +0000 (17:08 +0200)]
Be more careful to not lose sync in the FE/BE protocol.

If any error occurred while we were in the middle of reading a protocol
message from the client, we could lose sync, and incorrectly try to
interpret a part of another message as a new protocol message. That will
usually lead to an "invalid frontend message" error that terminates the
connection. However, this is a security issue because an attacker might
be able to deliberately cause an error, inject a Query message in what's
supposed to be just user data, and have the server execute it.

We were quite careful to not have CHECK_FOR_INTERRUPTS() calls or other
operations that could ereport(ERROR) in the middle of processing a message,
but a query cancel interrupt or statement timeout could nevertheless cause
it to happen. Also, the V2 fastpath and COPY handling were not so careful.
It's very difficult to recover in the V2 COPY protocol, so we will just
terminate the connection on error. In practice, that's what happened
previously anyway, as we lost protocol sync.

To fix, add a new variable in pqcomm.c, PqCommReadingMsg, that is set
whenever we're in the middle of reading a message. When it's set, we cannot
safely ERROR out and continue running, because we might've read only part
of a message. PqCommReadingMsg acts somewhat similarly to critical sections
in that if an error occurs while it's set, the error handler will force the
connection to be terminated, as if the error was FATAL. It's not
implemented by promoting ERROR to FATAL in elog.c, like ERROR is promoted
to PANIC in critical sections, because we want to be able to use
PG_TRY/CATCH to recover and regain protocol sync. pq_getmessage() takes
advantage of that to prevent an OOM error from terminating the connection.

To prevent unnecessary connection terminations, add a holdoff mechanism
similar to HOLD/RESUME_INTERRUPTS() that can be used hold off query cancel
interrupts, but still allow die interrupts. The rules on which interrupts
are processed when are now a bit more complicated, so refactor
ProcessInterrupts() and the calls to it in signal handlers so that the
signal handlers always call it if ImmediateInterruptOK is set, and
ProcessInterrupts() can decide to not do anything if the other conditions
are not met.

Reported by Emil Lenngren. Patch reviewed by Noah Misch and Andres Freund.
Backpatch to all supported versions.

Security: CVE-2015-0244

9 years agoPrevent Valgrind Memcheck errors around px_acquire_system_randomness().
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Prevent Valgrind Memcheck errors around px_acquire_system_randomness().

This function uses uninitialized stack and heap buffers as supplementary
entropy sources.  Mark them so Memcheck will not complain.  Back-patch
to 9.4, where Valgrind Memcheck cooperation first appeared.

Marko Tiikkaja

9 years agoCherry-pick security-relevant fixes from upstream imath library.
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Cherry-pick security-relevant fixes from upstream imath library.

This covers alterations to buffer sizing and zeroing made between imath
1.3 and imath 1.20.  Valgrind Memcheck identified the buffer overruns
and reliance on uninitialized data; their exploit potential is unknown.
Builds specifying --with-openssl are unaffected, because they use the
OpenSSL BIGNUM facility instead of imath.  Back-patch to 9.0 (all
supported versions).

Security: CVE-2015-0243

9 years agoFix buffer overrun after incomplete read in pullf_read_max().
Noah Misch [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
Fix buffer overrun after incomplete read in pullf_read_max().

Most callers pass a stack buffer.  The ensuing stack smash can crash the
server, and we have not ruled out the viability of attacks that lead to
privilege escalation.  Back-patch to 9.0 (all supported versions).

Marko Tiikkaja

Security: CVE-2015-0243

9 years agoport/snprintf(): fix overflow and do padding
Bruce Momjian [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
port/snprintf():  fix overflow and do padding

Prevent port/snprintf() from overflowing its local fixed-size
buffer and pad to the desired number of digits with zeros, even
if the precision is beyond the ability of the native sprintf().
port/snprintf() is only used on systems that lack a native
snprintf().

Reported by Bruce Momjian. Patch by Tom Lane. Backpatch to all
supported versions.

Security: CVE-2015-0242

9 years agoto_char(): prevent writing beyond the allocated buffer
Bruce Momjian [Mon, 2 Feb 2015 15:00:45 +0000 (10:00 -0500)]
to_char():  prevent writing beyond the allocated buffer

Previously very long localized month and weekday strings could
overflow the allocated buffers, causing a server crash.

Reported and patch reviewed by Noah Misch.  Backpatch to all
supported versions.

Security: CVE-2015-0241

9 years agoto_char(): prevent accesses beyond the allocated buffer
Bruce Momjian [Mon, 2 Feb 2015 15:00:44 +0000 (10:00 -0500)]
to_char():  prevent accesses beyond the allocated buffer

Previously very long field masks for floats could access memory
beyond the existing buffer allocated to hold the result.

Reported by Andres Freund and Peter Geoghegan. Backpatch to all
supported versions.

Security: CVE-2015-0241

9 years agoDoc: fix syntax description for psql's \setenv.
Tom Lane [Mon, 2 Feb 2015 05:18:54 +0000 (00:18 -0500)]
Doc: fix syntax description for psql's \setenv.

The variable name isn't optional --- looks like a copy-and-paste-o from
the \set command, where it is.

Dilip Kumar

9 years agoTranslation updates
Peter Eisentraut [Mon, 2 Feb 2015 04:23:40 +0000 (23:23 -0500)]
Translation updates

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 19c72ea8d856d7b1d4f5d759a766c8206bf9ce53

9 years agodoc: Improve claim about location of pg_service.conf
Peter Eisentraut [Mon, 2 Feb 2015 03:36:44 +0000 (22:36 -0500)]
doc: Improve claim about location of pg_service.conf

The previous wording claimed that the file was always in /etc, but of
course this varies with the installation layout.  Write instead that it
can be found via `pg_config --sysconfdir`.  Even though this is still
somewhat incorrect because it doesn't account of moved installations, it
at least conveys that the location depends on the installation.

9 years agoRelease notes for 9.4.1, 9.3.6, 9.2.10, 9.1.15, 9.0.19.
Tom Lane [Sun, 1 Feb 2015 21:50:31 +0000 (16:50 -0500)]
Release notes for 9.4.1, 9.3.6, 9.2.10, 9.1.15, 9.0.19.

9 years agoFix documentation of psql's ECHO all mode.
Tom Lane [Sat, 31 Jan 2015 23:35:13 +0000 (18:35 -0500)]
Fix documentation of psql's ECHO all mode.

"ECHO all" is ignored for interactive input, and has been for a very long
time, though possibly not for as long as the documentation has claimed the
opposite.  Fix that, and also note that empty lines aren't echoed, which
while dubious is another longstanding behavior (it's embedded in our
regression test files for one thing).  Per bug #12721 from Hans Ginzel.

In HEAD, also improve the code comments in this area, and suppress an
unnecessary fflush(stdout) when we're not echoing.  That would likely
be safe to back-patch, but I'll not risk it mere hours before a release
wrap.

9 years agoFirst-draft release notes for 9.4.1 et al.
Tom Lane [Sat, 31 Jan 2015 22:30:30 +0000 (17:30 -0500)]
First-draft release notes for 9.4.1 et al.

As usual, the release notes for older branches will be made by cutting
these down, but put them up for community review first.

Note: a significant fraction of these items don't apply to 9.4.1, only to
older branches, because the fixes already appeared in 9.4.0.  These can be
distinguished by noting the branch commits in the associated SGML comments.
This will be adjusted tomorrow while copying items into the older
release-X.Y.sgml files.  In a few cases I've made two separate entries with
different wordings for 9.4 than for the equivalent commits in the older
branches.

9 years agoUpdate time zone data files to tzdata release 2015a.
Tom Lane [Sat, 31 Jan 2015 03:45:44 +0000 (22:45 -0500)]
Update time zone data files to tzdata release 2015a.

DST law changes in Chile and Mexico (state of Quintana Roo).
Historical changes for Iceland.

9 years agoPolicy documentation improvements
Stephen Frost [Fri, 30 Jan 2015 21:09:41 +0000 (16:09 -0500)]
Policy documentation improvements

In ALTER POLICY, use 'check_expression' instead of 'expression' for the
parameter, to match up with the recent CREATE POLICY change.

In CREATE POLICY, frame the discussion as granting access to rows
instead of limiting access to rows.  Further, clarify that the
expression must return true for rows to be visible/allowed and that a
false or NULL result will mean the row is not visible/allowed.

Per discussion with Dean Rasheed and Robert.

9 years agoFix jsonb Unicode escape processing, and in consequence disallow \u0000.
Tom Lane [Fri, 30 Jan 2015 19:44:46 +0000 (14:44 -0500)]
Fix jsonb Unicode escape processing, and in consequence disallow \u0000.

We've been trying to support \u0000 in JSON values since commit
78ed8e03c67d7333, and have introduced increasingly worse hacks to try to
make it work, such as commit 0ad1a816320a2b53.  However, it fundamentally
can't work in the way envisioned, because the stored representation looks
the same as for \\u0000 which is not the same thing at all.  It's also
entirely bogus to output \u0000 when de-escaped output is called for.

The right way to do this would be to store an actual 0x00 byte, and then
throw error only if asked to produce de-escaped textual output.  However,
getting to that point seems likely to take considerable work and may well
never be practical in the 9.4.x series.

To preserve our options for better behavior while getting rid of the nasty
side-effects of 0ad1a816320a2b53, revert that commit in toto and instead
throw error if \u0000 is used in a context where it needs to be de-escaped.
(These are the same contexts where non-ASCII Unicode escapes throw error
if the database encoding isn't UTF8, so this behavior is by no means
without precedent.)

In passing, make both the \u0000 case and the non-ASCII Unicode case report
ERRCODE_UNTRANSLATABLE_CHARACTER / "unsupported Unicode escape sequence"
rather than claiming there's something wrong with the input syntax.

Back-patch to 9.4, where we have to do something because 0ad1a816320a2b53
broke things for many cases having nothing to do with \u0000.  9.3 also has
bogus behavior, but only for that specific escape value, so given the lack
of field complaints it seems better to leave 9.3 alone.

9 years agodoc: Remove superfluous table column
Peter Eisentraut [Fri, 30 Jan 2015 18:30:35 +0000 (13:30 -0500)]
doc: Remove superfluous table column

9 years agoFix Coverity warning about contrib/pgcrypto's mdc_finish().
Tom Lane [Fri, 30 Jan 2015 18:04:56 +0000 (13:04 -0500)]
Fix Coverity warning about contrib/pgcrypto's mdc_finish().

Coverity points out that mdc_finish returns a pointer to a local buffer
(which of course is gone as soon as the function returns), leaving open
a risk of misbehaviors possibly as bad as a stack overwrite.

In reality, the only possible call site is in process_data_packets()
which does not examine the returned pointer at all.  So there's no
live bug, but nonetheless the code is confusing and risky.  Refactor
to avoid the issue by letting process_data_packets() call mdc_finish()
directly instead of going through the pullf_read() API.

Although this is only cosmetic, it seems good to back-patch so that
the logic in pgp-decrypt.c stays in sync across all branches.

Marko Kreen

9 years agoProvide a way to supress the "out of memory" error when allocating.
Robert Haas [Fri, 30 Jan 2015 17:56:48 +0000 (12:56 -0500)]
Provide a way to supress the "out of memory" error when allocating.

Using the new interface MemoryContextAllocExtended, callers can
specify MCXT_ALLOC_NO_OOM if they are prepared to handle a NULL
return value.

Michael Paquier, reviewed and somewhat revised by me.

9 years agoFix assorted oversights in range selectivity estimation.
Tom Lane [Fri, 30 Jan 2015 17:30:38 +0000 (12:30 -0500)]
Fix assorted oversights in range selectivity estimation.

calc_rangesel() failed outright when comparing range variables to empty
constant ranges with < or >=, as a result of missing cases in a switch.
It also produced a bogus estimate for > comparison to an empty range.

On top of that, the >= and > cases were mislabeled throughout.  For
nonempty constant ranges, they managed to produce the right answers
anyway as a result of counterbalancing typos.

Also, default_range_selectivity() omitted cases for elem <@ range,
range &< range, and range &> range, so that rather dubious defaults
were applied for these operators.

In passing, rearrange the code in rangesel() so that the elem <@ range
case is handled in a less opaque fashion.

Report and patch by Emre Hasegeli, some additional work by me

9 years agoFix query-duration memory leak with GIN rescans.
Heikki Linnakangas [Fri, 30 Jan 2015 16:58:23 +0000 (17:58 +0100)]
Fix query-duration memory leak with GIN rescans.

The requiredEntries / additionalEntries arrays were not freed in
freeScanKeys() like other per-key stuff.

It's not obvious, but startScanKey() was only ever called after the keys
have been initialized with ginNewScanKey(). That's why it doesn't need to
worry about freeing existing arrays. The ginIsNewKey() test in gingetbitmap
was never true, because ginrescan free's the existing keys, and it's not OK
to call gingetbitmap twice in a row without calling ginrescan in between.
To make that clear, remove the unnecessary ginIsNewKey(). And just to be
extra sure that nothing funny happens if there is an existing key after all,
call freeScanKeys() to free it if it exists. This makes the code more
straightforward.

(I'm seeing other similar leaks in testing a query that rescans an GIN index
scan, but that's a different issue. This just fixes the obvious leak with
those two arrays.)

Backpatch to 9.4, where GIN fast scan was added.

9 years agoAllow pg_dump to use jobs and serializable transactions together.
Kevin Grittner [Fri, 30 Jan 2015 14:57:24 +0000 (08:57 -0600)]
Allow pg_dump to use jobs and serializable transactions together.

Since 9.3, when the --jobs option was introduced, using it together
with the --serializable-deferrable option generated multiple
errors.  We can get correct behavior by allowing the connection
which acquires the snapshot to use SERIALIZABLE, READ ONLY,
DEFERRABLE and pass that to the workers running the other
connections using REPEATABLE READ, READ ONLY.  This is a bit of a
kluge since the SERIALIZABLE behavior is achieved by running some
of the participating connections at a different isolation level,
but it is a simple and safe change, suitable for back-patching.

This will be followed by a proposal for a more invasive fix with
some slight behavioral changes on just the master branch, based on
suggestions from Andres Freund, but the kluge will be applied to
master until something is agreed along those lines.

Back-patched to 9.3, where the --jobs option was added.

Based on report from Alexander Korotkov

9 years agoFix BuildIndexValueDescription for expressions
Stephen Frost [Fri, 30 Jan 2015 02:59:34 +0000 (21:59 -0500)]
Fix BuildIndexValueDescription for expressions

In 804b6b6db4dcfc590a468e7be390738f9f7755fb we modified
BuildIndexValueDescription to pay attention to which columns are visible
to the user, but unfortunatley that commit neglected to consider indexes
which are built on expressions.

Handle error-reporting of violations of constraint indexes based on
expressions by not returning any detail when the user does not have
table-level SELECT rights.

Backpatch to 9.0, as the prior commit was.

Pointed out by Tom.

9 years agodoc: clarify libpq's 'verify-full' host name check
Bruce Momjian [Fri, 30 Jan 2015 02:20:21 +0000 (21:20 -0500)]
doc:  clarify libpq's 'verify-full' host name check

Report by David Guyot

9 years agoHandle unexpected query results, especially NULLs, safely in connectby().
Tom Lane [Fri, 30 Jan 2015 01:18:33 +0000 (20:18 -0500)]
Handle unexpected query results, especially NULLs, safely in connectby().

connectby() didn't adequately check that the constructed SQL query returns
what it's expected to; in fact, since commit 08c33c426bfebb32 it wasn't
checking that at all.  This could result in a null-pointer-dereference
crash if the constructed query returns only one column instead of the
expected two.  Less excitingly, it could also result in surprising data
conversion failures if the constructed query returned values that were
not I/O-conversion-compatible with the types specified by the query
calling connectby().

In all branches, insist that the query return at least two columns;
this seems like a minimal sanity check that can't break any reasonable
use-cases.

In HEAD, insist that the constructed query return the types specified by
the outer query, including checking for typmod incompatibility, which the
code never did even before it got broken.  This is to hide the fact that
the implementation does a conversion to text and back; someday we might
want to improve that.

In back branches, leave that alone, since adding a type check in a minor
release is more likely to break things than make people happy.  Type
inconsistencies will continue to work so long as the actual type and
declared type are I/O representation compatible, and otherwise will fail
the same way they used to.

Also, in all branches, be on guard for NULL results from the constructed
query, which formerly would cause null-pointer dereference crashes.
We now print the row with the NULL but don't recurse down from it.

In passing, get rid of the rather pointless idea that
build_tuplestore_recursively() should return the same tuplestore that's
passed to it.

Michael Paquier, adjusted somewhat by me

9 years agoProperly terminate the array returned by GetLockConflicts().
Andres Freund [Thu, 29 Jan 2015 16:49:03 +0000 (17:49 +0100)]
Properly terminate the array returned by GetLockConflicts().

GetLockConflicts() has for a long time not properly terminated the
returned array. During normal processing the returned array is zero
initialized which, while not pretty, is sufficient to be recognized as
a invalid virtual transaction id. But the HotStandby case is more than
aesthetically broken: The allocated (and reused) array is neither
zeroed upon allocation, nor reinitialized, nor terminated.

Not having a terminating element means that the end of the array will
not be recognized and that recovery conflict handling will thus read
ahead into adjacent memory. Only terminating when hitting memory
content that looks like a invalid virtual transaction id.  Luckily
this seems so far not have caused significant problems, besides making
recovery conflict more expensive.

Discussion: 20150127142713.GD29457@awork2.anarazel.de

Backpatch into all supported branches.

9 years agoAlign buffer descriptors to cache line boundaries.
Andres Freund [Thu, 29 Jan 2015 16:49:03 +0000 (17:49 +0100)]
Align buffer descriptors to cache line boundaries.

Benchmarks has shown that aligning the buffer descriptor array to
cache lines is important for scalability; especially on bigger,
multi-socket, machines.

Currently the array sometimes already happens to be aligned by
happenstance, depending how large previous shared memory allocations
were. That can lead to wildly varying performance results after minor
configuration changes.

In addition to aligning the start of descriptor array, also force the
size of individual descriptors to be of a common cache line size (64
bytes). That happens to already be the case on 64bit platforms, but
this way we can change the struct BufferDesc more easily.

As the alignment primarily matters in highly concurrent workloads
which probably all are 64bit these days, and the space wastage of
element alignment would be a bit more noticeable on 32bit systems, we
don't force the stride to be cacheline sized on 32bit platforms for
now. If somebody does actual performance testing, we can reevaluate
that decision by changing the definition of BUFFERDESC_PADDED_SIZE.

Discussion: 20140202151319.GD32123@awork2.anarazel.de

Per discussion with Bruce Momjan, Tom Lane, Robert Haas, and Peter
Geoghegan.

9 years agoFix #ifdefed'ed out code to compile again.
Andres Freund [Thu, 29 Jan 2015 16:49:03 +0000 (17:49 +0100)]
Fix #ifdefed'ed out code to compile again.

9 years agoFix bug where GIN scan keys were not initialized with gin_fuzzy_search_limit.
Heikki Linnakangas [Thu, 29 Jan 2015 17:35:55 +0000 (19:35 +0200)]
Fix bug where GIN scan keys were not initialized with gin_fuzzy_search_limit.

When gin_fuzzy_search_limit was used, we could jump out of startScan()
without calling startScanKey(). That was harmless in 9.3 and below, because
startScanKey()() didn't do anything interesting, but in 9.4 it initializes
information needed for skipping entries (aka GIN fast scans), and you
readily get a segfault if it's not done. Nevertheless, it was clearly wrong
all along, so backpatch all the way to 9.1 where the early return was
introduced.

(AFAICS startScanKey() did nothing useful in 9.3 and below, because the
fields it initialized were already initialized in ginFillScanKey(), but I
don't dare to change that in a minor release. ginFillScanKey() is always
called in gingetbitmap() even though there's a check there to see if the
scan keys have already been initialized, because they never are; ginrescan()
free's them.)

In the passing, remove unnecessary if-check from the second inner loop in
startScan(). We already check in the first loop that the condition is true
for all entries.

Reported by Olaf Gawenda, bug #12694, Backpatch to 9.1 and above, although
AFAICS it causes a live bug only in 9.4.

9 years agoMove out-of-memory error checks from aset.c to mcxt.c
Robert Haas [Thu, 29 Jan 2015 15:23:38 +0000 (10:23 -0500)]
Move out-of-memory error checks from aset.c to mcxt.c

This potentially allows us to add mcxt.c interfaces that do something
other than throw an error when memory cannot be allocated.  We'll
handle adding those interfaces in a separate commit.

Michael Paquier, with minor changes by me

9 years agoReword CREATE POLICY parameter descriptions
Stephen Frost [Thu, 29 Jan 2015 04:21:54 +0000 (23:21 -0500)]
Reword CREATE POLICY parameter descriptions

The parameter description for the using_expression and check_expression
in CREATE POLICY were unclear and arguably included a typo.  Clarify
and improve the consistency of that language.

Pointed out by Dean Rasheed.

9 years agoCREATE POLICY expression -> using_expression
Stephen Frost [Thu, 29 Jan 2015 03:59:03 +0000 (22:59 -0500)]
CREATE POLICY expression -> using_expression

The syntax for CREATE POLICY simply used "expression" for the USING
expression, while the WITH CHECK expression was "check_expression".
Given that we have two expressions, it's sensible to explcitly name both
to maintain clarity.

This patch simply changes the generic "expression" to be
"using_expression".

Pointed out by Peter Geoghegan.

9 years agoImprove CREATE POLICY documentation
Stephen Frost [Thu, 29 Jan 2015 03:16:24 +0000 (22:16 -0500)]
Improve CREATE POLICY documentation

The CREATE POLICY documention didn't sufficiently clarify what happens
when a given command type (eg: ALL or UPDATE) accepts both USING and
WITH CHECK clauses, but only the USING clause is defined.  Add language
to clarify that, in such a case, the USING clause will be used for both
USING and WITH CHECK cases.

Pointed out by Peter Geoghegan.

9 years agoAdd usebypassrls to pg_user and pg_shadow
Stephen Frost [Thu, 29 Jan 2015 02:47:15 +0000 (21:47 -0500)]
Add usebypassrls to pg_user and pg_shadow

The row level security patches didn't add the 'usebypassrls' columns to
the pg_user and pg_shadow views on the belief that they were deprecated,
but we havn't actually said they are and therefore we should include it.

This patch corrects that, adds missing documentation for rolbypassrls
into the system catalog page for pg_authid, along with the entries for
pg_user and pg_shadow, and cleans up a few other uses of 'row-level'
cases to be 'row level' in the docs.

Pointed out by Amit Kapila.

Catalog version bump due to system view changes.

9 years agoClean up range-table building in copy.c
Stephen Frost [Wed, 28 Jan 2015 22:42:28 +0000 (17:42 -0500)]
Clean up range-table building in copy.c

Commit 804b6b6db4dcfc590a468e7be390738f9f7755fb added the build of a
range table in copy.c to initialize the EState es_range_table since it
can be needed in error paths.  Unfortunately, that commit didn't
appreciate that some code paths might end up not initializing the rte
which is used to build the range table.

Fix that and clean up a couple others things along the way- build it
only once and don't explicitly set it on the !is_from path as it
doesn't make any sense there (cstate is palloc0'd, so this isn't an
issue from an initializing standpoint either).

The prior commit went back to 9.0, but this only goes back to 9.1 as
prior to that the range table build happens immediately after building
the RTE and therefore doesn't suffer from this issue.

Pointed out by Robert.

9 years agoFix column-privilege leak in error-message paths
Stephen Frost [Mon, 12 Jan 2015 22:04:11 +0000 (17:04 -0500)]
Fix column-privilege leak in error-message paths

While building error messages to return to the user,
BuildIndexValueDescription, ExecBuildSlotValueDescription and
ri_ReportViolation would happily include the entire key or entire row in
the result returned to the user, even if the user didn't have access to
view all of the columns being included.

Instead, include only those columns which the user is providing or which
the user has select rights on.  If the user does not have any rights
to view the table or any of the columns involved then no detail is
provided and a NULL value is returned from BuildIndexValueDescription
and ExecBuildSlotValueDescription.  Note that, for key cases, the user
must have access to all of the columns for the key to be shown; a
partial key will not be returned.

Further, in master only, do not return any data for cases where row
security is enabled on the relation and row security should be applied
for the user.  This required a bit of refactoring and moving of things
around related to RLS- note the addition of utils/misc/rls.c.

Back-patch all the way, as column-level privileges are now in all
supported versions.

This has been assigned CVE-2014-8161, but since the issue and the patch
have already been publicized on pgsql-hackers, there's no point in trying
to hide this commit.

9 years agoFix typo in comment.
Heikki Linnakangas [Wed, 28 Jan 2015 08:26:30 +0000 (10:26 +0200)]
Fix typo in comment.

9 years agoRemove dead NULL-pointer checks in GiST code.
Heikki Linnakangas [Wed, 28 Jan 2015 07:47:39 +0000 (09:47 +0200)]
Remove dead NULL-pointer checks in GiST code.

gist_poly_compress() and gist_circle_compress() checked for a NULL-pointer
key argument, but that was dead code; the gist code never passes a
NULL-pointer to the "compress" method.

This commit also removes a documentation note added in commit a0a3883,
about doing NULL-pointer checks in the "compress" method. It was added
based on the fact that some implementations were doing NULL-pointer
checks, but those checks were unnecessary in the first place.

The NULL-pointer check in gbt_var_same() function was also unnecessary.
The arguments to the "same" method come from the "compress", "union", or
"picksplit" methods, but none of them return a NULL pointer.

None of this is to be confused with SQL NULL values. Those are dealt with
by the gist machinery, and are never passed to the GiST opclass methods.

Michael Paquier

9 years agoFix NUMERIC field access macros to treat NaNs consistently.
Tom Lane [Tue, 27 Jan 2015 17:06:31 +0000 (12:06 -0500)]
Fix NUMERIC field access macros to treat NaNs consistently.

Commit 145343534c153d1e6c3cff1fa1855787684d9a38 arranged to store numeric
NaN values as short-header numerics, but the field access macros did not
get the memo: they thought only "SHORT" numerics have short headers.

Most of the time this makes no difference because we don't access the
weight or dscale of a NaN; but numeric_send does that.  As pointed out
by Andrew Gierth, this led to fetching uninitialized bytes.

AFAICS this could not have any worse consequences than that; in particular,
an unaligned stored numeric would have been detoasted by PG_GETARG_NUMERIC,
so that there's no risk of a fetch off the end of memory.  Still, the code
is wrong on its own terms, and it's not hard to foresee future changes that
might expose us to real risks.  So back-patch to all affected branches.

9 years agoAdd a note to PG_TRY's documentation about volatile safety.
Tom Lane [Mon, 26 Jan 2015 20:53:37 +0000 (15:53 -0500)]
Add a note to PG_TRY's documentation about volatile safety.

We had better memorialize what the actual requirements are for this.

9 years agoFix volatile-safety issue in dblink's materializeQueryResult().
Tom Lane [Mon, 26 Jan 2015 20:17:33 +0000 (15:17 -0500)]
Fix volatile-safety issue in dblink's materializeQueryResult().

Some fields of the sinfo struct are modified within PG_TRY and then
referenced within PG_CATCH, so as with recent patch to async.c, "volatile"
is necessary for strict POSIX compliance; and that propagates to a couple
of subroutines as well as materializeQueryResult() itself.  I think the
risk of actual issues here is probably higher than in async.c, because
storeQueryResult() is likely to get inlined into materializeQueryResult(),
leaving the compiler free to conclude that its stores into sinfo fields are
dead code.

9 years agoRe-enable abbreviated keys on Windows.
Robert Haas [Mon, 26 Jan 2015 19:28:14 +0000 (14:28 -0500)]
Re-enable abbreviated keys on Windows.

Commit 1be4eb1b2d436d1375899c74e4c74486890d8777 disabled this, but I
think the real problem here was fixed by commit
b181a91981203f6ec9403115a2917bd3f9473707 and commit
d060e07fa919e0eb681e2fa2cfbe63d6c40eb2cf.  So let's try re-enabling
it now and see what happens.

9 years agoFix volatile-safety issue in pltcl_SPI_execute_plan().
Tom Lane [Mon, 26 Jan 2015 17:18:25 +0000 (12:18 -0500)]
Fix volatile-safety issue in pltcl_SPI_execute_plan().

The "callargs" variable is modified within PG_TRY and then referenced
within PG_CATCH, which is exactly the coding pattern we've now found
to be unsafe.  Marking "callargs" volatile would be problematic because
it is passed by reference to some Tcl functions, so fix the problem
by not modifying it within PG_TRY.  We can just postpone the free()
till we exit the PG_TRY construct, as is already done elsewhere in this
same file.

Also, fix failure to free(callargs) when exiting on too-many-arguments
error.  This is only a minor memory leak, but a leak nonetheless.

In passing, remove some unnecessary "volatile" markings in the same
function.  Those doubtless are there because gcc 2.95.3 whinged about
them, but we now know that its algorithm for complaining is many bricks
shy of a load.

This is certainly a live bug with compilers that optimize similarly
to current gcc, so back-patch to all active branches.

9 years agoFix volatile-safety issue in asyncQueueReadAllNotifications().
Tom Lane [Mon, 26 Jan 2015 16:57:33 +0000 (11:57 -0500)]
Fix volatile-safety issue in asyncQueueReadAllNotifications().

The "pos" variable is modified within PG_TRY and then referenced
within PG_CATCH, so for strict POSIX conformance it must be marked
volatile.  Superficially the code looked safe because pos's address
was taken, which was sufficient to force it into memory ... but it's
not sufficient to ensure that the compiler applies updates exactly
where the program text says to.  The volatility marking has to extend
into a couple of subroutines too, but I think that's probably a good
thing because the risk of out-of-order updates is mostly in those
subroutines not asyncQueueReadAllNotifications() itself.  In principle
the compiler could have re-ordered operations such that an error could
be thrown while "pos" had an incorrect value.

It's unclear how real the risk is here, but for safety back-patch
to all active branches.

9 years agoFurther cleanup of ReorderBufferCommit().
Tom Lane [Mon, 26 Jan 2015 03:49:56 +0000 (22:49 -0500)]
Further cleanup of ReorderBufferCommit().

On closer inspection, we can remove the "volatile" qualifier on
"using_subtxn" so long as we initialize that before the PG_TRY block,
which there's no particularly good reason not to do.
Also, push the "change" variable inside the PG_TRY so as to remove
all question of whether it needs "volatile", and remove useless
early initializations of "snapshow_now" and "using_subtxn".

9 years agoClean up assorted issues in ALTER SYSTEM coding.
Tom Lane [Mon, 26 Jan 2015 01:19:04 +0000 (20:19 -0500)]
Clean up assorted issues in ALTER SYSTEM coding.

Fix unsafe use of a non-volatile variable in PG_TRY/PG_CATCH in
AlterSystemSetConfigFile().  While at it, clean up a bundle of other
infelicities and outright bugs, including corner-case-incorrect linked list
manipulation, a poorly designed and worse documented parse-and-validate
function (which even included some randomly chosen hard-wired substitutes
for the specified elevel in one code path ... wtf?), direct use of open()
instead of fd.c's facilities, inadequate checking of write()'s return
value, and generally poorly written commentary.

9 years agoClean up some mess in row-security patches.
Tom Lane [Sat, 24 Jan 2015 21:16:22 +0000 (16:16 -0500)]
Clean up some mess in row-security patches.

Fix unsafe coding around PG_TRY in RelationBuildRowSecurity: can't change
a variable inside PG_TRY and then use it in PG_CATCH without marking it
"volatile".  In this case though it seems saner to avoid that by doing
a single assignment before entering the TRY block.

I started out just intending to fix that, but the more I looked at the
row-security code the more distressed I got.  This patch also fixes
incorrect construction of the RowSecurityPolicy cache entries (there was
not sufficient care taken to copy pass-by-ref data into the cache memory
context) and a whole bunch of sloppiness around the definition and use of
pg_policy.polcmd.  You can't use nulls in that column because initdb will
mark it NOT NULL --- and I see no particular reason why a null entry would
be a good idea anyway, so changing initdb's behavior is not the right
answer.  The internal value of '\0' wouldn't be suitable in a "char" column
either, so after a bit of thought I settled on using '*' to represent ALL.
Chasing those changes down also revealed that somebody wasn't paying
attention to what the underlying values of ACL_UPDATE_CHR etc really were,
and there was a great deal of lackadaiscalness in the catalogs.sgml
documentation for pg_policy and pg_policies too.

This doesn't pretend to be a complete code review for the row-security
stuff, it just fixes the things that were in my face while dealing with
the bugs in RelationBuildRowSecurity.

9 years agoFix unsafe coding in ReorderBufferCommit().
Tom Lane [Sat, 24 Jan 2015 18:25:19 +0000 (13:25 -0500)]
Fix unsafe coding in ReorderBufferCommit().

"iterstate" must be marked volatile since it's changed inside the PG_TRY
block and then used in the PG_CATCH stanza.  Noted by Mark Wilding of
Salesforce.  (We really need to see if we can't get the C compiler to warn
about this.)

Also, reset iterstate to NULL after the mainline ReorderBufferIterTXNFinish
call, to ensure the PG_CATCH block doesn't try to do that a second time.