aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Process shared_preload_libraries in single-user mode.Jeff Davis2022-07-20
| | | | | | | | | | | | | | | | Without processing shared_preload_libraries, it's impossible to recover if custom WAL resource managers are needed. It may also pose a problem running VACUUM on a table with a custom AM, if the module implementing the AM is expecting to be loaded by shared_preload_libraries. The reason this wasn't done before was just the general principle to do fewer things in single-user mode. But it's easy enough to just set shared_preload_libraries to empty, for the same effect. Discussion: https://postgr.es/m/9decc18a42634f8a2f15c97a385a0f51a752f396.camel%40j-davis.com Reviewed-by: Tom Lane, Andres Freund Backpatch-through: 15
* Fix assertion failure and segmentation fault in backup code.Fujii Masao2022-07-20
| | | | | | | | | | | | | | | | | | | | | | When a non-exclusive backup is canceled, do_pg_abort_backup() is called and resets some variables set by pg_backup_start (pg_start_backup in v14 or before). But previously it forgot to reset the session state indicating whether a non-exclusive backup is in progress or not in this session. This issue could cause an assertion failure when the session running BASE_BACKUP is terminated after it executed pg_backup_start and pg_backup_stop (pg_stop_backup in v14 or before). Also it could cause a segmentation fault when pg_backup_stop is called after BASE_BACKUP in the same session is canceled. This commit fixes the issue by making do_pg_abort_backup reset that session state. Back-patch to all supported branches. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Michael Paquier, Robert Haas Discussion: https://postgr.es/m/3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com
* Prevent BASE_BACKUP in the middle of another backup in the same session.Fujii Masao2022-07-20
| | | | | | | | | | | | | | | | | | | | | | | | Multiple non-exclusive backups are able to be run conrrently in different sessions. But, in the same session, only one non-exclusive backup can be run at the same moment. If pg_backup_start (pg_start_backup in v14 or before) is called in the middle of another non-exclusive backup in the same session, an error is thrown. However, previously, in logical replication walsender mode, even if that walsender session had already called pg_backup_start and started a non-exclusive backup, it could execute BASE_BACKUP command and start another non-exclusive backup. Which caused subsequent pg_backup_stop to throw an error because BASE_BACKUP unexpectedly reset the session state marked by pg_backup_start. This commit prevents BASE_BACKUP command in the middle of another non-exclusive backup in the same session. Back-patch to all supported branches. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Michael Paquier, Robert Haas Discussion: https://postgr.es/m/3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com
* Tweak detail and hint messages to be consistent with project policyMichael Paquier2022-07-20
| | | | | | | | | | | Detail and hint messages should be full sentences and should end with a period, but some of the messages newly-introduced in v15 did not follow that. Author: Justin Pryzby Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/20220719120948.GF12702@telsasoft.com Backpatch-through: 15
* Fix missed corner cases for grantable permissions on GUCs.Tom Lane2022-07-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We allow users to set the values of not-yet-loaded extension GUCs, remembering those values in "placeholder" GUC entries. When/if the extension is loaded later in the session, we need to verify that the user had permissions to set the GUC. That was done correctly before commit a0ffa885e, but as of that commit, we'd check the permissions of the active role when the LOAD happens, not the role that had set the value. (This'd be a security bug if it had made it into a released version.) In principle this is simple enough to fix: we just need to remember the exact role OID that set each GUC value, and use that not GetUserID() when verifying permissions. Maintaining that data in the guc.c data structures is slightly tedious, but fortunately it's all basically just copy-n-paste of the logic for tracking the GucSource of each setting, as we were already doing. Another oversight is that validate_option_array_item() hadn't been taught to check for granted GUC privileges. This appears to manifest only in that ALTER ROLE/DATABASE RESET ALL will fail to reset settings that the user should be allowed to reset. Patch by myself and Nathan Bossart, per report from Nathan Bossart. Back-patch to v15 where the faulty code came in. Discussion: https://postgr.es/m/20220706224727.GA2158260@nathanxps13
* Use STDOUT/STDERR_FILENO in most of syslogger.Andres Freund2022-07-18
| | | | | | | | | | | | | | | This fixes problems on windows when logging collector is used in a service, failing with: FATAL: could not redirect stderr: Bad file descriptor This is triggered by 76e38b37a5. The problem is that STDOUT/STDERR_FILENO aren't defined on windows, which lead us to use _fileno(stdout) etc, but that doesn't work if stdout/stderr are closed. Author: Andres Freund <andres@anarazel.de> Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers) Backpatch: 15-, where 76e38b37a5 came in
* windows: msvc: Define STDIN/OUT/ERR_FILENO.Andres Freund2022-07-18
| | | | | | | | | | | | Because they are not available we've used _fileno(stdin) in some places, but that doesn't reliably work, because stdin might be closed. This is the prerequisite of the subsequent commit, fixing a failure introduced in 76e38b37a5. Author: Andres Freund <andres@anarazel.de> Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers) Backpatch: 15-, where 76e38b37a5 came in
* pg_upgrade: Adjust quoting style in message to match guidelinesPeter Eisentraut2022-07-18
|
* Add another SQL/JSON error codePeter Eisentraut2022-07-18
| | | | | | | A code comment said that the standard does not define a number for ERRCODE_SQL_JSON_ITEM_CANNOT_BE_CAST_TO_TARGET_TYPE, but this was fixed in a later draft version of the standard, so use that number now.
* Fix omissions in support for the "regcollation" type.Tom Lane2022-07-17
| | | | | | | | | | | | | The patch that added regcollation doesn't seem to have been too thorough about supporting it everywhere that other reg* types are supported. Fix that. (The find_expr_references omission is moderately serious, since it could result in missing expression dependencies. The others are less exciting.) Noted while fixing bug #17483. Back-patch to v13 where regcollation was added. Discussion: https://postgr.es/m/1423433.1652722406@sss.pgh.pa.us
* Make dsm_impl_posix_resize more future-proof.Thomas Munro2022-07-16
| | | | | | | | | | | | | | | | | | | | Commit 4518c798 blocks signals for a short region of code, but it assumed that whatever called it had the signal mask set to UnBlockSig on entry. That may be true today (or may even not be, in extensions in the wild), but it would be better not to make that assumption. We should save-and-restore the caller's signal mask. The PG_SETMASK() portability macro couldn't be used for that, which is why it wasn't done before. But... considering that commit a65e0864 established back in 9.6 that supported POSIX systems have sigprocmask(), and that this is POSIX-only code, there is no reason not to use standard sigprocmask() directly to achieve that. Back-patch to all supported releases, like 4518c798 and 80845b7c. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGKx6Biq7_UuV0kn9DW%2B8QWcpJC1qwhizdtD9tN-fn0H0g%40mail.gmail.com
* Fix inconsistent parameter names between prototype and declarationDavid Rowley2022-07-15
| | | | | | | Noticed while working in this area. This code was introduced in PG15, which is still in beta, so backpatch to there for consistency. Backpatch-through: 15
* Don't clobber postmaster sigmask in dsm_impl_resize.Thomas Munro2022-07-15
| | | | | | | | | | | | Commit 4518c798 intended to block signals in regular backends that allocate DSM segments, but dsm_impl_resize() is also reached by dsm_postmaster_startup(). It's not OK to clobber the postmaster's signal mask, so only manipulate the signal mask when under the postmaster. Back-patch to all releases, like 4518c798. Discussion: https://postgr.es/m/CA%2BhUKGKNpK%3D2OMeea_AZwpLg7Bm4%3DgYWk7eDjZ5F6YbozfOf8w%40mail.gmail.com
* Block signals while allocating DSM memory.Thomas Munro2022-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On Linux, we call posix_fallocate() on shm_open()'d memory to avoid later potential SIGBUS (see commit 899bd785). Based on field reports of systems stuck in an EINTR retry loop there, there, we made it possible to break out of that loop via slightly odd coding where the CHECK_FOR_INTERRUPTS() call was somewhat removed from the loop (see commit 422952ee). On further reflection, that was not a great choice for at least two reasons: 1. If interrupts were held, the CHECK_FOR_INTERRUPTS() would do nothing and the EINTR error would be surfaced to the user. 2. If EINTR was reported but neither QueryCancelPending nor ProcDiePending was set, then we'd dutifully retry, but with a bit more understanding of how posix_fallocate() works, it's now clear that you can get into a loop that never terminates. posix_fallocate() is not a function that can do some of the job and tell you about progress if it's interrupted, it has to undo what it's done so far and report EINTR, and if signals keep arriving faster than it can complete (cf recovery conflict signals), you're stuck. Therefore, for now, we'll simply block most signals to guarantee progress. SIGQUIT is not blocked (see InitPostmasterChild()), because its expected handler doesn't return, and unblockable signals like SIGCONT are not expected to arrive at a high rate. For good measure, we'll include the ftruncate() call in the blocked region, and add a retry loop. Back-patch to all supported releases. Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Nicola Contu <nicola.contu@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220701154105.jjfutmngoedgiad3%40alvherre.pgsql
* Plug memory leakAlvaro Herrera2022-07-13
| | | | | | | Commit 054325c5eeb3 created a memory leak in PQsendQueryInternal in case an error occurs while sending the message. Repair. Backpatch to 14, like that commit. Reported by Coverity.
* Small cleanup of create_list_bounds()David Rowley2022-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When checking for interleaved partitions, we mark the partition as interleaved when; 1. we find an earlier partition index when looping over the sorted-by-Datum indexes[] array, or; 2. we find that the NULL partition allows some non-NULL Datum value. In the code, as it was written in db632fbca we'll continue to check for case 2 when we've already marked the partition as interleaved for case 1. Here we make it so we don't bother marking the partition as interleaved for case 2 when it's already been marked due to case 1. Really all this saves is a useless call to bms_add_member(), but since this code is new to PG15, it seems worth fixing it now to save anyone the trouble of complaining at some time in the future. We have the opportunity to improve this now before PG15 is out. This might ease some future back-patching pain. Per report and patch by Zhihong Yu. However, I slightly revised the comments and altered the bms_add_member() code to match in both locations. We already know that index is equal to boundinfo->null_index from the if condition. Author: Zhihong Yu Discussion: https://postgr.es/m/CALNJ-vQbZR0pYxz9zQ5bqXVcwtGgNgVupeEpNT65HZ+yWZnc4g@mail.gmail.com Backpatch-through: 15, same as db632fbca.
* Tidy up code in get_cheapest_group_keys_order()David Rowley2022-07-13
| | | | | | | | | | | | | | | | | | | | | | | There are a few things that we could do a little better within get_cheapest_group_keys_order(): 1. We should be using list_free() rather than pfree() on a List. 2. We should use for_each_from() instead of manually coding a for loop to skip the first n elements of a List 3. list_truncate(list_copy(...), n) is not a great way to copy the first n elements of a list. Let's invent list_copy_head() for that. That way we don't need to copy the entire list just to truncate it directly afterwards. 4. We can simplify finding the cheapest cost by setting the cheapest cost variable to DBL_MAX. That allows us to skip special-casing the initial iteration of the loop. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvrGyL3ft8waEkncG9y5HDMu5TFFJB1paoTC8zi9YK97Nw@mail.gmail.com Backpatch-through: 15, where get_cheapest_group_keys_order was added.
* Invent qsort_interruptible().Tom Lane2022-07-12
| | | | | | | | | | | | | | | | | | | | | | | | Justin Pryzby reported that some scenarios could cause gathering of extended statistics to spend many seconds in an un-cancelable qsort() operation. To fix, invent qsort_interruptible(), which is just like qsort_arg() except that it will also do CHECK_FOR_INTERRUPTS every so often. This bloats the backend by a couple of kB, which seems like a good investment. (We considered just enabling CHECK_FOR_INTERRUPTS in the existing qsort and qsort_arg functions, but there are some callers for which that'd demonstrably be unsafe. Opt-in seems like a better way.) For now, just apply qsort_interruptible() in statistics collection. There's probably more places where it could be useful, but we can always change other call sites as we find problems. Back-patch to v14. Before that we didn't have extended stats on expressions, so that the problem was less severe. Also, this patch depends on the sort_template infrastructure introduced in v14. Tom Lane and Justin Pryzby Discussion: https://postgr.es/m/20220509000108.GQ28830@telsasoft.com
* Fix lock assertions in dshash.c.Thomas Munro2022-07-11
| | | | | | | | | | | | | | | | | | | | | | | | | dshash.c previously maintained flags to be able to assert that you didn't hold any partition lock. These flags could get out of sync with reality in error scenarios. Get rid of all that, and make assertions about the locks themselves instead. Since LWLockHeldByMe() loops internally, we don't want to put that inside another loop over all partition locks. Introduce a new debugging-only interface LWLockAnyHeldByMe() to avoid that. This problem was noted by Tom and Andres while reviewing changes to support the new shared memory stats system, and later showed up in reality while working on commit 389869af. Back-patch to 11, where dshash.c arrived. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Andres Freund <andres@anarazel.de> Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220311012712.botrpsikaufzteyt@alap3.anarazel.de Discussion: https://postgr.es/m/CA%2BhUKGJ31Wce6HJ7xnVTKWjFUWQZPBngxfJVx4q0E98pDr3kAw%40mail.gmail.com
* Fix two portability issues with the tests of test_oat_hooksMichael Paquier2022-07-11
| | | | | | | | | | | | | | | | | | | | | | | | This addresses two issues in the tests of test_oat_hooks: - The role regress_test_user was being left behind, preventing the test to succeed on repeated runs. It makes sense to leave some objects behind to have more coverage for pg_upgrade (as does test_pg_dump), but the role dropped here does not own any objects so there is no reason to keep it. - GRANT SET ON PARAMETER is issued, creating an entry in pg_parameter_acl without cleaning up the entry created. This causes an overlap with unsafe_tests as both use work_mem, making the latter fail. This commit adds an extra REVOKE SET ON PARAMETER to clean the contents of pg_parameter_acl, switching to maintenance_work_mem rather than work_mem to avoid an overlap between both tests. The tests of test_oat_hooks cannot use installcheck yet as these are proving to be unstable with caching and the namespace search hooks, so the issues fixed here cannot be reached yet, but they would be once the hook issue is addressed and installcheck is allowed again in test_oat_hooks. Discussion: https://postgr.es/m/YrpVkADAY0knF6vM@paquier.xyz Backpatch-through: 15
* Improve error message with JSON_SERIALIZE()Michael Paquier2022-07-11
| | | | | | | | | | The error message introduced in 3c633f3 can share the same format string with an existing message used for JSON(), reducing the translation effort. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20220708.154135.2123613118233840495.horikyota.ntt@gmail.com Backpatch-through: 15
* Fix \watch's interaction with libedit on ^C.Thomas Munro2022-07-10
| | | | | | | | | | | | | | When you hit ^C, the terminal driver in Unix-like systems echoes "^C" as well as sending an interrupt signal (depending on stty settings). At least libedit (but maybe also libreadline) is then confused about the current cursor location, and corrupts the display if you try to scroll back. Fix, by moving to a new line before the next prompt is displayed. Back-patch to all supported released. Author: Pavel Stehule <pavel.stehule@gmail.com> Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3278793.1626198638%40sss.pgh.pa.us
* Preserve relfilenode of pg_largeobject and its index across pg_upgrade.Robert Haas2022-07-08
| | | | | | | | | | | | | | | | | | | | Commit 9a974cbcba005256a19991203583a94b4f9a21a9 did this for user tables, but pg_upgrade treats pg_largeobject as a user table, and so needs the same treatment. Without this fix, if you rewrite the pg_largeobject table and then perform an upgrade with pg_upgrade, the table will apparently be empty on the new cluster, while all of your objects will end up with an orphaned file. With this fix, instead of the old cluster's pg_largeobject files ending up orphaned, the original files fro the new cluster do. That's mostly harmless because we expect the table to be empty, but we might want to arrange to remove the as part of the upgrade. Since we're still debating the best way of doing that, I (rhaas) have decided to postpone dealing with that problem and get the basic fix committed. Justin Pryzby, reviewed by Shruthi Gowda and by me. Discussion: http://postgr.es/m/20220701231413.GI13040@telsasoft.com
* Only allow returning string types or bytea from json_serializeAndrew Dunstan2022-07-07
| | | | | | | | | | These are documented to be the allowed types for the RETURNING clause, but the restriction was not being enforced, which caused a segfault if another type was specified. Add some testing for this. Per report from a.kozhemyakin Backpatch to release 15.
* Fix alias matching in transformLockingClause().Dean Rasheed2022-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | When locking a specific named relation for a FOR [KEY] UPDATE/SHARE clause, transformLockingClause() finds the relation to lock by scanning the rangetable for an RTE with a matching eref->aliasname. However, it failed to account for the visibility rules of a join RTE. If a join RTE doesn't have a user-supplied alias, it will have a generated eref->aliasname of "unnamed_join" that is not visible as a relation name in the parse namespace. Such an RTE needs to be skipped, otherwise it might be found in preference to a regular base relation with a user-supplied alias of "unnamed_join", preventing it from being locked. In addition, if a join RTE doesn't have a user-supplied alias, but does have a join_using_alias, then the RTE needs to be matched using that alias rather than the generated eref->aliasname, otherwise a misleading "relation not found" error will be reported rather than a "join cannot be locked" error. Backpatch all the way, except for the second part which only goes back to 14, where JOIN USING aliases were added. Dean Rasheed, reviewed by Tom Lane. Discussion: https://postgr.es/m/CAEZATCUY_KOBnqxbTSPf=7fz9HWPnZ5Xgb9SwYzZ8rFXe7nb=w@mail.gmail.com
* Re-order disable_on_error in tab-complete.Amit Kapila2022-07-07
| | | | | | | | | | | | | | By convention, the tab-complete subscription parameters are listed in the COMPLETE_WITH lists in alphabetical order, but when the "disable_on_error" parameter was introduced this was not done. This patch just tidies that up. Reported-by: Peter Smith Author: Peter Smith Reviewed-by: Euler Taveira, Takamichi Osumi Backpatch-through: 15, where it was introduced Discussion: https://postgr.es/m/CAHut+PucvKZgg_eJzUW--iL6DXHg1Jwj6F09tQziE3kUF67uLg@mail.gmail.com
* pgstat: slru: remove outdated commentAndres Freund2022-07-06
| | | | | | | | That comment might have been true at some point during development, but definitely isn't anymore. Reported-By: Melanie Plageman <melanieplageman@gmail.com> Backpatch: 15-
* Fix wrong field order in _readMergeWhenClause().Tom Lane2022-07-06
| | | | | | | | | | | We hadn't noticed this because it's dead code: there is no situation where we read raw parse trees from text format. So maybe the right fix is to remove the function altogether, but I'll forbear for now; it's not the only dead code in readfuncs.c, I think. Noted while comparing existing code to the results of Peter's auto-generation script.
* Overload index_form_tuple to allow the memory context to be suppliedDavid Rowley2022-07-07
| | | | | | | | | | | | | | | | | | | | | | | 40af10b57 changed things so we make use of a generation memory context for storing tuples to be sorted by tuplesort.c. That change does not play nicely with the changes made in 9f03ca915 (back in 2014). That commit changed things so that index_form_tuple() is called while switched into the tuplestore's tuplecontext. In order to fetch the tuple from the index, index_form_tuple() must do various memory allocations which are unrelated to the storage of the final returned tuple. Although all of these allocations are pfree'd, the fact that we now use a generation context means that the memory for these pfree'd allocations won't be used again by any other allocation due to generation.c's lack of freelists. This could result in sorts used for building indexes exceeding maintenance_work_mem by a very large amount. Here we fix it so we no longer allocate anything apart from the tuple itself into the generation context by adding a new version of index_form_tuple named index_form_tuple_context, which can be called to specify the MemoryContext to allocate the tuple into. Discussion: https://postgr.es/m/CAApHDvrHQkiFRHiGiAS-LMOvJN-eK-s762=tVzBz8ZqUea-a_A@mail.gmail.com Backpatch-through: 15, where 40af10b57 was added.
* pgstat: drop subscription stats regardless of slot, fix commentAndres Freund2022-07-06
| | | | | | | | | | There's no reason anymore to only drop subscription stats if associated with a slot, now that stats drops are transactional. Additionally, the comment referring to autovacuum cleaning up stats was clearly outdated. Author: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoAwiby3HeJE7vJe16Gr75RFfJ640dyHqvsiUhyKJTXPtw@mail.gmail.com Backpatch: 15-
* Remove size increase in ExprEvalStep caused by hashed saopsDavid Rowley2022-07-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 50e17ad28 increased the size of ExprEvalStep from 64 bytes up to 88 bytes. Lots of effort was spent during the development of the current expression evaluation code to make an instance of this struct as small as possible. Making this struct larger than needed reduces CPU cache efficiency during expression evaluation which causes noticeable slowdowns during query execution. In order to reduce the size of the struct, here we remove the fn_addr field. The values from this field can be obtained via fcinfo, just with some extra pointer dereferencing. The extra indirection does not seem to cause any noticeable slowdowns. Various other fields have been moved into the ScalarArrayOpExprHashTable struct. These fields are only used when the ScalarArrayOpExprHashTable pointer has already been dereferenced, so no additional pointer dereferences occur for these. Here we also make hash_fcinfo_data the last field in ScalarArrayOpExprHashTable so that we can avoid a further pointer dereference to get the FunctionCallInfoBaseData. This also saves a call to palloc(). 50e17ad28 was added in 14, but it's too late to adjust the size of the ExprEvalStep in that version, so here we just backpatch to 15, which is currently in beta. Author: Andres Freund, David Rowley Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch-through: 15
* Tighten pg_upgrade's new check for non-upgradable anyarray usages.Tom Lane2022-07-05
| | | | | | | We only need to reject cases when the aggregate or operator is itself declared with a polymorphic type. Per buildfarm. Discussion: https://postgr.es/m/3383880.QJadu78ljV@vejsadalnx
* pgstat: reduce timer overhead by leaving timer running.Andres Freund2022-07-05
| | | | | | | | | | | | | | | | | | | | Previously the timer was enabled whenever there were any pending stats after executing a statement, just to then be disabled again when not idle anymore. That lead to an increase in GetCurrentTimestamp() calls from within timeout.c compared to 14. To avoid that increase, leave the timer enabled until stats are reported, rather than until idle. The timer is only disabled once the pending stats have been reported. For me this fixes the increase in GetCurrentTimestamp() calls, there now are fewer calls in 15 than in 14, in the previously slowed down workload. While at it, also update assertion in pgstat_report_stat() to be more precise. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch: 15-
* expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size.Andres Freund2022-07-05
| | | | | | | | | | | | | | The new expression step types increased the size of ExprEvalStep by ~4 for all types of expression steps, slowing down expression evaluation noticeably. Move them out of line. There's other issues with these expression steps, but addressing them is largely independent of this aspect. Author: Andres Freund <andres@anarazel.de> Reviewed-By: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch: 15-
* Revert 019_replslot_limit.pl related debugging aids.Andres Freund2022-07-05
| | | | | | | | | | | | | | This reverts most of 91c0570a791, f28bf667f60, fe0972ee5e6, afdeff10526. The only thing left is the retry loop in 019_replslot_limit.pl that avoids spurious failures by retrying a couple times. We haven't seen any hard evidence that this is caused by anything but slow process shutdown. We did not find any cases where walsenders did not vanish after waiting for longer. Therefore there's no reason for this debugging code to remain. Discussion: https://postgr.es/m/20220530190155.47wr3x2prdwyciah@alap3.anarazel.de Backpatch: 15-
* Rename pg_checkpointer predefined role to pg_checkpoint.Robert Haas2022-07-05
| | | | | | | | | This is more consistent with how other predefined roles that confer specific privileges are named. Nathan Bosart Discussion: http://postgr.es/m/CA+TgmoatH7+yYe+A8uJFNogg3VUDtFE6c-77yHAY8TRWR7oqyw@mail.gmail.com
* Fix pg_upgrade to detect non-upgradable anyarray usages.Tom Lane2022-07-05
| | | | | | | | | | | | | | | | | When we changed some built-in functions to use anycompatiblearray instead of anyarray, we created a dump/restore hazard for user-defined operators and aggregates relying on those functions: the user objects have to be modified to change their signatures similarly. This causes pg_upgrade to fail partway through if the source installation contains such objects. We generally try to have pg_upgrade detect such hazards and fail before it does anything exciting, so add logic to detect this case too. Back-patch to v14 where the change was made. Justin Pryzby, reviewed by Andrey Borodin Discussion: https://postgr.es/m/3383880.QJadu78ljV@vejsadalnx
* Fix errors in copyfuncs/equalfuncs support for JSON node types.Tom Lane2022-07-05
| | | | | | | | | | | Noted while comparing existing code to the output of the proposed patch to automate creation of these functions. Some of the changes are just cosmetic, but others represent real bugs. I've not attempted to analyze the user-visible impact. Back-patch to v15 where this code came in. Discussion: https://postgr.es/m/1794155.1656984188@sss.pgh.pa.us
* libpq: Improve idle state handling in pipeline modeAlvaro Herrera2022-07-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We were going into IDLE state too soon when executing queries via PQsendQuery in pipeline mode, causing several scenarios to misbehave in different ways -- most notably, as reported by Daniele Varrazzo, that a warning message is produced by libpq: message type 0x33 arrived from server while idle But it is also possible, if queries are sent and results consumed not in lockstep, for the expected mediating NULL result values from PQgetResult to be lost (a problem which has not been reported, but which is more serious). Fix this by introducing two new concepts: one is a command queue element PGQUERY_CLOSE to tell libpq to wait for the CloseComplete server response to the Close message that is sent by PQsendQuery. Because the application is not expecting any PGresult from this, the mechanism to consume it is a bit hackish. The other concept, authored by Horiguchi-san, is a PGASYNC_PIPELINE_IDLE state for libpq's state machine to differentiate "really idle" from merely "the idle state that occurs in between reading results from the server for elements in the pipeline". This makes libpq not go fully IDLE when the libpq command queue contains entries; in normal cases, we only go IDLE once at the end of the pipeline, when the server response to the final SYNC message is received. (However, there are corner cases it doesn't fix, such as terminating the query sequence by PQsendFlushRequest instead of PQpipelineSync; this sort of scenario is what requires PGQUERY_CLOSE bit above.) This last bit helps make the libpq state machine clearer; in particular we can get rid of an ugly hack in pqParseInput3 to avoid considering IDLE as such when the command queue contains entries. A new test mode is added to libpq_pipeline.c to tickle some related problematic cases. Reported-by: Daniele Varrazzo <daniele.varrazzo@gmail.com> Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CA+mi_8bvD0_CW3sumgwPvWdNzXY32itoG_16tDYRu_1S2gV2iw@mail.gmail.com
* Use a short socket directory path in pg_upgrade testing.Tom Lane2022-07-03
| | | | | | | | | | | | | | | | | | | | | Several buildfarm members are failing the pg_upgrade test in REL_15_STABLE, though the identical test is fine in HEAD. On thorntail it's possible to see that the problem is an overlength socket path name, and I bet the same is true on the others. The normally-started postmasters used in the test are already set up with short socket directory paths, but we neglected to tell pg_upgrade itself to do likewise when starting child postmasters, and indeed it seems to be explicitly selecting the test directory instead. Back-patch to v15 where the current test script was introduced. (The previous script might have the same issue, because I don't see any use of -s/--socketdir in it either; but we've had no complaints, so leave it alone for now.) Discussion: https://postgr.es/m/1410025.1656890531@sss.pgh.pa.us
* Remove %error-verbose directive from jsonpath parserAndrew Dunstan2022-07-03
| | | | | | | | | | | None of the other bison parsers contains this directive, and it gives rise to some unfortunate and impenetrable messages, so just remove it. Backpatch to release 12, where it was introduced. Per gripe from Erik Rijkers Discussion: https://postgr.es/m/ba069ce2-a98f-dc70-dc17-2ccf2a9bf7c7@xs4all.nl
* Add Windows file version information to libpq/test programs.Noah Misch2022-07-03
| | | | | | Back-patch to v15, the first version to install these programs. Discussion: https://postgr.es/m/20220501080706.GA1542365@rfd.leadboat.com
* Simplify tab completion of extension versions, redux.Tom Lane2022-07-03
| | | | | | | | | | | | | | | | | | | After commit 662dbe2c8, psql tab completion didn't conveniently support the case of "ALTER EXTENSION foo UPDATE". It'd always add "TO", which is fine if you want to specify a target version but not if you don't ... and surely the latter is the much more common case. To fix, remove "TO" from the initially offered completion; you now need to press TAB one additional time to get that. We won't try to duplicate the old behavior of attempting initial completion on the target version along with TO. It's too squirrelly to get the quoting right, and this is such an infrequent usage that it doesn't seem worth expending a lot of effort and special code on. Noted by Noah Misch. Back-patch to v15. Discussion: https://postgr.es/m/20220703083217.GB2476530@rfd.leadboat.com
* Fix previous commit's ecpg_clocale for ppc Darwin.Noah Misch2022-07-02
| | | | | | | | | | Per buildfarm member prairiedog, this platform rejects uninitialized global variables in shared libraries. Back-patch to v10, like the addition of the variable. Reviewed by Tom Lane. Discussion: https://postgr.es/m/20220703030619.GB2378460@rfd.leadboat.com
* ecpglib: call newlocale() once per process.Noah Misch2022-07-02
| | | | | | | | | | | | | | | | | | | | | | | | | | ecpglib has been calling it once per SQL query and once per EXEC SQL GET DESCRIPTOR. Instead, if newlocale() has not succeeded before, call it while establishing a connection. This mitigates three problems: - If newlocale() failed in EXEC SQL GET DESCRIPTOR, the command silently proceeded without the intended locale change. - On AIX, each newlocale()+freelocale() cycle leaked memory. - newlocale() CPU usage may have been nontrivial. Fail the connection attempt if newlocale() fails. Rearrange ecpg_do_prologue() to validate the connection before its uselocale(). The sort of program that may regress is one running in an environment where newlocale() fails. If that program establishes connections without running SQL statements, it will stop working in response to this change. I'm betting against the importance of such an ECPG use case. Most SQL execution (any using ECPGdo()) has long required newlocale() success, so there's little a connection could do without newlocale(). Back-patch to v10 (all supported versions). Reviewed by Tom Lane. Reported by Guillaume Lelarge. Discussion: https://postgr.es/m/20220101074055.GA54621@rfd.leadboat.com
* Default to dynamic_shared_memory_type=sysv on Solaris.Thomas Munro2022-07-02
| | | | | | | | | | | | | | | | | POSIX shm_open() can sleep for a long time and fail spuriously because of contention on an internal lock file on Solaris (and presumably illumos). Commit 389869af fixed the main problem with this, namely that we could crash, but it's now clear that "posix" is not a good default. Therefore, choose "sysv" at initdb time on Solaris and illumos. Other choices are still available by editing the postgresql.conf file. Back-patch only to 15, because contention is much less likely further back, and it doesn't seem like a good idea to change this in released branches. This should clear up the failures on build farm animal margay. Discussion: https://postgr.es/m/CA%2BhUKGKqKrCV5xKWfh9rnm%3Do%3DDwZLTLtnsj_XpUi9g5%3DV%2B9oyg%40mail.gmail.com
* Fix race condition in t/028_pitr_timelines.pl.Noah Misch2022-07-01
| | | | | | | Per buildfarm members sungazer and mylodon. Back-patch to v15, which introduced this test. Discussion: https://postgr.es/m/20220627070457.GA2176699@rfd.leadboat.com
* Add missing GETTEXT_FLAGS entryPeter Eisentraut2022-07-01
|
* Harden dsm_impl.c against unexpected EEXIST.Thomas Munro2022-07-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we trusted the OS not to report EEXIST unless we'd passed in IPC_CREAT | IPC_EXCL or O_CREAT | O_EXCL, as appropriate. Solaris's shm_open() can in fact do that, causing us to crash because we didn't ereport and then we blithely assumed the mapping was successful. Let's treat EEXIST just like any other error, unless we're actually trying to create a new segment. This applies to shm_open(), where this behavior has been seen, and also to the equivalent operations for our sysv and mmap modes just on principle. Based on the underlying reason for the error, namely contention on a lock file managed by Solaris librt for each distinct name, this problem is only likely to happen on 15 and later, because the new shared memory stats system produces shm_open() calls for the same path from potentially large numbers of backends concurrently during authentication. Earlier releases only shared memory segments between a small number of parallel workers under one Gather node. You could probably hit it if you tried hard enough though, and we should have been more defensive in the first place. Therefore, back-patch to all supported releases. Per build farm animal margay. This isn't the end of the story, though, it just changes random crashes into random "File exists" errors; more work needed for a green build farm. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGKqKrCV5xKWfh9rnm%3Do%3DDwZLTLtnsj_XpUi9g5%3DV%2B9oyg%40mail.gmail.com
* pgindent run prior to branching v15.Tom Lane2022-06-30
| | | | pgperltidy and reformat-dat-files too. Not many changes.