aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Make checksum_impl.h safe to compile with -fstrict-aliasing.Tom Lane2018-08-31
| | | | | | | | | | | | | | | | | | | | In general, Postgres requires -fno-strict-aliasing with compilers that implement C99 strict aliasing rules. There's little hope of getting rid of that overall. But it seems like it would be a good idea if storage/checksum_impl.h in particular didn't depend on it, because that header is explicitly intended to be included by external programs. We don't have a lot of control over the compiler switches that an external program might use, as shown by Michael Banck's report of failure in a privately-modified version of pg_verify_checksums. Hence, switch to using a union in place of willy-nilly pointer casting inside this file. I think this makes the code a bit more readable anyway. checksum_impl.h hasn't changed since it was introduced in 9.3, so back-patch all the way. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de
* Stop bgworkers during fast shutdown with postmaster in startup phaseMichael Paquier2018-08-29
| | | | | | | | | | | | | | When a postmaster gets into its phase PM_STARTUP, it would start background workers using BgWorkerStart_PostmasterStart mode immediately, which would cause problems for a fast shutdown as the postmaster forgets to send SIGTERM to already-started background workers. With smart and immediate shutdowns, this correctly happened, and fast shutdown is the only mode missing the shot. Author: Alexander Kukushkin Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAFh8B=mvnD8+DZUfzpi50DoaDfZRDfd7S=gwj5vU9GYn8UvHkA@mail.gmail.com Backpatch-through: 9.5
* Make pg_restore's identify_locking_dependencies() more bulletproof.Tom Lane2018-08-28
| | | | | | | | | | | | | | | | | This function had a blacklist of dump object types that it believed needed exclusive lock ... but we hadn't maintained that, so that it was missing ROW SECURITY, POLICY, and INDEX ATTACH items, all of which need (or should be treated as needing) exclusive lock. Since the same oversight seems likely in future, let's reverse the sense of the test so that the code has a whitelist of safe object types; better to wrongly assume a command can't be run in parallel than the opposite. Currently the only POST_DATA object type that's safe is CREATE INDEX ... and that list hasn't changed in a long time. Back-patch to 9.5 where RLS came in. Discussion: https://postgr.es/m/11450.1535483506@sss.pgh.pa.us
* Avoid quadratic slowdown in regexp match/split functions.Andrew Gierth2018-08-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | regexp_matches, regexp_split_to_table and regexp_split_to_array all work by compiling a list of match positions as character offsets (NOT byte positions) in the source string. Formerly, they then used text_substr to extract the matched text; but in a multi-byte encoding, that counts the characters in the string, and the characters needed to reach the starting byte position, on every call. Accordingly, the performance degraded as the product of the input string length and the number of match positions, such that splitting a string of a few hundred kbytes could take many minutes. Repair by keeping the wide-character copy of the input string available (only in the case where encoding_max_length is not 1) after performing the match operation, and extracting substrings from that instead. This reduces the complexity to being linear in the number of result bytes, discounting the actual regexp match itself (which is not affected by this patch). In passing, remove cleanup using retail pfree() which was obsoleted by commit ff428cded (Feb 2008) which made cleanup of SRF multi-call contexts automatic. Also increase (to ~134 million) the maximum number of matches and provide an error message when it is reached. Backpatch all the way because this has been wrong forever. Analysis and patch by me; review by Kaiting Chen. Discussion: https://postgr.es/m/87pnyn55qh.fsf@news-spur.riddles.org.uk see also https://postgr.es/m/87lg996g4r.fsf@news-spur.riddles.org.uk
* Fix missing dependency for pg_dump's ENABLE ROW LEVEL SECURITY items.Tom Lane2018-08-27
| | | | | | | | | | | | | | The archive should show a dependency on the item's table, but it failed to include one. This could cause failures in parallel restore due to emitting ALTER TABLE ... ENABLE ROW LEVEL SECURITY before restoring the table's data. In practice the odds of a problem seem low, since you would typically need to have set FORCE ROW LEVEL SECURITY as well, and you'd also need a very high --jobs count to have any chance of this happening. That probably explains the lack of field reports. Still, it's a bug, so back-patch to 9.5 where RLS was introduced. Discussion: https://postgr.es/m/19784.1535390902@sss.pgh.pa.us
* Make syslogger more robust against failures in opening CSV log files.Tom Lane2018-08-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous coding figured it'd be good enough to postpone opening the first CSV log file until we got a message we needed to write there. This is unsafe, though, because if the open fails we end up in infinite recursion trying to report the failure. Instead make the CSV log file management code look as nearly as possible like the longstanding logic for the stderr log file. In particular, open it immediately at postmaster startup (if enabled), or when we get a SIGHUP in which we find that log_destination has been changed to enable CSV logging. It seems OK to fail if a postmaster-start-time open attempt fails, as we've long done for the stderr log file. But we can't die if we fail to open a CSV log file during SIGHUP, so we're still left with a problem. In that case, write any output meant for the CSV log file to the stderr log file. (This will also cover race-condition cases in which backends send CSV log data before or after we have the CSV log file open.) This patch also fixes an ancient oversight that, if CSV logging was turned off during a SIGHUP, we never actually closed the last CSV log file. In passing, remember to reset whereToSendOutput = DestNone during syslogger start, since (unlike all other postmaster children) it's forked before the postmaster has done that. This made for a platform-dependent difference in error reporting behavior between the syslogger and other children: except on Windows, it'd report problems to the original postmaster stderr as well as the normal error log file(s). It's barely possible that that was intentional at some point; but it doesn't seem likely to be desirable in production, and the platform dependency definitely isn't desirable. Per report from Alexander Kukushkin. It's been like this for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAFh8B==iLUD_gqC-dAENS0V+kVrCeGiKujtKqSQ7++S-caaChw@mail.gmail.com
* doc: "Latest checkpoint location" will not match in pg_upgradeBruce Momjian2018-08-25
| | | | | | | | | | | | Mention that "Latest checkpoint location" will not match in pg_upgrade if the standby server is still running during the upgrade, which is possible. "Match" text first appeared in PG 9.5. Reported-by: Paul Bonaud Discussion: https://postgr.es/m/c7268794-edb4-1772-3bfd-04c54585c24e@trainline.com Backpatch-through: 9.5
* doc: add doc link for 'applicable_roles'Bruce Momjian2018-08-25
| | | | | | | | | | Reported-by: Ashutosh Sharma Discussion: https://postgr.es/m/CAE9k0PnhnL6MNDLuvkk8USzOa_DpzDzFQPAM_uaGuXbh9HMKYw@mail.gmail.com Author: Ashutosh Sharma Backpatch-through: 9.3
* docs: clarify plpython SD and GD dictionary behaviorBruce Momjian2018-08-25
| | | | | | | | Reported-by: Adam Bielański Discussion: https://postgr.es/m/153484305538.1370.7605856225879294548@wrigleys.postgresql.org Backpatch-through: 9.3
* Fix lexing of standard multi-character operators in edge cases.Andrew Gierth2018-08-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits c6b3c939b (which fixed the precedence of >=, <=, <> operators) and 865f14a2d (which added support for the standard => notation for named arguments) created a class of lexer tokens which look like multi-character operators but which have their own token IDs distinct from Op. However, longest-match rules meant that following any of these tokens with another operator character, as in (1<>-1), would cause them to be incorrectly returned as Op. The error here isn't immediately obvious, because the parser would usually still find the correct operator via the Op token, but there were more subtle problems: 1. If immediately followed by a comment or +-, >= <= <> would be given the old precedence of Op rather than the correct new precedence; 2. If followed by a comment, != would be returned as Op rather than as NOT_EQUAL, causing it not to be found at all; 3. If followed by a comment or +-, the => token for named arguments would be lexed as Op, causing the argument to be mis-parsed as a simple expression, usually causing an error. Fix by explicitly checking for the operators in the {operator} code block in addition to all the existing special cases there. Backpatch to 9.5 where the problem was introduced. Analysis and patch by me; review by Tom Lane. Discussion: https://postgr.es/m/87va851ppl.fsf@news-spur.riddles.org.uk
* Reduce an unnecessary O(N^3) loop in lexer.Andrew Gierth2018-08-23
| | | | | | | | | | | | The lexer's handling of operators contained an O(N^3) hazard when dealing with long strings of + or - characters; it seems hard to prevent this case from being O(N^2), but the additional N multiplier was not needed. Backpatch all the way since this has been there since 7.x, and it presents at least a mild hazard in that trying to do Bind, PREPARE or EXPLAIN on a hostile query could take excessive time (without honouring cancels or timeouts) even if the query was never executed.
* Fix set of NLS translation issuesMichael Paquier2018-08-21
| | | | | | | | | | | | | | | | While monitoring the code, a couple of issues related to string translation has showed up: - Some routines for auto-updatable views return an error string, which sometimes missed the shot. A comment regarding string translation is added for each routine to help with future features. - GSSAPI authentication missed two translations. - vacuumdb handles non-translated strings. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20180810.152131.31921918.horiguchi.kyotaro@lab.ntt.co.jp Backpatch-through: 9.3
* Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands.Tom Lane2018-08-17
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this code blindly followed the common coding pattern of passing PQserverVersion(AH->connection) as the server-version parameter of fmtQualifiedId. That works as long as we have a connection; but in pg_restore with text output, we don't. Instead we got a zero from PQserverVersion, which fmtQualifiedId interpreted as "server is too old to have schemas", and so the name went unqualified. That still accidentally managed to work in many cases, which is probably why this ancient bug went undetected for so long. It only became obvious in the wake of the changes to force dump/restore to execute with restricted search_path. In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server- version behavioral dependency, and just making it schema-qualify all the time. We no longer support pg_dump from servers old enough to need the ability to omit schema name, let alone restoring to them. (Also, the few callers outside pg_dump already didn't work with pre-schema servers.) In older branches, that's not an acceptable solution, so instead just tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify its output regardless of server version. Per bug #15338 from Oleg somebody. Back-patch to all supported branches. Discussion: https://postgr.es/m/153452458706.1316.5328079417086507743@wrigleys.postgresql.org
* Set scan direction appropriately for SubPlans (bug #15336)Andrew Gierth2018-08-17
| | | | | | | | | | | | | | | | | | | | When executing a SubPlan in an expression, the EState's direction field was left alone, resulting in an attempt to execute the subplan backwards if it was encountered during a backwards scan of a cursor. Also, though much less likely, it was possible to reach the execution of an InitPlan while in backwards-scan state. Repair by saving/restoring estate->es_direction and forcing forward scan mode in the relevant places. Backpatch all the way, since this has been broken since 8.3 (prior to commit c7ff7663e, SubPlans had their own EStates rather than sharing the parent plan's, so there was no confusion over scan direction). Per bug #15336 reported by Vladimir Baranoff; analysis and patch by me, review by Tom Lane. Discussion: https://postgr.es/m/153449812167.1304.1741624125628126322@wrigleys.postgresql.org
* pg_upgrade: issue helpful error message for use on standbysBruce Momjian2018-08-17
| | | | | | | | | | | | | Commit 777e6ddf1723306bd2bf8fe6f804863f459b0323 checked for a shut down message from a standby and allowed it to continue. This patch reports a helpful error message in these cases, suggesting to use rsync as documented. Diagnosed-by: Martín Marqués Discussion: https://postgr.es/m/CAPdiE1xYCow-reLjrhJ9DqrMu-ppNq0ChUUEvVdxhdjGRD5_eA@mail.gmail.com Backpatch-through: 9.3
* Mention ownership requirements for REFRESH MATERIALIZED VIEW in docsMichael Paquier2018-08-17
| | | | | | Author: Dian Fay Discussion: https://postgr.es/m/745abbd2-a1a0-ead8-2cb2-768c16747d97@gmail.com Backpatch-through: 9.3
* Close the file descriptor in ApplyLogicalMappingFileTomas Vondra2018-08-16
| | | | | | | | | | | | | | The function was forgetting to close the file descriptor, resulting in failures like this: ERROR: 53000: exceeded maxAllocatedDescs (492) while trying to open file "pg_logical/mappings/map-4000-4eb-1_60DE1E08-5376b5-537c6b" LOCATION: OpenTransientFile, fd.c:2161 Simply close the file at the end, and backpatch to 9.4 (where logical decoding was introduced). While at it, fix a nearby typo. Discussion: https://www.postgresql.org/message-id/flat/738a590a-2ce5-9394-2bef-7b1caad89b37%402ndquadrant.com
* Make snprintf.c follow the C99 standard for snprintf's result value.Tom Lane2018-08-15
| | | | | | | | | | | | | | | | | | | | | | | | C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
* Clean up assorted misuses of snprintf()'s result value.Tom Lane2018-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
* pg_upgrade: fix shutdown check for standby serversBruce Momjian2018-08-14
| | | | | | | | | | | | Commit 244142d32afd02e7408a2ef1f249b00393983822 only tested for the pg_controldata output for primary servers, but standby servers have different "Database cluster state" output, so check for that too. Diagnosed-by: Michael Paquier Discussion: https://postgr.es/m/20180810164240.GM13638@paquier.xyz Backpatch-through: 9.3
* docs: Only first instance of a PREPARE parameter sets data typeBruce Momjian2018-08-09
| | | | | | | | | | | | If the first reference to $1 is "($1 = col) or ($1 is null)", the data type can be determined, but not for "($1 is null) or ($1 = col)". This change documents this. Reported-by: Morgan Owens Discussion: https://postgr.es/m/153233728858.1404.15268121695358514937@wrigleys.postgresql.org Backpatch-through: 9.3
* Don't run atexit callbacks in quickdie signal handlers.Heikki Linnakangas2018-08-08
| | | | | | | | | | | | | | | | | exit() is not async-signal safe. Even if the libc implementation is, 3rd party libraries might have installed unsafe atexit() callbacks. After receiving SIGQUIT, we really just want to exit as quickly as possible, so we don't really want to run the atexit() callbacks anyway. The original report by Jimmy Yih was a self-deadlock in startup_die(). However, this patch doesn't address that scenario; the signal handling while waiting for the startup packet is more complicated. But at least this alleviates similar problems in the SIGQUIT handlers, like that reported by Asim R P later in the same thread. Backpatch to 9.3 (all supported versions). Discussion: https://www.postgresql.org/message-id/CAOMx_OAuRUHiAuCg2YgicZLzPVv5d9_H4KrL_OFsFP%3DVPekigA%40mail.gmail.com
* Don't record FDW user mappings as members of extensions.Tom Lane2018-08-07
| | | | | | | | | | | | | | | | | CreateUserMapping has a recordDependencyOnCurrentExtension call that's been there since extensions were introduced (very possibly my fault). However, there's no support anywhere else for user mappings as members of extensions, nor are they listed as a possible member object type in the documentation. Nor does it really seem like a good idea for user mappings to belong to extensions when roles don't. Hence, remove the bogus call. (As we saw in bug #15310, the lack of any pg_dump support for this case ensures that any such membership record would silently disappear during pg_upgrade. So there's probably no need for us to do anything else about cleaning up after this mistake.) Discussion: https://postgr.es/m/27952.1533667213@sss.pgh.pa.us
* Fix incorrect initialization of BackendActivityBuffer.Tom Lane2018-08-07
| | | | | | | | | | | | | Since commit c8e8b5a6e, this has been zeroed out using the wrong length. In practice the length would always be too small, leading to not zeroing the whole buffer rather than clobbering additional memory; and that's pretty harmless, both because shmem would likely start out as zeroes and because we'd reinitialize any given entry before use. Still, it's bogus, so fix it. Reported by Petru-Florin Mihancea (bug #15312) Discussion: https://postgr.es/m/153363913073.1303.6518849192351268091@wrigleys.postgresql.org
* Fix pg_upgrade to handle event triggers in extensions correctly.Tom Lane2018-08-07
| | | | | | | | | | | pg_dump with --binary-upgrade must emit ALTER EXTENSION ADD commands for all objects that are members of extensions. It forgot to do so for event triggers, as per bug #15310 from Nick Barnes. Back-patch to 9.3 where event triggers were introduced. Haribabu Kommi Discussion: https://postgr.es/m/153360083872.1395.4593932457718151600@wrigleys.postgresql.org
* Ensure pg_dump_sort.c sorts null vs non-null namespace consistently.Tom Lane2018-08-07
| | | | | | | | | | | | | | | | | | | | The original coding here (which is, I believe, my fault) supposed that it didn't need to concern itself with the possibility that one object of a given type-priority has a namespace while another doesn't. But that's not reliably true anymore, if it ever was; and if it does happen then it's possible that DOTypeNameCompare returns self-inconsistent comparison results. That leads to unspecified behavior in qsort() and a resultant weird output order from pg_dump. This should end up being only a cosmetic problem, because any ordering constraints that actually matter should be enforced by the later dependency-based sort. Still, it's a bug, so back-patch. Report and fix by Jacob Champion, though I editorialized on his patch to the extent of making NULL sort after non-NULL, for consistency with our usual sorting definitions. Discussion: https://postgr.es/m/CABAq_6Hw+V-Kj7PNfD5tgOaWT_-qaYkc+SRmJkPLeUjYXLdxwQ@mail.gmail.com
* Stamp 9.5.14.REL9_5_14Tom Lane2018-08-06
|
* Translation updatesPeter Eisentraut2018-08-06
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 901dbb2f9c08846927a0f103adf87e234bc47844
* Last-minute updates for release notes.Tom Lane2018-08-06
| | | | Security: CVE-2018-10915, CVE-2018-10925
* Fix failure to reset libpq's state fully between connection attempts.Tom Lane2018-08-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The logic in PQconnectPoll() did not take care to ensure that all of a PGconn's internal state variables were reset before trying a new connection attempt. If we got far enough in the connection sequence to have changed any of these variables, and then decided to try a new server address or server name, the new connection might be completed with some state that really only applied to the failed connection. While this has assorted bad consequences, the only one that is clearly a security issue is that password_needed didn't get reset, so that if the first server asked for a password and the second didn't, PQconnectionUsedPassword() would return an incorrect result. This could be leveraged by unprivileged users of dblink or postgres_fdw to allow them to use server-side login credentials that they should not be able to use. Other notable problems include the possibility of forcing a v2-protocol connection to a server capable of supporting v3, or overriding "sslmode=prefer" to cause a non-encrypted connection to a server that would have accepted an encrypted one. Those are certainly bugs but it's harder to paint them as security problems in themselves. However, forcing a v2-protocol connection could result in libpq having a wrong idea of the server's standard_conforming_strings setting, which opens the door to SQL-injection attacks. The extent to which that's actually a problem, given the prerequisite that the attacker needs control of the client's connection parameters, is unclear. These problems have existed for a long time, but became more easily exploitable in v10, both because it introduced easy ways to force libpq to abandon a connection attempt at a late stage and then try another one (rather than just giving up), and because it provided an easy way to specify multiple target hosts. Fix by rearranging PQconnectPoll's state machine to provide centralized places to reset state properly when moving to a new target host or when dropping and retrying a connection to the same host. Tom Lane, reviewed by Noah Misch. Our thanks to Andrew Krasichkov for finding and reporting the problem. Security: CVE-2018-10915
* Release notes for 10.5, 9.6.10, 9.5.14, 9.4.19, 9.3.24.Tom Lane2018-08-05
|
* Doc: fix incorrectly stated argument list for pgcrypto's hmac() function.Tom Lane2018-08-05
| | | | | | | The bytea variant takes (bytea, bytea, text). Per unsigned report. Discussion: https://postgr.es/m/153344327294.1404.654155870612982042@wrigleys.postgresql.org
* Fix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.Tom Lane2018-08-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When expanding an updatable view that is an INSERT's target, the rewriter failed to rewrite Vars in the ON CONFLICT UPDATE clause. This accidentally worked if the view was just "SELECT * FROM ...", as the transformation would be a no-op in that case. With more complicated view targetlists, this omission would often lead to "attribute ... has the wrong type" errors or even crashes, as reported by Mario De Frutos Dieguez. Fix by adding code to rewriteTargetView to fix up the data structure correctly. The easiest way to update the exclRelTlist list is to rebuild it from scratch looking at the new target relation, so factor the code for that out of transformOnConflictClause to make it sharable. In passing, avoid duplicate permissions checks against the EXCLUDED pseudo-relation, and prevent useless view expansion of that relation's dummy RTE. The latter is only known to happen (after this patch) in cases where the query would fail later due to not having any INSTEAD OF triggers for the view. But by exactly that token, it would create an unintended and very poorly tested state of the query data structure, so it seems like a good idea to prevent it from happening at all. This has been broken since ON CONFLICT was introduced, so back-patch to 9.5. Dean Rasheed, based on an earlier patch by Amit Langote; comment-kibitzing and back-patching by me Discussion: https://postgr.es/m/CAFYwGJ0xfzy8jaK80hVN2eUWr6huce0RU8AgU04MGD00igqkTg@mail.gmail.com
* Reset properly errno before calling write()Michael Paquier2018-08-05
| | | | | | | | | | | | 6cb3372 enforces errno to ENOSPC when less bytes than what is expected have been written when it is unset, though it forgot to properly reset errno before doing a system call to write(), causing errno to potentially come from a previous system call. Reported-by: Tom Lane Author: Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/31797.1533326676@sss.pgh.pa.us
* Add table relcache invalidation to index builds.Peter Geoghegan2018-08-03
| | | | | | | | | | | | | | | | | | | | | | | | It's necessary to make sure that owning tables have a relcache invalidation prior to advancing the command counter to make newly-entered catalog tuples for the index visible. inval.c must be able to maintain the consistency of the local caches in the event of transaction abort. There is usually only a problem when CREATE INDEX transactions abort, since there is a generic invalidation once we reach index_update_stats(). This bug is of long standing. Problems were made much more likely by the addition of parallel CREATE INDEX (commit 9da0cc35284), but it is strongly suspected that similar problems can be triggered without involving plan_create_index_workers(). (plan_create_index_workers() triggers a relcache build or rebuild, which previously only happened in rare edge cases.) Author: Peter Geoghegan Reported-By: Luca Ferrari Diagnosed-By: Andres Freund Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAKoxK+5fVodiCtMsXKV_1YAKXbzwSfp7DgDqUmcUAzeAhf=HEQ@mail.gmail.com Backpatch: 9.3-
* Fix pg_replication_slot example outputAlvaro Herrera2018-08-03
| | | | | | | | | The example output of pg_replication_slot is wrong. Correct it and make the output stable by explicitly listing columns to output. Author: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20180731.190909.42582169.horiguchi.kyotaro@lab.ntt.co.jp
* pg_upgrade: fix --check for live source server checksBruce Momjian2018-07-31
| | | | | | Fix for commit 244142d32afd02e7408a2ef1f249b00393983822. Backpatch-through: 9.3
* Further fixes for quoted-list GUC values in pg_dump and ruleutils.c.Tom Lane2018-07-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits 742869946 et al turn out to be a couple bricks shy of a load. We were dumping the stored values of GUC_LIST_QUOTE variables as they appear in proconfig or setconfig catalog columns. However, although that quoting rule looks a lot like SQL-identifier double quotes, there are two critical differences: empty strings ("") are legal, and depending on which variable you're considering, values longer than NAMEDATALEN might be valid too. So the current technique fails altogether on empty-string list entries (as reported by Steven Winfield in bug #15248) and it also risks truncating file pathnames during dump/reload of GUC values that are lists of pathnames. To fix, split the stored value without any downcasing or truncation, and then emit each element as a SQL string literal. This is a tad annoying, because we now have three copies of the comma-separated-string splitting logic in varlena.c as well as a fourth one in dumputils.c. (Not to mention the randomly-different-from-those splitting logic in libpq...) I looked at unifying these, but it would be rather a mess unless we're willing to tweak the API definitions of SplitIdentifierString, SplitDirectoriesString, or both. That might be worth doing in future; but it seems pretty unsafe for a back-patched bug fix, so for now accept the duplication. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/7585.1529435872@sss.pgh.pa.us
* Fix pg_dump's failure to dump REPLICA IDENTITY for constraint indexes.Tom Lane2018-07-30
| | | | | | | | | | | | pg_dump knew about printing ALTER TABLE ... REPLICA IDENTITY USING INDEX for indexes declared as indexes, but it failed to print that for indexes declared as unique or primary-key constraints. Per report from Achilleas Mantzios. This has been broken since the feature was introduced, AFAICS. Back-patch to 9.4. Discussion: https://postgr.es/m/1e6cc5ad-b84a-7c07-8c08-a4d0c3cdc938@matrix.gatewaynet.com
* Doc: fix oversimplified example for CREATE POLICY.Tom Lane2018-07-30
| | | | | | | | | | | | | | As written, this policy constrained only the post-image not the pre-image of rows, meaning that users could delete other users' rows or take ownership of such rows, contrary to what the docs claimed would happen. We need two separate policies to achieve the documented effect. While at it, try to explain what's happening a bit more fully. Per report from Олег Самойлов. Back-patch to 9.5 where this was added. Thanks to Stephen Frost for off-list discussion. Discussion: https://postgr.es/m/3298321532002010@sas1-2b3c3045b736.qloud-c.yandex.net
* Fix earthdistance test suite function name typo.Noah Misch2018-07-29
| | | | | | Affected test queries have been testing the wrong thing since their introduction in commit 4c1383efd132e4f532213c8a8cc63a455f55e344. Back-patch to 9.3 (all supported versions).
* Document security implications of qualified names.Noah Misch2018-07-28
| | | | | | | | | | | Commit 5770172cb0c9df9e6ce27c507b449557e5b45124 documented secure schema usage, and that advice suffices for using unqualified names securely. Document, in typeconv-func primarily, the additional issues that arise with qualified names. Back-patch to 9.3 (all supported versions). Reviewed by Jonathan S. Katz. Discussion: https://postgr.es/m/20180721012446.GA1840594@rfd.leadboat.com
* pgtest: run clean, build, and check stages separatelyBruce Momjian2018-07-28
| | | | | | This allows for cleaner error reporting. Backpatch-through: 9.5
* pg_upgrade: check for clean server shutdownsBruce Momjian2018-07-28
| | | | | | | | | | | | | 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
* pgtest: grab possible warnings from install.logBruce Momjian2018-07-28
| | | | | | | Since PG 9.5, 'make check' records the build output in install.log, so look in there for warnings too. Backpatch-through: 9.5
* doc: Fix reference to "decoder" to instead be the correct "output plugin".Andres Freund2018-07-24
| | | | | | Author: Jonathan Katz Discussion: https://postgr.es/m/DD02DD86-5989-4BFD-8712-468541F68383@postgresql.org Backpatch: 9.4-, where logical decoding was added
* Further portability hacking in pg_upgrade's test script.Tom Lane2018-07-21
| | | | | | | | | | | | | | | | | | | | | | 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
* Fix handling of empty uncompressed posting list pages in GINAlexander Korotkov2018-07-19
| | | | | | | | | | | | | | PostgreSQL 9.4 introduces posting list compression in GIN. This feature supports online upgrade, so that after pg_upgrade uncompressed posting lists are compressed on-the-fly. Underlying code appears to always expect at least one item on uncompressed posting list page. But there could be completely empty pages, because VACUUM never deletes leftmost and rightmost pages from posting trees. This commit fixes that. Reported-by: Sivasubramanian Ramasubramanian Discussion: https://postgr.es/m/1531867212836.63354%40amazon.com Author: Sivasubramanian Ramasubramanian, Alexander Korotkov Backpatch-through: 9.4
* Fix print of Path nodes when using OPTIMIZER_DEBUGMichael Paquier2018-07-19
| | | | | | | | | | | GatherMergePath (introduced in 10) and CustomPath (introduced in 9.5) have gone missing. The order of the Path nodes was inconsistent with what is listed in nodes.h, so make the order consistent at the same time to ease future checks and additions. Author: Sawada Masahiko Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAD21AoBQMLoc=ohH-oocuAPsELrmk8_EsRJjOyR8FQLZkbE0wA@mail.gmail.com
* Fix misc typos, mostly in comments.Heikki Linnakangas2018-07-18
| | | | | | | | 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.