1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-30 11:03:19 +03:00
Commit Graph

2574 Commits

Author SHA1 Message Date
dfc71a1b25 pg_upgrade: Add SYSTEMQUOTE for Windows paths with spaces
Another fix for commit 244142d32a.  We
added SYSTEMQUOTE to the win32 popen() compatibility function in PG 9.4.

Backpatch-through: 9.3 only
2018-07-31 19:06:32 -04:00
a1d383ab57 pg_upgrade: fix --check for live source server checks
Fix for commit 244142d32a.

Backpatch-through: 9.3
2018-07-31 18:10:06 -04:00
003132d178 Fix earthdistance test suite function name typo.
Affected test queries have been testing the wrong thing since their
introduction in commit 4c1383efd1.
Back-patch to 9.3 (all supported versions).
2018-07-29 12:02:12 -07:00
a5c84e0b7f pg_upgrade: check for clean server shutdowns
Previously pg_upgrade checked for the pid file and started/stopped the
server to force a clean shutdown.  However, "pg_ctl -m immediate"
removes the pid file but doesn't do a clean shutdown, so check
pg_controldata for a clean shutdown too.

Diagnosed-by: Vimalraj A

Discussion: https://postgr.es/m/CAFKBAK5e4Q-oTUuPPJ56EU_d2Rzodq6GWKS3ncAk3xo7hAsOZg@mail.gmail.com

Backpatch-through: 9.3
2018-07-28 15:01:55 -04:00
b782b7c57f Further portability hacking in pg_upgrade's test script.
I blew the dust off a Bourne shell (file date 1996, yea verily) and
tried to run test.sh with it.  It mostly worked, but I found that the
temp-directory creation code introduced by commit be76a6d39 was not
compatible, for a couple of reasons: this shell thinks "set -e" should
force an exit if a command within backticks fails, and it also thinks code
within braces should be executed by a sub-shell, meaning that variable
settings don't propagate back up to the parent shell.  In view of Victor
Wagner's report that Solaris is still using pre-POSIX shells, seems like
we oughta make this case work.  It's not like the code is any less
idiomatic this way; the prior coding technique appeared nowhere else.

