aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Fix issues around EXPLAIN with JIT.Andres Freund2018-10-03
| | | | | | | | | | | | | | | | | I (Andres) was more than a bit hasty in committing 33001fd7a7072d48327 after last minute changes, leading to a number of problems (jit output was only shown for JIT in parallel workers, and just EXPLAIN without ANALYZE didn't work). Lukas luckily found these issues quickly. Instead of combining instrumentation in in standard_ExecutorEnd(), do so on demand in the new ExplainPrintJITSummary(). Also update a documentation example of the JIT output, changed in 52050ad8ebec8d831. Author: Lukas Fittl, with minor changes by me Discussion: https://postgr.es/m/CAP53PkxmgJht69pabxBXJBM+0oc6kf3KHMborLP7H2ouJ0CCtQ@mail.gmail.com Backpatch: 11, where JIT compilation was introduced
* MAXALIGN the target address where we store flattened value.Amit Kapila2018-10-03
| | | | | | | | | | | | The API (EOH_flatten_into) that flattens the expanded value representation expects the target address to be maxaligned. All it's usage adhere to that principle except when serializing datums for parallel query. Fix that usage. Diagnosed-by: Tom Lane Author: Tom Lane and Amit Kapila Backpatch-through: 9.6 Discussion: https://postgr.es/m/11629.1536550032@sss.pgh.pa.us
* Set snprintf.c's maximum number of NL arguments to be 31.Tom Lane2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | Previously, we used the platform's NL_ARGMAX if any, otherwise 16. The trouble with this is that the platform value is hugely variable, ranging from the POSIX-minimum 9 to as much as 64K on recent FreeBSD. Values of more than a dozen or two have no practical use and slow down the initialization of the argtypes array. Worse, they cause snprintf.c to consume far more stack space than was the design intention, possibly resulting in stack-overflow crashes. Standardize on 31, which is comfortably more than we need (it looks like no existing translatable message has more than about 10 parameters). I chose that, not 32, to make the array sizes powers of 2, for some possible small gain in speed of the memset. The lack of reported crashes suggests that the set of platforms we use snprintf.c on (in released branches) may have no overlap with the set where NL_ARGMAX has unreasonably large values. But that's not entirely clear, so back-patch to all supported branches. Per report from Mateusz Guzik (via Thomas Munro). Discussion: https://postgr.es/m/CAEepm=3VF=PUp2f8gU8fgZB22yPE_KBS0+e1AHAtQ=09schTHg@mail.gmail.com
* Fix corner-case failures in has_foo_privilege() family of functions.Tom Lane2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The variants of these functions that take numeric inputs (OIDs or column numbers) are supposed to return NULL rather than failing on bad input; this rule reduces problems with snapshot skew when queries apply the functions to all rows of a catalog. has_column_privilege() had careless handling of the case where the table OID didn't exist. You might get something like this: select has_column_privilege(9999,'nosuchcol','select'); ERROR: column "nosuchcol" of relation "(null)" does not exist or you might get a crash, depending on the platform's printf's response to a null string pointer. In addition, while applying the column-number variant to a dropped column returned NULL as desired, applying the column-name variant did not: select has_column_privilege('mytable','........pg.dropped.2........','select'); ERROR: column "........pg.dropped.2........" of relation "mytable" does not exist It seems better to make this case return NULL as well. Also, the OID-accepting variants of has_foreign_data_wrapper_privilege, has_server_privilege, and has_tablespace_privilege didn't follow the principle of returning NULL for nonexistent OIDs. Superusers got TRUE, everybody else got an error. Per investigation of Jaime Casanova's report of a new crash in HEAD. These behaviors have been like this for a long time, so back-patch to all supported branches. Patch by me; thanks to Stephen Frost for discussion and review Discussion: https://postgr.es/m/CAJGNTeP=-6Gyqq5TN9OvYEydi7Fv1oGyYj650LGTnW44oAzYCg@mail.gmail.com
* Fix documentation of pgrowlocks using "lock_type" instead of "modes"Michael Paquier2018-10-02
| | | | | | | | | | The example used in the documentation is outdated as well. This is an oversight from 0ac5ad5, which bumped up pgrowlocks but forgot some bits of the documentation. Reported-by: Chris Wilson Discussion: https://postgr.es/m/153838692816.2950.12001142346234155699@wrigleys.postgresql.org Backpatch-through: 9.3
* Change PROCEDURE to FUNCTION in CREATE EVENT TRIGGER syntaxPeter Eisentraut2018-10-01
| | | | | | | This was claimed to have been done in 0a63f996e018ac508c858e87fa39cc254a5db49f, but that actually only changed the documentation and not the grammar. (That commit did fully change it for CREATE TRIGGER.)
* Fix tuple_data_split() to not open a relation without any lock.Tom Lane2018-10-01
| | | | | | | | | | | | | | | | | contrib/pageinspect's tuple_data_split() function thought it could get away with opening the referenced relation with NoLock. In practice there's no guarantee that the current session holds any lock on that rel (even if we just read a page from it), so that this is unsafe. Switch to using AccessShareLock. Also, postpone closing the relation, so that we needn't copy its tupdesc. Also, fix unsafe use of att_isnull() for attributes past the end of the tuple. Per testing with a patch that complains if we open a relation without holding any lock on it. I don't plan to back-patch that patch, but we should close the holes it identifies in all supported branches. Discussion: https://postgr.es/m/2038.1538335244@sss.pgh.pa.us
* Fix ALTER COLUMN TYPE to not open a relation without any lock.Tom Lane2018-10-01
| | | | | | | | | | | | | | | | | | | If the column being modified is referenced by a foreign key constraint of another table, ALTER TABLE would open the other table (to re-parse the constraint's definition) without having first obtained a lock on it. This was evidently intentional, but that doesn't mean it's really safe. It's especially not safe in 9.3, which pre-dates use of MVCC scans for catalog reads, but even in current releases it doesn't seem like a good idea. We know we'll need AccessExclusiveLock shortly to drop the obsoleted constraint, so just get that a little sooner to close the hole. Per testing with a patch that complains if we open a relation without holding any lock on it. I don't plan to back-patch that patch, but we should close the holes it identifies in all supported branches. Discussion: https://postgr.es/m/2038.1538335244@sss.pgh.pa.us
* Fix detection of the result type of strerror_r().Tom Lane2018-09-30
| | | | | | | | | | | | | | | | | | | | The method we've traditionally used, of redeclaring strerror_r() to see if the compiler complains of inconsistent declarations, turns out not to work reliably because some compilers only report a warning, not an error. Amazingly, this has gone undetected for years, even though it certainly breaks our detection of whether strerror_r succeeded. Let's instead test whether the compiler will take the result of strerror_r() as a switch() argument. It's possible this won't work universally either, but it's the best idea I could come up with on the spur of the moment. Back-patch of commit 751f532b9. Buildfarm results indicate that only icc-on-Linux actually has an issue here; perhaps the lack of field reports indicates that people don't build PG for production that way. Discussion: https://postgr.es/m/10877.1537993279@sss.pgh.pa.us
* Improve error reporting for unsupported effective_io_concurrency setting.Tom Lane2018-09-28
| | | | | | | | | | | | | | Give a specific error complaining about lack of posix_fadvise() when someone tries to set effective_io_concurrency > 0 on platforms without that. This probably isn't worth extensive back-patching, but I (tgl) felt cramming it into v11 was reasonable. James Robinson Discussion: https://postgr.es/m/153771876450.14994.560017943128223619@wrigleys.postgresql.org Discussion: https://postgr.es/m/A3942987-5BC7-4F05-B54D-2A0EC2914B33@jlr-photo.com
* Fix assertion failure when updating full_page_writes for checkpointer.Amit Kapila2018-09-28
| | | | | | | | | | | | | | | | | | When the checkpointer receives a SIGHUP signal to update its configuration, it may need to update the shared memory for full_page_writes and need to write a WAL record for it. Now, it is quite possible that the XLOG machinery has not been initialized by that time and it will lead to assertion failure while doing that. Fix is to allow the initialization of the XLOG machinery outside critical section. This bug has been introduced by the commit 2c03216d83 which added the XLOG machinery initialization in RecoveryInProgress code path. Reported-by: Dilip Kumar Author: Dilip Kumar Reviewed-by: Michael Paquier and Amit Kapila Backpatch-through: 9.5 Discussion: https://postgr.es/m/CAFiTN-u4BA8KXcQUWDPNgaKAjDXC=C2whnzBM8TAcv=stckYUw@mail.gmail.com
* Fix WAL recycling on standbys depending on archive_modeMichael Paquier2018-09-28
| | | | | | | | | | | | | | | | | | | | | | | | A restart point or a checkpoint recycling WAL segments treats segments marked with neither ".done" (archiving is done) or ".ready" (segment is ready to be archived) in archive_status the same way for archive_mode being "on" or "always". While for a primary this is fine, a standby running a restart point with archive_mode = on would try to mark such a segment as ready for archiving, which is something that will never happen except after the standby is promoted. Note that this problem applies only to WAL segments coming from the local pg_wal the first time archive recovery is run. Segments part of a self-contained base backup are the most common case where this could happen, however even in this case normally the .done markers would be most likely part of the backup. Segments recovered from an archive are marked as .ready or .done by the startup process, and segments finished streaming are marked as such by the WAL receiver, so they are handled already. Reported-by: Haruka Takatsuka Author: Michael Paquier Discussion: https://postgr.es/m/15402-a453c90ed4cf88b2@postgresql.org Backpatch-through: 9.5, where archive_mode = always has been added.
* Fix assorted bugs in pg_get_partition_constraintdef().Tom Lane2018-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It failed if passed a nonexistent relation OID, or one that was a non-heap relation, because of blindly applying heap_open to a user-supplied OID. This is not OK behavior for a SQL-exposed function; we have a project policy that we should return NULL in such cases. Moreover, since pg_get_partition_constraintdef ought now to work on indexes, restricting it to heaps is flat wrong anyway. The underlying function generate_partition_qual() wasn't on board with indexes having partition quals either, nor for that matter with rels having relispartition set but yet null relpartbound. (One wonders whether the person who wrote the function comment blocks claiming that these functions allow a missing relpartbound had ever tested it.) Fix by testing relispartition before opening the rel, and by using relation_open not heap_open. (If any other relkinds ever grow the ability to have relispartition set, the code will work with them automatically.) Also, don't reject null relpartbound in generate_partition_qual. Back-patch to v11, and all but the null-relpartbound change to v10. (It's not really necessary to change generate_partition_qual at all in v10, but I thought s/heap_open/relation_open/ would be a good idea anyway just to keep the code in sync with later branches.) Per report from Justin Pryzby. Discussion: https://postgr.es/m/20180927200020.GJ776@telsasoft.com
* Recurse to sequences on ownership change for all relkindsPeter Eisentraut2018-09-26
| | | | | | | | | | | | | | When a table ownership is changed, we must apply that also to any owned sequences. (Otherwise, it would result in a situation that cannot be restored, because linked sequences must have the same owner as the table.) But this was previously only applied to regular tables and materialized views. But it should also apply to at least foreign tables. This patch removes the relkind check altogether, because it doesn't save very much and just introduces the possibility of similar omissions. Bug: #15238 Reported-by: Christoph Berg <christoph.berg@credativ.de>
* Rework activation of commit timestamps during recoveryMichael Paquier2018-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | The activation and deactivation of commit timestamp tracking has not been handled consistently for a primary or standbys at recovery. The facility can be activated at three different moments of recovery: - The beginning, where a primary would use the GUC value for the decision-making, and where a standby relies on the contents of the control file. - When replaying a XLOG_PARAMETER_CHANGE record at redo. - The end, where both primary and standby rely on the GUC value. Using the GUC value for a primary at the beginning of recovery causes problems with commit timestamp access when doing crash recovery. Particularly, when replaying transaction commits, it could be possible that an attempt to read commit timestamps is done for a transaction which committed at a moment when track_commit_timestamp was disabled. A test case is added to reproduce the failure. The test works down to v11 as it takes advantage of transaction commits within procedures. Reported-by: Hailong Li Author: Masahiko Sawasa, Michael Paquier Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/11224478-a782-203b-1f17-e4797b39bdf0@qunar.com Backpatch-through: 9.5, where commit timestamps have been introduced.
* Remove obsolete commentAlvaro Herrera2018-09-25
| | | | | The documented shortcoming was actually fixed in 4c728f3829 so the comment is not true anymore.
* Collect JIT instrumentation from workers.Andres Freund2018-09-25
| | | | | | | | | | | | | | | | Previously, when using parallel query, EXPLAIN (ANALYZE)'s JIT compilation timings did not include the overhead from doing so on the workers. Fix that. We do so by simply aggregating the cost of doing JIT compilation on workers and the leader together. Arguably that's not quite accurate, because the total time spend doing so is spent in parallel - but it's hard to do much better. For additional detail, when VERBOSE is specified, the stats for workers are displayed separately. Author: Amit Khandekar and Andres Freund Discussion: https://postgr.es/m/CAJ3gD9eLrz51RK_gTkod+71iDcjpB_N8eC6vU2AW-VicsAERpQ@mail.gmail.com Backpatch: 11-
* Make some fixes to allow building Postgres on macOS 10.14 ("Mojave").Tom Lane2018-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Apple's latest rearrangements of the system-supplied headers have broken building of PL/Perl and PL/Tcl. The only practical way to fix PL/Tcl is to start using the "-isysroot" compiler flag to point to SDK-supplied headers, as Apple expects. We must also start distinguishing where to find Perl's headers from where to find its shared library; but that seems like good cleanup anyway. Extensions that formerly did something like -I$(perl_archlibexp)/CORE should now do -I$(perl_includedir)/CORE instead. perl_archlibexp is still the place to look for libperl.so, though. If for some reason you don't like the default -isysroot setting, you can override that by setting PG_SYSROOT in configure's arguments. I don't currently think people would need to do so, unless maybe for cross-version build purposes. In addition, teach configure where to find tclConfig.sh. Our traditional method of searching $auto_path hasn't worked for the last couple of macOS releases, and it now seems clear that Apple's not going to change that. The workaround of manually specifying --with-tclconfig was annoying already, but Mojave's made it a lot more so because the sysroot path now has to be included as well. Let's just wire the knowledge into configure instead. To avoid breaking builds against non-default Tcl installations (e.g. MacPorts) wherein the $auto_path method probably still works, arrange to try the additional case only after all else has failed. Back-patch to all supported versions, since at least the buildfarm cares about that. The changes are set up to not do anything on macOS releases that are old enough to not have functional sysroot trees.
* Ignore publication tables when --no-publications is usedMichael Paquier2018-09-25
| | | | | | | | | | | 96e1cb4 has added support for --no-publications in pg_dump, pg_dumpall and pg_restore, but forgot the fact that publication tables also need to be ignored when this option is used. Author: Gilles Darold Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/3f48e812-b0fa-388e-2043-9a176bdee27e@dalibo.com Backpatch-through: 10, where publications have been added.
* Revoke pg_stat_statements_reset() permissionsMichael Paquier2018-09-25
| | | | | | | | | | | | Commit 25fff40 has granted execute permission of the function pg_stat_statements_reset() to default role "pg_read_all_stats", but this role is meant to read statistics, and not to reset them. The permissions on this function are revoked from "pg_read_all_stats". The version of pg_stat_statements is bumped up in consequence. Author: Haribabu Kommi Reviewed-by: Michael Paquier, Amit Kapila Discussion: https://postgr.es/m/CAJrrPGf5fCnKqXObpwGN9nMyD--tzOf-7LFCJiz59Z1wJ5qj9A@mail.gmail.com
* auto_explain: Include JIT information if applicable.Andres Freund2018-09-24
| | | | | | | | | | Due to my (Andres') omission auto_explain did not include information about JIT compilation. Fix that. Author: Lukas Fittl Discussion: https://postgr.es/m/CAP53PkzgSyoTCau0-5FNaM484B=uO8nLzma7L1ncWLb1=oVJQA@mail.gmail.com Backpatch: 11-, where JIT compilation was introduced
* Make EXPLAIN output for JIT compilation more dense.Andres Freund2018-09-24
| | | | | | | | | | | | | | | | | | A discussion about also reporting JIT compilation overhead on workers brought unhappiness with the verbosity of the current explain format to light. Make the text format more dense, and restructure the structured output to mirror that more closely. As we're re-jiggering the output format anyway: The denser format allows us to report all flags for JIT compilation (now also reporting PGJIT_EXPR and PGJIT_DEFORM), and report the total time in addition to the individual times. Per complaint from Tom Lane. Author: Andres Freund Discussion: https://postgr.es/m/27812.1537221015@sss.pgh.pa.us Backpatch: 11-, where JIT compilation was introduced
* Fast default trigger and expand_tuple fixesAndrew Dunstan2018-09-24
| | | | | | | | | | | | | | Ensure that triggers get properly filled in tuples for the OLD value. Also fix the logic of detecting missing null values. The previous logic failed to detect a missing null column before the first missing column with a default. Fixing this has simplified the logic a bit. Regression tests are added to test changes. This should ensure better coverage of expand_tuple(). Original bug reports, and some code and test scripts from Tomas Vondra Backpatch to release 11.
* Fix over-allocation of space for array_out()'s result string.Tom Lane2018-09-24
| | | | | | | | | | | | | | | | | | | | | | | | array_out overestimated the space needed for its output, possibly by a very substantial amount if the array is multi-dimensional, because of wrong order of operations in the loop that counts the number of curly-brace pairs needed. While the output string is normally short-lived, this could still cause problems in extreme cases. An additional minor error was that it counted one more delimiter than is actually needed. Repair those errors, add an Assert that the space is now correctly calculated, and make some minor improvements in the comments. I also failed to resist the temptation to get rid of an integer modulus operation per array element; a simple comparison is sufficient. This bug dates clear back to Berkeley days, so back-patch to all supported versions. Keiichi Hirobe, minor additional work by me Discussion: https://postgr.es/m/CAH=EFxE9W0tRvQkixR2XJRRCToUYUEDkJZk6tnADXugPBRdcdg@mail.gmail.com
* Initialize random() in bootstrap/stand-alone postgres and in initdb.Noah Misch2018-09-23
| | | | | | | | | | | | | | | | | | | | | | This removes a difference between the standard IsUnderPostmaster execution environment and that of --boot and --single. In a stand-alone backend, "SELECT random()" always started at the same seed. On a system capable of using posix shared memory, initdb could still conclude "selecting dynamic shared memory implementation ... sysv". Crashed --boot or --single postgres processes orphaned shared memory objects having names that collided with the not-actually-random names that initdb probed. The sysv fallback appeared after ten crashes of --boot or --single postgres. Since --boot and --single are rare in production use, systems used for PostgreSQL development are the principal candidate to notice this symptom. Back-patch to 9.3 (all supported versions). PostgreSQL 9.4 introduced dynamic shared memory, but 9.3 does share the "SELECT random()" problem. Reviewed by Tom Lane and Kyotaro HORIGUCHI. Discussion: https://postgr.es/m/20180915221546.GA3159382@rfd.leadboat.com
* Doc: warn against using parallel restore with --load-via-partition-root.Tom Lane2018-09-23
| | | | | | | This isn't terribly safe, and making it so doesn't seem like a small project, so for the moment just warn against it. Discussion: https://postgr.es/m/13624.1535486019@sss.pgh.pa.us
* Fix failure in WHERE CURRENT OF after rewinding the referenced cursor.Tom Lane2018-09-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In a case where we have multiple relation-scan nodes in a cursor plan, such as a scan of an inheritance tree, it's possible to fetch from a given scan node, then rewind the cursor and fetch some row from an earlier scan node. In such a case, execCurrent.c mistakenly thought that the later scan node was still active, because ExecReScan hadn't done anything to make it look not-active. We'd get some sort of failure in the case of a SeqScan node, because the node's scan tuple slot would be pointing at a HeapTuple whose t_self gets reset to invalid by heapam.c. But it seems possible that for other relation scan node types we'd actually return a valid tuple TID to the caller, resulting in updating or deleting a tuple that shouldn't have been considered current. To fix, forcibly clear the ScanTupleSlot in ExecScanReScan. Another issue here, which seems only latent at the moment but could easily become a live bug in future, is that rewinding a cursor does not necessarily lead to *immediately* applying ExecReScan to every scan-level node in the plan tree. Upper-level nodes will think that they can postpone that call if their child node is already marked with chgParam flags. I don't see a way for that to happen today in a plan tree that's simple enough for execCurrent.c's search_plan_tree to understand, but that's one heck of a fragile assumption. So, add some logic in search_plan_tree to detect chgParam flags being set on nodes that it descended to/through, and assume that that means we should consider lower scan nodes to be logically reset even if their ReScan call hasn't actually happened yet. Per bug #15395 from Matvey Arye. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/153764171023.14986.280404050547008575@wrigleys.postgresql.org
* docs: remove use of escape strings and use bytea hex outputBruce Momjian2018-09-21
| | | | | | | | | | | standard_conforming_strings defaulted to 'on' in PG 9.1. bytea_output defaulted to 'hex' in PG 9.0. Reported-by: André Hänsel Discussion: https://postgr.es/m/12e601d447ac$345994a0$9d0cbde0$@webkr.de Backpatch-through: 9.3
* Fix bogus tab-completion rule for CREATE PUBLICATION.Tom Lane2018-09-21
| | | | | | | | | | | | | | You can't use "FOR TABLE" as a single Matches argument, because readline will consider that input to be two words not one. It's necessary to make the pattern contain two arguments. The case accidentally worked anyway because the words_after_create test fired ... but only for the first such table name. Noted by Edmund Horner, though this isn't exactly his proposed fix. Backpatch to v10 where the faulty code came in. Discussion: https://postgr.es/m/CAMyN-kDe=gBmHgxWwUUaXuwK+p+7g1vChR7foPHRDLE592nJPQ@mail.gmail.com
* Use size_t consistently in dsa.{ch}.Thomas Munro2018-09-22
| | | | | | | | Takeshi Ideriha complained that there is a mixture of Size and size_t in dsa.c and corresponding header. Let's use size_t. Back-patch to 10 where dsa.c landed, to make future back-patching easy. Discussion: https://postgr.es/m/4E72940DA2BF16479384A86D54D0988A6F19ABD9%40G01JPEXMBKW04
* Document lock taken on referenced table when adding a foreign keyMichael Paquier2018-09-21
| | | | | | | | This can happen for CREATE TABLE and ALTER TABLE, so a mention is added to both of them in the concerned subsections. Author: Adrien Nayrat Discussion: https://postgr.es/m/c4e8af11-1dfc-766a-c953-76979b9fcdaa@anayrat.info
* Error out for clang on x86-32 without SSE2 support, no -fexcess-precision.Andres Freund2018-09-20
| | | | | | | | | | | | | | | | | | | | | | | As clang currently doesn't support -fexcess-precision=standard, compiling x86-32 code with SSE2 disabled, can lead to problems with floating point overflow checks and the like. This issue was noticed because clang, on at least some BSDs, defaults to i386 compatibility, whereas it defaults to pentium4 on Linux. Our forced usage of __builtin_isinf() lead to some overflow checks not triggering when compiling for i386, e.g. when the result of the calculation didn't overflow in 80bit registers, but did so in 64bit. While we could just fall back to a non-builtin isinf, it seems likely that the use of 80bit registers leads to other problems (which is why we force the flag for GCC already). Therefore error out when detecting clang in that situation. Reported-By: Victor Wagner Analyzed-By: Andrew Gierth and Andres Freund Author: Andres Freund Discussion: https://postgr.es/m/20180905005130.ewk4xcs5dgyzcy45@alap3.anarazel.de Backpatch: 9.3-, all supported versions are affected
* Fix segment_bins corruption in dsa.c.Thomas Munro2018-09-20
| | | | | | | | | | | | | | | | If a segment has been freed by dsa.c because it is entirely empty, other backends must make sure to unmap it before following links to new segments that might happen to have the same index number, or they could finish up looking at a defunct segment and then corrupt the segment_bins lists. The correct protocol requires checking freed_segment_counter after acquiring the area lock and before resolving any index number to a segment. Add the missing checks and an assertion. Back-patch to 10, where dsa.c first arrived. Author: Thomas Munro Reported-by: Tomas Vondra Discussion: https://postgr.es/m/CAEepm%3D0thg%2Bja5zGVa7jBy-uqyHrTqTm8HGhEOtMmigGrAqTbw%40mail.gmail.com
* Defer restoration of libraries in parallel workers.Thomas Munro2018-09-20
| | | | | | | | | | | | | | | | | | | | | | Several users of extensions complained of crashes in parallel workers that turned out to be due to syscache access from their _PG_init() functions. Reorder the initialization of parallel workers so that libraries are restored after the caches are initialized, and inside a transaction. This was reported in bug #15350 and elsewhere. We don't consider it to be a bug: extensions shouldn't do that, because then they can't be used in shared_preload_libraries. However, it's a fairly obscure hazard and these extensions worked in practice before parallel query came along. So let's make it work. Later commits might add a warning message and eventually an error. Back-patch to 9.6, where parallel query landed. Author: Thomas Munro Reviewed-by: Amit Kapila Reported-by: Kieran McCusker, Jimmy Discussion: https://postgr.es/m/153512195228.1489.8545997741965926448%40wrigleys.postgresql.org
* Fix minor error message style guide violation.Tom Lane2018-09-19
| | | | | | | | No periods at the ends of primary error messages, please. Daniel Gustafsson Discussion: https://postgr.es/m/43E004C0-18C6-42B4-A313-003B43EB0571@yesql.se
* Don't ignore locktable-full failures in StandbyAcquireAccessExclusiveLock.Tom Lane2018-09-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 37c54863c removed the code in StandbyAcquireAccessExclusiveLock that checked the return value of LockAcquireExtended. That created a bug, because it's still passing reportMemoryError = false to LockAcquireExtended, meaning that LOCKACQUIRE_NOT_AVAIL will be returned if we're out of shared memory for the lock table. In such a situation, the startup process would believe it had acquired an exclusive lock even though it hadn't, with potentially dire consequences. To fix, just drop the use of reportMemoryError = false, which allows us to simplify the call into a plain LockAcquire(). It's unclear that the locktable-full situation arises often enough that it's worth having a better recovery method than crash-and-restart. (I strongly suspect that the only reason the code path existed at all was that it was relatively simple to do in the pre-37c54863c implementation. But now it's not.) LockAcquireExtended's reportMemoryError parameter is now dead code and could be removed. I refrained from doing so, however, because there was some interest in resurrecting the behavior if we do get reports of locktable-full failures in the field. Also, it seems unwise to remove the parameter concurrently with shipping commit f868a8143, which added a parameter; if there are any third-party callers of LockAcquireExtended, we want them to get a wrong-number-of-parameters compile error rather than a possibly-silent misinterpretation of its last parameter. Back-patch to 9.6 where the bug was introduced. Discussion: https://postgr.es/m/6202.1536359835@sss.pgh.pa.us
* Revert "Allow concurrent-safe open() and fopen() in frontend code for Windows"REL_11_BETA4Tom Lane2018-09-18
| | | | | | | | | | | | | | | | This reverts commit f02259fe93e75d5443a2fabe2f2f38b81924ab36, in the v11 branch only. The hack this required in initdb.c should probably have clued us that it wasn't really ready, but we didn't get the hint. Subsequent developments have made clear that it affected text-vs-binary behavior in a lot of places, and there's no reason to think that any of those behavioral changes are desirable. There's no time to fix this before 11beta4, so just revert for the moment. We can keep working on this in HEAD, and maybe reconsider a back-patch once we're satisfied things are stable. (I take the blame for this fiasco, having encouraged Michael to back-patch a change at the last possible moment before beta wrap.)
* Fix some probably-minor oversights in readfuncs.c.Tom Lane2018-09-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The system expects TABLEFUNC RTEs to have coltypes, coltypmods, and colcollations lists, but outfuncs doesn't dump them and readfuncs doesn't restore them. This doesn't cause obvious failures, because the only things that look at those fields are expandRTE() and get_rte_attribute_type(), which are mostly used during parse analysis, before anything would've passed the parsetree through outfuncs/readfuncs. But expandRTE() is used in build_physical_tlist(), which means that that function will return a wrong answer for a TABLEFUNC RTE that came from a view. Very accidentally, this doesn't cause serious problems, because what it will return is NIL which callers will interpret as "couldn't build a physical tlist because of dropped columns". So you still get a plan that works, though it's marginally less efficient than it could be. There are also some other expandRTE() calls associated with transformation of whole-row Vars in the planner. I have been unable to exhibit misbehavior from that, and it may be unreachable in any case that anyone would care about ... but I'm not entirely convinced, so this seems like something we should back- patch a fix for. Fortunately, we can fix it without forcing a change of stored rules and a catversion bump, because we can just copy these lists from the subsidiary TableFunc object. readfuncs.c was also missing support for NamedTuplestoreScan plan nodes. This accidentally fails to break parallel query because a query using a named tuplestore would never be considered parallel-safe anyway. However, project policy since parallel query came in is that all plan node types should have outfuncs/readfuncs support, so this is clearly an oversight that should be repaired. Noted while fooling around with a patch to test outfuncs/readfuncs more thoroughly. That exposed some other issues too, but these are the only ones that seem worth back-patching. Back-patch to v10 where both of these features came in. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us
* Allow DSM allocation to be interrupted.Thomas Munro2018-09-18
| | | | | | | | | | | | | | | | Chris Travers reported that the startup process can repeatedly try to cancel a backend that is in a posix_fallocate()/EINTR loop and cause it to loop forever. Teach the retry loop to give up if an interrupt is pending. Don't actually check for interrupts in that loop though, because a non-local exit would skip some clean-up code in the caller. Back-patch to 9.4 where DSM was added (and posix_fallocate() was later back-patched). Author: Chris Travers Reviewed-by: Ildar Musin, Murat Kabilov, Oleksii Kliukin Tested-by: Oleksii Kliukin Discussion: https://postgr.es/m/CAN-RpxB-oeZve_J3SM_6%3DHXPmvEG%3DHX%2B9V9pi8g2YR7YW0rBBg%40mail.gmail.com
* Stamp 11beta4.Tom Lane2018-09-17
|
* Fix parsetree representation of XMLTABLE(XMLNAMESPACES(DEFAULT ...)).Tom Lane2018-09-17
| | | | | | | | | | | | | | | | | | | | | | | The original coding for XMLTABLE thought it could represent a default namespace by a T_String Value node with a null string pointer. That's not okay, though; in particular outfuncs.c/readfuncs.c are not on board with such a representation, meaning you'll get a null pointer crash if you try to store a view or rule containing this construct. To fix, change the parsetree representation so that we have a NULL list element, instead of a bogus Value node. This isn't really a functional limitation since default XML namespaces aren't yet implemented in the executor; you'd just get "DEFAULT namespace is not supported" anyway. But crashes are not nice, so back-patch to v10 where this syntax was added. Ordinarily we'd consider a parsetree representation change to be un-backpatchable; but since existing releases would crash on the way to storing such constructs, there can't be any existing views/rules to be incompatible with. Per report from Andrey Lepikhov. Discussion: https://postgr.es/m/3690074f-abd2-56a9-144a-aa5545d7a291@postgrespro.ru
* Fix pgbench lexer's "continuation" rule to cope with Windows newlines.Tom Lane2018-09-17
| | | | | | | | | | | | | | Our general practice in frontend code is to accept input with either Unix-style newlines (\n) or DOS-style (\r\n). pgbench was mostly down with that, but its rule for line continuations (backslash-newline) was not. This had been masked on Windows buildfarm machines before commit 0ba06e0bf by use of Windows text mode to read files. We could have fixed it by forcing text mode again, but it's better to fix the parsing code so that Windows-style text files on Unix systems don't cause problems. Back-patch to v10 where pgbench grew line continuations. Discussion: https://postgr.es/m/17194.1537191697@sss.pgh.pa.us
* Allow concurrent-safe open() and fopen() in frontend code for WindowsMichael Paquier2018-09-17
| | | | | | | | | | | | | | | | | | | | 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
* Translation updatesPeter Eisentraut2018-09-17
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: be9925199917aac824dd4b472bdce3b97dbc90ca
* Add list of acknowledgments to release notesPeter Eisentraut2018-09-16
| | | | | | | This contains all individuals mentioned in the commit messages during PostgreSQL 11 development. current through 7a2f70f0e5e83871d091ee479abea4b8f850dd29
* Fix out-of-tree build for transform modules.Andrew Gierth2018-09-16
| | | | | | | | | | | | | | | | | | Neither plperl nor plpython installed sufficient header files to permit transform modules to be built out-of-tree using PGXS. Fix that by installing all plperl and plpython header files (other than those with special purposes such as generated data tables), and also install plpython's special .mk file for mangling regression tests. (This commit does not fix the windows install, which does not currently install _any_ plperl or plpython headers.) Also fix the existing transform modules for hstore and ltree so that their cross-module #include directives work as anticipated by commit df163230b9 et seq. This allows them to serve as working examples of how to reference other modules when doing separate out-of-tree builds. Discussion: https://postgr.es/m/87o9ej8bgl.fsf%40news-spur.riddles.org.uk
* Add outfuncs.c support for RawStmt nodes.Tom Lane2018-09-16
| | | | | | | | | | | | | | | I noticed while poking at a report from Andrey Lepikhov that the recent addition of RawStmt nodes at the top of raw parse trees makes it impossible to print any raw parse trees whatsoever, because outfuncs.c doesn't know RawStmt and hence fails to descend into it. While we generally lack outfuncs.c support for utility statements, there is reasonably complete support for what you can find in a raw SELECT statement. It was not my intention to make that all dead code ... so let's add support for RawStmt. Back-patch to v10 where RawStmt appeared.
* doc: clarify pg_basebackup's -C/--create-slot descriptionBruce Momjian2018-09-16
| | | | | | The previous text was overly complex. Backpatch-through: 11
* In v11, disable JIT by default (it's still enabled by default in HEAD).Tom Lane2018-09-15
| | | | | | | | | | | Per discussion, JIT isn't quite mature enough to ship enabled-by-default. I failed to resist the temptation to do a bunch of copy-editing on the related documentation. Also, clean up some inconsistencies in which section of config.sgml the JIT GUCs are documented in vs. what guc.c and postgresql.config.sample had. Discussion: https://postgr.es/m/20180914222657.mw25esrzbcnu6qlu@alap3.anarazel.de
* Fix failure with initplans used conditionally during EvalPlanQual rechecks.Tom Lane2018-09-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The EvalPlanQual machinery assumes that any initplans (that is, uncorrelated sub-selects) used during an EPQ recheck would have already been evaluated during the main query; this is implicit in the fact that execPlan pointers are not copied into the EPQ estate's es_param_exec_vals. But it's possible for that assumption to fail, if the initplan is only reached conditionally. For example, a sub-select inside a CASE expression could be reached during a recheck when it had not been previously, if the CASE test depends on a column that was just updated. This bug is old, appearing to date back to my rewrite of EvalPlanQual in commit 9f2ee8f28, but was not detected until Kyle Samson reported a case. To fix, force all not-yet-evaluated initplans used within the EPQ plan subtree to be evaluated at the start of the recheck, before entering the EPQ environment. This could be inefficient, if such an initplan is expensive and goes unused again during the recheck --- but that's piling one layer of improbability atop another. It doesn't seem worth adding more complexity to prevent that, at least not in the back branches. It was convenient to use the new-in-v11 ExecEvalParamExecParams function to implement this, but I didn't like either its name or the specifics of its API, so revise that. Back-patch all the way. Rather than rewrite the patch to avoid depending on bms_next_member() in the oldest branches, I chose to back-patch that function into 9.4 and 9.3. (This isn't the first time back-patches have needed that, and it exhausted my patience.) I also chose to back-patch some test cases added by commits 71404af2a and 342a1ffa2 into 9.4 and 9.3, so that the 9.x versions of eval-plan-qual.spec are all the same. Andrew Gierth diagnosed the problem and contributed the added test cases, though the actual code changes are by me. Discussion: https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com