aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Introduce transaction_timeoutAlexander Korotkov2024-02-15
| | | | | | | | | | | | | | | | | | | This commit adds timeout that is expected to be used as a prevention of long-running queries. Any session within the transaction will be terminated after spanning longer than this timeout. However, this timeout is not applied to prepared transactions. Only transactions with user connections are affected. Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com Author: Andrey Borodin <amborodin@acm.org> Author: Japin Li <japinli@hotmail.com> Author: Junwang Zhao <zhjwpku@gmail.com> Reviewed-by: Nikolay Samokhvalov <samokhvalov@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: bt23nguyent <bt23nguyent@oss.nttdata.com> Reviewed-by: Yuhang Qiu <iamqyh@gmail.com>
* Doc: improve a couple of comments in postgresql.conf.sample.Tom Lane2024-02-15
| | | | | | | | | Clarify comments associated with max_parallel_workers and related settings. Per bug #18343 from Christopher Kline. Discussion: https://postgr.es/m/18343-3a5e903d1d3692ab@postgresql.org
* Pull up ANY-SUBLINK with the necessary lateral support.Alexander Korotkov2024-02-15
| | | | | | | | | | | | | | | | For ANY-SUBLINK, we adopted a two-stage pull-up approach to handle different types of scenarios. In the first stage, the sublink is pulled up as a subquery. Because of this, when writing this code, we did not have the ability to perform lateral joins, and therefore, we were unable to pull up Var with varlevelsup=1. Now that we have the ability to use lateral joins, we can eliminate this limitation. Author: Andy Fan <zhihui.fan1213@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru> Reviewed-by: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
* Allow passing extra options to initdb for testsPeter Eisentraut2024-02-15
| | | | | | | | | | | | | | | Setting the environment variable PG_TEST_INITDB_EXTRA_OPTS passes extra options to initdb run by pg_regress or PostgreSQL::Test::Cluster's init. This can be useful for a wide variety of uses, like running all tests with checksums enabled, or with JIT enabled, or with different GUC settings, or with different locale settings. (Not all tests are going to pass with arbitrary options, but it is useful to run this against specific test suites.) Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/d4d2ad9f-1c1d-47a1-bb4d-c10a747d4f15%40eisentraut.org
* Another try to fix BF failure introduced in commit ddd5f4f54a.Amit Kapila2024-02-15
| | | | | | | | | | Before attempting to sync the slot on standby by pg_sync_replication_slots(), ensure that on the primary restart_lsn for the slot has moved to a recent WAL position, by re-creating the subscription and the logical slot. Author: Hou Zhijie Discussion: https://postgr.es/m/CAA4eK1+d5Lne8vCAn0un4SP9x-ZBr2-xfxg01uSfeBTSCKFZoQ@mail.gmail.com
* Simplify PathKey checking codeDavid Rowley2024-02-15
| | | | | | | | | | | | | | | | | | | | | | pathkeys_useful_for_ordering() contained some needless checks to return 0 when either root->query_pathkeys or pathkeys lists were empty. This is already handled by pathkeys_count_contained_in(), so let's have it do the work instead of having redundant checks. Similarly, in pathkeys_useful_for_grouping(), checking pathkeys is an empty list just before looping over it isn't required. Technically, neither is the list empty check for group_pathkeys, but I felt a bit more work would have to be done to get the equivalent behavior if we'd left it up to the foreach loop to call list_member_ptr(). This was noticed by Andy while he was reviewing a patch to improve the UNION planner. Since that patch adds another function similar to pathkeys_useful_for_ordering() and since I wasn't planning to copy these redundant checks over to the new function, let's adjust the existing code so that both functions will be consistent. Author: Andy Fan Discussion: https://postgr.es/m/87o7cti48f.fsf@163.com
* Clarify the 'rows' parameter in create_append_pathDavid Rowley2024-02-15
| | | | | | | | | | This is extracted from a larger patch to improve the UNION planner. While working on that, I found myself having to check what the 'rows' parameter is for. It's not obvious that passing a negative number is the way to have the rows estimate calculated and to find that out you need to read code in create_append_path() and in cost_append(). Discussion: https://postgr.es/m/CAApHDvpb_63XQodmxKUF8vb9M7CxyUyT4sWvEgqeQU-GB7QFoQ@mail.gmail.com
* Remove obsolete check in SIGTERM handler for the startup process.Nathan Bossart2024-02-14
| | | | | | | | | | | | Thanks to commit 3b00fdba9f, this check in the SIGTERM handler for the startup process is now obsolete and can be removed. Instead of leaving around the dead function write_stderr_signal_safe(), I've opted to just remove it for now. This partially reverts commit 97550c0711. Reviewed-by: Andres Freund, Noah Misch Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
* Centralize logic for restoring errno in signal handlers.Nathan Bossart2024-02-14
| | | | | | | | | | | | | | | | Presently, we rely on each individual signal handler to save the initial value of errno and then restore it before returning if needed. This is easily forgotten and, if missed, often goes undetected for a long time. In commit 3b00fdba9f, we introduced a wrapper signal handler function that checks whether MyProcPid matches getpid(). This commit moves the aforementioned errno restoration code from the individual signal handlers to the new wrapper handler so that we no longer need to worry about missing it. Reviewed-by: Andres Freund, Noah Misch Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
* Check that MyProcPid == getpid() in backend signal handlers.Nathan Bossart2024-02-14
| | | | | | | | | | | | | | | | | | | | | In commit 97550c0711, we added a similar check to the SIGTERM handler for the startup process. This commit adds this check to backend signal handlers installed with pqsignal(). This is done by using a wrapper function that performs the check before calling the actual handler. The hope is that this will offer more general protection against child processes of Postgres backends inadvertently modifying shared memory due to inherited signal handlers. Another potential follow-up improvement is to use this wrapper handler function to restore errno instead of relying on each individual handler function to do so. This commit makes the changes in commit 97550c0711 obsolete but leaves reverting it for a follow-up commit. Reviewed-by: Andres Freund, Noah Misch Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
* Allow pg_monitor to execute pg_current_logfile().Nathan Bossart2024-02-14
| | | | | | | | | | | | We allow roles with privileges of pg_monitor to execute functions like pg_ls_logdir(), so it seems natural that such roles would also be able to execute this function. Bumps catversion. Co-authored-by: Pavlo Golub Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CAK7ymcLmEYWyQkiCZ64WC-HCzXAB0omM%3DYpj9B3rXe8vUAFMqw%40mail.gmail.com
* Fix multiranges to behave more like dependent types.Tom Lane2024-02-14
| | | | | | | | | | | | | | | | | | | | | For most purposes, multiranges act like dependent objects of the associated range type: you can't create them separately or drop them separately. This is like the way that autogenerated array types behave. However, a couple of points were overlooked: array types automatically track the ownership of their base type, and array types do not have their own permissions but use those of the base type, while multiranges didn't emulate those behaviors. This is fairly broken, mainly because pg_dump doesn't think it needs to worry about multiranges as separate objects, and thus it fails to dump/restore ownership or permissions of multiranges. There's no apparent value in letting a multirange diverge from its parent's ownership or permissions, so let's make them act like arrays in these respects. However, we continue to let multiranges be renamed or moved to a different schema independently of their parent, since that doesn't break anything. Discussion: https://postgr.es/m/1580383.1705343264@sss.pgh.pa.us
* Fix BF introduced in commit ddd5f4f54a.Amit Kapila2024-02-14
| | | | | | | | | | | The failure is that the remote slot is not synchronized after the same slot on standby gets invalidated. The reason was that remote_slot's restart_lsn was lagged behind the standby's oldest WAL segment. The test didn't ensure that remote_slot's LSN was advanced to the latest position before we tried to sync the slots via new the function pg_sync_replication_slots(). Discussion: https://postgr.es/m/CAA4eK1JLBi3HzenB6do3_hd78kN0UDD1mz-vumWE52XHHEq5Bw@mail.gmail.com
* Add a slot synchronization function.Amit Kapila2024-02-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit introduces a new SQL function pg_sync_replication_slots() which is used to synchronize the logical replication slots from the primary server to the physical standby so that logical replication can be resumed after a failover or planned switchover. A new 'synced' flag is introduced in pg_replication_slots view, indicating whether the slot has been synchronized from the primary server. On a standby, synced slots cannot be dropped or consumed, and any attempt to perform logical decoding on them will result in an error. The logical replication slots on the primary can be synchronized to the hot standby by using the 'failover' parameter of pg-create-logical-replication-slot(), or by using the 'failover' option of CREATE SUBSCRIPTION during slot creation, and then calling pg_sync_replication_slots() on standby. For the synchronization to work, it is mandatory to have a physical replication slot between the primary and the standby aka 'primary_slot_name' should be configured on the standby, and 'hot_standby_feedback' must be enabled on the standby. It is also necessary to specify a valid 'dbname' in the 'primary_conninfo'. If a logical slot is invalidated on the primary, then that slot on the standby is also invalidated. If a logical slot on the primary is valid but is invalidated on the standby, then that slot is dropped but will be recreated on the standby in the next pg_sync_replication_slots() call provided the slot still exists on the primary server. It is okay to recreate such slots as long as these are not consumable on standby (which is the case currently). This situation may occur due to the following reasons: - The 'max_slot_wal_keep_size' on the standby is insufficient to retain WAL records from the restart_lsn of the slot. - 'primary_slot_name' is temporarily reset to null and the physical slot is removed. The slot synchronization status on the standby can be monitored using the 'synced' column of pg_replication_slots view. A functionality to automatically synchronize slots by a background worker and allow logical walsenders to wait for the physical will be done in subsequent commits. Author: Hou Zhijie, Shveta Malik, Ajin Cherian based on an earlier version by Peter Eisentraut Reviewed-by: Masahiko Sawada, Bertrand Drouvot, Peter Smith, Dilip Kumar, Nisha Moond, Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
* Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"Michael Paquier2024-02-14
| | | | | | | | | | | | | | This reverts commit 95fb5b49024, for reasons similar to what led to 1aa8324b81fa. In this case, the callback was called once per row, which is less worse than the previous callback introduced for COPY TO called once per argument for each row, still the patch set discussed to plug in custom routines to the COPY paths would be able to know which subroutine to use depending on its CopyFromState, so this led to a suboptimal approach at the end. For now, this part is reverted to consider better which approach to use. Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de
* Improve comment about query_id_enabled in queryjumblefuncs.cMichael Paquier2024-02-14
| | | | | | | | | | | | | The comment was inexact because query_id_enabled will not be switched to "true" even if compute_query_id is "on", unless a module requests for it. While on it, this adds a comment to mention that IsQueryIdEnabled() should be used to check if query ID computation is enabled or not. Author: Yugo Nagata Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/20240209153823.e29a68cadb14225f1362a2cf@sraoss.co.jp
* Catch overflow when rounding intervals in AdjustIntervalForTypmod.Tom Lane2024-02-13
| | | | | | | | | | | | | Previously, an interval microseconds field close to INT64_MAX or INT64_MIN could overflow, producing a result with not even the correct sign, while being rounded to match a precision specification. This seems worth fixing, but not worth back-patching, in part because the ereturn() notation doesn't exist very far back. Report and patch by Joseph Koshakow (some cosmetic mods by me) Discussion: https://postgr.es/m/CAAvxfHfpuLgqJYzkUcher466Z1LpmE+5Sm+zc8L6zKCOQ+6TDQ@mail.gmail.com
* Fix 'mmap' DSM implementation with allocations larger than 4 GBHeikki Linnakangas2024-02-13
| | | | | | Fixes bug #18341. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/18341-ce16599e7fd6228c@postgresql.org
* Use a safer outfuncs/readfuncs representation for BitStrings.Tom Lane2024-02-13
| | | | | | | | | | | | | | | | | | | | | | | | For a long time, our outfuncs.c code has supposed that the string contents of a BitString node could just be printed literally with no concern for quoting/escaping. Now, that's okay if the string literal contains only valid binary or hex digits ... but our lexer doesn't check that, preferring to let bitin() be the sole authority on what's valid. So we could have raw parse trees that contain incorrect BitString literals, and that can result in failures when WRITE_READ_PARSE_PLAN_TREES debugging is enabled. Fix by using outToken() to print the string field, and debackslash() to read it. This results in a change in the emitted representation only in cases that would have failed before, and don't represent valid SQL in the first place. Between that and the fact that we don't store raw parse trees in the catalogs, I judge this safe to apply without a catversion bump. Per bug #18340 from Alexander Lakhin. Back-patch to v16; before that, we lacked readfuncs support for BitString nodes, so that the problem was only cosmetic. Discussion: https://postgr.es/m/18340-4aa1ae6ed4121912@postgresql.org
* Skip .DS_Store files in server side utilsDaniel Gustafsson2024-02-13
| | | | | | | | | | | | | | | | | | The macOS Finder application creates .DS_Store files in directories when opened, which creates problems for serverside utilities which expect all files to be PostgreSQL specific files. Skip these files when encountered in pg_checksums, pg_rewind and pg_basebackup. This was extracted from a larger patchset for skipping hidden files and system files, where the concencus was to just skip these. Since this is equally likely to happen in every version, backpatch to all supported versions. Reported-by: Mark Guertin <markguertin@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Tobias Bussmann <t.bussmann@gmx.net> Discussion: https://postgr.es/m/E258CE50-AB0E-455D-8AAD-BB4FE8F882FB@gmail.com Backpatch-through: v12
* Use correct format placeholder for timeline IDsPeter Eisentraut2024-02-13
| | | | Should be %u rather than %d.
* Read WAL directly from WAL buffers.Jeff Davis2024-02-12
| | | | | | | | | | | | If available, read directly from WAL buffers, avoiding the need to go through the filesystem. Only for physical replication for now, but can be expanded to other callers. In preparation for replicating unflushed WAL data. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACXKKK%3DwbiG5_t6dGao5GoecMwRkhr7GjVBM_jg54%2BNa%3DQ%40mail.gmail.com Reviewed-by: Andres Freund, Alvaro Herrera, Nathan Bossart, Dilip Kumar, Nitin Jadhav, Melih Mutlu, Kyotaro Horiguchi
* Remove "#ifdef WIN32" guards from src/port/win32*.cHeikki Linnakangas2024-02-12
| | | | | | | | | These files are only compiled on Windows, and most of them didn't have "#ifdef WIN32" guards. Remove them from the few that did, for consistency. Author: Tristan Partin Discussion: https://www.postgresql.org/message-id/CXGM9RYSXA2J.1DBO4MRXGZA9P@neon.tech
* Remove unnecessary smgropen() callsHeikki Linnakangas2024-02-12
| | | | | | | | Now that RelationCreateStorage() returns the SmgrRelation (since commit 5c1560606dc), use that. Author: Japin Li Discussion: https://www.postgresql.org/message-id/ME3P282MB316600FA62F6605477F26F6AB6742@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
* Use heap_inplace_update() to unset pg_database.dathasloginevtAlexander Korotkov2024-02-12
| | | | | | | | | | | | | Doing this instead of regular updates serves two purposes. First, that avoids possible waiting on the row-level lock. Second, that avoids dealing with TOAST. It's known that changes made by heap_inplace_update() may be lost due to concurrent normal updates. However, we are OK with that. The subsequent connections will still have a chance to set "dathasloginevt" to false. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/e2a0248e-5f32-af0c-9832-a90d303c2c61%40gmail.com
* Remove obsolete script related to MSVC build systemPeter Eisentraut2024-02-11
|
* Fix gai_strerror() thread-safety on Windows.Thomas Munro2024-02-12
| | | | | | | | | | | | | | | | | | | | Commit 5579388d removed code that supplied a fallback implementation of getaddrinfo(), which was dead code on modern systems. One tiny piece of the removed code was still doing something useful on Windows, though: that OS's own gai_strerror()/gai_strerrorA() function returns a pointer to a static buffer that it overwrites each time, so it's not thread-safe. In rare circumstances, a multi-threaded client program could get an incorrect or corrupted error message. Restore the replacement gai_strerror() function, though now that it's only for Windows we can put it into a win32-specific file and cut it down to the errors that Windows documents. The error messages here are taken from FreeBSD, because Windows' own messages seemed too verbose. Back-patch to 16. Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGKz%2BF9d2PTiXwfYV7qJw%2BWg2jzACgSDgPizUw7UG%3Di58A%40mail.gmail.com
* Use extensible buffers to assemble command linesPeter Eisentraut2024-02-11
| | | | | | | | | | | | | This makes use of StringInfo to assemble command lines, instead of using fixed-size buffers and the (remote) possibility of "command too long" errors. Also makes the code a bit simpler. This covers the test driver programs pg_regress and pg_isolation_regress. Similar to the changes done for pg_rewind in a33e17f210. Discussion: https://www.postgresql.org/message-id/2be4fee5-738f-4749-b9f8-b452032c7ade%40eisentraut.org
* Disallow jsonpath methods involving TZ in immutable functionsAndrew Dunstan2024-02-10
| | | | | | | | | | | Timezones are not immutable and so neither is any function that relies on them. In commit 66ea94e8, we introduced a few methods which do casting from one time to another and thus may involve the current timezone. To preserve the immutability of jsonpath functions currently marked immutable, disallow these methods from being called from non-TZ aware functions. Jeevan Chalke, per a report from Jian He.
* Remove race condition in pg_get_expr().Tom Lane2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | Since its introduction, pg_get_expr() has intended to silently return NULL if called with an invalid relation OID, as can happen when scanning the catalogs concurrently with relation drops. However, there is a race condition: we check validity of the OID at the start, but it could get dropped just afterward, leading to failures. This is the cause of some intermittent instability we're seeing in a proposed new test case, and presumably it's a hazard in the field as well. We can fix this by AccessShareLock-ing the target relation for the duration of pg_get_expr(). Since we don't require any permissions on the target relation, this is semantically a bit undesirable. But it turns out that the set_relation_column_names() subroutine already takes a transient AccessShareLock on that relation, and has done since commit 2ffa740be in 2012. Given the lack of complaints about that, it seems like there should be no harm in holding the lock a bit longer. Back-patch to all supported branches. Discussion: https://postgr.es/m/31ddcc01-a71b-4e8c-9948-01d1c47293ca@eisentraut.org
* Avoid concurrent calls to bindtextdomain().Tom Lane2024-02-09
| | | | | | | | | | | | | | | | | | | | | | We previously supposed that it was okay for different threads to call bindtextdomain() concurrently (cf. commit 1f655fdc3). It now emerges that there's at least one gettext implementation in which that triggers an abort() crash, so let's stop doing that. Add mutexes guarding libpq's and ecpglib's calls, which are the only ones that need worry about multithreaded callers. Note: in libpq, we could perhaps have piggybacked on default_threadlock() to avoid defining a new mutex variable. I judge that not terribly safe though, since libpq_gettext could be called from code that is holding the default mutex. If that were the first such call in the process, it'd fail. An extra mutex is cheap insurance against unforeseen interactions. Per bug #18312 from Christian Maurer. Back-patch to all supported versions. Discussion: https://postgr.es/m/18312-bbbabc8113592b78@postgresql.org Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us
* Clean up Windows-specific mutex code in libpq and ecpglib.Tom Lane2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix pthread-win32.h and pthread-win32.c to provide a more complete emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER and make sure that pthread_mutex_lock() can operate on a mutex object that's been initialized that way. Then we don't need the duplicative platform-specific logic in default_threadlock() and pgtls_init(), which we'd otherwise need yet a third copy of for an upcoming bug fix. Also, since default_threadlock() supposes that pthread_mutex_lock() cannot fail, try to ensure that that's actually true, by getting rid of the malloc call that was formerly involved in initializing an emulated mutex. We can define an extra state for the spinlock field instead. Also, replace the similar code in ecpglib/misc.c with this version. While ecpglib's version at least had a POSIX-compliant API, it also had the potential of failing during mutex init (but here, because of CreateMutex failure rather than malloc failure). Since all of misc.c's callers ignore failures, it seems like a wise idea to avoid failures here too. A further improvement in this area could be to unify libpq's and ecpglib's implementations into a src/port/pthread-win32.c file. But that doesn't seem like a bug fix, so I'll desist for now. In preparation for the aforementioned bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us
* Refactor pipe_read_line to return the full lineDaniel Gustafsson2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | | Commit 5b2f4afffe6 refactored find_other_exec() and in the process created pipe_read_line() into a static routine for reading a single line of output, aimed at reading version numbers. Commit a7e8ece41 later exposed it externally in order to read a postgresql.conf GUC using "postgres -C ..". Further, f06b1c598 also made use of it for reading a version string much like find_other_exec(). The internal variable remained "pgver", even when used for other purposes. Since the function requires passing a buffer and its size, and at most size - 1 bytes will be read via fgets(), there is a truncation risk when using this for reading GUCs (like how pg_rewind does, though the risk in this case is marginal). To keep this as generic functionality for reading a line from a pipe, this refactors pipe_read_line() into returning an allocated buffer containing all of the line to remove the risk of silent truncation. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: John Naylor <johncnaylorls@gmail.com> Discussion: https://postgr.es/m/DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se
* Fix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()Alexander Korotkov2024-02-09
| | | | | | | | | | | | | | | | group_keys_reorder_by_pathkeys() function searched for matching pathkeys within root->group_pathkeys. That could lead to picking an aggregate pathkey and using its pathkey->pk_eclass->ec_sortref as an argument of get_sortgroupref_clause_noerr(). Given that ec_sortref of an aggregate pathkey references aggregate targetlist not query targetlist, this leads to incorrect query optimization. Fix this by looking for matching pathkeys only within the first num_groupby_pathkeys pathkeys. Reported-by: David G. Johnston Discussion: https://postgr.es/m/CAKFQuwY3Ek%3DcLThgd8FdaSc5JRDVt0FaV00gMcWra%2BTAR4gGUw%40mail.gmail.com Author: Andrei Lepikhov, Alexander Korotkov
* Fix propagation of persistence to sequences in ALTER TABLE / ADD COLUMNPeter Eisentraut2024-02-09
| | | | | | | | | | | | | | | | Fix for 344d62fb9a9: That commit introduced unlogged sequences and made it so that identity/serial sequences automatically get the persistence level of their owning table. But this works only for CREATE TABLE and not for ALTER TABLE / ADD COLUMN. The latter would always create the sequence as logged (default), independent of the persistence setting of the table. This is fixed here. Note: It is allowed to change the persistence of identity sequences directly using ALTER SEQUENCE. So mistakes in existing databases can be fixed manually. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/c4b6e2ed-bcdf-4ea7-965f-e49761094827%40eisentraut.org
* Fix indentation of copyto.cMichael Paquier2024-02-09
| | | | | | Issue introduced by b619852086ed. Per buildfarm member koel.
* Improve COPY TO performance when server and client encodings matchMichael Paquier2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | | This commit fixes an oversight introduced in c61a2f58418e, where COPY TO would attempt to do encoding conversions even if the encodings of the client and the server matched for multi-byte encodings. All conversions go through pg_any_to_server() that makes the conversion a no-op when the encodings of the client and the server match, even for multi-byte encodings. The logic was fine, but setting CopyToStateData->need_transcoding would cause strlen() to be called for nothing for each attribute of all the rows copied, and that was showing high in some profiles (more attributes make that easier to reach). This change improves the runtime of some worst-case COPY TO queries by 15%~ (number present at least here). This is a performance improvement, so no backpatch is done out of caution as this is not a regression. Reported-by: Andres Freund Analyzed-by: Andres Freund Author: Michael Paquier Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/20240206020504.edijzczkgd25ek6z@awork3.anarazel.de
* Apply pg_dump test cleanups to test_pg_dump as wellPeter Eisentraut2024-02-08
| | | | | | | | Apply the changes from 41a284411e0 to the test_pg_dump module as well. Here, we just apply the new test consistency checks, but we don't need to fix any existing tests. Discussion: https://www.postgresql.org/message-id/flat/1f8cb371-e84e-434e-0367-6b716fb16fa1@eisentraut.org
* Fix gcc >= 10 warningAlexander Korotkov2024-02-08
| | | | | Reported-by: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVQoFXxFm2kCmhHcdM7DjA84_bOjoM8HVAKHbE%2BKrZ1uA%40mail.gmail.com
* Fix wrong logic in TransactionIdInRecentPast()Alexander Korotkov2024-02-08
| | | | | | | | | | | | | | | | | | | The TransactionIdInRecentPast() should return false for all the transactions older than TransamVariables->oldestClogXid. However, the function contains a bug in comparison FullTransactionId to TransactionID allowing full transactions between nextXid - 2^32 and oldestClogXid - 2^31. This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into FullTransactionId first, then performing the comparison. Backpatch to all supported versions. Reported-by: Egor Chindyaskin Bug: 18212 Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org Author: Karina Litskevich Reviewed-by: Kyotaro Horiguchi Backpatch-through: 12
* Fix meson installation of xid_wraparound test.Masahiko Sawada2024-02-08
| | | | | | | | Fix for e255b646a, to prevent installation of xid_wraparound test module during main install. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/84cd416a-0e37-4019-8380-1c8a3cdd8c5c%40eisentraut.org
* Fix warnings in cpluspluscheckJohn Naylor2024-02-08
| | | | | | | | | | Various int variables were compared to macros that are of type size_t, which caused -Wsign-compare warnings in cpluspluscheck. Change those to size_t, which also better describes their purpose. Per report from Peter Eisentraut Discussion: https://postgr.es/m/486847dc-6de5-464a-938e-bac98ec2438b%40eisentraut.org
* Rename static function to avoid conflicting namesDaniel Gustafsson2024-02-07
| | | | | | | | | | | Commit a4fd3aa719e moved setup_cancel_handler out of psql and exporeted it as a global function. While pg_dump isn't using the header it's exported in, having a conflicting name still risks causing confusion when grepping the code for callsites, so rename the static function in pg_dump to avoid this. Author: Yugo Nagata <nagata@sraoss.co.jp> Discussion: https://postgr.es/m/20240126094245.cf6718cc659273765f3ab69a@sraoss.co.jp
* Remove Start* macros in postmaster.c.Nathan Bossart2024-02-07
| | | | | | | | | | | These macros are just shorthands for calling StartChildProcess() with the appropriate process type, and they arguably make the code harder to understand. Suggested-by: Andres Freund Author: Reid Thompson Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/e88934c02a5c66f5e8caab2025f85da6b9026d0b.camel%40crunchydata.com
* Adjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT relDavid Rowley2024-02-07
| | | | | | | | | | | | | | | | | | | | | | A comment in grouping_planner() claimed that the PlannerInfo upper_targets array was not used in core code. However, the code that generated the paths for the UPPERREL_PARTIAL_DISTINCT rel made that comment untrue. Here we adjust the create_distinct_paths() function signature to pass down the PathTarget the same as is done for create_grouping_paths(), thus making the aforementioned comment true again. In passing adjust the order of the upper_targets[] assignments. These seem to be following the reverse enum order apart from UPPERREL_PARTIAL_DISTINCT. Also, update the header comment for generate_gather_paths() to mention the function is also used to create gather paths for partial distinct paths. Author: Richard Guo, David Rowley Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com
* Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.Amit Kapila2024-02-07
| | | | | | | | | | | Commit 861f86beea used REGBUF_NO_CHANGE at one of the places in the hash index to register the clean buffers but forgot to avoid setting LSN in that case. Reported-by: Michael Paquier Author: Kuroda Hayato Reviewed-by: Amit Kapila, Michael Paquier Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz
* Clean-ups for 776621a5e4 and 7329240437.Amit Kapila2024-02-07
| | | | | | | | | | | | | | | | Following are a few clean-ups related to failover option support in slots: 1. Improve the documentation in create_subscription.sgml. 2. Remove the spurious blank line in subscriptioncmds.c. 3. Remove the NOTICE for alter_replication_slot in subscriptioncmds.c as we would sometimes print it even when nothing has changed. One can find the change by enabling log_replication_commands on the publisher. 4. Optimize ReplicationSlotAlter() function to prevent disk flushing when the slot's data remains unchanged. Author: Hou Zhijie Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com Discussion: https://postgr.es/m/OS0PR01MB57164904651FB588A518E98894472@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Simplify signature of CopyAttributeOutCSV() in copyto.cMichael Paquier2024-02-07
| | | | | | | | This has come up in 2889fd23be56, reverted later on, and is still useful on its own to reduce a bit the differences between the code paths dedicated to CSV and text. Discussion: https://postgr.es/m/ZcCKwAeFrlOqPBuN@paquier.xyz
* Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"Michael Paquier2024-02-07
| | | | | | | | | | | | This reverts commit 2889fd23be56, following a discussion with Andres Freund as this callback, being called once per attribute when sending a relation's row, can involve a lot of indirect function calls (more attributes to deal with means more impact). The effects of a dispatch at this level would become more visible when improving the per-row code execution of COPY TO, impacting future potential performance improvements. Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de
* Change initial use of pg_atomic_write_u64 to initAlvaro Herrera2024-02-06
| | | | | | This only matters when using atomics emulation with semaphores. Per buildfarm member rorqual.