(There is a remaining bash-ism here, which is that $RANDOM doesn't do
what the code hopes in non-bash shells.  But the use of $$ elsewhere in
that path should be enough to ensure uniqueness and some amount of
randomness, so I think it's okay as-is.)

Back-patch to all supported branches, as the previous commit was.

Discussion: https://postgr.es/m/20180720153820.69e9ae6c@fafnir.local.vm
2018-07-21 15:40:52 -04:00
b0a7a41db5 Fix misc typos, mostly in comments.
A collection of typos I happened to spot while reading code, as well as
grepping for common mistakes.

Backpatch to all supported versions, as applicable, to avoid conflicts
when backporting other commits in the future.
2018-07-18 16:54:40 +03:00
cbbe75fe67 Fix crash in contrib/ltree's lca() function for empty input array.
lca_inner() wasn't prepared for the possibility of getting no inputs.
Fix that, and make some cosmetic improvements to the code while at it.

Also, I thought the documentation of this function as returning the
"longest common prefix" of the paths was entirely misleading; it really
returns a path one shorter than the longest common prefix, for the typical
definition of "prefix".  Don't use that term in the docs, and adjust the
examples to clarify what really happens.

This has been broken since its beginning, so back-patch to all supported
branches.

Per report from Hailong Li.  Thanks to Pierre Ducroquet for diagnosing
and for the initial patch, though I whacked it around some and added
test cases.

Discussion: https://postgr.es/m/5b0d8e4f-f2a3-1305-d612-e00e35a7be66@qunar.com
2018-07-13 18:45:30 -04:00
f6f75539d2 Prevent accidental linking of system-supplied copies of libpq.so etc.
Back-patch commit dddfc4cb2, which broke LDFLAGS and related Makefile
variables into two parts, one for within-build-tree library references and
one for external libraries, to ensure that the order of -L flags has all
of the former before all of the latter.  This turns out to fix a problem
recently noted on buildfarm member peripatus, that we attempted to
incorporate code from libpgport.a into a shared library.  That will fail on
platforms that are sticky about putting non-PIC code into shared libraries.
(It's quite surprising we hadn't seen such failures before, since the code
in question has been like that for a long time.)

I think that peripatus' problem could have been fixed with just a subset
of this patch; but since the previous issue of accidentally linking to the
wrong copy of a Postgres shlib seems likely to bite people in the field,
let's just back-patch the whole change.  Now that commit dddfc4cb2 has
survived some beta testing, I'm less afraid to back-patch it than I was
at the time.

This also fixes undesired inclusion of "-DFRONTEND" in pg_config's CPPFLAGS
output (in 9.6 and up) and undesired inclusion of "-L../../src/common" in
its LDFLAGS output (in all supported branches).

Back-patch to v10 and older branches; this is already in v11.

Discussion: https://postgr.es/m/20180704234304.bq2dxispefl65odz@ler-imac.local
2018-07-09 17:23:32 -04:00
3243cbc085 Use snprintf not sprintf in pg_waldump's timestamptz_to_str.
This could only cause an issue if strftime returned a ridiculously
long timezone name, which seems unlikely; and it wouldn't qualify
as a security problem even then, since pg_waldump (nee pg_xlogdump)
is a debug tool not part of the server.  But gcc 8 has started issuing
warnings about it, so let's use snprintf and be safe.

Backpatch to 9.3 where this code was added.

Discussion: https://postgr.es/m/21789.1529170195@sss.pgh.pa.us
2018-06-16 14:45:47 -04:00
594ff3d7d7 Allow MSYS as well as MINGW in Msys uname
Msys2's uname -s outputs a string beginning MSYS rather than MINGW as is
output by Msys. Allow either in pg_upgrade's test.sh.

Backpatch to all live branches.
2018-05-04 15:05:28 -04:00
9ed1c8ca25 Fix potentially-unportable code in contrib/adminpack.
Spelling access(2)'s second argument as "2" is just horrid.
POSIX makes no promises as to the numeric values of W_OK and related
macros.  Even if it accidentally works as intended on every supported
platform, it's still unreadable and inconsistent with adjacent code.

In passing, don't spell "NULL" as "0" either.  Yes, that's legal C;
no, it's not project style.

Back-patch, just in case the unportability is real and not theoretical.
(Most likely, even if a platform had different bit assignments for
access()'s modes, there'd not be an observable behavior difference
here; but I'm being paranoid today.)
2018-04-15 13:02:12 -04:00
0bea99bd92 Fix assorted issues in convert_to_scalar().
If convert_to_scalar is passed a pair of datatypes it can't cope with,
its former behavior was just to elog(ERROR).  While this is OK so far as
the core code is concerned, there's extension code that would like to use
scalarltsel/scalargtsel/etc as selectivity estimators for operators that
work on non-core datatypes, and this behavior is a show-stopper for that
use-case.  If we simply allow convert_to_scalar to return FALSE instead of
outright failing, then the main logic of scalarltsel/scalargtsel will work
fine for any operator that behaves like a scalar inequality comparison.
The lack of conversion capability will mean that we can't estimate to
better than histogram-bin-width precision, since the code will effectively
assume that the comparison constant falls at the middle of its bin.  But
that's still a lot better than nothing.  (Someday we should provide a way
for extension code to supply a custom version of convert_to_scalar, but
today is not that day.)

While poking at this issue, we noted that the existing code for handling
type bytea in convert_to_scalar is several bricks shy of a load.
It assumes without checking that if the comparison value is type bytea,
the bounds values are too; in the worst case this could lead to a crash.
It also fails to detoast the input values, so that the comparison result is
complete garbage if any input is toasted out-of-line, compressed, or even
just short-header.  I'm not sure how often such cases actually occur ---
the bounds values, at least, are probably safe since they are elements of
an array and hence can't be toasted.  But that doesn't make this code OK.

Back-patch to all supported branches, partly because author requested that,
but mostly because of the bytea bugs.  The change in API for the exposed
routine convert_network_to_scalar() is theoretically a back-patch hazard,
but it seems pretty unlikely that any third-party code is calling that
function directly.

Tomas Vondra, with some adjustments by me

Discussion: https://postgr.es/m/b68441b6-d18f-13ab-b43b-9a72188a4e02@2ndquadrant.com
2018-03-03 20:31:35 -05:00
10102c91ea Rename base64 routines to avoid conflict with Solaris built-in functions.
Solaris 11.4 has built-in functions named b64_encode and b64_decode.
Rename ours to something else to avoid the conflict (fortunately,
ours are static so the impact is limited).

One could wish for less duplication of code in this area, but that
would be a larger patch and not very suitable for back-patching.
Since this is a portability fix, we want to put it into all supported
branches.

Report and initial patch by Rainer Orth, reviewed and adjusted a bit
by Michael Paquier

Discussion: https://postgr.es/m/ydd372wk28h.fsf@CeBiTec.Uni-Bielefeld.DE
2018-02-28 18:33:45 -05:00
3db38b0cef Empty search_path in Autovacuum and non-psql/pgbench clients.
This makes the client programs behave as documented regardless of the
connect-time search_path and regardless of user-created objects.  Today,
a malicious user with CREATE permission on a search_path schema can take
control of certain of these clients' queries and invoke arbitrary SQL
functions under the client identity, often a superuser.  This is
exploitable in the default configuration, where all users have CREATE
privilege on schema "public".

This changes behavior of user-defined code stored in the database, like
pg_index.indexprs and pg_extension_config_dump().  If they reach code
bearing unqualified names, "does not exist" or "no schema has been
selected to create in" errors might appear.  Users may fix such errors
by schema-qualifying affected names.  After upgrading, consider watching
server logs for these errors.

The --table arguments of src/bin/scripts clients have been lax; for
example, "vacuumdb -Zt pg_am\;CHECKPOINT" performed a checkpoint.  That
now fails, but for now, "vacuumdb -Zt 'pg_am(amname);CHECKPOINT'" still
performs a checkpoint.

Back-patch to 9.3 (all supported versions).

Reviewed by Tom Lane, though this fix strategy was not his first choice.
Reported by Arseniy Sharoglazov.

Security: CVE-2018-1058
2018-02-26 07:39:48 -08:00
a7a6051cfd Allow auto_explain.log_min_duration to go up to INT_MAX.
The previous limit of INT_MAX / 1000 seems to have been cargo-culted in
from somewhere else.  Or possibly the value was converted to microseconds
at some point; but in all supported releases, it's just compared to other
values, so there's no need for the restriction.  This change raises the
effective limit from ~35 minutes to ~24 days, which conceivably is useful
to somebody, and anyway it's more consistent with the range of the core
log_min_duration_statement GUC.

Per complaint from Kevin Bloch.  Back-patch to all supported releases.

Discussion: https://postgr.es/m/8ea82d7e-cb78-8e05-0629-73aa14d2a0ca@codingthat.com
2018-02-23 14:39:23 -05:00
d3b0a23a20 Repair pg_upgrade's failure to preserve relfrozenxid for matviews.
This oversight led to data corruption in matviews, manifesting as
"could not access status of transaction" before our most recent releases,
and "found xmin from before relfrozenxid" errors since then.

The proximate cause of the problem seems to have been confusion between
the task of preserving dropped-column status and the task of preserving
frozenxid status.  Those are required for distinct sets of relkinds,
and the reasoning was entirely undocumented in the source code.  In hopes
of forestalling future errors of the same kind, try to improve the
commentary in this area.

In passing, also improve the remarkably unhelpful comments around
pg_upgrade's set_frozenxids().  That's not actually buggy AFAICS,
but good luck figuring out what it does from the old comments.

Per report from Claudio Freire.  It appears that bug #14852 from Alexey
Ermakov is an earlier report of the same issue, and there may be other
cases that we failed to identify at the time.

Patch by me based on analysis by Andres Freund.  The bug dates back
to the introduction of matviews, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAGTBQpbrY9CdRGGhyBZ9yqY4jWaGC85rUF4X+R7d-aim=mBNsw@mail.gmail.com
Discussion: https://postgr.es/m/20171013115320.28049.86457@wrigleys.postgresql.org
2018-02-21 18:40:24 -05:00
9c59e48a22 Ensure that all temp files made during pg_upgrade are non-world-readable.
pg_upgrade has always attempted to ensure that the transient dump files
it creates are inaccessible except to the owner.  However, refactoring
in commit 76a7650c4 broke that for the file containing "pg_dumpall -g"
output; since then, that file was protected according to the process's
default umask.  Since that file may contain role passwords (hopefully
encrypted, but passwords nonetheless), this is a particularly unfortunate
oversight.  Prudent users of pg_upgrade on multiuser systems would
probably run it under a umask tight enough that the issue is moot, but
perhaps some users are depending only on pg_upgrade's umask changes to
protect their data.

To fix this in a future-proof way, let's just tighten the umask at
process start.  There are no files pg_upgrade needs to write at a
weaker security level; and if there were, transiently relaxing the
umask around where they're created would be a safer approach.

Report and patch by Tom Lane; the idea for the fix is due to Noah Misch.
Back-patch to all supported branches.

Security: CVE-2018-1053
2018-02-05 10:58:27 -05:00
4ab8ded9fb pgcrypto's encrypt() supports AES-128, AES-192, and AES-256
Previously, only 128 was mentioned, but the others are also supported.

Thomas Munro, reviewed by Michael Paquier and extended a bit by me.

Discussion: http://postgr.es/m/CAEepm=1XbBHXYJKofGjnM2Qfz-ZBVqhGU4AqvtgR+Hegy4fdKg@mail.gmail.com
2018-01-31 16:33:46 -05:00
6e9ac0a2e2 Avoid formally-undefined use of memcpy() in hstoreUniquePairs().
hstoreUniquePairs() often called memcpy with equal source and destination
pointers.  Although this is almost surely harmless in practice, it's
undefined according to the letter of the C standard.  Some versions of
valgrind will complain about it, and some versions of libc as well
(cf. commit ad520ec4a).  Tweak the code to avoid doing that.

Noted by Tomas Vondra.  Back-patch to all supported versions because
of the hazard of libc assertions.

Discussion: https://postgr.es/m/bf84d940-90d4-de91-19dd-612e011007f4@fuzzy.cz
2017-11-25 14:42:46 -05:00
77b76fea94 Provide modern examples of how to auto-start Postgres on macOS.
The scripts in contrib/start-scripts/osx don't work at all on macOS
10.10 (Yosemite) or later, because they depend on SystemStarter which
Apple deprecated long ago and removed in 10.10.  Add a new subdirectory
contrib/start-scripts/macos with scripts that use the newer launchd
infrastructure.

Since this problem is independent of which Postgres version you're using,
back-patch to all supported branches.

Discussion: https://postgr.es/m/31338.1510763554@sss.pgh.pa.us
2017-11-17 12:47:44 -05:00
b500297680 start-scripts: switch to $PGUSER before opening $PGLOG.
By default, $PGUSER has permission to unlink $PGLOG.  If $PGUSER
replaces $PGLOG with a symbolic link, the server will corrupt the
link-targeted file by appending log messages.  Since these scripts open
$PGLOG as root, the attack works regardless of target file ownership.

"make install" does not install these scripts anywhere.  Users having
manually installed them in the past should repeat that process to
acquire this fix.  Most script users have $PGLOG writable to root only,
located in $PGDATA.  Just before updating one of these scripts, such
users should rename $PGLOG to $PGLOG.old.  The script will then recreate
$PGLOG with proper ownership.

Reviewed by Peter Eisentraut.  Reported by Antoine Scemama.

Security: CVE-2017-12172
2017-11-06 07:11:13 -08:00
6d9de660dc Stabilize postgres_fdw regression tests.
The new test cases added in commit 8bf58c0d9 turn out to have output
that can vary depending on the lc_messages setting prevailing on the
test server.  Hide the remote end's error messages to ensure stable
output.  This isn't a terribly desirable solution; we'd rather know
that the connection failed for the expected reason and not some other
one.  But there seems little choice for the moment.

Per buildfarm.

Discussion: https://postgr.es/m/18419.1500658570@sss.pgh.pa.us
2017-07-21 14:20:43 -04:00
69711415e6 Re-establish postgres_fdw connections after server or user mapping changes.
Previously, postgres_fdw would keep on using an existing connection even
if the user did ALTER SERVER or ALTER USER MAPPING commands that should
affect connection parameters.  Teach it to watch for catcache invals
on these catalogs and re-establish connections when the relevant catalog
entries change.  Per bug #14738 from Michal Lis.

In passing, clean up some rather crufty decisions in commit ae9bfc5d6
about where fields of ConnCacheEntry should be reset.  We now reset
all the fields whenever we open a new connection.

Kyotaro Horiguchi, reviewed by Ashutosh Bapat and myself.
Back-patch to 9.3 where postgres_fdw appeared.

Discussion: https://postgr.es/m/20170710113917.7727.10247@wrigleys.postgresql.org
2017-07-21 12:51:38 -04:00
5c890645d9 pg_upgrade: start/stop new server after pg_resetwal
When commit 0f33a719fd removed the
instructions to start/stop the new cluster before running rsync, it was
now possible for pg_resetwal/pg_resetxlog to leave the final WAL record
at wal_level=minimum, preventing upgraded standby servers from
reconnecting.

This patch fixes that by having pg_upgrade unconditionally start/stop
the new cluster after pg_resetwal/pg_resetxlog has run.

Backpatch through 9.2 since, though the instructions were added in PG
9.5, they worked all the way back to 9.2.

Discussion: https://postgr.es/m/20170620171844.GC24975@momjian.us

Backpatch-through: 9.2
2017-06-20 13:20:02 -04:00
99090e977d Fix low-probability leaks of PGresult objects in the backend.
We had three occurrences of essentially the same coding pattern
wherein we tried to retrieve a query result from a libpq connection
without blocking.  In the case where PQconsumeInput failed (typically
indicating a lost connection), all three loops simply gave up and
returned, forgetting to clear any previously-collected PGresult
object.  Since those are malloc'd not palloc'd, the oversight results
in a process-lifespan memory leak.

One instance, in libpqwalreceiver, is of little significance because
the walreceiver process would just quit anyway if its connection fails.
But we might as well fix it.

The other two instances, in postgres_fdw, are somewhat more worrisome
because at least in principle the scenario could be repeated, allowing
the amount of memory leaked to build up to something worth worrying
about.  Moreover, in these cases the loops contain CHECK_FOR_INTERRUPTS
calls, as well as other calls that could potentially elog(ERROR),
providing another way to exit without having cleared the PGresult.
Here we need to add PG_TRY logic similar to what exists in quite a
few other places in postgres_fdw.

Coverity noted the libpqwalreceiver bug; I found the other two cases
by checking all calls of PQconsumeInput.

Back-patch to all supported versions as appropriate (9.2 lacks
postgres_fdw, so this is really quite unexciting for that branch).

Discussion: https://postgr.es/m/22620.1497486981@sss.pgh.pa.us
2017-06-15 15:03:57 -04:00
fc267a0c3c postgres_fdw: Allow cancellation of transaction control commands.
Commit f039eaac71, later back-patched
with commit 1b812afb0e, allowed many of
the queries issued by postgres_fdw to fetch remote data to respond to
cancel interrupts in a timely fashion.  However, it didn't do anything
about the transaction control commands, which remained
noninterruptible.

Improve the situation by changing do_sql_command() to retrieve query
results using pgfdw_get_result(), which uses the asynchronous
interface to libpq so that it can check for interrupts every time
libpq returns control.  Since this might result in a situation
where we can no longer be sure that the remote transaction state
matches the local transaction state, add a facility to force all
levels of the local transaction to abort if we've lost track of
the remote state; without this, an apparently-successful commit of
the local transaction might fail to commit changes made on the
remote side.  Also, add a 60-second timeout for queries issue during
transaction abort; if that expires, give up and mark the state of
the connection as unknown.  Drop all such connections when we exit
the local transaction.  Together, these changes mean that if we're
aborting the local toplevel transaction anyway, we can just drop the
remote connection in lieu of waiting (possibly for a very long time)
for it to complete an abort.

This still leaves quite a bit of room for improvement.  PQcancel()
has no asynchronous interface, so if we get stuck sending the cancel
request we'll still hang.  Also, PQsetnonblocking() is not used, which
means we could block uninterruptibly when sending a query.  There
might be some other optimizations possible as well.  Nonetheless,
this allows us to escape a wait for an unresponsive remote server
quickly in many more cases than previously.

Report by Suraj Kharage.  Patch by me and Rafia Sabih.  Review
and testing by Amit Kapila and Tushar Ahuja.

Discussion: http://postgr.es/m/CAF1DzPU8Kx+fMXEbFoP289xtm3bz3t+ZfxhmKavr98Bh-C0TqQ@mail.gmail.com
2017-06-07 15:41:05 -04:00
80a06b5acd Fix new warnings from GCC 7
This addresses the new warning types -Wformat-truncation
-Wformat-overflow that are part of -Wall, via -Wformat, in GCC 7.
2017-05-15 13:31:38 -04:00
3aa16b117a Allow queries submitted by postgres_fdw to be canceled.
Back-patch of commits f039eaac71 and
1b812afb0e, which arranged (in 9.6+) to
make remote queries interruptible.  It was known at the time that the
same problem existed in the back-branches, but I did not back-patch
for lack of a user complaint.

Michael Paquier and Etsuro Fujita, adjusted for older branches by me.
Per gripe from Suraj Kharage.  This doesn't directly addresss Suraj's
gripe, but since the patch that will do so builds up on top of this
work, it seems best to back-patch this part first.

Discussion: http://postgr.es/m/CAF1DzPU8Kx+fMXEbFoP289xtm3bz3t+ZfxhmKavr98Bh-C0TqQ@mail.gmail.com
2017-05-06 22:17:35 -04:00
899960b9fe Avoid depending on non-POSIX behavior of fcntl(2).
The POSIX standard does not say that the success return value for
fcntl(F_SETFD) and fcntl(F_SETFL) is zero; it says only that it's not -1.
We had several calls that were making the stronger assumption.  Adjust
them to test specifically for -1 for strict spec compliance.

The standard further leaves open the possibility that the O_NONBLOCK
flag bit is not the only active one in F_SETFL's argument.  Formally,
therefore, one ought to get the current flags with F_GETFL and store
them back with only the O_NONBLOCK bit changed when trying to change
the nonblock state.  In port/noblock.c, we were doing the full pushup
in pg_set_block but not in pg_set_noblock, which is just weird.  Make
both of them do it properly, since they have little business making
any assumptions about the socket they're handed.  The other places
where we're issuing F_SETFL are working with FDs we just got from
pipe(2), so it's reasonable to assume the FDs' properties are all
default, so I didn't bother adding F_GETFL steps there.

Also, while pg_set_block deserves some points for trying to do things
right, somebody had decided that it'd be even better to cast fcntl's
third argument to "long".  Which is completely loony, because POSIX
clearly says the third argument for an F_SETFL call is "int".

Given the lack of field complaints, these missteps apparently are not
of significance on any common platforms.  But they're still wrong,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/30882.1492800880@sss.pgh.pa.us
2017-04-21 15:55:56 -04:00
4e91330dac Support OpenSSL 1.1.0 in 9.3 and 9.2.
This commit back-patches the equivalent of the 9.5-branch commits
e2838c580 and 48e5ba61e, so that we can work with OpenSSL 1.1.0
in all supported branches.

Original patches by Andreas Karlsson and Heikki Linnakangas,
back-patching work by Andreas Karlsson.

Patch: https://postgr.es/m/0c817abb-3f7d-20fb-583a-58f7593a0bea@proxel.se
Discussion: https://postgr.es/m/5129.1492293840@sss.pgh.pa.us
2017-04-17 13:52:42 -04:00
fad06b2873 Further fix pg_trgm's extraction of trigrams from regular expressions.
Commit 9e43e8714 turns out to have been insufficient: not only is it
necessary to track tentative parent links while considering a set of
arc removals, but it's necessary to track tentative flag additions
as well.  This is because we always merge arc target states into
arc source states; therefore, when considering a merge of the final
state with some other, it is the other state that will acquire a new
TSTATE_FIN bit.  If there's another arc for the same color trigram
that would cause merging of that state with the initial state, we
failed to recognize the problem.  The test cases for the prior commit
evidently only exercised situations where a tentative merge with the
initial state occurs before one with the final state.  If it goes the
other way around, we'll happily merge the initial and final states,
either producing a broken final graph that would never match anything,
or triggering the Assert added by the prior commit.

It's tempting to consider switching the merge direction when the merge
involves the final state, but I lack the time to analyze that idea in
detail.  Instead just keep track of the flag changes that would result
from proposed merges, in the same way that the prior commit tracked
proposed parent links.

Along the way, add some more debugging support, because I'm not entirely
confident that this is the last bug here.  And tweak matters so that
the transformed.dot file uses small integers rather than pointer values
to identify states; that makes it more readable if you're just eyeballing
it rather than fooling with Graphviz.  And rename a couple of identically
named struct fields to reduce confusion.

Per report from Corey Csuhta.  Add a test case based on his example.
(Note: this case does not trigger the bug under 9.3, apparently because
its different measurement of costs causes it to stop merging states before
it hits the failure.  I spent some time trying to find a variant that would
fail in 9.3, without success; but I'm sure such cases exist.)

Like the previous patch, back-patch to 9.3 where this code was added.

Report: https://postgr.es/m/E2B01A4B-4530-406B-8D17-2F67CF9A16BA@csuhta.com
2017-04-14 14:52:03 -04:00
5be58cc89a Fix regexport.c to behave sanely with lookaround constraints.
regexport.c thought it could just ignore LACON arcs, but the correct
behavior is to treat them as satisfiable while consuming zero input
(rather reminiscently of commit 9f1e642d5).  Otherwise, the emitted
simplified-NFA representation may contain no paths leading from initial
to final state, which unsurprisingly confuses pg_trgm, as seen in
bug #14623 from Jeff Janes.

Since regexport's output representation has no concept of an arc that
consumes zero input, recurse internally to find the next normal arc(s)
after any LACON transitions.  We'd be forced into changing that
representation if a LACON could be the last arc reaching the final
state, but fortunately the regex library never builds NFAs with such
a configuration, so there always is a next normal arc.

Back-patch to 9.3 where this logic was introduced.

Discussion: https://postgr.es/m/20170413180503.25948.94871@wrigleys.postgresql.org
2017-04-13 17:18:35 -04:00
649cd9085e Silence compiler warning in sepgsql
<selinux/label.h> includes <stdbool.h>, which creates an incompatible
We don't care if <stdbool.h> redefines "true"/"false"; those are close
enough.

Complaint and initial patch by Mike Palmiotto. Final approach per
Tom Lane's suggestion, as discussed on hackers. Backpatching to
all supported branches.

Discussion: https://postgr.es/m/flat/623bcaae-112e-ced0-8c22-a84f75ae0c53%40joeconway.com
2017-04-06 14:24:50 -07:00
a7dcd95dab Spelling fixes
From: Josh Soref <jsoref@gmail.com>
2017-03-14 13:45:45 -04:00
e03c6d93bf Fix pg_file_write() error handling.
Detect fclose() failures; given "ln -s /dev/full $PGDATA/devfull",
"pg_file_write('devfull', 'x', true)" now fails as it should.  Don't
leak a stream when fwrite() fails.  Remove a born-ineffective test that
aimed to skip zero-length writes.  Back-patch to 9.2 (all supported
versions).
2017-03-12 19:36:15 -04:00
f6cfc14e5a Fix ancient connection leak in dblink
When using unnamed connections with dblink, every time a new
connection is made, the old one is leaked. Fix that.

This has been an issue probably since dblink was first committed.
Someone complained almost ten years ago, but apparently I decided
not to pursue it at the time, and neither did anyone else, so it
slipped between the cracks. Now that someone else has complained,
fix in all supported branches.

Discussion: (orig) https://postgr.es/m/flat/F680AB59-6D6F-4026-9599-1BE28880273D%40decibel.org#F680AB59-6D6F-4026-9599-1BE28880273D@decibel.org
Discussion: (new) https://postgr.es/m/flat/0A3221C70F24FB45833433255569204D1F6ADF8C@G01JPEXMBYT05
Reported by: Jim Nasby and Takayuki Tsunakawa
2017-03-11 13:33:22 -08:00
5c7ce2d03a pg_xlogdump: Remove extra newline in error message
fatal_error() already prints out a trailing newline.
2017-03-08 09:59:52 -05:00
53b5a8c131 Fix contrib/pg_trgm's extraction of trigrams from regular expressions.
The logic for removing excess trigrams from the result was faulty.
It intends to avoid merging the initial and final states of the NFA,
which is necessary, but in testing whether removal of a specific trigram
would cause that, it failed to consider the combined effects of all the
state merges that that trigram's removal would cause.  This could result
in a broken final graph that would never match anything, leading to GIN
or GiST indexscans not finding anything.

To fix, add a "tentParent" field that is used only within this loop,
and set it to show state merges that we are tentatively going to do.
While examining a particular arc, we must chase up through tentParent
links as well as regular parent links (the former can only appear atop
the latter), and we must account for state init/fin flag merges that
haven't actually been done yet.

To simplify the latter, combine the separate init and fin bool fields
into a bitmap flags field.  I also chose to get rid of the "children"
state list, which seems entirely inessential.

Per bug #14563 from Alexey Isayko, which the added test cases are based on.
Back-patch to 9.3 where this code was added.

Report: https://postgr.es/m/20170222111446.1256.67547@wrigleys.postgresql.org
Discussion: https://postgr.es/m/8816.1487787594@sss.pgh.pa.us
2017-02-22 15:04:07 -05:00
e9e2bde2d2 Back-patch 9.4-era compiler warning fixes into older branches.
This applies portions of commits b64b5ccb6 and b1aebbb6a to the older
branches, in hopes of getting -Werror builds to succeed there.  The
applied changes simply remove useless tests, eg checking an unsigned
variable to see if it is >= 0.  Recent versions of clang warn about
such tests by default.
2017-02-17 16:58:59 -05:00
822115db8f Fix typo also in expected output.
Commit 181bdb90ba fixed the typo in the .sql file, but forgot to update the
expected output.
2017-02-06 12:04:35 +02:00
e8f9fe4ae7 Fix typos in comments.
Backpatch to all supported versions, where applicable, to make backpatching
of future fixes go more smoothly.

Josh Soref

Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com
2017-02-06 11:34:28 +02:00
a8191800a6 Invalidate cached plans on FDW option changes.
This fixes problems where a plan must change but fails to do so,
as seen in a bug report from Rajkumar Raghuwanshi.

For ALTER FOREIGN TABLE OPTIONS, do this through the standard method of
forcing a relcache flush on the table.  For ALTER FOREIGN DATA WRAPPER
and ALTER SERVER, just flush the whole plan cache on any change in
pg_foreign_data_wrapper or pg_foreign_server.  That matches the way
we handle some other low-probability cases such as opclass changes, and
it's unclear that the case arises often enough to be worth working harder.
Besides, that gives a patch that is simple enough to back-patch with
confidence.

Back-patch to 9.3.  In principle we could apply the code change to 9.2 as
well, but (a) we lack postgres_fdw to test it with, (b) it's doubtful that
anyone is doing anything exciting enough with FDWs that far back to need
this desperately, and (c) the patch doesn't apply cleanly.

Patch originally by Amit Langote, reviewed by Etsuro Fujita and Ashutosh
Bapat, who each contributed substantial changes as well.

Discussion: https://postgr.es/m/CAKcux6m5cA6rRPTKkqVdJ-R=KKDfe35Q_ZuUqxDSV_4hwga=og@mail.gmail.com
2017-01-06 14:12:52 -05:00
9b8507bfae Make dblink try harder to form useful error messages
When libpq encounters a connection-level error, e.g. runs out of memory
while forming a result, there will be no error associated with PGresult,
but a message will be placed into PGconn's error buffer. postgres_fdw
takes care to use the PGconn error message when PGresult does not have
one, but dblink has been negligent in that regard. Modify dblink to mirror
what postgres_fdw has been doing.

Back-patch to all supported branches.

Author: Joe Conway
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/02fa2d90-2efd-00bc-fefc-c23c00eb671e%40joeconway.com
2016-12-22 09:47:25 -08:00
bd46cce214 Protect dblink from invalid options when using postgres_fdw server
When dblink uses a postgres_fdw server name for its connection, it
is possible for the connection to have options that are invalid
with dblink (e.g. "updatable"). The recommended way to avoid this
problem is to use dblink_fdw servers instead. However there are use
cases for using postgres_fdw, and possibly other FDWs, for dblink
connection options, therefore protect against trying to use any
options that do not apply by using is_valid_dblink_option() when
building the connection string from the options.

Back-patch to 9.3. Although 9.2 supports FDWs for connection info,
is_valid_dblink_option() did not yet exist, and neither did
postgres_fdw, at least in the postgres source tree. Given the lack
of previous complaints, fixing that seems too invasive/not worth it.

Author: Corey Huinker
Reviewed-By: Joe Conway
Discussion: https://postgr.es/m/CADkLM%3DfWyXVEyYcqbcRnxcHutkP45UHU9WD7XpdZaMfe7S%3DRwA%40mail.gmail.com
2016-12-22 09:18:50 -08:00
3272be3468 Improve dblink error message when remote does not provide it
When dblink or postgres_fdw detects an error on the remote side of the
connection, it will try to construct a local error message as best it
can using libpq's PQresultErrorField(). When no primary message is
available, it was bailing out with an unhelpful "unknown error". Make
that message better and more style guide compliant. Per discussion
on hackers.

Backpatch to 9.2 except postgres_fdw which didn't exist before 9.3.

Discussion: https://postgr.es/m/19872.1482338965%40sss.pgh.pa.us
2016-12-21 15:51:46 -08:00
4bff35cca5 Retry opening new segments in pg_xlogdump --folllow
There is a small window between when the server closes out the existing
segment and the new one is created. Put a loop around the open call in
this case to make sure we wait for the new file to actually appear.
2016-09-30 11:23:25 +02:00
3c97a7ca46 worker_spi: Call pgstat_report_stat.
Without this, statistics changes accumulated by the worker never get
reported to the stats collector, which is bad.

Julien Rouhaud
2016-09-28 12:40:23 -04:00
7baa8bfca5 Fix -e option in contrib/intarray/bench/bench.pl.
As implemented, -e ran an EXPLAIN but then discarded the output, which
certainly seems pointless.  Make it print to stdout instead.  It's been
like that forever, so back-patch to all supported branches.

Daniel Gustafsson, reviewed by Andreas Scherbaum

Patch: <B97BDCB7-A3B3-4734-90B5-EDD586941629@yesql.se>
2016-08-17 15:51:10 -04:00
d292af0ac0 Fix typo 2016-08-09 19:08:30 -04:00
95a6855c55 Obstruct shell, SQL, and conninfo injection via database and role names.
Due to simplistic quoting and confusion of database names with conninfo
strings, roles with the CREATEDB or CREATEROLE option could escalate to
superuser privileges when a superuser next ran certain maintenance
commands.  The new coding rule for PQconnectdbParams() calls, documented
at conninfo_array_parse(), is to pass expand_dbname=true and wrap
literal database names in a trivial connection string.  Escape
zero-length values in appendConnStrVal().  Back-patch to 9.1 (all
supported versions).

Nathan Bossart, Michael Paquier, and Noah Misch.  Reviewed by Peter
Eisentraut.  Reported by Nathan Bossart.

Security: CVE-2016-5424
2016-08-08 10:07:52 -04:00