aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Don't allow LIMIT/OFFSET clause within sub-selects to be pushed to workers.Amit Kapila2018-09-14
| | | | | | | | | | | | | | Allowing sub-select containing LIMIT/OFFSET in workers can lead to inconsistent results at the top-level as there is no guarantee that the row order will be fully deterministic. The fix is to prohibit pushing LIMIT/OFFSET within sub-selects to workers. Reported-by: Andrew Fletcher Bug: 15324 Author: Amit Kapila Reviewed-by: Dilip Kumar Backpatch-through: 9.6 Discussion: https://postgr.es/m/153417684333.10284.11356259990921828616@wrigleys.postgresql.org
* Allow concurrent-safe open() and fopen() in frontend code for WindowsMichael Paquier2018-09-14
| | | | | | | | | | | | | | | | | | | | PostgreSQL uses a custom wrapper for open() and fopen() which is concurrent-safe, allowing multiple processes to open and work on the same file. This has a couple of advantages: - pg_test_fsync does not handle O_DSYNC correctly otherwise, leading to false claims that disks are unsafe. - TAP tests can run into race conditions when a postmaster and pg_ctl open postmaster.pid, fixing some random failures in the buildfam. pg_upgrade is one frontend tool using workarounds to bypass file locking issues with the log files it generates, however the interactions with pg_ctl are proving to be tedious to get rid of, so this is left for later. Author: Laurenz Albe Reviewed-by: Michael Paquier, Kuntal Ghosh Discussion: https://postgr.es/m/1527846213.2475.31.camel@cybertec.at Discussion: https://postgr.es/m/16922.1520722108@sss.pgh.pa.us
* Improve autovacuum logging for aggressive and anti-wraparound runsMichael Paquier2018-09-14
| | | | | | | | | | | | A log message was being generated when log_min_duration is reached for autovacuum on a given relation to indicate if it was an aggressive run, and missed the point of mentioning if it is doing an anti-wrapround run. The log message generated is improved so as one, both or no extra details are added depending on the option set. Author: Sergei Kornilov Reviewed-by: Masahiko Sawada, Michael Paquier Discussion: https://postgr.es/m/11587951532155118@sas1-19a94364928d.qloud-c.yandex.net
* Message style improvementsPeter Eisentraut2018-09-13
| | | | | | | | | Fix one untranslatable string concatenation in pg_rewind. Fix one message in pg_verify_checksums to use a style use elsewhere and avoid plural issues. Fix one gratuitous abbreviation in psql.
* Detect LLVM 7 without specifying binaries explicitly.Andres Freund2018-09-13
| | | | | | | | | | | | Before this commit LLVM 7 was supported, but only if one explicitly provided LLVM_CONFIG= and CLANG= paths. As LLVM 7 is the first version that includes our upstreamed debugging and profiling features, and as debian is planning to default to 7 due to wider architecture support, it seems good to support auto-detecting that version. Author: Christoph Berg Discussion: https://postgr.es/m/20180912124517.GD24584@msg.df7cb.de Backpatch: 11, where LLVM was introduced
* Attempt to identify system timezone by reading /etc/localtime symlink.Tom Lane2018-09-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On many modern platforms, /etc/localtime is a symlink to a file within the IANA database. Reading the symlink lets us find out the name of the system timezone directly, without going through the brute-force search embodied in scan_available_timezones(). This shortens the runtime of initdb by some tens of ms, which is helpful for the buildfarm, and it also allows us to reliably select the same zone name the system was actually configured for, rather than possibly choosing one of IANA's many zone aliases. (For example, in a system configured for "Asia/Tokyo", the brute-force search would not choose that name but its alias "Japan", on the grounds of the latter string being shorter. More surprisingly, "Navajo" is preferred to either "America/Denver" or "US/Mountain", as seen in an old complaint from Josh Berkus.) If /etc/localtime doesn't exist, or isn't a symlink, or we can't make sense of its contents, or the contents match a zone we know but that zone doesn't match the observed behavior of localtime(), fall back to the brute-force search. Also, tweak initdb so that it prints the zone name it selected. In passing, replace the last few references to the "Olson" database in code comments with "IANA", as that's been our preferred term since commit b2cbced9e. Patch by me, per a suggestion from Robert Haas; review by Michael Paquier Discussion: https://postgr.es/m/7408.1525812528@sss.pgh.pa.us
* Attach FPI to the first record after full_page_writes is turned on.Amit Kapila2018-09-13
| | | | | | | | | | | | | | | | XLogInsert fails to attach a required FPI to the first record after full_page_writes is turned on by the last checkpoint. This bug got introduced in 9.5 due to code rearrangement in commits 2c03216d83 and 2076db2aea. Fix it by ensuring that XLogInsertRecord performs a recomputation when the given record is generated with FPW as off but found that the flag has been turned on while actually inserting the record. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi Reviewed-by: Amit Kapila Backpatch-through: 9.5 where this problem was introduced Discussion: https://postgr.es/m/20180420.151043.74298611.horiguchi.kyotaro@lab.ntt.co.jp
* Simplify static function in extension.cMichael Paquier2018-09-13
| | | | | | | | | | | An extra argument for the filename defining the extension script location was present, aimed at being used for error reporting, but has never been used. This was around since extensions have been added in d9572c4. Author: Yugo Nagata Reviewed-by: Tatsuo Ishii Discussion: https://postgr.es/m/20180907180504.1ff19e1675bb44a67e9c7ab1@sraoss.co.jp
* Simplify index tuple descriptor initializationPeter Eisentraut2018-09-13
| | | | | | | | | | | | | | | | We have two code paths for initializing the tuple descriptor for a new index: For a normal index, we copy the tuple descriptor from the table and reset a number of fields that are not applicable to indexes. For an expression index, we make a blank tuple descriptor and fill in the needed fields based on the provided expressions. As pg_attribute has grown over time, the number of fields that we need to reset in the first case is now bigger than the number of fields we actually want to copy, so it's sensible to do it the other way around: Make a blank descriptor and copy just the fields we need. This also allows more code sharing between the two branches, and it avoids having to touch this code for almost every unrelated change to the pg_attribute structure. Reviewed-by: Arthur Zakirov <a.zakirov@postgrespro.ru>
* Minor fixes for psql tab completion.Tom Lane2018-09-12
| | | | | | | | | | | | | | | * Include partitioned tables in what's offered after ANALYZE. * Include toast_tuple_target in what's offered after ALTER TABLE ... SET|RESET. * Include HASH in what's offered after PARTITION BY. This is extracted from a larger patch; these bits seem like uncontroversial bug fixes for v11 features, so back-patch them into v11. Justin Pryzby Discussion: https://postgr.es/m/20180529000623.GA21896@telsasoft.com
* Repair bug in regexp split performance improvements.Andrew Gierth2018-09-12
| | | | | | | | | | | | | | | | Commit c8ea87e4b introduced a temporary conversion buffer for substrings extracted during regexp splits. Unfortunately the code that sized it was failing to ignore the effects of ignored degenerate regexp matches, so for regexp_split_* calls it could under-size the buffer in such cases. Fix, and add some regression test cases (though those will only catch the bug if run in a multibyte encoding). Backpatch to 9.3 as the faulty code was. Thanks to the PostGIS project, Regina Obe and Paul Ramsey for the report (via IRC) and assistance in analysis. Patch by me.
* ecpg: Change --version output to common stylePeter Eisentraut2018-09-12
| | | | | | | | When we removed the ecpg-specific versions, we also removed the "(PostgreSQL)" from the --version output, which we show in other programs. Reported-by: Ioseph Kim <pgsql-kr@postgresql.kr>
* Add PQresultMemorySize function to report allocated size of a PGresult.Tom Lane2018-09-11
| | | | | | | | | This number can be useful for application memory management, and the overhead to track it seems pretty trivial. Lars Kanis, reviewed by Pavel Stehule, some mods by me Discussion: https://postgr.es/m/fa16a288-9685-14f2-97c8-b8ac84365a4f@greiz-reinsdorf.de
* Parse more strictly integer parameters from connection strings in libpqMichael Paquier2018-09-12
| | | | | | | | | | | | | | | | | | | | The following parameters have been parsed in lossy ways when specified in a connection string processed by libpq: - connect_timeout - keepalives - keepalives_count - keepalives_idle - keepalives_interval - port Overflowing values or the presence of incorrect characters were not properly checked, leading to libpq trying to use such values and fail with unhelpful error messages. This commit hardens the parsing of those parameters so as it is possible to find easily incorrect values. Author: Fabien Coelho Reviewed-by: Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/alpine.DEB.2.21.1808171206180.20841@lancre
* doc: adjust PG 11 release notesBruce Momjian2018-09-11
| | | | | | Fixes for channel binding, SQL procedures, and pg_trgm. Backpatch-through: 11
* Remove ruleutils.c's special case for BIT [VARYING] literals.Tom Lane2018-09-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, get_const_expr() insisted on prefixing BIT and VARBIT literals with 'B'. That's not really necessary, because we always append explicit-cast syntax to identify the constant's type. Moreover, it's subtly wrong for VARBIT, because the parser will interpret B'...' as '...'::"bit"; see make_const() which explicitly assigns type BITOID for a T_BitString literal. So what had been a simple VARBIT literal is reconstructed as ('...'::"bit")::varbit, which is not the same thing, at least not before constant folding. This results in odd differences after dump/restore, as complained of by the patch submitter, and it could result in actual failures in partitioning or inheritance DDL operations (see commit 542320c2b, which repaired similar misbehaviors for some other data types). Fixing it is pretty easy: just remove the special case and let the default code path handle these types. We could have kept the special case for BIT only, but there seems little point in that. Like the previous patch, I judge that back-patching this into stable branches wouldn't be a good idea. However, it seems not quite too late for v11, so let's fix it there. Paul Guo, reviewed by Davy Machado and John Naylor, minor adjustments by me Discussion: https://postgr.es/m/CABQrizdTra=2JEqA6+Ms1D1k1Kqw+aiBBhC9TreuZRX2JzxLAA@mail.gmail.com
* Repair double-free in SP-GIST rescan (bug #15378)Andrew Gierth2018-09-11
| | | | | | | | | | | | | | | | | | | | | | spgrescan would first reset traversalCxt, and then traverse a potentially non-empty stack containing pointers to traversalValues which had been allocated in those contexts, freeing them a second time. This bug originates in commit ccd6eb49a where traversalValue was introduced. Repair by traversing the stack before the context reset; this isn't ideal, since it means doing retail pfree in a context that's about to be reset, but the freeing of a stack entry is also done in other places in the code during the scan so it's not worth trying to refactor it further. Regression test added. Backpatch to 9.6 where the problem was introduced. Per bug #15378; analysis and patch by me, originally from a report on IRC by user velix; see also PostGIS ticket #4174; review by Alexander Korotkov. Discussion: https://postgr.es/m/153663176628.23136.11901365223750051490@wrigleys.postgresql.org
* Use -Bsymbolic for shared libraries on HP-UX and Solaris.Tom Lane2018-09-10
| | | | | | | | | | | These platforms are also subject to the mis-linking problem addressed in commit e3d77ea6b. It's not clear whether we could solve it with a solution equivalent to GNU ld's version scripts, but -Bsymbolic appears to fix it, so let's use that. Like the previous commit, back-patch as far as v10. Discussion: https://postgr.es/m/153626613985.23143.4743626885618266803@wrigleys.postgresql.org
* Hide a static inline from FRONTEND code.Tom Lane2018-09-10
| | | | | | | | | | | For some reason pg_waldump is including tuptable.h, and the recent addition of a static inline function to it is causing problems on older buildfarm members that fail to optimize such functions away completely. I wonder if this situation doesn't mean that some header refactoring is called for ... but as a band-aid, wrap the static function in "#ifndef FRONTEND". Discussion: https://postgr.es/m/20180824154237.mabsv6fsz5q37bma@alap3.anarazel.de
* Prevent mis-linking of src/port and src/common functions on *BSD.Tom Lane2018-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On ELF-based platforms (and maybe others?) it's possible for a shared library, when dynamically loaded into the backend, to call the backend versions of src/port and src/common functions rather than the frontend versions that are actually linked into the shlib. This is the cause of bug #15367 from Jeremy Evans, and is likely to lead to more problems in future; it's accidental that we've failed to notice any bad effects up to now. The recommended way to fix this on ELF-based platforms is to use a linker "version script" that makes the shlib's versions of the functions local. (Apparently, -Bsymbolic would fix it as well, but with other side effects that we don't want.) Doing so has the additional benefit that we can make sure the shlib only exposes the symbols that are meant to be part of its API, and not ones that are just for cross-file references within the shlib. So we'd already been using a version script for libpq on popular platforms, but it's now apparent that it's necessary for correctness on every ELF-based platform. Hence, add appropriate logic to the openbsd, freebsd, and netbsd stanzas of Makefile.shlib; this is just a copy-and-paste from the linux stanza. There may be additional work to do if commit ed0cdf0e0 reveals that the problem exists elsewhere, but this is all that is known to be needed right now. Back-patch to v10 where SCRAM support came in. The problem is ancient, but analysis suggests that there were no really severe consequences in older branches. Hence, I won't take the risk of such a large change in the build process for older branches. In passing, remove a rather opaque comment about -Bsymbolic; I don't think it's very on-point about why we don't use that, if indeed that's what it's talking about at all. Patch by me; thanks to Andrew Gierth for helping to diagnose the problem, and for additional testing. Discussion: https://postgr.es/m/153626613985.23143.4743626885618266803@wrigleys.postgresql.org
* Improve behavior of to_timestamp()/to_date() functionsAlexander Korotkov2018-09-09
| | | | | | | | | | | | | | | | | | | | | | to_timestamp()/to_date() functions were introduced mainly for Oracle compatibility, and became very popular among PostgreSQL users. However, some behavior of to_timestamp()/to_date() functions are both incompatible with Oracle and confusing for our users. This behavior is related to handling of spaces and separators in non FX (fixed format) mode. This commit reworks this behavior making less confusing, better documented and more compatible with Oracle. Nevertheless, there are still following incompatibilities with Oracle. 1) We don't insist that there are no format string patterns unmatched to input string. 2) In FX mode we don't insist space and separators in format string to exactly match input string. 3) When format string patterns are divided by mix of spaces and separators, we don't distinguish them, while Oracle takes into account only last group of spaces/separators. Discussion: https://postgr.es/m/1873520224.1784572.1465833145330.JavaMail.yahoo%40mail.yahoo.com Author: Artur Zakirov, Alexander Korotkov, Liudmila Mantrova Review: Amul Sul, Robert Haas, Tom Lane, Dmitry Dolgov, David G. Johnston
* Fix past pd_upper write in ginRedoRecompress()Alexander Korotkov2018-09-09
| | | | | | | | | | | | | | | | ginRedoRecompress() replays actions over compressed segments of posting list in-place. However, it might lead to write past pg_upper, because intermediate state during playing the changes can take more space than both original state and final state. This commit fixes that by refuse from in-place modification. Instead page tail is copied once modification is started, and then it's used as the source of original segments. Backpatch to 9.4 where posting list compression was introduced. Reported-by: Sivasubramanian Ramasubramanian Discussion: https://postgr.es/m/1536091151804.6588%40amazon.com Author: Alexander Korotkov based on patch from and ideas by Sivasubramanian Ramasubramanian Review: Sivasubramanian Ramasubramanian Backpatch-through: 9.4
* Work around stdbool problem in dfmgr.c.Tom Lane2018-09-09
| | | | | | | | | | | | Commit 842cb9fa6 refactored things so that dfmgr.c includes <dlfcn.h>, which before that had only been directly included in platform-specific stub files. It turns out that on macOS, <dlfcn.h> includes <stdbool.h>, and that causes problems on platforms where _Bool is not char-sized ... which happens to include the PPC versions of macOS. Work around it much as we have in plperl.h, by #undef'ing bool after including the problematic file, but only if we're not using stdbool-style booleans. Discussion: https://postgr.es/m/E1fxqjl-0003YS-NS@gemulon.postgresql.org
* Install a check for mis-linking of src/port and src/common functions.Tom Lane2018-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | On ELF-based platforms (and maybe others?) it's possible for a shared library, when dynamically loaded into the backend, to call the backend versions of src/port and src/common functions rather than the frontend versions that are actually linked into the shlib. This is definitely not what we want, because the frontend versions often behave slightly differently. Up to now it's been "slight" enough that nobody noticed; but with the addition of SCRAM support functions in src/common, we're observing crashes due to the difference between palloc and malloc memory allocation rules, as reported in bug #15367 from Jeremy Evans. The purpose of this patch is to create a direct test for this type of mis-linking, so that we know whether any given platform requires extra measures to prevent using the wrong functions. If the test fails, it will lead to connection failures in the contrib/postgres_fdw regression test. At the moment, *BSD platforms using ELF format are known to have the problem and can be expected to fail; but we need to know whether anything else does, and we need a reliable ongoing check for future platforms. Actually fixing the problem will be the subject of later commit(s). Discussion: https://postgr.es/m/153626613985.23143.4743626885618266803@wrigleys.postgresql.org
* Allow ENOENT in check_mode_recursive().Noah Misch2018-09-08
| | | | | | | Buildfarm member tern failed src/bin/pg_ctl/t/001_start_stop.pl when a check_mode_recursive() call overlapped a server's startup-time deletion of pg_stat/global.stat. Just warn. Also, include errno in the message. Back-patch to v11, where check_mode_recursive() first appeared.
* Fix logical subscriber wait in test.Noah Misch2018-09-08
| | | | | | Buildfarm members sungazer and tern revealed this deficit. Back-patch to v10, like commit 4f10e7ea7b2231f453bb18b6e710ac333eaf121b, which introduced the test.
* Minor cleanup/future-proofing for pg_saslprep().Tom Lane2018-09-08
| | | | | | | | | | | | | | | | | Ensure that pg_saslprep() initializes its output argument to NULL in all failure paths, and then remove the redundant initialization that some (not all) of its callers did. This does not fix any live bug, but it reduces the odds of future bugs of omission. Also add a comment about why the existing failure-path coding is adequate. Back-patch so as to keep the function's API consistent across branches, again to forestall future bug introduction. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/16558.1536407783@sss.pgh.pa.us
* Remove duplicated words split across lines in commentsMichael Paquier2018-09-08
| | | | | | | | This has been detected using some interesting tricks with sed, and the method used is mentioned in details in the discussion below. Author: Justin Pryzby Discussion: https://postgr.es/m/20180908013109.GB15350@telsasoft.com
* Save/restore SPI's global variables in SPI_connect() and SPI_finish().Tom Lane2018-09-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch removes two sources of interference between nominally independent functions when one SPI-using function calls another, perhaps without knowing that it does so. Chapman Flack pointed out that xml.c's query_to_xml_internal() expects SPI_tuptable and SPI_processed to stay valid across datatype output function calls; but it's possible that such a call could involve re-entrant use of SPI. It seems likely that there are similar hazards elsewhere, if not in the core code then in third-party SPI users. Previously SPI_finish() reset SPI's API globals to zeroes/nulls, which would typically make for a crash in such a situation. Restoring them to the values they had at SPI_connect() seems like a considerably more useful behavior, and it still meets the design goal of not leaving any dangling pointers to tuple tables of the function being exited. Also, cause SPI_connect() to reset these variables to zeroes/nulls after saving them. This prevents interference in the opposite direction: it's possible that a SPI-using function that's only ever been tested standalone contains assumptions that these variables start out as zeroes. That was the case as long as you were the outermost SPI user, but not so much for an inner user. Now it's consistent. Report and fix suggestion by Chapman Flack, actual patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net
* Limit depth of forced recursion for CLOBBER_CACHE_RECURSIVELY.Tom Lane2018-09-07
| | | | | | | | | | | | It's somewhat surprising that we got away with this before. (Actually, since nobody tests this routinely AFAIK, it might've been broken for awhile. But it's definitely broken in the wake of commit f868a8143.) It seems sufficient to limit the forced recursion to a small number of levels. Back-patch to all supported branches, like the preceding patch. Discussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us
* Fix longstanding recursion hazard in sinval message processing.Tom Lane2018-09-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | LockRelationOid and sibling routines supposed that, if our session already holds the lock they were asked to acquire, they could skip calling AcceptInvalidationMessages on the grounds that we must have already read any remote sinval messages issued against the relation being locked. This is normally true, but there's a critical special case where it's not: processing inside AcceptInvalidationMessages might attempt to access system relations, resulting in a recursive call to acquire a relation lock. Hence, if the outer call had acquired that same system catalog lock, we'd fall through, despite the possibility that there's an as-yet-unread sinval message for that system catalog. This could, for example, result in failure to access a system catalog or index that had just been processed by VACUUM FULL. This is the explanation for buildfarm failures we've been seeing intermittently for the past three months. The bug is far older than that, but commits a54e1f158 et al added a new recursion case within AcceptInvalidationMessages that is apparently easier to hit than any previous case. To fix this, we must not skip calling AcceptInvalidationMessages until we have *finished* a call to it since acquiring a relation lock, not merely acquired the lock. (There's already adequate logic inside AcceptInvalidationMessages to deal with being called recursively.) Fortunately, we can implement that at trivial cost, by adding a flag to LOCALLOCK hashtable entries that tracks whether we know we have completed such a call. There is an API hazard added by this patch for external callers of LockAcquire: if anything is testing for LOCKACQUIRE_ALREADY_HELD, it might be fooled by the new return code LOCKACQUIRE_ALREADY_CLEAR into thinking the lock wasn't already held. This should be a fail-soft condition, though, unless something very bizarre is being done in response to the test. Also, I added an additional output argument to LockAcquireExtended, assuming that that probably isn't called by any outside code given the very limited usefulness of its additional functionality. Back-patch to all supported branches. Discussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us
* Improve handling of corrupted two-phase state files at recoveryMichael Paquier2018-09-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When a corrupted two-phase state file is found by WAL replay, be it for crash recovery or archive recovery, then the file is simply skipped and a WARNING is logged to the user, causing the transaction to be silently lost. Facing an on-disk WAL file which is corrupted is as likely to happen as what is stored in WAL records, but WAL records are already able to fail hard if there is a CRC mismatch. On-disk two-phase state files, on the contrary, are simply ignored if corrupted. Note that when restoring the initial two-phase data state at recovery, files newer than the horizon XID are discarded hence no files present in pg_twophase/ should be torned and have been made durable by a previous checkpoint, so recovery should never see any corrupted two-phase state file by design. The situation got better since 978b2f6 which has added two-phase state information directly in WAL instead of using on-disk files, so the risk is limited to two-phase transactions which live across at least one checkpoint for long periods. Backups having legit two-phase state files on-disk could also lose silently transactions when restored if things get corrupted. This behavior exists since two-phase commit has been introduced, no back-patch is done for now per the lack of complaints about this problem. Author: Michael Paquier Discussion: https://postgr.es/m/20180709050309.GM1467@paquier.xyz
* Refactor installation of extension headers.Andrew Gierth2018-09-07
| | | | | | | | | | | | | | | | Commit be54b3777 failed on gmake 3.80 due to a chained conditional, which on closer examination could be removed entirely with some refactoring elsewhere for a net simplification and more robustness against empty expansions. Along the way, add some more comments. Also make explicit in the documentation and comments that built headers are not removed by 'make clean', since we don't typically want that for headers generated by a separate ./configure step, and it's much easier to add your own 'distclean' rule or use EXTRA_CLEAN than to try and override a deletion rule in pgxs.mk. Per buildfarm member prariedog and comments by Michael Paquier, though all the actual changes are my fault.
* libpq: Change "options" dispchar to normalPeter Eisentraut2018-09-07
| | | | | | | | | | | | | | | | | | libpq connection options as returned by PQconndefaults() have a "dispchar" field that determines (among other things) whether an option is a "debug" option, which shouldn't be shown by default to clients. postgres_fdw makes use of that to control which connection options to accept from a foreign server configuration. Curiously, the "options" option, which allows passing configuration settings to the backend server, was listed as a debug option, which prevented it from being used by postgres_fdw. Maybe it was once meant for debugging, but it's clearly in general use nowadays. So change the dispchar for it to be the normal non-debug case. Also remove the "debug" reference from its label field. Reported-by: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
* Use C99 designated initializers for some structsPeter Eisentraut2018-09-07
| | | | | | | These are just a few particularly egregious cases that were hard to read and write, and error prone because of many similar adjacent types. Discussion: https://www.postgresql.org/message-id/flat/4c9f01be-9245-2148-b569-61a8562ef190%402ndquadrant.com
* Fix inconsistent argument naming.Tom Lane2018-09-06
| | | | Typo in commit 842cb9fa6.
* Make contrib/unaccent's unaccent() function work when not in search path.Tom Lane2018-09-06
| | | | | | | | | | | | | | | | | | | | | | | | Since the fixes for CVE-2018-1058, we've advised people to schema-qualify function references in order to fix failures in code that executes under a minimal search_path setting. However, that's insufficient to make the single-argument form of unaccent() work, because it looks up the "unaccent" text search dictionary using the search path. The most expedient answer seems to be to remove the search_path dependency by making it look in the same schema that the unaccent() function itself is declared in. This will definitely work for the normal usage of this function with the unaccent dictionary provided by the extension. It's barely possible that there are people who were relying on the search-path-dependent behavior to select other dictionaries with the same name; but if there are any such people at all, they can still get that behavior by writing unaccent('unaccent', ...), or possibly unaccent('unaccent'::text::regdictionary, ...) if the lookup has to be postponed to runtime. Per complaint from Gunnlaugur Thor Briem. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAPs+M8LCex6d=DeneofdsoJVijaG59m9V0ggbb3pOH7hZO4+cQ@mail.gmail.com
* Refactor dlopen() supportPeter Eisentraut2018-09-06
| | | | | | | | | | Nowadays, all platforms except Windows and older HP-UX have standard dlopen() support. So having a separate implementation per platform under src/backend/port/dynloader/ is a bit excessive. Instead, treat dlopen() like other library functions that happen to be missing sometimes and put a replacement implementation under src/port/. Discussion: https://www.postgresql.org/message-id/flat/e11a49cb-570a-60b7-707d-7084c8de0e61%402ndquadrant.com#54e735ae37476a121abb4e33c2549b03
* Fix the overrun in hash index metapage for smaller block sizes.Amit Kapila2018-09-06
| | | | | | | | | | | | | | | | | | The commit 620b49a1 changed the value of HASH_MAX_BITMAPS with the intent to allow many non-unique values in hash indexes without worrying to reach the limit of the number of overflow pages. At that time, this didn't occur to us that it can overrun the block for smaller block sizes. Choose the value of HASH_MAX_BITMAPS based on BLCKSZ such that it gives the same answer as now for the cases where the overrun doesn't occur, and some other sufficiently-value for the cases where an overrun currently does occur. This allows us not to change the behavior in any case that currently works, so there's really no reason for a HASH_VERSION bump. Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/CAA4eK1LtF4VmU4mx_+i72ff1MdNZ8XaJMGkt2HV8+uSWcn8t4A@mail.gmail.com
* Allow extensions to install built as well as unbuilt headers.Andrew Gierth2018-09-05
| | | | | | | | | | | | | | | | Commit df163230b overlooked the case that an out-of-tree extension might need to build its header files (e.g. via ./configure). If it is also doing a VPATH build, the HEADERS_* rules in the original commit would then fail to find the files, since they would be looking only under $(srcdir) and not in the build directory. Fix by adding HEADERS_built and HEADERS_built_$(MODULE) which behave like DATA_built in that they look in the build dir rather than the source dir (and also make the files dependencies of the "all" target). No Windows support appears to be needed for this, since it is only relevant to out-of-tree builds (no support exists in Mkvcbuild.pm to build extension header files in any case).
* Remove no-longer-used variable.Tom Lane2018-09-05
| | | | Oversight in 2fbdf1b38. Per buildfarm.
* Make argument names of pg_get_object_address consistent, and fix docs.Tom Lane2018-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | pg_get_object_address and pg_identify_object_as_address are supposed to be inverses, but they disagreed as to the names of the arguments representing the textual form of an object address. Moreover, the documented argument names didn't agree with reality at all, either for these functions or pg_identify_object. In HEAD and v11, I think we can get away with renaming the input arguments of pg_get_object_address to match the outputs of pg_identify_object_as_address. In theory that might break queries using named-argument notation to call pg_get_object_address, but it seems really unlikely that anybody is doing that, or that they'd have much trouble adjusting if they were. In older branches, we'll just live with the lack of consistency. Aside from fixing the documentation of these functions to match reality, I couldn't resist the temptation to do some copy-editing. Per complaint from Jean-Pierre Pelletier. Back-patch to 9.5 where these functions were introduced. (Before v11, this is a documentation change only.) Discussion: https://postgr.es/m/CANGqjDnWH8wsTY_GzDUxbt4i=y-85SJreZin4Hm8uOqv1vzRQA@mail.gmail.com
* Simplify partitioned table creation vs. relcacheAlvaro Herrera2018-09-05
| | | | | | | | | | | | | | | | | | | In the original code, we were storing the pg_inherits row for a partitioned table too early: enough that we had a hack for relcache to avoid falling flat on its face while reading such a partial entry. If we finish the pg_class creation first and *then* store the pg_inherits entry, we don't need that hack. Also recognize that pg_class.relpartbound is not marked NOT NULL and therefore it's entirely possible to read null values, so having only Assert() protection isn't enough. Change those to if/elog tests instead. This qualifies as a robustness fix, so backpatch to pg11. In passing, remove one access that wasn't actually needed, and reword one message to be like all the others that check for the same thing. Reviewed-by: Amit Langote Discussion: https://postgr.es/m/20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql
* PL/Python: Remove use of simple slicing APIPeter Eisentraut2018-09-05
| | | | | | | | | | The simple slicing API (sq_slice, sq_ass_slice) has been deprecated since Python 2.0 and has been removed altogether in Python 3, so remove those functions from the PLyResult class. Instead, the non-slice mapping functions mp_subscript and mp_ass_subscript can take slice objects as an index. Since we just pass the index through to the underlying list object, we already support that. Test coverage was already in place.
* docs: improve AT TIME ZONE descriptionBruce Momjian2018-09-04
| | | | | | | | | The previous description was unclear. Also add a third example, change use of time zone acronyms to more verbose descriptions, and add a mention that using 'time' with AT TIME ZONE uses the current time zone rules. Backpatch-through: 9.3
* Improve some error message strings and errcodesMichael Paquier2018-09-04
| | | | | | | | | | | | | | | | This makes a bit less work for translators, by unifying error strings a bit more with what the rest of the code does, this time for three error strings in autoprewarm and one in base backup code. After some code review of slot.c, some file-access errcodes are reported but lead to an incorrect internal error, while corrupted data makes the most sense, similarly to the previous work done in e41d0a1. Also, after calling rmtree(), a WARNING gets reported, which is a duplicate of what the internal call report, so make the code more consistent with all other code paths calling this function. Author: Michael Paquier Discussion: https://postgr.es/m/20180902200747.GC1343@paquier.xyz
* Fully enforce uniqueness of constraint names.Tom Lane2018-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's been true for a long time that we expect names of table and domain constraints to be unique among the constraints of that table or domain. However, the enforcement of that has been pretty haphazard, and it missed some corner cases such as creating a CHECK constraint and then an index constraint of the same name (as per recent report from André Hänsel). Also, due to the lack of an actual unique index enforcing this, duplicates could be created through race conditions. Moreover, the code that searches pg_constraint has been quite inconsistent about how to handle duplicate names if one did occur: some places checked and threw errors if there was more than one match, while others just processed the first match they came to. To fix, create a unique index on (conrelid, contypid, conname). Since either conrelid or contypid is zero, this will separately enforce uniqueness of constraint names among constraints of any one table and any one domain. (If we ever implement SQL assertions, and put them into this catalog, more thought might be needed. But it'd be at least as reasonable to put them into a new catalog; having overloaded this one catalog with two kinds of constraints was a mistake already IMO.) This index can replace the existing non-unique index on conrelid, though we need to keep the one on contypid for query performance reasons. Having done that, we can simplify the logic in various places that either coped with duplicates or neglected to, as well as potentially improve lookup performance when searching for a constraint by name. Also, as per our usual practice, install a preliminary check so that you get something more friendly than a unique-index violation report in the case complained of by André. And teach ChooseIndexName to avoid choosing autogenerated names that would draw such a failure. While it's not possible to make such a change in the back branches, it doesn't seem quite too late to put this into v11, so do so. Discussion: https://postgr.es/m/0c1001d4428f$0942b430$1bc81c90$@webkr.de
* Clean up after TAP tests in oid2name and vacuumlo.Tom Lane2018-09-04
| | | | | | | | Oversights in commits 1aaf532de and bfea331a5. Unlike the case for traditional-style REGRESS tests, pgxs.mk doesn't have any builtin support for TAP tests, so it doesn't realize it should remove tmp_check/. Maybe we should build some actual pgxs infrastructure for TAP tests ... but for the moment, just remove explicitly.
* Prohibit pushing subqueries containing window function calculation toAmit Kapila2018-09-04
| | | | | | | | | | | | | | | | | | | | workers. Allowing window function calculation in workers leads to inconsistent results because if the input row ordering is not fully deterministic, the output of window functions might vary across workers. The fix is to treat them as parallel-restricted. In the passing, improve the coding pattern in max_parallel_hazard_walker so that it has a chain of mutually-exclusive if ... else if ... else if ... else if ... IsA tests. Reported-by: Marko Tiikkaja Bug: 15324 Author: Amit Kapila Reviewed-by: Tom Lane Backpatch-through: 9.6 Discussion: https://postgr.es/m/CAL9smLAnfPJCDUUG4ckX2iznj53V7VSMsYefzZieN93YxTNOcw@mail.gmail.com
* During the split, set checksum on an empty hash index page.Amit Kapila2018-09-04
| | | | | | | | | | | | | | | | On a split, we allocate a new splitpoint's worth of bucket pages wherein we initialize the last page with zeros which is fine, but we forgot to set the checksum for that last page. We decided to back-patch this fix till 10 because we don't have an easy way to test it in prior versions. Another reason is that the hash-index code is changed heavily in 10, so it is not advisable to push the fix without testing it in prior versions. Author: Amit Kapila Reviewed-by: Yugo Nagata Backpatch-through: 10 Discussion: https://postgr.es/m/5d03686d-727c-dbf8-0064-bf8b97ffe850@2ndquadrant.com