aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* 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
* 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.
* 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
* 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
* 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
* Remove pg_constraint.conincludingAlvaro Herrera2018-09-03
| | | | | | | | | | | | This column was added in commit 8224de4f42cc ("Indexes with INCLUDE columns and their support in B-tree") to ease writing the ruleutils.c supporting code for that feature, but it turns out to be unnecessary -- we can do the same thing with just one more syscache lookup. Even the documentation for the new column being removed in this commit is awkward. Discussion: https://postgr.es/m/20180902165018.33otxftp3olgtu4t@alvherre.pgsql
* Fix memory leak in TRUNCATE decodingTomas Vondra2018-09-03
| | | | | | | | | | | | | | | | | | | | | | | | When decoding a TRUNCATE record, the relids array was being allocated in the main ReorderBuffer memory context, but not released with the change resulting in a memory leak. The array was also ignored when serializing/deserializing the change, assuming all the information is stored in the change itself. So when spilling the change to disk, we've only we have serialized only the pointer to the relids array. Thanks to never releasing the array, the pointer however remained valid even after loading the change back to memory, preventing an actual crash. This fixes both the memory leak and (de)serialization. The relids array is still allocated in the main ReorderBuffer memory context (none of the existing ones seems like a good match, and adding an extra context seems like an overkill). The allocation is wrapped in a new ReorderBuffer API functions, to keep the details within reorderbuffer.c, just like the other ReorderBufferGet methods do. Author: Tomas Vondra Discussion: https://www.postgresql.org/message-id/flat/66175a41-9342-2845-652f-1bd4c3ee50aa%402ndquadrant.com Backpatch: 11, where decoding of TRUNCATE was introduced
* Fix initial sync of slot parent directory when restoring statusMichael Paquier2018-09-02
| | | | | | | | | | | | | | | | At the beginning of recovery, information from replication slots is recovered from disk to memory. In order to ensure the durability of the information, the status file as well as its parent directory are synced. It happens that the sync on the parent directory was done directly using the status file path, which is logically incorrect, and the current code has been doing a sync on the same object twice in a row. Reported-by: Konstantin Knizhnik Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/9eb1a6d5-b66f-2640-598d-c5ea46b8f68a@postgrespro.ru Backpatch-through: 9.4-
* Avoid using potentially-under-aligned page buffers.Tom Lane2018-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | There's a project policy against using plain "char buf[BLCKSZ]" local or static variables as page buffers; preferred style is to palloc or malloc each buffer to ensure it is MAXALIGN'd. However, that policy's been ignored in an increasing number of places. We've apparently got away with it so far, probably because (a) relatively few people use platforms on which misalignment causes core dumps and/or (b) the variables chance to be sufficiently aligned anyway. But this is not something to rely on. Moreover, even if we don't get a core dump, we might be paying a lot of cycles for misaligned accesses. To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlock that the compiler must allocate with sufficient alignment, and use those in place of plain char arrays. I used these types even for variables where there's no risk of a misaligned access, since ensuring proper alignment should make kernel data transfers faster. I also changed some places where we had been palloc'ing short-lived buffers, for coding style uniformity and to save palloc/pfree overhead. Since this seems to be a live portability hazard (despite the lack of field reports), back-patch to all supported versions. Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de
* Ignore server-side delays when enforcing wal_sender_timeout.Noah Misch2018-08-31
| | | | | | | | | | | Healthy clients of servers having poor I/O performance, such as buildfarm members hamster and tern, saw unexpected timeouts. That disagreed with documentation. This fix adds one gettimeofday() call whenever ProcessRepliesIfAny() finds no client reply messages. Back-patch to 9.4; the bug's symptom is rare and mild, and the code all moved between 9.3 and 9.4. Discussion: https://postgr.es/m/20180826034600.GA1105084@rfd.leadboat.com
* Fix 8a934d677 for libc++ and make more include order resistant.Andres Freund2018-08-31
| | | | | | | | | | | | | | | | | | | | | The previous definition was used in C++ mode, which causes problems when using clang with libc++ (rather than libstdc++), due to bugs therein. So just avoid in C++ mode. A second problem is that depending on include order and implicit includes the previous definition did not guarantee that the current hack was effective by the time isinf was used, fix that by forcing math.h to be included. This can cause clang using builds, or gcc using ones with JIT enabled, to slow down noticably. It's likely that we at some point want a better solution for the performance problem, but while it's there it should better work. Reported-By: Steven Winfield Bug: #15270 Discussion: https://postgr.es/m/153116283147.1401.360416241833049560@wrigleys.postgresql.org Author: Andres Freund Backpatch: 11, like the previous commit.
* Fix psql's \dC command to annotate I/O conversion casts as such.Tom Lane2018-08-31
| | | | | | | | | | | | | A cast declared WITH INOUT was described as '(binary coercible)', which seems pretty inaccurate; let's print '(with inout)' instead. Per complaint from Jean-Pierre Pelletier. This definitely seems like a bug fix, but given that it's been wrong since 8.4 and nobody complained before, I'm hesitant to back-patch a behavior change into stable branches. It doesn't seem too late for v11 though. Discussion: https://postgr.es/m/5b887023.1c69fb81.ff96e.6a1d@mx.google.com
* Ensure correct minimum consistent point on standbysMichael Paquier2018-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Startup process has improved its calculation of incorrect minimum consistent point in 8d68ee6, which ensures that all WAL available gets replayed when doing crash recovery, and has introduced an incorrect calculation of the minimum recovery point for non-startup processes, which can cause incorrect page references on a standby when for example the background writer flushed a couple of pages on-disk but was not updating the control file to let a subsequent crash recovery replay to where it should have. The only case where this has been reported to be a problem is when a standby needs to calculate the latest removed xid when replaying a btree deletion record, so one would need connections on a standby that happen just after recovery has thought it reached a consistent point. Using a background worker which is started after the consistent point is reached would be the easiest way to get into problems if it connects to a database. Having clients which attempt to connect periodically could also be a problem, but the odds of seeing this problem are much lower. The fix used is pretty simple, as the idea is to give access to the minimum recovery point written in the control file to non-startup processes so as they use a reference, while the startup process still initializes its own references of the minimum consistent point so as the original problem with incorrect page references happening post-promotion with a crash do not show up. Reported-by: Alexander Kukushkin Diagnosed-by: Alexander Kukushkin Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi, Alexander Kukushkin Discussion: https://postgr.es/m/153492341830.1368.3936905691758473953@wrigleys.postgresql.org Backpatch-through: 9.3
* Code review for pg_verify_checksums.c.Tom Lane2018-08-31
| | | | | | | | | | | | | | | | | | Use postgres_fe.h, since this is frontend code. Pretend that we've heard of project style guidelines for, eg, #include order. Use BlockNumber not int arithmetic for block numbers, to avoid misbehavior with relations exceeding 2^31 blocks. Avoid an unnecessary strict-aliasing warning (per report from Michael Banck). Const-ify assorted stuff. Avoid scribbling on the output of readdir() -- perhaps that's safe in practice, but POSIX forbids it, and this code has so far earned exactly zero credibility portability-wise. Editorialize on an ambiguously-worded message. I did not touch the problem of the "buf" local variable being possibly insufficiently aligned; that's not specific to this code, and seems like it should be fixed as part of a different, larger patch. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de
* 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
* Disable support for partitionwise joins in problematic cases.Etsuro Fujita2018-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit f49842d, which added support for partitionwise joins, built the child's tlist by applying adjust_appendrel_attrs() to the parent's. So in the case where the parent's included a whole-row Var for the parent, the child's contained a ConvertRowtypeExpr. To cope with that, that commit added code to the planner, such as setrefs.c, but some code paths still assumed that the tlist for a scan (or join) rel would only include Vars and PlaceHolderVars, which was true before that commit, causing errors: * When creating an explicit sort node for an input path for a mergejoin path for a child join, prepare_sort_from_pathkeys() threw the 'could not find pathkey item to sort' error. * When deparsing a relation participating in a pushed down child join as a subquery in contrib/postgres_fdw, get_relation_column_alias_ids() threw the 'unexpected expression in subquery output' error. * When performing set_plan_references() on a local join plan generated by contrib/postgres_fdw for EvalPlanQual support for a pushed down child join, fix_join_expr() threw the 'variable not found in subplan target lists' error. To fix these, two approaches have been proposed: one by Ashutosh Bapat and one by me. While the former keeps building the child's tlist with a ConvertRowtypeExpr, the latter builds it with a whole-row Var for the child not to violate the planner assumption, and tries to fix it up later, But both approaches need more work, so refuse to generate partitionwise join paths when whole-row Vars are involved, instead. We don't need to handle ConvertRowtypeExprs in the child's tlists for now, so this commit also removes the changes to the planner. Previously, partitionwise join computed attr_needed data for each child separately, and built the child join's tlist using that data, which also required an extra step for adding PlaceHolderVars to that tlist, but it would be more efficient to build it from the parent join's tlist through the adjust_appendrel_attrs() transformation. So this commit builds that list that way, and simplifies build_joinrel_tlist() and placeholder.c as well as part of set_append_rel_size() to basically what they were before partitionwise join went in. Back-patch to PG11 where partitionwise join was introduced. Report by Rajkumar Raghuwanshi. Analysis by Ashutosh Bapat, who also provided some of regression tests. Patch by me, reviewed by Robert Haas. Discussion: https://postgr.es/m/CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com
* Fix pg_verify_checksums on Windows.Amit Kapila2018-08-31
| | | | | | | | | | | | | To verify the checksums, we open the file in text mode which doesn't work on Windows as WIN32 treats Control-Z as EOF in files opened in text mode. This leads to "short read of block .." error in some cases. Fix it by opening the files in the binary mode. Author: Amit Kapila Reviewed-by: Magnus Hagander Backpatch-through: 11 Discussion: https://postgr.es/m/CAA4eK1+LOnzod+h85FGmyjWzXKy-XV1FYwEyP-Tky2WpD5cxwA@mail.gmail.com
* Remove extra word from src/backend/optimizer/READMEEtsuro Fujita2018-08-31
|
* pg_verify_checksums: rename -d to --verboseAlvaro Herrera2018-08-30
| | | | | | | | | | | | | | Using -d is odd, because we normally reserve that for a database argument, so rename it to -v and add long version --verbose. Also, reduce it to emit one line per file checked rather than one line per block. Per a complaint from Michael Banck. Author: Yugo Nagata <nagata@sraoss.co.jp> Reviewed-by: Michael Banck <michael.banck@credativ.de> Discussion: https://postgr.es/m/20180827113411.GA22768@nighthawk.caipicrew.dd-dns.de
* Fix IndexInfo comments.Heikki Linnakangas2018-08-30
| | | | | | | | Recently, ii_KeyAttrNumbers was renamed to ii_IndexAttrNumbers, and ii_Am field was added, but the comments were not updated. Author: Yugo Nagata Discussion: https://www.postgresql.org/message-id/20180830134831.e35a91b8b978b248c16c8f7b@sraoss.co.jp
* 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
* Code review for pg_dump's handling of ALTER INDEX ATTACH PARTITION.Tom Lane2018-08-28
| | | | | | | | | | | | | | | Ensure the TOC entry is marked with the correct schema, so that its name is as unique as the index's is. Fix the dependencies: we want dependencies from this TOC entry to the two indexes it depends on, and we don't care (at least not for this purpose) what order the indexes are created in. Also, add dependencies on the indexes' underlying tables. Those might seem pointless given the index dependencies, but they are helpful to cue parallel restore to avoid running the ATTACH PARTITION in parallel with other DDL on the same tables. Discussion: https://postgr.es/m/10817.1535494963@sss.pgh.pa.us
* Include contrib modules in the temp installation even without REGRESS.Tom Lane2018-08-28
| | | | | | | | | | | | | | Now that we have TAP tests, a contrib module may have something useful to do in "make check" even if it has no pg_regress-style regression scripts, and hence no REGRESS setting. But the TAP tests will fail, or else test the wrong installed files, unless we install the contrib module into the temp installation. So move the bit about adding to EXTRA_INSTALL so that it applies regardless. We might want this in back branches in future, but for the moment I only risked adding it to v11. Discussion: https://postgr.es/m/12438.1535488750@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
* pg_verify_checksums: Message style improvements and NLS supportPeter Eisentraut2018-08-28
| | | | | | The source code was already set up for NLS support, so just a nls.mk file needed to be added. Also, fix the old problem of putting the int64 format specifier right into the string, which breaks NLS.
* Fix snapshot leak warning for some proceduresPeter Eisentraut2018-08-27
| | | | | | | | | | | | | | | | | | | | The problem arises with the combination of CALL with output parameters and doing a COMMIT inside the procedure. When a CALL has output parameters, the portal uses the strategy PORTAL_UTIL_SELECT instead of PORTAL_MULTI_QUERY. Using PORTAL_UTIL_SELECT causes the portal's snapshot to be registered with the current resource owner (portal->holdSnapshot); see 9ee1cf04ab6bcefe03a11837b53f29ca9dc24c7a for the reason. Normally, PortalDrop() unregisters the snapshot. If not, then ResourceOwnerRelease() will print a warning about a snapshot leak on transaction commit. A transaction commit normally drops all portals (PreCommit_Portals()), except the active portal. So in case of the active portal, we need to manually release the snapshot to avoid the warning. Reported-by: Prabhat Sahu <prabhat.sahu@enterprisedb.com> Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org>
* 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
* Fix typos.Thomas Munro2018-08-27
| | | | | Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f8du35u5DprpykWvgNEScxapbWYJdHq%2Bz06Wj3Y2KFPbw%40mail.gmail.com
* 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
* LLVMJIT: LLVMGetHostCPUFeatures now is upstream, use LLMV version if available.Andres Freund2018-08-24
| | | | | | | Noticed thanks to buildfarm animal seawasp. Author: Andres Freund Backpatch: v11-, where LLVM based JIT compliation was introduced.
* Suppress uninitialized-variable warning in new SCRAM code.Tom Lane2018-08-24
| | | | | | | While we generally don't sweat too much about "may be used uninitialized" warnings from older compilers, I noticed that there's a fair number of buildfarm animals that are producing such a warning *only* for this variable. So it seems worth silencing.
* 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.
* In libpq, don't look up all the hostnames at once.Tom Lane2018-08-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, we looked up the target hostname in connectDBStart, so that PQconnectPoll did not need to do DNS name resolution. The patches that added multiple-target-host support to libpq preserved this division of labor; but it's really nonsensical now, because it means that if any one of the target hosts fails to resolve in DNS, the connection fails. That negates the no-single-point-of-failure goal of the feature. Additionally, DNS lookups aren't exactly cheap, but the code did them all even if the first connection attempt succeeds. Hence, rearrange so that PQconnectPoll does the lookups, and only looks up a hostname when it's time to try that host. This does mean that PQconnectPoll could block on a DNS lookup --- but if you wanted to avoid that, you should be using hostaddr, as the documentation has always specified. It seems fairly unlikely that any applications would really care whether the lookup occurs inside PQconnectStart or PQconnectPoll. In addition to calling out that fact explicitly, do some other minor wordsmithing in the docs around the multiple-target-host feature. Since this seems like a bug in the multiple-target-host feature, backpatch to v10 where that was introduced. In the back branches, avoid moving any existing fields of struct pg_conn, just in case any third-party code is looking into that struct. Tom Lane, reviewed by Fabien Coelho Discussion: https://postgr.es/m/4913.1533827102@sss.pgh.pa.us
* Copy-editing of pg_verify_checksums help and ref pagePeter Eisentraut2018-08-23
| | | | | Reformat synopsis, put options into better order, make the desciption line a bit shorter, and put more details into the description.
* PL/pgSQL: Extend test casePeter Eisentraut2018-08-23
| | | | | | | | This test was supposed to check the interaction of INOUT and default parameters in a procedure call, but it only checked the case where the parameter was not supplied. Now it also checks the case where the parameter was supplied. It was already working correctly, so no code changes required.
* Change PROCEDURE to FUNCTION in CREATE TRIGGER syntaxPeter Eisentraut2018-08-22
| | | | | | | | | | | | | Since procedures are now a different thing from functions, change the CREATE TRIGGER and CREATE EVENT TRIGGER syntax to use FUNCTION in the clause that specifies the function. PROCEDURE is still accepted for compatibility. pg_dump and ruleutils.c output is not changed yet, because that would require a change in information_schema.sql and thus a catversion change. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* Change PROCEDURE to FUNCTION in CREATE OPERATOR syntaxPeter Eisentraut2018-08-22
| | | | | | | | | Since procedures are now a different thing from functions, change the CREATE OPERATOR syntax to use FUNCTION in the clause that specifies the function. PROCEDURE is still accepted for compatibility. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* doc: Update uses of the word "procedure"Peter Eisentraut2018-08-22
| | | | | | | | | | | | | | | | | | | Historically, the term procedure was used as a synonym for function in Postgres/PostgreSQL. Now we have procedures as separate objects from functions, so we need to clean up the documentation to not mix those terms. In particular, mentions of "trigger procedures" are changed to "trigger functions", and access method "support procedures" are changed to "support functions". (The latter already used FUNCTION in the SQL syntax anyway.) Also, the terminology in the SPI chapter has been cleaned up. A few tests, examples, and code comments are also adjusted to be consistent with documentation changes, but not everything. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* Do not dump identity sequences with excluded parent tableMichael Paquier2018-08-22
| | | | | | | | | | | | | | This commit prevents a crash of pg_dump caused by the exclusion of a table which has identity columns, as the table would be correctly excluded but not its identity sequence. In order to fix that, identity sequences are excluded if the parent table is defined as such. Knowing about such sequences has no meaning without their parent table anyway. Reported-by: Andy Abelisto Author: David Rowley Reviewed-by: Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/153479393218.1316.8472285660264976457@wrigleys.postgresql.org Backpatch-through: 10
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 17:45:49 +0000