aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Close yet another race condition in replication slot test codeAlvaro Herrera2021-07-29
| | | | | | | | | | | | Buildfarm shows that this test has a further failure mode when a checkpoint starts earlier than expected, so we detect a "checkpoint completed" line that's not the one we want. Change the config to try and prevent this. Per buildfarm Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20210729.162038.534808353849568395.horikyota.ntt@gmail.com
* Add missing exit() in pg_verifybackup when failing to find pg_waldumpMichael Paquier2021-07-29
| | | | | | | | | | | | pg_verifybackup needs by default pg_waldump to check after a range of WAL segments required for a backup, except if --no-parse-wal is specified. The code checked for the presence of the binary pg_waldump in an installation and reported an error, but it forgot to properly exit(). This could lead to confusing errors reported. Reviewed-by: Robert Haas, Fabien Coelho Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz Backpatch-through: 13
* Update minimum recovery point on truncation during WAL replay of abort record.Fujii Masao2021-07-29
| | | | | | | | | | | | | | | | If a file is truncated, we must update minRecoveryPoint. Once a file is truncated, there's no going back; it would not be safe to stop recovery at a point earlier than that anymore. Commit 7bffc9b7bf changed xact_redo_commit() so that it updates minRecoveryPoint on truncation, but forgot to change xact_redo_abort(). Back-patch to all supported versions. Reported-by: mengjuan.cmj@alibaba-inc.com Author: Fujii Masao Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/b029fce3-4fac-4265-968e-16f36ff4d075.mengjuan.cmj@alibaba-inc.com
* Set pg_setting.pending_restart when pertinent config lines are removedAlvaro Herrera2021-07-27
| | | | | | | | | | | | | | | | | This changes the behavior of examining the pg_file_settings view after changing a config option that requires restart. The user needs to know that any change of such options does not take effect until a restart, and this worked correctly if the line is edited without removing it. However, for the case where the line is removed altogether, the flag doesn't get set, because a flag was only set in set_config_option, but that's not called for lines removed. Repair. (Ref.: commits 62d16c7fc561 and a486e35706ea) Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/202107262302.xsfdfc5sb7sh@alvherre.pgsql
* Avoid using ambiguous word "non-negative" in error messages.Fujii Masao2021-07-28
| | | | | | | | | | | | | | | | | | | | | | The error messages using the word "non-negative" are confusing because it's ambiguous about whether it accepts zero or not. This commit improves those error messages by replacing it with less ambiguous word like "greater than zero" or "greater than or equal to zero". Also this commit added the note about the word "non-negative" to the error message style guide, to help writing the new error messages. When postgres_fdw option fetch_size was set to zero, previously the error message "fetch_size requires a non-negative integer value" was reported. This error message was outright buggy. Therefore back-patch to all supported versions where such buggy error message could be thrown. Reported-by: Hou Zhijie Author: Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
* pg_resetxlog: add option to set oldest xid & use by pg_upgradeBruce Momjian2021-07-26
| | | | | | | | | | | | | | | | | | Add pg_resetxlog -u option to set the oldest xid in pg_control. Previously -x set this value be -2 billion less than the -x value. However, this causes the server to immediately scan all relation's relfrozenxid so it can advance pg_control's oldest xid to be inside the autovacuum_freeze_max_age range, which is inefficient and might disrupt diagnostic recovery. pg_upgrade will use this option to better create the new cluster to match the old cluster. Reported-by: Jason Harvey, Floris Van Nee Discussion: https://postgr.es/m/20190615183759.GB239428@rfd.leadboat.com, 87da83168c644fd9aae38f546cc70295@opammb0562.comp.optiver.com Author: Bertrand Drouvot Backpatch-through: 9.6
* Fix a couple of memory leaks in src/bin/pg_basebackup/Michael Paquier2021-07-26
| | | | | | | | | | These have been introduced by 7fbe0c8, and could happen for pg_basebackup and pg_receivewal. Per report from Coverity for the ones in walmethods.c, I have spotted the ones in receivelog.c after more review. Backpatch-through: 10
* Get rid of artificial restriction on hash table sizes on Windows.Tom Lane2021-07-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The point of introducing the hash_mem_multiplier GUC was to let users reproduce the old behavior of hash aggregation, i.e. that it could use more than work_mem at need. However, the implementation failed to get the job done on Win64, where work_mem is clamped to 2GB to protect various places that calculate memory sizes using "long int". As written, the same clamp was applied to hash_mem. This resulted in severe performance regressions for queries requiring a bit more than 2GB for hash aggregation, as they now spill to disk and there's no way to stop that. Getting rid of the work_mem restriction seems like a good idea, but it's a big job and could not conceivably be back-patched. However, there's only a fairly small number of places that are concerned with the hash_mem value, and it turns out to be possible to remove the restriction there without too much code churn or any ABI breaks. So, let's do that for now to fix the regression, and leave the larger task for another day. This patch does introduce a bit more infrastructure that should help with the larger task, namely pg_bitutils.h support for working with size_t values. Per gripe from Laurent Hasson. Back-patch to v13 where the behavior change came in. Discussion: https://postgr.es/m/997817.1627074924@sss.pgh.pa.us Discussion: https://postgr.es/m/MN2PR15MB25601E80A9B6D1BA6F592B1985E39@MN2PR15MB2560.namprd15.prod.outlook.com
* Make the standby server promptly handle interrupt signals.Fujii Masao2021-07-25
| | | | | | | | | | | | | | | | | | | This commit changes the startup process in the standby server so that it handles the interrupt signals after waiting for wal_retrieve_retry_interval on the latch and resetting it, before entering another wait on the latch. This change causes the standby server to promptly handle interrupt signals. Otherwise, previously, there was the case where the standby needs to wait extra five seconds to shutdown when the shutdown request arrived while the startup process was waiting for wal_retrieve_retry_interval on the latch. Author: Fujii Masao, but implementation idea is from Soumyadeep Chakraborty Reviewed-by: Soumyadeep Chakraborty Discussion: https://postgr.es/m/9d7e6ab0-8a53-ddb9-63cd-289bcb25fe0e@oss.nttdata.com Per discussion of BUG #17073, back-patch to all supported versions. Discussion: https://postgr.es/m/17073-1a5fdaed0fa5d4d0@postgresql.org
* Fix check for conflicting session- vs transaction-level locks.Tom Lane2021-07-24
| | | | | | | | | | | | | | | | | | | | | | | We have an implementation restriction that PREPARE TRANSACTION can't handle cases where both session-lifespan and transaction-lifespan locks are held on the same lockable object. (That's because we'd otherwise need to acquire a new PROCLOCK entry during post-prepare cleanup, which is an operation that might fail. The situation can only arise with odd usages of advisory locks, so removing the restriction is probably not worth the amount of effort it would take.) AtPrepare_Locks attempted to enforce this, but its logic was many bricks shy of a load, because it only detected cases where the session and transaction locks had the same lockmode. Locks of different modes on the same object would lead to the rather unhelpful message "PANIC: we seem to have dropped a bit somewhere". To fix, build a transient hashtable with one entry per locktag, not one per locktag + mode, and use that to detect conflicts. Per bug #17122 from Alexander Pyhalov. This bug is ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/17122-04f3c32098a62233@postgresql.org
* Make printf("%s", NULL) print "(null)" instead of crashing.Tom Lane2021-07-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We previously took a hard-line attitude that callers should never print a null string pointer, and doing so is worthy of an assertion failure or crash. However, we've long since flushed out any easy-to-find bugs of that nature. What remains is a lot of code that perhaps could fail that way in hard-to-reach corner cases. For example, in something as simple as ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" for table \"%s\" does not exist", conname, get_rel_name(relid)))); one must wonder whether it's completely guaranteed that get_rel_name cannot return NULL in this context. If such a situation did occur, the existing policy converts what might be a pretty minor bug into a server crash condition. This is not good for robustness. Hence, let's follow the lead of glibc and print "(null)" instead of failing. We should, of course, still consider it a bug if that behavior is reachable in ordinary use; but crashing seems less desirable than not crashing. This fix works across-the-board in v12 and up, where we always use src/port/snprintf.c. Before that, on most platforms we're at the mercy of the local libc, but it appears that Solaris 10 is the only supported platform where we'd still get a crash. Most other platforms such as *BSD, macOS, and Solaris 11 have adopted glibc's behavior at some point. (AIX and HPUX just print "" not "(null)", but that's close enough.) I've not checked what Windows' native printf would do, but it doesn't matter because we've long used snprintf.c on that platform. In v12 and up, also const-ify related code so that we're not casting away const on the constant string. This is just neatnik-ism, since next to no compilers will warn about that. Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org
* jit: Don't inline functions that access thread-locals.Thomas Munro2021-07-22
| | | | | | | | | | | | Code inlined by LLVM can crash or fail with "Relocation type not implemented yet!" if it tries to access thread local variables. Don't inline such code. Back-patch to 11, where LLVM arrived. Bug #16696. Author: Dmitry Marakasov <amdmi3@amdmi3.ru> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/16696-29d944a33801fbfe@postgresql.org
* Document "B" and "us" as accepted units in postgres.conf.sampleJohn Naylor2021-07-21
| | | | | | | | | | | | | | In postgresql.conf, memory and file size GUCs can be specified with "B" (bytes) as of b06d8e58b. Likewise, time GUCs can be specified with "us" (microseconds) as of caf626b2c. Update postgres.conf.sample to reflect that fact. Pavel Luzanov Backpatch to v12, which is the earliest version that allows both of these units. A separate commit will document the "B" case for v11. Discussion: https://www.postgresql.org/message-id/flat/f10d16fc-8fa0-1b3c-7371-cb3a35a13b7a%40postgrespro.ru
* Fix corner-case uninitialized-variable issues in plpgsql.Tom Lane2021-07-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If an error was raised during our initial attempt to check whether a successfully-compiled expression is "simple", subsequent calls of exec_stmt_execsql would suppose that stmt->mod_stmt was already computed when it had not been. This could lead to assertion failures in debug builds; in production builds the effect would typically be to act as if INTO STRICT had been specified even when it had not been. Of course that only matters if the subsequent attempt to execute the expression succeeds, so that the problem can only be reached by fixing a failure in some referenced, inline-able SQL function and then retrying the calling plpgsql function in the same session. (There might be even-more-obscure ways to change the expression's behavior without changing the plpgsql function, but that one seems like the only one people would be likely to hit in practice.) The most foolproof way to fix this would be to arrange for exec_prepare_plan to not set expr->plan until we've finished the subsidiary simple-expression check. But it seems hard to do that without creating reference-count leak issues. So settle for documenting the hazard in a comment and fixing exec_stmt_execsql to test separately for whether it's computed stmt->mod_stmt. (That adds a test-and-branch per execution, but hopefully that's negligible in context.) In v11 and up, also fix exec_stmt_call which had a variant of the same issue. Per bug #17113 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17113-077605ce00e0e7ec@postgresql.org
* Fix some issues with WAL segment opening for pg_receivewal --compressMichael Paquier2021-07-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The logic handling the opening of new WAL segments was fuzzy when using --compress if a partial, non-compressed, segment with the same base name existed in the repository storing those files. In this case, using --compress would cause the code to first check for the existence and the size of a non-compressed segment, followed by the opening of a new compressed, partial, segment. The code was accidentally working correctly on most platforms as the buildfarm has proved, except bowerbird where gzflush() could fail in this code path. It is wrong anyway to take the code path used pre-padding when creating a new partial, non-compressed, segment, so let's fix it. Note that this issue exists when users mix successive runs of pg_receivewal with or without compression, as discovered with the tests introduced by ffc9dda. While on it, this refactors the code so as code paths that need to know about the ".gz" suffix are down from four to one in walmethods.c, easing a bit the introduction of new compression methods. This addresses a second issue where log messages generated for an unexpected failure would not show the compressed segment name involved, which was confusing, printing instead the name of the non-compressed equivalent. Reported-by: Georgios Kokolatos Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz Backpatch-through: 10
* Make new replication slot test code even less racyAlvaro Herrera2021-07-19
| | | | | | | | | Further fix the test code in ead9e51e8236, this time by waiting until the checkpoint has completed before moving on; this ensures that the WAL segment removal has already happened when we create the next slot. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20210719.111318.2042379313472032754.horikyota.ntt@gmail.com
* Don't allow to set replication slot_name as ''.Amit Kapila2021-07-19
| | | | | | | | | | | | | We don't allow to create replication slot_name as an empty string ('') via SQL API pg_create_logical_replication_slot() but it is allowed to be set via Alter Subscription command. This will lead to apply worker repeatedly keep trying to stream data via slot_name '' and the user is not allowed to create the slot with that name. Author: Japin Li Reviewed-By: Ranier Vilela, Amit Kapila Backpatch-through: 10, where it was introduced Discussion: https://postgr.es/m/MEYP282MB1669CBD98E721C77CA696499B61A9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Make new replication slot test code less racyAlvaro Herrera2021-07-17
| | | | | | | | | | | The new test code added in ead9e51e8236 is racy -- it hinges on shared-memory state, which changes before the WARNING message is logged. Put it the other way around. Backpatch to 13. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202107161809.zclasccpfcg3@alvherre.pgsql
* Fix pg_dump for disabled triggers on partitioned tablesAlvaro Herrera2021-07-16
| | | | | | | | | | | | | | | | pg_dump failed to preserve the 'enabled' flag (which can be not only disabled, but also REPLICA or ALWAYS) for partitions which had it changed from their respective parents. Attempt to handle that by including a definition for such triggers in the dump, but replace the standard CREATE TRIGGER line with an ALTER TRIGGER line. Backpatch to 11, where these triggers can exist. In branches 11 and 12, pick up a few test lines from commit b9b408c48724 to verify that pg_upgrade is okay with these arrangements. Co-authored-by: Justin Pryzby <pryzby@telsasoft.com> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com
* Preserve firing-on state when cloning row triggers to partitionsAlvaro Herrera2021-07-16
| | | | | | | | | | | | | | | When triggers are cloned from partitioned tables to their partitions, the 'tgenabled' flag (origin/replica/always/disable) was not propagated. Make it so that the flag on the trigger on partition is initially set to the same value as on the partitioned table. Add a test case to verify the behavior. Backpatch to 11, where this appeared in commit 86f575948c77. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com
* Advance old-segment horizon properly after slot invalidationAlvaro Herrera2021-07-16
| | | | | | | | | | | | | | | | When some slots are invalidated due to the max_slot_wal_keep_size limit, the old segment horizon should move forward to stay within the limit. However, in commit c6550776394e we forgot to call KeepLogSeg again to recompute the horizon after invalidating replication slots. In cases where other slots remained, the limits would be recomputed eventually for other reasons, but if all slots were invalidated, the limits would not move at all afterwards. Repair. Backpatch to 13 where the feature was introduced. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reported-by: Marcin Krupowicz <mk@071.ovh> Discussion: https://postgr.es/m/17103-004130e8f27782c9@postgresql.org
* Ensure HAVE_DECL_XXX macros in MSVC builds match those in Unix.Tom Lane2021-07-15
| | | | | | | | | | | | | | | | Autoconf's AC_CHECK_DECLS() always defines HAVE_DECL_whatever as 1 or 0, but some of the entries in msvc/Solution.pm showed such symbols as "undef" instead of 0. Fix that for consistency. There's no live bug in current usages AFAICS, but it's not hard to imagine one creeping in if more-complex #if tests get added. Back-patch to v13, which is as far back as Solution.pm contains this data. The inconsistency still exists in the manually-filled pg_config_ext.h.win32 files of older branches; but as long as the problem is only latent, it doesn't seem worth the trouble to clean things up there. Discussion: https://postgr.es/m/3185430.1626133592@sss.pgh.pa.us
* Fix unexpected error messages for various flavors of ALTER TABLEMichael Paquier2021-07-14
| | | | | | | | | | | | | | | | | | | | | Some commands of ALTER TABLE could fail with the following error: ERROR: "tab" is of the wrong type This error is unexpected, as all the code paths leading to ATWrongRelkindError() should use a supported set of relkinds to generate correct error messages. This commit closes the gap with such mistakes, by adding all the missing relkind combinations. Tests are added to check all the problems found. Note that some combinations are not used, but these are left around as it could have an impact on applications relying on this code. 2ed532e has done a much larger refactoring on HEAD to make such error messages easier to manage in the long-term, so nothing is needed there. Author: Kyotaro Horiguchi Reviewed-by: Peter Eisentraut, Ahsan Hadi, Michael Paquier Discussion: https://postgr.es/m/20210216.181415.368926598204753659.horikyota.ntt@gmail.com Backpatch-through: 11
* Robustify tuplesort's free_sort_tuple functionDavid Rowley2021-07-13
| | | | | | | | | | | 41469253e went to the trouble of removing a theoretical bug from free_sort_tuple by checking if the tuple was NULL before freeing it. Let's make this a little more robust by also setting the tuple to NULL so that should we be called again we won't end up doing a pfree on the already pfree'd tuple. Per advice from Tom Lane. Discussion: https://postgr.es/m/3188192.1626136953@sss.pgh.pa.us Backpatch-through: 9.6, same as 41469253e
* Fix theoretical bug in tuplesortDavid Rowley2021-07-13
| | | | | | | | | | | | | | | | | | | | | This fixes a theoretical bug in tuplesort.c which, if a bounded sort was used in combination with a byval Datum sort (tuplesort_begin_datum), when switching the sort to a bounded heap in make_bounded_heap(), we'd call free_sort_tuple(). The problem was that when sorting Datums of a byval type, the tuple is NULL and free_sort_tuple() would free the memory for it regardless of that. This would result in a crash. Here we fix that simply by adding a check to see if the tuple is NULL before trying to disassociate and free any memory belonging to it. The reason this bug is only theoretical is that nowhere in the current code base do we do tuplesort_set_bound() when performing a Datum sort. However, let's backpatch a fix for this as if any extension uses the code in this way then it's likely to cause problems. Author: Ronan Dunklau Discussion: https://postgr.es/m/CAApHDvpdoqNC5FjDb3KUTSMs5dg6f+XxH4Bg_dVcLi8UYAG3EQ@mail.gmail.com Backpatch-through: 9.6, oldest supported version
* Remove dead assignment to local variable.Heikki Linnakangas2021-07-12
| | | | | | | | | | | | This should have been removed in commit 7e30c186da, which split the loop into two. Only the first loop uses the 'from' variable; updating it in the second loop is bogus. It was never read after the first loop, so this was harmless and surely optimized away by the compiler, but let's be tidy. Backpatch to all supported versions. Author: Ranier Vilela Discussion: https://www.postgresql.org/message-id/CAEudQAoWq%2BAL3BnELHu7gms2GN07k-np6yLbukGaxJ1vY-zeiQ%40mail.gmail.com
* Lock the extension during ALTER EXTENSION ADD/DROP.Tom Lane2021-07-11
| | | | | | | | | | | | | | | | | Although we were careful to lock the object being added or dropped, we failed to get any sort of lock on the extension itself. This allowed the ALTER to proceed in parallel with a DROP EXTENSION, which is problematic for a couple of reasons. If both commands succeeded we'd be left with a dangling link in pg_depend, which would cause problems later. Also, if the ALTER failed for some reason, it might try to print the extension's name, and that could result in a crash or (in older branches) a silly error message complaining about extension "(null)". Per bug #17098 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org
* Fix assign_record_type_typmod().Jeff Davis2021-07-10
| | | | | | | | | | | | | | | | | | If an error occurred in the wrong place, it was possible to leave an unintialized entry in the hash table, leading to a crash. Fixed. Also, be more careful about the order of operations so that an allocation error doesn't leak memory in CacheMemoryContext or unnecessarily advance NextRecordTypmod. Backpatch through version 11. Earlier versions (prior to 35ea75632a5) do not exhibit the problem, because an uninitialized hash entry contains a valid empty list. Author: Sait Talha Nisanci <Sait.Nisanci@microsoft.com> Reviewed-by: Andres Freund Discussion: https://postgr.es/m/HE1PR8303MB009069D476225B9A9E194B8891779@HE1PR8303MB0090.EURPRD83.prod.outlook.com Backpatch-through: 11
* Fix numeric_mul() overflow due to too many digits after decimal point.Dean Rasheed2021-07-10
| | | | | | | | | This fixes an overflow error when using the numeric * operator if the result has more than 16383 digits after the decimal point by rounding the result. Overflow errors should only occur if the result has too many digits *before* the decimal point. Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
* Avoid creating a RESULT RTE that's marked LATERAL.Tom Lane2021-07-09
| | | | | | | | | | | | | | | Commit 7266d0997 added code to pull up simple constant function results, converting the RTE_FUNCTION RTE to a dummy RTE_RESULT RTE since it no longer need be scanned. But I forgot to clear the LATERAL flag if the RTE has it set. If the function reduced to a constant, it surely contains no lateral references so this simplification is logically OK. It's needed because various other places will Assert that RESULT RTEs aren't LATERAL. Per bug #17097 from Yaoguang Chen. Back-patch to v13 where the faulty code came in. Discussion: https://postgr.es/m/17097-3372ef9f798fc94f@postgresql.org
* Update configure's probe for libldap to work with OpenLDAP 2.5.Tom Lane2021-07-09
| | | | | | | | | | | | | | | The separate libldap_r is gone and libldap itself is now always thread-safe. Unfortunately there seems no easy way to tell by inspection whether libldap is thread-safe, so we have to take it on faith that libldap is thread-safe if there's no libldap_r. That should be okay, as it appears that libldap_r was a standard part of the installation going back at least 20 years. Report and patch by Adrian Ho. Back-patch to all supported branches, since people might try to build any of them with a newer OpenLDAP. Discussion: https://postgr.es/m/17083-a19190d9591946a7@postgresql.org
* Reject cases where a query in WITH rewrites to just NOTIFY.Tom Lane2021-07-09
| | | | | | | | | | | | | | | | | Since the executor can't cope with a utility statement appearing as a node of a plan tree, we can't support cases where a rewrite rule inserts a NOTIFY into an INSERT/UPDATE/DELETE command appearing in a WITH clause of a larger query. (One can imagine ways around that, but it'd be a new feature not a bug fix, and so far there's been no demand for it.) RewriteQuery checked for this, but it missed the case where the DML command rewrites to *only* a NOTIFY. That'd lead to crashes later on in planning. Add the missed check, and improve the level of testing of this area. Per bug #17094 from Yaoguang Chen. It's been busted since WITH was introduced, so back-patch to all supported branches. Discussion: https://postgr.es/m/17094-bf15dff55eaf2e28@postgresql.org
* Remove more obsolete comments about semaphores.Thomas Munro2021-07-09
| | | | | | | | | Commit 6753333f stopped using semaphores as the sleep/wake mechanism for heavyweight locks, but some obsolete references to that scheme remained in comments. As with similar commit 25b93a29, back-patch all the way. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CA%2BhUKGLafjB1uzXcy%3D%3D2L3cy7rjHkqOVn7qRYGBjk%3D%3DtMJE7Yg%40mail.gmail.com
* Add missing Int64GetDatum macro in dbsize.cDavid Rowley2021-07-09
| | | | | | | | I accidentally missed adding this when adjusting 55fe60938 for back patching. This adjustment was made for 9.6 to 13. 14 and master are not affected. Discussion: https://postgr.es/m/CAApHDvp=twCsGAGQG=A=cqOaj4mpknPBW-EZB-sd+5ZS5gCTtA@mail.gmail.com
* Fix incorrect return value in pg_size_pretty(bigint)David Rowley2021-07-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Due to how pg_size_pretty(bigint) was implemented, it's possible that when given a negative number of bytes that the returning value would not match the equivalent positive return value when given the equivalent positive number of bytes. This was due to two separate issues. 1. The function used bit shifting to convert the number of bytes into larger units. The rounding performed by bit shifting is not the same as dividing. For example -3 >> 1 = -2, but -3 / 2 = -1. These two operations are only equivalent with positive numbers. 2. The half_rounded() macro rounded towards positive infinity. This meant that negative numbers rounded towards zero and positive numbers rounded away from zero. Here we fix #1 by dividing the values instead of bit shifting. We fix #2 by adjusting the half_rounded macro always to round away from zero. Additionally, adjust the pg_size_pretty(numeric) function to be more explicit that it's using division rather than bit shifting. A casual observer might have believed bit shifting was used due to a static function being named numeric_shift_right. However, that function was calculating the divisor from the number of bits and performed division. Here we make that more clear. This change is just cosmetic and does not affect the return value of the numeric version of the function. Here we also add a set of regression tests both versions of pg_size_pretty() which test the values directly before and after the function switches to the next unit. This bug was introduced in 8a1fab36a. Prior to that negative values were always displayed in bytes. Author: Dean Rasheed, David Rowley Discussion: https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.com Backpatch-through: 9.6, where the bug was introduced.
* Reduce overhead of cache-clobber testing in LookupOpclassInfo().Tom Lane2021-07-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 03ffc4d6d added logic to bypass all caching behavior in LookupOpclassInfo when CLOBBER_CACHE_ALWAYS is enabled. It doesn't look like I stopped to think much about what that would cost, but recent investigation shows that the cost is enormous: it roughly doubles the time needed for cache-clobber test runs. There does seem to be value in this behavior when trying to test the opclass-cache loading logic itself, but for other purposes the cost is excessive. Hence, let's back off to doing this only when debug_invalidate_system_caches_always is at least 3; or in older branches, when CLOBBER_CACHE_RECURSIVELY is defined. While here, clean up some other minor issues in LookupOpclassInfo. Re-order the code so we aren't left with broken cache entries (leading to later core dumps) in the unlikely case that we suffer OOM while trying to allocate space for a new entry. (That seems to be my oversight in 03ffc4d6d.) Also, in >= v13, stop allocating one array entry too many. That's evidently left over from sloppy reversion in 851b14b0c. Back-patch to all supported branches, mainly to reduce the runtime of cache-clobbering buildfarm animals. Discussion: https://postgr.es/m/1370856.1625428625@sss.pgh.pa.us
* Don't try to print data type names in slot_store_error_callback().Tom Lane2021-07-02
| | | | | | | | | | | | | | | | | | | | | | The existing code tried to do syscache lookups in an already-failed transaction, which is problematic to say the least. After some consideration of alternatives, the best fix seems to be to just drop type names from the error message altogether. The table and column names seem like sufficient localization. If the user is unsure what types are involved, she can check the local and remote table definitions. Having done that, we can also discard the LogicalRepTypMap hash table, which had no other use. Arguably, LOGICAL_REP_MSG_TYPE replication messages are now obsolete as well; but we should probably keep them in case some other use emerges. (The complexity of removing something from the replication protocol would likely outweigh any savings anyhow.) Masahiko Sawada and Bharath Rupireddy, per complaint from Andres Freund. Back-patch to v10 where this code originated. Discussion: https://postgr.es/m/20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
* Fix prove_installcheck to use correct paths when used with PGXSAndrew Dunstan2021-07-01
| | | | | | | | | | | | | | The prove_installcheck recipe in src/Makefile.global.in was emitting bogus paths for a couple of elements when used with PGXS. Here we create a separate recipe for the PGXS case that does it correctly. We also take the opportunity to make the make the file more readable by breaking up the prove_installcheck and prove_check recipes across several lines, and to remove the setting for REGRESS_SHLIB to src/test/recovery/Makefile, which is the only set of tests that actually need it. Backpatch to all live branches Discussion: https://postgr.es/m/f2401388-936b-f4ef-a07c-a0bcc49b3300@dunslane.net
* Fix incorrect PITR message for transaction ROLLBACK PREPAREDMichael Paquier2021-06-30
| | | | | | | | | | | Reaching PITR on such a transaction would cause the generation of a LOG message mentioning a transaction committed, not aborted. Oversight in 4f1b890. Author: Simon Riggs Discussion: https://postgr.es/m/CANbhV-GJ6KijeCgdOrxqMCQ+C8QiK657EMhCy4csjrPcEUFv_Q@mail.gmail.com Backpatch-through: 9.6
* Don't use abort(3) in libpq's fe-print.c.Tom Lane2021-06-28
| | | | | | | | | | | | | | | | Causing a core dump on out-of-memory seems pretty unfriendly, and surely is far outside the expected behavior of a general-purpose library. Just print an error message (as we did already) and return. These functions unfortunately don't have an error return convention, but code using them is probably just looking for a quick-n-dirty print method and wouldn't bother to check anyway. Although these functions are semi-deprecated, it still seems appropriate to back-patch this. In passing, also back-patch b90e6cef1, just to reduce cosmetic differences between the branches. Discussion: https://postgr.es/m/3122443.1624735363@sss.pgh.pa.us
* Don't depend on -fwrapv semantics in pgbench's random() function.Tom Lane2021-06-28
| | | | | | | | | | | | | | | | | | | | | | | Instead use the common/int.h functions to check for integer overflow in a more C-standard-compliant fashion. This is motivated by recent failures on buildfarm member moonjelly, where it appears that development-tip gcc is optimizing without regard to the -fwrapv switch. Presumably that's a gcc bug that will be fixed soon, but we might as well install cleaner coding here rather than wait. (This does not address the question of whether we'll ever be able to get rid of using -fwrapv. Testing shows that this spot is the only place where doing so creates visible regression test failures, but unfortunately that proves very little.) Back-patch to v12. The common/int.h functions exist in v11, but that branch doesn't use them in any client-side code. I judge that this case isn't interesting enough in the real world to take even a small risk of issues from being the first such use. Tom Lane and Fabien Coelho Discussion: https://postgr.es/m/73927.1624815543@sss.pgh.pa.us
* Fix race condition in TransactionGroupUpdateXidStatus().Amit Kapila2021-06-28
| | | | | | | | | | | | | | | | | | | When we cannot immediately acquire XactSLRULock in exclusive mode at commit time, we add ourselves to a list of processes that need their XIDs status update. We do this if the clog page where we need to update the current transaction status is the same as the group leader's clog page, otherwise, we allow the caller to clear it by itself. Now, when we can't add ourselves to any group, we were not clearing the current proc if it has already become a member of some group which was leading to an assertion failure when the same proc was assigned to another backend after the current backend exits. Reported-by: Alexander Lakhin Bug: 17072 Author: Amit Kapila Tested-By: Alexander Lakhin Backpatch-through: 11, where it was introduced Discussion: https://postgr.es/m/17072-2f8764857ef2c92a@postgresql.org
* Add test for CREATE INDEX CONCURRENTLY with not-so-immutable predicateMichael Paquier2021-06-28
| | | | | | | | | | | | 83158f7 has improved index_set_state_flags() so as it is possible to use transactional updates when updating pg_index state flags, but there was not really a test case which stressed directly the possibility it fixed. This commit adds such a test, using a predicate that looks valid in appearance but calls a stable function. Author: Andrey Lepikhov Discussion: https://postgr.es/m/9b905019-5297-7372-0ad2-e1a4bb66a719@postgrespro.ru Backpatch-through: 9.6
* Make index_set_state_flags() transactionalMichael Paquier2021-06-28
| | | | | | | | | | | | | | | | | | | | | 3c84046 is the original commit that introduced index_set_state_flags(), where the presence of SnapshotNow made necessary the use of an in-place update. SnapshotNow has been removed in 813fb03, so there is no actual reasons to not make this operation transactional. As reported by Andrey, it is possible to trigger the assertion of this routine expecting no transactional updates when switching the pg_index state flags, using a predicate mark as immutable but calling stable or volatile functions. 83158f7 has been around for a couple of months on HEAD now with no issues found related to it, so it looks safe enough for a backpatch. Reported-by: Andrey Lepikhov Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200903080440.GA8559@paquier.xyz Discussion: https://postgr.es/m/9b905019-5297-7372-0ad2-e1a4bb66a719@postgrespro.ru Backpatch-through: 9.6
* Remove memory leaks in isolationtester.Tom Lane2021-06-27
| | | | | | | | | | | | | | | specscanner.l leaked a kilobyte of memory per token of the spec file. Apparently somebody thought that the introductory code block would be executed once; but it's once per yylex() call. A couple of functions in isolationtester.c leaked small amounts of memory due to not bothering to free one-time allocations. Might as well improve these so that valgrind gives this program a clean bill of health. Also get rid of an ugly static variable. Coverity complained about one of the one-time leaks, which led me to try valgrind'ing isolationtester, which led to discovery of the larger leak.
* Remove non-existing variable reference in MSVC's Solution.pmMichael Paquier2021-06-26
| | | | | | | | | | | The version string is grabbed from PACKAGE_VERSION in pg_config.h in the MSVC build since 8f4fb4c6, but an error message referenced a variable that existed before that. This had no consequences except if one messes up enough with the version number of the build. Author: Anton Voloshin Discussion: https://postgr.es/m/af79ee1b-9962-b299-98e1-f90a289e19e6@postgrespro.ru Backpatch-through: 13
* Remove some useless logs from the TAP tests of pgbenchMichael Paquier2021-06-26
| | | | | | | | | | | | | | | | 002_pgbench_no_server was printing some array pointers instead of the actual contents of those arrays for the expected outputs of stdout and stderr for a tested command. This does not add any new information that can help with debugging as the test names allow to track failure locations, if any. This commit simply removes those logs as the rest of the printed information is redundant with command_checks_all(). Per discussion with Andrew Dunstan and Álvaro Herrera. Discussion: https://postgr.es/m/YNXNFaG7IgkzZanD@paquier.xyz Backpatch-through: 11
* Remove unnecessary failure cases in RemoveRoleFromObjectPolicy().Tom Lane2021-06-25
| | | | | | | | | | | | | | | | | | | | | | | | It's not really necessary for this function to open or lock the relation associated with the pg_policy entry it's modifying. The error checks it's making on the rel are if anything counterproductive (e.g., if we don't want to allow installation of policies on system catalogs, here is not the place to prevent that). In particular, it seems just wrong to insist on an ownership check. That has the net effect of forcing people to use superuser for DROP OWNED BY, which surely is not an effect we want. Also there is no point in rebuilding the dependencies of the policy expressions, which aren't being changed. Lastly, locking the table also seems counterproductive; it's not helping to prevent race conditions, since we failed to re-read the pg_policy row after acquiring the lock. That means that concurrent DDL would likely result in "tuple concurrently updated/deleted" errors; which is the same behavior this code will produce, with less overhead. Per discussion of bug #17062. Back-patch to all supported versions, as the failure cases this eliminates seem just as undesirable in 9.6 as in HEAD. Discussion: https://postgr.es/m/1573181.1624220108@sss.pgh.pa.us
* Make walsenders show their replication commands in pg_stat_activity.Tom Lane2021-06-25
| | | | | | | | | | | | | | | | | | | | | | | | | | A walsender process that has executed a SQL command left the text of that command in pg_stat_activity.query indefinitely, which is quite confusing if it's in RUNNING state but not doing that query. An easy and useful fix is to treat replication commands as if they were SQL queries, and show them in pg_stat_activity according to the same rules as for regular queries. While we're at it, it seems also sensible to set debug_query_string, allowing error logging and debugging to see the replication command. While here, clean up assorted silliness in exec_replication_command: * Clean up SQLCmd code path, and fix its only-accidentally-not-buggy memory management. * Remove useless duplicate call of SnapBuildClearExportedSnapshot(). * replication_scanner_finish() was never called. Back-patch of commit f560209c6 into v10-v13. I'd originally felt that this didn't merit back-patching, but subsequent confusion while debugging walsender problems suggests that it'll be useful. Also, the original commit has now aged long enough to provide some comfort that it won't cause problems. Discussion: https://postgr.es/m/2673480.1624557299@sss.pgh.pa.us Discussion: https://postgr.es/m/880181.1600026471@sss.pgh.pa.us
* Cleanup some code related to pgbench log checks in TAP testsMichael Paquier2021-06-25
| | | | | | | | | | | | | | | | | | This fixes a couple of problems within the so-said code of this commit subject: - Replace the use of open() with slurp_file(), fixing an issue reported by buildfarm member fairywren whose perl installation keep around CRLF characters, causing the matching patterns for the logs to fail. - Remove the eval block, which is not really necessary. This set of issues has come into light after fixing a different issue with c13585fe, and this is wrong since this code has been introduced. Reported-by: Andrew Dunstan, and buildfarm member fairywren Author: Michael Paquier Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/0f49303e-7784-b3ee-200b-cbf67be2eb9e@dunslane.net Backpatch-through: 11
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-30 10:37:07 +0000