aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* docs: remove reference to src/tools/threadBruce Momjian2020-10-27
| | | | | | | | | | | This directory and the ability to build the thread test independently were removed in commit 8a2121185b. Reported-by: e.indrupskaya@postgrespro.ru Discussion: https://postgr.es/m/160379609706.24746.7506163279454026608@wrigleys.postgresql.org Backpatch-through: 9.5
* doc: simplify wording of function error affectsBruce Momjian2020-10-26
| | | | | | | | Reported-by: bob.henkel@gmail.com Discussion: https://postgr.es/m/160324449781.693.8298142858847611071@wrigleys.postgresql.org Backpatch-through: 9.5
* doc: make blooms docs match realityBruce Momjian2020-10-26
| | | | | | | | | | | | | | Parallel execution changed the way bloom queries are executed, so update the EXPLAIN output, and restructure the docs to be clearer and more accurate. Reported-by: Daniel Westermann Discussion: https://postgr.es/m/ZR0P278MB0122119FAE78721A694C30C8D2340@ZR0P278MB0122.CHEP278.PROD.OUTLOOK.COM Author: Daniel Westermann and me Backpatch-through: 9.6
* Fix corner case for a BEFORE ROW UPDATE trigger returning OLD.Tom Lane2020-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | If the old row has any "missing" attributes that are supposed to be retrieved from an associated tuple descriptor, the wrong things happened because the trigger result is shoved directly into an executor slot that lacks the missing-attribute data. Notably, CHECK-constraint verification would incorrectly see those columns as NULL, and so would RETURNING-list evaluation. Band-aid around this by forcibly expanding the tuple before passing it to the trigger function. (IMO it was a fundamental misdesign to put the missing-attribute data into tuple constraints, which so much of the system considers to be optional. But we're probably stuck with that now, and will have to continue to apply band-aids as we find other places with similar issues.) Back-patch to v12. v11 would also have the issue, except that commit 920311ab1 already applied a similar band-aid. That forced expansion in more cases than seem really necessary, though, so this isn't a directly equivalent fix. Amit Langote, with some cosmetic changes by me Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
* Fix incorrect parameter name in a function header commentDavid Rowley2020-10-25
| | | | | | Author: Zhijie Hou Discussion: https://postgr.es/m/14cd74ea00204cc8a7ea5d738ac82cd1@G08CNEXMBPEKD05.g08.fujitsu.local Backpatch-through: 12, where the mistake was introduced
* Fix ancient bug in ecpg's pthread_once() emulation for Windows.Tom Lane2020-10-24
| | | | | | | | | | | | | | | | We must not set the "done" flag until after we've executed the initialization function. Otherwise, other threads can fall through the initial unlocked test before initialization is really complete. This has been seen to cause rare failures of ecpg's thread/descriptor test, and it could presumably cause other sorts of misbehavior in threaded ECPG-using applications, since ecpglib relies on pthread_once() in several places. Diagnosis and patch by me, based on investigation by Alexander Lakhin. Back-patch to all supported branches (the bug dates to 2007). Discussion: https://postgr.es/m/16685-d6cd241872c101d3@postgresql.org
* Fix broken XML formatting in EXPLAIN output for incremental sorts.Tom Lane2020-10-23
| | | | | | | | | | The ExplainCloseGroup arguments for incremental sort usage data didn't match the corresponding ExplainOpenGroup. This only matters for XML-format output, which is probably why we'd not noticed. Daniel Gustafsson, per bug #16683 from Frits Jalvingh Discussion: https://postgr.es/m/16683-8005033324ad34e9@postgresql.org
* Update time zone data files to tzdata release 2020d.Tom Lane2020-10-22
| | | | | DST law changes in Palestine, with a whopping 120 hours' notice. Also some historical corrections for Palestine.
* Sync our copy of the timezone library with IANA release tzcode2020d.Tom Lane2020-10-22
| | | | | | | | There's no functional change at all here, but I'm curious to see whether this change successfully shuts up Coverity's warning about a useless strcmp(), which appeared with the previous update. Discussion: http://mm.icann.org/pipermail/tz/2020-October/029370.html
* Fix connection string handling in psql's \connect command.Tom Lane2020-10-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | psql's \connect claims to be able to re-use previous connection parameters, but in fact it only re-uses the database name, user name, host name (and possibly hostaddr, depending on version), and port. This is problematic for assorted use cases. Notably, pg_dump[all] emits "\connect databasename" commands which we would like to have re-use all other parameters. If such a script is loaded in a psql run that initially had "-d connstring" with some non-default parameters, those other parameters would be lost, potentially causing connection failure. (Thus, this is the same kind of bug addressed in commits a45bc8a4f and 8e5793ab6, although the details are much different.) To fix, redesign do_connect() so that it pulls out all properties of the old PGconn using PQconninfo(), and then replaces individual properties in that array. In the case where we don't wish to re-use anything, get libpq's default settings using PQconndefaults() and replace entries in that, so that we don't need different code paths for the two cases. This does result in an additional behavioral change for cases where the original connection parameters allowed multiple hosts, say "psql -h host1,host2", and the \connect request allows re-use of the host setting. Because the previous coding relied on PQhost(), it would only permit reconnection to the same host originally selected. Although one can think of scenarios where that's a good thing, there are others where it is not. Moreover, that behavior doesn't seem to meet the principle of least surprise, nor was it documented; nor is it even clear it was intended, since that coding long pre-dates the addition of multi-host support to libpq. Hence, this patch is content to drop it and re-use the host list as given. Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Use fast checkpoint in PostgresNode::backup()Alvaro Herrera2020-10-21
| | | | Should cause tests to be a bit faster
* Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursionAlvaro Herrera2020-10-20
| | | | | | | | | | | | | | | | More precisely, correctly handle the ONLY flag indicating not to recurse. This was implemented in 86f575948c77 by recursing in trigger.c, but that's the wrong place; use ATSimpleRecursion instead, which behaves properly. However, because legacy inheritance has never recursed in that situation, make sure to do that only for new-style partitioning. I noticed this problem while testing a fix for another bug in the vicinity. This has been wrong all along, so backpatch to 11. Discussion: https://postgr.es/m/20201016235925.GA29829@alvherre.pgsql
* Avoid invalid alloc size error in shm_mqPeter Eisentraut2020-10-20
| | | | | | | | | | | | In shm_mq_receive(), a huge payload could trigger an unjustified "invalid memory alloc request size" error due to the way the buffer size is increased. Add error checks (documenting the upper limit) and avoid the error by limiting the allocation size to MaxAllocSize. Author: Markus Wanner <markus.wanner@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/3bb363e7-ac04-0ac4-9fe8-db1148755bfa%402ndquadrant.com
* Fix typo in commit 99ae342fc4.Amit Kapila2020-10-20
| | | | | | | | In v13, the id for max_parallel_maintenance_workers is defined differently as compared to HEAD in docs, so adjust the docs accordingly. Reported-by: Magnus Hagander and Tom Lane Discussion: https://postgr.es/m/CABUevEyAFQZ_jvjY_KtRUWbci4YMyQC1QAMzDQAbLs=XCo3m5Q@mail.gmail.com
* Fix connection string handling in src/bin/scripts/ programs.Tom Lane2020-10-19
| | | | | | | | | | | | | | | | | | | | | When told to process all databases, clusterdb, reindexdb, and vacuumdb would reconnect by replacing their --maintenance-db parameter with the name of the target database. If that parameter is a connstring (which has been allowed for a long time, though we failed to document that before this patch), we'd lose any other options it might specify, for example SSL or GSS parameters, possibly resulting in failure to connect. Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and pg_restore. We can fix it in the same way, by using libpq's rules for handling multiple "dbname" parameters to add the target database name separately. I chose to apply the same refactoring approach as in that patch, with a struct to handle the command line parameters that need to be passed through to connectDatabase. (Maybe someday we can unify the very similar functions here and in pg_dump/pg_restore.) Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Fix list-munging bug that broke SQL function result coercions.Tom Lane2020-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 913bbd88d, check_sql_fn_retval() can either insert type coercion steps in-line in the Query that produces the SQL function's results, or generate a new top-level Query to perform the coercions, if modifying the Query's output in-place wouldn't be safe. However, it appears that the latter case has never actually worked, because the code tried to inject the new Query back into the query list it was passed ... which is not the list that will be used for later processing when we execute the SQL function "normally" (without inlining it). So we ended up with no coercion happening at run-time, leading to wrong results or crashes depending on the datatypes involved. While the regression tests look like they cover this area well enough, through a huge bit of bad luck all the test cases that exercise the separate-Query path were checking either inline-able cases (which accidentally didn't have the bug) or cases that are no-ops at runtime (e.g., varchar to text), so that the failure to perform the coercion wasn't obvious. The fact that the cases that don't work weren't allowed at all before v13 probably contributed to not noticing the problem sooner, too. To fix, get rid of the separate "flat" list of Query nodes and instead pass the real two-level list that is going to be used later. I chose to make the same change in check_sql_fn_statements(), although that has no actual bug, just so that we don't need that data structure at all. This is an API change, as evidenced by the adjustments needed to callers outside functions.c. That's a bit scary to be doing in a released branch, but so far as I can tell from a quick search, there are no outside callers of these functions (and they are sufficiently specific to our semantics for SQL-language functions that it's not apparent why any extension would need to call them). In any case, v13 already changed the API of check_sql_fn_retval() compared to prior branches. Per report from pinker. Back-patch to v13 where this code came in. Discussion: https://postgr.es/m/1603050466566-0.post@n3.nabble.com
* Misc documentation fixes.Heikki Linnakangas2020-10-19
| | | | | | | | | | | | | | | | | | | - Misc grammar and punctuation fixes. - Stylistic cleanup: use spaces between function arguments and JSON fields in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after last END in a few PL/pgSQL examples that were missing them. - Make sentence that talked about "..." and ".." operators more clear, by avoiding to end the sentence with "..". That makes it look the same as "..." - Fix syntax description for HAVING: HAVING conditions cannot be repeated Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all supported versions, to the extent that the patch applies easily. Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
* Fix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional.Heikki Linnakangas2020-10-19
| | | | | | | | | ALTER SEQUENCE RESTART was made transactional in commit 3d79013b97. Backpatch to v10, where that was introduced. Patch by Justin Pryzby, per Yaroslav Schekin's report. Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
* Fix output of tsquery example in docs.Heikki Linnakangas2020-10-19
| | | | | | | | | The output for this query changed in commit 4e2477b7b8. Backport to 9.6 like that commit. Patch by Justin Pryzby, per Yaroslav Schekin's report. Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
* In libpq for Windows, call WSAStartup once and WSACleanup not at all.Tom Lane2020-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Windows documentation insists that every WSAStartup call should have a matching WSACleanup call. However, if that ever had actual relevance, it wasn't in this century. Every remotely-modern Windows kernel is capable of cleaning up when a process exits without doing that, and must be so to avoid resource leaks in case of a process crash. Moreover, Postgres backends have done WSAStartup without WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any indication of a problem with that. libpq's habit of doing WSAStartup during connection start and WSACleanup during shutdown is also rather inefficient, since a series of non-overlapping connection requests leads to repeated, quite expensive DLL unload/reload cycles. We document a workaround for that (having the application call WSAStartup for itself), but that's just a kluge. It's also worth noting that it's far from uncommon for applications to exit without doing PQfinish, and we've not heard reports of trouble from that either. However, the real reason for acting on this is that recent experiments by Alexander Lakhin show that calling WSACleanup during PQfinish is triggering the symptom we occasionally see that a process using libpq fails to emit expected stdio output. Therefore, let's change libpq so that it calls WSAStartup only once per process, during the first connection attempt, and never calls WSACleanup at all. While at it, get rid of the only other WSACleanup call in our code tree, in pg_dump/parallel.c; that presumably is equally useless. Back-patch of HEAD commit 7d00a6b2d. Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
* Fix doc for full text search distance operator.Heikki Linnakangas2020-10-19
| | | | | | | | | Commit 028350f619 changed its behavior from "at most" to "exactly", but forgot to update the documentation. Backpatch to 9.6. Patch by Justin Pryzby, per Yaroslav Schekin's report. Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
* Update link for plluaMagnus Hagander2020-10-19
| | | | | Author: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/A05874AE-8771-4C61-A24E-0B6249B8F3C2@yesql.se
* Relax some asserts in merge join costing codeDavid Rowley2020-10-20
| | | | | | | | | | | | | | | | | | | | | | | | In the planner, it was possible, given an extreme enough case containing a large number of joins for the number of estimated rows to become infinite. This could cause problems in initial_cost_mergejoin() where we perform some calculations based on those row estimates. A problem case, presented by Onder Kalaci showed an Assert failure from an Assert checking outerstartsel <= outerendsel. In his test case this was effectively NaN <= Inf, which is false. The NaN outerstartsel came from multiplying the infinite outer_path_rows by 0.0. In master, this problem was fixed by a90c950fc, however, that fix was too invasive for the backbranches. Here we just relax the Asserts to allow them to pass. The worst that appears to happen from this is that we show NaN cost values and infinite row estimates in EXPLAIN. add_path() would have had a hard time doing anything useful with such costs, but that does not really matter as if the row estimates were even close to accurate, such plan would not complete this side of the heat death of the universe. Reported-by: Onder Kalaci Backpatch: 9.5 to 13 Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
* Change the docs for PARALLEL option of Vacuum.Amit Kapila2020-10-19
| | | | | | | | | | The rules to choose the number of parallel workers to perform parallel vacuum operation were not clearly specified. Reported-by: Peter Eisentraut Author: Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/36aa8aea-61b7-eb3c-263b-648e0cb117b7@2ndquadrant.com
* Fix potential memory leak in pgcryptoMichael Paquier2020-10-19
| | | | | | | | | | | | | | | | | | | | | When allocating a EVP context, it would have been possible to leak some memory allocated directly by OpenSSL, that PostgreSQL lost track of if the initialization of the context allocated failed. The cleanup can be done with EVP_MD_CTX_destroy(). Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree equivalent implementations for older versions since ce9b75d (code removed with 9b7cd59a as of 10~). However, in 9.5 and 9.6, the existing code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without an equivalent implementation when building the tree with OpenSSL 0.9.6 or older, meaning that this code is in reality broken with such versions since it got introduced in e2838c5. As we have heard no complains about that, it does not seem worth bothering with in 9.5 and 9.6, so I have left that out for simplicity. Author: Michael Paquier Discussion: https://postgr.es/m/20201015072212.GC2305@paquier.xyz Backpatch-through: 9.5
* Doc: caution against misuse of 'now' and related datetime literals.Tom Lane2020-10-17
| | | | | | | | | | | | | | Section 8.5.1.4, which defines these literals, made only a vague reference to the fact that they might be evaluated too soon to be safe in non-interactive contexts. Provide a more explicit caution against misuse. Also, generalize the wording in the related tip in section 9.9.4: while it clearly described this problem, it implied (or really, stated outright) that the problem only applies to table DEFAULT clauses. Per gripe from Tijs van Dam. Back-patch to all supported branches. Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com
* Update time zone data files to tzdata release 2020c.Tom Lane2020-10-16
| | | | | | DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island, Casey Station (Antarctica). Historical corrections for France, Hungary, Monaco.
* Sync our copy of the timezone library with IANA release tzcode2020c.Tom Lane2020-10-16
| | | | | | | | | | | | | | | This changes zic's default output format from "-b fat" to "-b slim". We were already using "slim" in v13/HEAD, so those branches drop the explicit -b switch in the Makefiles. Instead, add an explicit "-b fat" in v12 and before, so that we don't change the output file format in those branches. (This is perhaps excessively conservative, but we decided not to do so in a12079109, and I'll stick with that.) Other non-cosmetic changes are to drop support for zic's long-obsolete "-y" switch, and to ensure that strftime() does not change errno unless it fails. As usual with tzcode changes, back-patch to all supported branches.
* Add missing error check in pgcrypto/crypt-md5.c.Tom Lane2020-10-16
| | | | | | | | | | | | | | | In theory, the second px_find_digest call in px_crypt_md5 could fail even though the first one succeeded, since resource allocation is required. Don't skip testing for a failure. (If one did happen, the likely result would be a crash rather than clean recovery from an OOM failure.) The code's been like this all along, so back-patch to all supported branches. Daniel Gustafsson Discussion: https://postgr.es/m/AA8D6FE9-4AB2-41B4-98CB-AE64BA668C03@yesql.se
* Doc: tweak column widths in synchronous-commit-matrix table.Tom Lane2020-10-16
| | | | | | | | | | | | Commit a97e85f2b caused "exceed the available area" warnings in PDF builds. Fine-tune colwidth values to avoid that. Back-patch to 9.6, like the prior patch. (This is of dubious value before v13, since we were far from free of such warnings in older branches. But we might as well keep the SGML looking the same in all branches.) Per buildfarm.
* llvmjit: Work around bug in LLVM 3.9 causing crashes after 72559438f92.Andres Freund2020-10-15
| | | | | | | | | | | | | | Unfortunately in LLVM 3.9 LLVMGetAttributeCountAtIndex(func, index) crashes when called with an index that has 0 attributes. Since there's no way to work around this in the C API, add a small C++ wrapper doing so. The only reason this didn't fail before 72559438f92 is that there always are function attributes... Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20201016001254.w2nfj7gd74jmb5in@alap3.anarazel.de Backpatch: 11-, like 72559438f92
* pg_upgrade: remove C99 compiler req. from commit 3c0471b5fdBruce Momjian2020-10-15
| | | | | | | | | | | | | | This commit required support for inline variable definition, which is not a requirement. RELEASE NOTE AUTHOR: the author of commit 3c0471b5fd (pg_upgrade/tablespaces) was Justin Pryzby, not me. Reported-by: Andres Freund Discussion: https://postgr.es/m/20201016001959.h24fkywfubkv2pc5@alap3.anarazel.de Backpatch-through: 9.5
* pg_upgrade: generate check error for left-over new tablespaceBruce Momjian2020-10-15
| | | | | | | | | | | | | Previously, if pg_upgrade failed, and the user recreated the cluster but did not remove the new cluster tablespace directory, a later pg_upgrade would fail since the new tablespace directory would already exists. This adds error reporting for this during check. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200925005531.GJ23631@telsasoft.com Backpatch-through: 9.5
* llvmjit: Also copy parameter / return value attributes from template functions.Andres Freund2020-10-15
| | | | | | | | | | | | | | | | | | | | | Previously we only copied the function attributes. That caused problems at least on s390x: Because we didn't copy the 'zeroext' attribute for ExecAggTransReparent()'s *IsNull parameters, expressions invoking it didn't ensure that the upper bytes of the registers were zeroed. In the - relatively rare - cases where not, ExecAggTransReparent() wrongly ended up in the newValueIsNull branch due to the register not being zero. Subsequently causing a crash. It's quite possible that this would cause problems on other platforms, and in other places than just ExecAggTransReparent() on s390x. Thanks to Christoph (and the Debian project) for providing me with access to a s390x machine, allowing me to debug this. Reported-By: Christoph Berg Author: Andres Freund Discussion: https://postgr.es/m/20201015083246.kie5726xerdt3ael@alap3.anarazel.de Backpatch: 11-, where JIT was added
* doc: improve description of synchronous_commit modesBruce Momjian2020-10-15
| | | | | | | | | | | | Previously it wasn't clear exactly what each of the synchronous_commit modes accomplished. This clarifies that, and adds a table describing it. Only backpatched through 9.6 since 9.5 doesn't have all the options. Reported-by: kghost0@gmail.com Discussion: https://postgr.es/m/159741195522.14321.13812604195366728976@wrigleys.postgresql.org Backpatch-through: 9.6
* Fix query in new test to check tables are syncedAlvaro Herrera2020-10-15
| | | | | | | | Rather than looking for tablesync workers, it is more reliable to see the sync state of the tables. Per note from Amit Kapila. Discussion: https://postgr.es/m/CAA4eK1JSSD7FVwq+_rOme86jUZTQFzjsNU06hQ4-LiRt1xFmSg@mail.gmail.com
* Handle EACCES errors from kevent() better.Thomas Munro2020-10-15
| | | | | | | | | | | | | | | | While registering for postmaster exit events, we have to handle a couple of edge cases where the postmaster is already gone. Commit 815c2f09 missed one: EACCES must surely imply that PostmasterPid no longer belongs to our postmaster process (or alternatively an unexpected permissions model has been imposed on us). Like ESRCH, this should be treated as a WL_POSTMASTER_DEATH event, rather than being raised with ereport(). No known problems reported in the wild. Per code review from Tom Lane. Back-patch to 13. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3624029.1602701929%40sss.pgh.pa.us
* doc: Mention that toast_tuple_target affects also column marked as Main.Fujii Masao2020-10-15
| | | | | | | | | | | | Previously it was documented that toast_tuple_target affected column marked as only External or Extended. But this description is not correct and toast_tuple_target affects also column marked as Main. Back-patch to v11 where toast_tuple_target reloption was introduced. Author: Shinya Okano Reviewed-by: Tatsuhito Kasahara, Fujii Masao Discussion: https://postgr.es/m/93f46e311a67422e89e770d236059817@oss.nttdata.com
* Restore replication protocol's duplicate command tagsAlvaro Herrera2020-10-14
| | | | | | | | | | | | | | | | | | | | I removed the duplicate command tags for START_REPLICATION inadvertently in commit 07082b08cc5d, but the replication protocol requires them. The fact that the replication protocol was broken was not noticed because all our test cases use an optimized code path that exits early, failing to verify that the behavior is correct for non-optimized cases. Put them back. Also document this protocol quirk. Add a test case that shows the failure. It might still succeed even without the patch when run on a fast enough server, but it suffices to show the bug in enough cases that it would be noticed in buildfarm. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Henry Hinze <henry.hinze@gmail.com> Reviewed-by: Petr Jelínek <petr.jelinek@2ndquadrant.com> Discussion: https://postgr.es/m/16643-eaadeb2a1a58d28c@postgresql.org
* Make WL_POSTMASTER_DEATH level-triggered on kqueue builds.Thomas Munro2020-10-15
| | | | | | | | | | | | If WaitEventSetWait() reports that the postmaster has gone away, later calls to WaitEventSetWait() should continue to report that. Otherwise further waits that occur in the proc_exit() path after we already noticed the postmaster's demise could block forever. Back-patch to 13, where the kqueue support landed. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3624029.1602701929%40sss.pgh.pa.us
* Paper over regression failures in infinite_recurse() on PPC64 Linux.Tom Lane2020-10-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our infinite_recurse() test to verify sane stack-overrun behavior is affected by a bug of the Linux kernel on PPC64: it will get SIGSEGV if it receives a signal when the stack depth is (a) over 1MB and (b) within a few kB of filling the current physical stack allocation. See https://bugzilla.kernel.org/show_bug.cgi?id=205183. Since this test is a bit time-consuming and we run it in parallel with test scripts that do a lot of DDL, it can be expected to get an sinval catchup interrupt at some point, leading to failure if the timing is wrong. This has caused more than 100 buildfarm failures over the past year or so. While a fix exists for the kernel bug, it might be years before that propagates into all production kernels, particularly in some of the older distros we have in the buildfarm. For now, let's just back off and not run this test on Linux PPC64; that loses nothing in test coverage so far as our own code is concerned. To do that, split this test into a new script infinite_recurse.sql and skip the test when the platform name is powerpc64...-linux-gnu. Back-patch to v12. Branches before that have not been seen to get this failure. No doubt that's because the "errors" test was not run in parallel with other tests before commit 798070ec0, greatly reducing the odds of an sinval catchup being necessary. I also back-patched 3c8553547 into v12, just so the new regression script would look the same in all branches having it. Discussion: https://postgr.es/m/3479046.1602607848@sss.pgh.pa.us Discussion: https://postgr.es/m/20190723162703.GM22387%40telsasoft.com
* Fix GiST buffering build to work when there are included columns.Tom Lane2020-10-12
| | | | | | | | | | | | | | gistRelocateBuildBuffersOnSplit did not get the memo about which attribute count to use. This could lead to a crash if there were included columns and buffering build was chosen. (Because there are random page-split decisions elsewhere in GiST index build, the crashes are not entirely deterministic.) Back-patch to v12 where GiST gained support for included columns. Pavel Borisov Discussion: https://postgr.es/m/CALT9ZEECCV5m7wvxg46PC-7x-EybUmnpupBGhSFMoAAay+r6HQ@mail.gmail.com
* Fix memory leak when guc.c decides a setting can't be applied now.Tom Lane2020-10-12
| | | | | | | | | | | | | | | | | | | | The prohibitValueChange code paths in set_config_option(), which are executed whenever we re-read a PGC_POSTMASTER variable from postgresql.conf, neglected to free anything before exiting. Thus we'd leak the proposed new value of a PGC_STRING variable, as noted by BoChen in bug #16666. For all variable types, if the check hook creates an "extra" chunk, we'd also leak that. These are malloc not palloc chunks, so there is no mechanism for recovering the leaks before process exit. Fortunately, the values are typically not very large, meaning you'd have to go through an awful lot of SIGHUP configuration-reload cycles to make the leakage amount to anything. Still, for a long-lived postmaster process it could potentially be a problem. Oversight in commit 2594cf0e8. Back-patch to all supported branches. Discussion: https://postgr.es/m/16666-2c41a4eec61b03e1@postgresql.org
* Choose ppc compare_exchange constant path for more operand values.Noah Misch2020-10-11
| | | | | | | | | | | | | The implementation uses smaller code when the "expected" operand is a small constant, but the implementation needlessly defined the set of acceptable constants more narrowly than the ABI does. Core PostgreSQL and PGXN don't use the constant path at all, so this is future-proofing. Back-patch to v13, where commit 30ee5d17c20dbb282a9952b3048d6ad52d56c371 introduced this code. Reviewed by Tom Lane. Reported by Christoph Berg. Discussion: https://postgr.es/m/20201009092825.GD889580@msg.df7cb.de
* For ppc gcc, implement 64-bit compare_exchange and fetch_add with asm.Noah Misch2020-10-11
| | | | | | | | | | | While xlc defines __64BIT__, gcc does not. Due to this oversight in commit 30ee5d17c20dbb282a9952b3048d6ad52d56c371, gcc builds continued implementing 64-bit atomics by way of intrinsics. Back-patch to v13, where that commit first appeared. Reviewed by Tom Lane. Discussion: https://postgr.es/m/20201011051043.GA1724101@rfd.leadboat.com
* Fix optimization hazard in gram.y's makeOrderedSetArgs(), redux.Tom Lane2020-10-07
| | | | | | | | | | | | | | | | It appears that commit cf63c641c, which intended to prevent misoptimization of the result-building step in makeOrderedSetArgs, didn't go far enough: buildfarm member hornet's version of xlc is now optimizing back to the old, broken behavior in which list_length(directargs) is fetched only after list_concat() has changed that value. I'm not entirely convinced whether that's an undeniable compiler bug or whether it can be justified by a sufficiently aggressive interpretation of C sequence points. So let's just change the code to make it harder to misinterpret. Back-patch to all supported versions, just in case. Discussion: https://postgr.es/m/1830491.1601944935@sss.pgh.pa.us
* Prevent internal overflows in date-vs-timestamp and related comparisons.Tom Lane2020-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The date-vs-timestamp, date-vs-timestamptz, and timestamp-vs-timestamptz comparators all worked by promoting the first type to the second and then doing a simple same-type comparison. This works fine, except when the conversion result is out of range, in which case we throw an entirely avoidable error. The sources of such failures are (a) type date can represent dates much farther in the future than the timestamp types can; (b) timezone rotation might cause a just-in-range timestamp value to become a just-out-of-range timestamptz value. Up to now we just ignored these corner-case issues, but now we have an actual user complaint (bug #16657 from Huss EL-Sheikh), so let's do something about it. It turns out that commit 52ad1e659 already built all the necessary infrastructure to support error-free comparisons, but neglected to actually use it in the main-line code paths. Fix that, do a little bit of code style review, and remove the now-duplicate logic in jsonpath_exec.c. Back-patch to v13 where 52ad1e659 came in. We could take this back further by back-patching said infrastructure, but given the small number of complaints so far, I don't feel a great need to. Discussion: https://postgr.es/m/16657-cde2f876d8cc7971@postgresql.org
* Rethink recent fix for pg_dump's handling of extension config tables.Tom Lane2020-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 3eb3d3e78 was a few bricks shy of a load: while it correctly set the table's "interesting" flag when deciding to dump the data of an extension config table, it was not correct to clear that flag if we concluded we shouldn't dump the data. This led to the crash reported in bug #16655, because in fact we'll traverse dumpTableSchema anyway for all extension tables (to see if they have user-added seclabels or RLS policies). The right thing to do is to force "interesting" true in makeTableDataInfo, and otherwise leave the flag alone. (Doing it there is more future-proof in case additional calls are added, and it also avoids setting the flag unnecessarily if that function decides the table is non-dumpable.) This investigation also showed that while only the --inserts code path had an obvious failure in the case considered by 3eb3d3e78, the COPY code path also has a problem with not having loaded table subsidiary data. That causes fmtCopyColumnList to silently return an empty string instead of the correct column list. That accidentally mostly works, which perhaps is why we didn't notice this before. It would only fail if the restore column order is different from the dump column order, which only happens in weird inheritance cases, so it's not surprising nobody had hit the case with an extension config table. Nonetheless, it's a bug, and it goes a long way back, not just to v12 where the --inserts code path started to have a problem with this. In hopes of catching such cases a bit sooner in future, add some Asserts that "interesting" has been set in both dumpTableData and dumpTableSchema. Adjust the test case added by 3eb3d3e78 so that it checks the COPY rather than INSERT form of that bug, allowing it to detect the longer-standing symptom. Per bug #16655 from Cameron Daniel. Back-patch to all supported branches. Discussion: https://postgr.es/m/16655-5c92d6b3a9438137@postgresql.org Discussion: https://postgr.es/m/18048b44-3414-b983-8c7c-9165b177900d@2ndQuadrant.com
* pg_upgrade: remove pre-8.4 code and >= 8.4 checkBruce Momjian2020-10-06
| | | | | | | | | | We only support upgrading from >= 8.4 so no need for this code or tests. Reported-by: Magnus Hagander Discussion: https://postgr.es/m/CABUevEx-D0PNVe00tkeQRGennZQwDtBJn=493MJt-x6sppbUxA@mail.gmail.com Backpatch-through: 9.5
* pg_upgrade; change major version comparisons to use <=, not <Bruce Momjian2020-10-06
| | | | | | This makes checking for older major versions more consistent. Backpatch-through: 9.5