aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Remove unnecessary break in pg_logical_replication_slot_advance()Michael Paquier2023-10-23
| | | | | | | | | | | | | | | pg_logical_replication_slot_advance() included a break condition to stop when a targeted LSN is reached, when processing a series of WAL records with XLogReadRecord(). Since 38a957316d7e, it matched with the check of its main while loop. This condition saved from an extra CFI check, actually pointless, so let's remove this condition and simplify the code. In passing, fix an incorrect comment. Author: Bharath Rupireddy Reviewed-by: Tom Lane, Gurjeet Singh Discussion: https://postgr.es/m/CALj2ACWfGDLQ2cy7ZKwxnJqbDkO6Yvqqrqxne5ZN4HYm=PRTGg@mail.gmail.com
* Fix min_dynamic_shared_memory on Windows.Thomas Munro2023-10-22
| | | | | | | | | | | | | | | | | | When min_dynamic_shared_memory is set above 0, we try to find space in a pre-allocated region of the main shared memory area instead of calling dsm_impl_XXX() routines to allocate more. The dsm_pin_segment() and dsm_unpin_segment() routines had a bug: they called dsm_impl_XXX() routines even for main region segments. Nobody noticed before now because those routines do nothing on Unix, but on Windows they'd fail while attempting to duplicate an invalid Windows HANDLE. Add the missing gating. Back-patch to 14, where commit 84b1c63a added this feature. Fixes pgsql-bugs bug #18165. Reported-by: Maxime Boyer <maxime.boyer@cra-arc.gc.ca> Tested-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/18165-bf4f525cea6e51de%40postgresql.org
* Allow ALTER SYSTEM to set unrecognized custom GUCs.Tom Lane2023-10-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, ALTER SYSTEM failed if the target GUC wasn't present in the session's GUC hashtable. That is a reasonable behavior for core (single-part) GUC names, and for custom GUCs for which we have loaded an extension that's reserved the prefix. But it's unnecessarily restrictive otherwise, and it also causes inconsistent behavior: you can "ALTER SYSTEM SET foo.bar" only if you did "SET foo.bar" earlier in the session. That's fairly silly. Hence, refactor things so that we can execute ALTER SYSTEM even if the variable doesn't have a GUC hashtable entry, as long as the name meets the custom-variable naming requirements and does not have a reserved prefix. (It's safe to do this even if the variable belongs to an extension we currently don't have loaded. A bad value will at worst cause a WARNING when the extension does get loaded.) Also, adjust GRANT ON PARAMETER to have the same opinions about whether to allow an unrecognized GUC name, and to throw the same errors if not (it previously used a one-size-fits-all message for several distinguishable conditions). By default, only a superuser will be allowed to do ALTER SYSTEM SET on an unrecognized name, but it's possible to GRANT the ability to do it. Patch by me, pursuant to a documentation complaint from Gavin Panella. Arguably this is a bug fix, but given the lack of other complaints I'll refrain from back-patching. Discussion: https://postgr.es/m/2617358.1697501956@sss.pgh.pa.us Discussion: https://postgr.es/m/169746329791.169914.16613647309012285391@wrigleys.postgresql.org
* Make some error strings more genericAlvaro Herrera2023-10-20
| | | | | It's undesirable to have SQL commands or configuration options in a translatable error string, so take some of these out.
* Extend ALTER OPERATOR to allow setting more optimization attributes.Tom Lane2023-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Allow the COMMUTATOR, NEGATOR, MERGES, and HASHES attributes to be set by ALTER OPERATOR. However, we don't allow COMMUTATOR/NEGATOR to be changed once set, nor allow the MERGES/HASHES flags to be unset once set. Changes like that might invalidate plans already made, and dealing with the consequences seems like more trouble than it's worth. The main use-case we foresee for this is to allow addition of missed properties in extension update scripts, such as extending an existing operator to support hashing. So only transitions from not-set to set states seem very useful. This patch also causes us to reject some incorrect cases that formerly resulted in inconsistent catalog state, such as trying to set the commutator of an operator to be some other operator that already has a (different) commutator. While at it, move the InvokeObjectPostCreateHook call for CREATE OPERATOR to not occur until after we've fixed up commutator or negator links as needed. The previous ordering could only be justified by thinking of the OperatorUpd call as a kind of ALTER OPERATOR step; but we don't call InvokeObjectPostAlterHook therein. It seems better to let the hook see the final state of the operator object. In the documentation, move the discussion of how to establish commutator pairs from xoper.sgml to the CREATE OPERATOR ref page. Tommy Pavlicek, reviewed and editorialized a bit by me Discussion: https://postgr.es/m/CAEhP-W-vGVzf4udhR5M8Bdv88UYnPrhoSkj3ieR3QNrsGQoqdg@mail.gmail.com
* Remove unnecessary dependencies to wal_level=logical in TAP testsMichael Paquier2023-10-20
| | | | | | | | | | | | | A couple of TAP tests make use of wal_level=logical for nodes that do not need to do any kind of logical decoding operations, like subscription nodes on which changes are only applied. This can be confusing when reading these tests as setup examples, so let's remove this configuration where not required (contrary to two-way logical replication and similar more complex cases). This simplifies the tests a bit, making them slightly cheaper with less WAL generated overall. Author: Hayato Kuroda Discussion: https://postgr.es/m/TYAPR01MB5866946BCEB747ABE513ACC6F5D5A@TYAPR01MB5866.jpnprd01.prod.outlook.com
* During online checkpoints, insert XLOG_CHECKPOINT_REDO at redo point.Robert Haas2023-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | This allows tools that read the WAL sequentially to identify (possible) redo points when they're reached, rather than only being able to detect them in retrospect when XLOG_CHECKPOINT_ONLINE is found, possibly much later in the WAL stream. There are other possible applications as well; see the discussion links below. Any redo location that precedes the checkpoint location should now point to an XLOG_CHECKPOINT_REDO record, so add a cross-check to verify this. While adjusting the code in CreateCheckPoint() for this patch, I made it call WALInsertLockAcquireExclusive a bit later than before, since there appears to be no need for it to be held while checking whether the system is idle, whether this is an end-of-recovery checkpoint, or what the current timeline is. Bump XLOG_PAGE_MAGIC. Patch by me, based in part on earlier work from Dilip Kumar. Review by Dilip Kumar, Amit Kapila, Andres Freund, and Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoYy-Vc6G9QKcAKNksCa29cv__czr+N9X_QCxEfQVpp_8w@mail.gmail.com Discussion: http://postgr.es/m/20230614194717.jyuw3okxup4cvtbt%40awork3.anarazel.de Discussion: http://postgr.es/m/CA+hUKG+b2ego8=YNW2Ohe9QmSiReh1-ogrv8V_WZpJTqP3O+2w@mail.gmail.com
* Doc: modernize comment for boolin().Tom Lane2023-10-19
| | | | | | | | | | Most of the behavior described by this comment was moved to parse_bool_with_len() some time ago. Move what's still valuable there too, and drop the rest. Peter Smith Discussion: https://postgr.es/m/CAHut+PtMJURKp=U8Z=Ktp0zV40sEb1f-iEk9FvY2GQe+5ZBnwg@mail.gmail.com
* Add local_blk_{read|write}_time I/O timing statistics for local blocksMichael Paquier2023-10-19
| | | | | | | | | | | | | There was no I/O timing statistics for counting read and write timings on local blocks, contrary to the counterparts for temp and shared blocks. This information is available when track_io_timing is enabled. The output of EXPLAIN is updated to show this information. An update of pg_stat_statements is planned next. Author: Nazir Bilal Yavuz Reviewed-by: Robert Haas, Melanie Plageman Discussion: https://postgr.es/m/CAN55FZ19Ss279mZuqGbuUNxka0iPbLgYuOQXqAKewrjNrp27VA@mail.gmail.com
* Rename I/O timing statistics columns to shared_blk_{read|write}_timeMichael Paquier2023-10-19
| | | | | | | | | | | | | | | | | | | | These two counters, defined in BufferUsage to track respectively the time spent while reading and writing blocks have historically only tracked data related to shared buffers, when track_io_timing is enabled. An upcoming patch to add specific counters for local buffers will take advantage of this rename as it has come up that no data is currently tracked for local buffers, and tracking local and shared buffers using the same fields would be inconsistent with the treatment done for temp buffers. Renaming the existing fields clarifies what the block type of each stats field is. pg_stat_statement is updated to reflect the rename. No extension version bump is required as 5a3423ad8ee17 has done one, affecting v17~. Author: Nazir Bilal Yavuz Reviewed-by: Robert Haas, Melanie Plageman Discussion: https://postgr.es/m/CAN55FZ19Ss279mZuqGbuUNxka0iPbLgYuOQXqAKewrjNrp27VA@mail.gmail.com
* Improve pglz_decompress's defenses against corrupt compressed data.Tom Lane2023-10-18
| | | | | | | | | | | | | | | | When processing a match tag, check to see if the claimed "off" is more than the distance back to the output buffer start. If it is, then the data is corrupt, and what's more we would fetch from outside the buffer boundaries and potentially incur a SIGSEGV. (Although the odds of that seem relatively low, given that "off" can't be more than 4K.) Back-patch to v13; before that, this function wasn't really trying to protect against bad data. Report and fix by Flavien Guedez. Discussion: https://postgr.es/m/01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net
* Install wait_event_types.h in VPATH buildsMichael Paquier2023-10-19
| | | | | | | | | | | An extra rule is needed in src/include/Makefile for VPATH builds to install any generated server-side include files, and wait_event_types.h was forgotten from the set. Issue introduced by fa88928470b5. Reported-by: Christoph Berg Discussion: https://postgr.es/m/ZTAA11u7CtX6NqlK@msg.df7cb.de
* jit: Changes for LLVM 17.Thomas Munro2023-10-19
| | | | | | | | | | | | Changes required by https://llvm.org/docs/NewPassManager.html. Back-patch to 12, leaving the final release of 11 unchanged, consistent with earlier decision not to back-patch LLVM 16 support either. Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BWXznXCyTgCADd%3DHWkP9Qksa6chd7L%3DGCnZo-MBgg9Lg%40mail.gmail.com
* jit: Supply LLVMGlobalGetValueType() for LLVM < 8.Thomas Munro2023-10-19
| | | | | | | | | | | Commit 37d5babb used this C API function while adding support for LLVM 16 and opaque pointers, but it's not available in LLVM 7 and older. Provide it in our own llvmjit_wrap.cpp. It just calls a C++ function that pre-dates LLVM 3.9, our minimum target. Back-patch to 12, like 37d5babb. Discussion: https://postgr.es/m/CA%2BhUKGKnLnJnWrkr%3D4mSGhE5FuTK55FY15uULR7%3Dzzc%3DwX4Nqw%40mail.gmail.com
* jit: Support opaque pointers in LLVM 16.Thomas Munro2023-10-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove use of LLVMGetElementType() and provide the type of all pointers to LLVMBuildXXX() functions when emitting IR, as required by modern LLVM versions[1]. * For LLVM <= 14, we'll still use the old LLVMBuildXXX() functions. * For LLVM == 15, we'll continue to do the same, explicitly opting out of opaque pointer mode. * For LLVM >= 16, we'll use the new LLVMBuildXXX2() functions that take the extra type argument. The difference is hidden behind some new IR emitting wrapper functions l_load(), l_gep(), l_call() etc. The change is mostly mechanical, except that at each site the correct type had to be provided. In some places we needed to do some extra work to get functions types, including some new wrappers for C++ APIs that are not yet exposed by in LLVM's C API, and some new "example" functions in llvmjit_types.c because it's no longer possible to start from the function pointer type and ask for the function type. Back-patch to 12, because it's a little tricker in 11 and we agreed not to put the latest LLVM support into the upcoming final release of 11. [1] https://llvm.org/docs/OpaquePointers.html Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Ronan Dunklau <ronan.dunklau@aiven.io> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKGKNX_%3Df%2B1C4r06WETKTq0G4Z_7q4L4Fxn5WWpMycDj9Fw%40mail.gmail.com
* pg_upgrade: Fix test name in 002_pg_upgrade.plMichael Paquier2023-10-18
| | | | | | Author: Hou Zhijie Discussion: https://postgr.es/m/TYAPR01MB5724A40D47E71F4717357EC694D5A@TYAPR01MB5724.jpnprd01.prod.outlook.com Backpatch-through: 15
* Clean up some pg_dump testsPeter Eisentraut2023-10-18
| | | | | | | | | | | | | | | | | 1) Remove useless entries from "unlike" lists. Runs that are not listed in "like" don't need to be excluded in "unlike". 2) Ensure there is always a "like" list, even if it is empty. This makes the test more self-documenting. 3) Use predefined lists such as %full_runs where appropriate, instead of listing all runs separately. Also add code that checks 1 and 2 automatically and dies with an error for violations. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/1f8cb371-e84e-434e-0367-6b716fb16fa1@eisentraut.org
* Count write times when extending relation files for shared buffersMichael Paquier2023-10-18
| | | | | | | | | | | | | | | | | | Relation files extended by multiple blocks at a time have been counting the number of blocks written, but forgot to increment the write time in this case, as single-block write and relation extension are treated as two different I/O operations in the shared stats: IOOP_EXTEND vs IOOP_WRITE. In this case IOOP_EXTEND was forgotten for normal (non-temporary) relations, still the number of blocks written was incremented according to the relation extend done. Write times are tracked when track_io_timing is enabled, which is not the case by default. Author: Nazir Bilal Yavuz Reviewed-by: Robert Haas, Melanie Plageman Discussion: https://postgr.es/m/CAN55FZ19Ss279mZuqGbuUNxka0iPbLgYuOQXqAKewrjNrp27VA@mail.gmail.com Backpatch-through: 16
* Add flush option to pg_logical_emit_message()Michael Paquier2023-10-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | Since its introduction, LogLogicalMessage() (via the SQL interface pg_logical_emit_message()) has never included a call to XLogFlush(), causing it to potentially lose messages on a crash when used in non-transactional mode. This has come up to me as a problem while playing with ideas to design a test suite for what has become 039_end_of_wal.pl introduced in bae868caf222 by Thomas Munro, because there are no direct ways to force a WAL flush via SQL. The default is false, to not flush messages and influence existing use-cases where this function could be used. If set to true, the message emitted is flushed before returning back to the caller, making the message durable on crash. This new option has no effect when using pg_logical_emit_message() in transactional mode, as the record's flush is guaranteed by the WAL record generated by the transaction committed. Two queries of test_decoding are tweaked to cover the new code path for the flush. Bump catalog version. Author: Michael Paquier Reviewed-by: Andres Freund, Amit Kapila, Fujii Masao, Tung Nguyen, Tomas Vondra Discussion: https://postgr.es/m/ZNsdThSe2qgsfs7R@paquier.xyz
* Dodge a compiler bug affecting timetz_zone/timetz_izone.Tom Lane2023-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | Use a modulo operator instead of implementing the same behavior with a loop. The loop solution is doubtless microscopically faster for the typical case of only wrapping into the very next day, but maybe not so much for large interval values. In any case, timetz is such a backwater that it's doubtful anybody would notice any performance change anyway. This avoids a compiler bug occurring in AIX's xlc, even in pretty late-model revisions. We did not have test coverage for the case where the initial result->time value is negative, so add that. For the moment, install this only in HEAD. My plan is to back-patch the test case, and then the code change assuming that buildfarm testing proves the bug occurs in the back branches. (That seems pretty likely, but let's find out for sure.) Per buildfarm results from commits 97957fdba and 2f0472030. Thanks to Michael Paquier for the idea to use a modulo operation to replace the faulty loop. Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com
* Avoid calling proc_exit() in processes forked by system().Nathan Bossart2023-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The SIGTERM handler for the startup process immediately calls proc_exit() for the duration of the restore_command, i.e., a call to system(). This system() call forks a new process to execute the shell command, and this child process inherits the parent's signal handlers. If both the parent and child processes receive SIGTERM, both will attempt to call proc_exit(). This can end badly. For example, both processes will try to remove themselves from the PGPROC shared array. To fix this problem, this commit adds a check in StartupProcShutdownHandler() to see whether MyProcPid == getpid(). If they match, this is the parent process, and we can proc_exit() like before. If they do not match, this is a child process, and we just emit a message to STDERR (in a signal safe manner) and _exit(), thereby skipping any problematic exit callbacks. This commit also adds checks in proc_exit(), ProcKill(), and AuxiliaryProcKill() that verify they are not being called within such child processes. Suggested-by: Andres Freund Reviewed-by: Thomas Munro, Andres Freund Discussion: https://postgr.es/m/Y9nGDSgIm83FHcad%40paquier.xyz Discussion: https://postgr.es/m/20230223231503.GA743455%40nathanxps13 Backpatch-through: 11
* Reword messages about impending (M)XID exhaustion.Robert Haas2023-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | First, we shouldn't recommend switching to single-user mode, because that's terrible advice. Especially on newer versions where VACUUM will enter emergency mode when nearing (M)XID exhaustion, it's perfectly fine to just VACUUM in multi-user mode. Doing it that way is less disruptive and avoids disabling the safeguards that prevent actual wraparound, so recommend that instead. Second, be more precise about what is going to happen (when we're nearing the limits) or what is happening (when we actually hit them). The database doesn't shut down, nor does it refuse all commands. It refuses commands that assign whichever of XIDs and MXIDs are nearly exhausted. No back-patch. The existing hint that advises going to single-user mode is sufficiently awful advice that removing it or changing it might be justifiable even though we normally avoid changing user-facing messages in back-branches, but I (rhaas) felt that it was better to be more conservative and limit this fix to master only. Aside from the usual risk of breaking translations, people might be used to the existing message, or even have monitoring scripts that look for it. Alexander Alekseev, John Naylor, Robert Haas, reviewed at various times by Peter Geoghegan, Hannu Krosing, and Andres Freund. Discussion: http://postgr.es/m/CA+TgmoZBg95FiR9wVQPAXpGPRkacSt2okVge+PKPPFppN7sfnQ@mail.gmail.com
* Talk about assigning, rather than generating, new MultiXactIds.Robert Haas2023-10-17
| | | | | | | | | The word "assign" is used in various places internally to describe what GetNewMultiXactId does, but the user-facing messages have previously said "generate". For consistency, standardize on "assign," which seems (at least to me) to be slightly clearer. Discussion: http://postgr.es/m/CA+TgmoaoE1_i3=4-7GCTtKLVZVQ2Gh6qESW2VG1OprtycxOHMA@mail.gmail.com
* Improve truncation of pg_serial/, removing "apparent wraparound" LOGsMichael Paquier2023-10-17
| | | | | | | | | | | | | | | | | | | | | | It is possible that the tail XID of pg_serial/ gets ahead of its head XID, which would cause the truncation of pg_serial/ done during checkpoints to show up as a "wraparound" LOG in SimpleLruTruncate(), which is confusing. This also wastes a bit of disk space until the head page is reclaimed again. CheckPointPredicate() is changed so as the cutoff page for the truncation is switched to the head page if the tail XID has advanced beyond the head XID, rather than the tail page. This prevents the confusing LOG message about a wraparound while allowing some truncation to be done to cut in disk space. This could be considered as a bug fix, but the original behavior is harmless as well, resulting only in disk space temporarily wasted, so no backpatch is done. Author: Sami Imseih Reviewed-by: Heikki Linnakangas, Michael Paquier Discussion: https://postgr.es/m/755E19CA-D02C-4A4C-80D3-74F775410C48@amazon.com
* Run 006_login_trigger.pl only with Unix-domain socketsAlexander Korotkov2023-10-17
| | | | | | Per report from buildfarm member drongo. Reported-by: Alexander Lakhin
* Restart the apply worker if the privileges have been revoked.Amit Kapila2023-10-17
| | | | | | | | | | | Restart the apply worker if the subscription owner's superuser privileges have been revoked. This is required so that the subscription connection string gets revalidated and use the password option to connect to the publisher for non-superusers, if required. Author: Vignesh C Reviewed-by: Amit Kapila Discussion: http://postgr.es/m/CALDaNm2Dxmhq08nr4P6G+24QvdBo_GAVyZ_Q1TcGYK+8NHs9xw@mail.gmail.com
* Add regression test coverage for timetz_izone().Tom Lane2023-10-16
| | | | | | | | | Extend the test added by commit 97957fdba so that it also covers timetz_izone(), that is the "AT TIME ZONE interval" case. This is mostly to see if xlc's apparent bug occurs there too, but more code coverage is always welcome. Discussion: https://postgr.es/m/2287835.1697464481@sss.pgh.pa.us
* Ensure we have a snapshot while dropping ON COMMIT DROP temp tables.Tom Lane2023-10-16
| | | | | | | | | | | | | | | | | | | | | | | | Dropping a temp table could entail TOAST table access to clean out toasted catalog entries, such as large pg_constraint.conbin strings for complex CHECK constraints. If we did that via ON COMMIT DROP, we triggered the assertion in init_toast_snapshot(), because there was no provision for setting up a snapshot for the drop actions. Fix that. (I assume here that the adjacent truncation actions for ON COMMIT DELETE ROWS don't have a similar problem: it doesn't seem like nontransactional truncations would need to touch any toasted fields. If that proves wrong, we could refactor a bit to have the same snapshot acquisition cover that too.) The test case added here does not fail before v15, because that assertion was added in 277692220 which was not back-patched. However, the race condition the assertion warns of surely exists further back, so back-patch to all supported branches. Per report from Richard Guo. Discussion: https://postgr.es/m/CAMbWs4-x26=_QxxgdJyNbiCDzvtr2WV5ZDso_v-CukKEe6cBZw@mail.gmail.com
* Move extra code out of the Pre/PostRestoreCommand() section.Nathan Bossart2023-10-16
| | | | | | | | | | | | | | | | | | | | If SIGTERM is received within this section, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required there (as such code is unlikely to be compatible with doing proc_exit() in a signal handler). This commit moves the code recently added to this section (see 1b06d7bac9 and 7fed801135) to outside of the section. This ensures that the startup process only calls proc_exit() in its SIGTERM handler for the duration of the system() call, which is how this code worked from v8.4 to v14. Reported-by: Michael Paquier, Thomas Munro Analyzed-by: Andres Freund Suggested-by: Tom Lane Reviewed-by: Michael Paquier, Robert Haas, Thomas Munro, Andres Freund Discussion: https://postgr.es/m/Y9nGDSgIm83FHcad%40paquier.xyz Discussion: https://postgr.es/m/20230223231503.GA743455%40nathanxps13 Backpatch-through: 15
* List 006_login_trigger.pl test for mesonAlexander Korotkov2023-10-16
| | | | | Reported-by: Thomas Munro Discussion: https://postgr.es/m/CA%2BhUKGLuqDUaYYhJnA1H1q5Z-k18kQHoEqZ5fiXtTi4038nspg%40mail.gmail.com
* worker_spi: Fix test failure with BGWORKER_BYPASS_ROLELOGINCHECKMichael Paquier2023-10-16
| | | | | | | | | | | | This is a consequence of 4817da51f69a that has bumped up max_worker_processes, where now the last worker started by the test would be able to start by itself a parallel worker because there are more slots available. This did not show up before as the number of bgworkers reached exactly 8, as known as the previous limit, at the end of the test. Per report from buildfarm member crake, reproducible with debug_parallel_query = regress in the same fashion as fd4d93d269c0.
* Try to handle torn reads of pg_control in frontend.Thomas Munro2023-10-16
| | | | | | | | | | | | | | | | | | | | | | | Some of our src/bin tools read the control file without any kind of interlocking against concurrent writes from the server. At least ext4 and ntfs can expose partially modified contents when you do that. For now, we'll try to tolerate this by retrying up to 10 times if the checksum doesn't match, until we get two reads in a row with the same bad checksum. This is not guaranteed to reach the right conclusion, but it seems very likely to. Thanks to Tom Lane for this suggestion. Various ideas for interlocking or atomicity were considered too complicated, unportable or expensive given the lack of field reports, but remain open for future reconsideration. Back-patch as far as 12. It doesn't seem like a good idea to put a heuristic change for a very rare problem into the final release of 11. Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru> Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de
* worker_spi: Bump up max_worker_processes in TAP testsMichael Paquier2023-10-16
| | | | | | | | | | | | | | | | mamba has detected a failure in the last test that should start a bgworker while bypassing the role login check. The buildfarm did not provide any information about its failure in the logs, but I suspect that this is caused by an exhaustion of the max_worker_processes slots set at 8 by default. In "normal" test runs, the number of bgworkers running at this stage of the test is already 7, so, if one of them spawns for example a parallel worker all the slots would be taken, preventing the last worker of the test to start. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/ZSyebsiub88pyJJO@paquier.xyz
* Rename 005_login_trigger.pl to 006_login_trigger.plAlexander Korotkov2023-10-16
| | | | In order to avoid numbering collision with 005_sspi.pl.
* Fix role names in src/test/authentication/t/005_login_trigger.plAlexander Korotkov2023-10-16
| | | | Per buildfarm member longfin.
* Fix code indentation violations in e83d1b0c40ccMichael Paquier2023-10-16
| | | | | koel has not reported this one yet, I have just bumped on it while looking at a different patch.
* Fix comment from commit 22655aa231.Thomas Munro2023-10-16
| | | | | Per automated complaint from BF animal koel this needed to be re-indented, but there was also a typo. Back-patch to 16.
* Add support event triggers on authenticated loginAlexander Korotkov2023-10-16
| | | | | | | | | | | | | | | | | | | | | This commit introduces trigger on login event, allowing to fire some actions right on the user connection. This can be useful for logging or connection check purposes as well as for some personalization of environment. Usage details are described in the documentation included, but shortly usage is the same as for other triggers: create function returning event_trigger and then create event trigger on login event. In order to prevent the connection time overhead when there are no triggers the commit introduces pg_database.dathasloginevt flag, which indicates database has active login triggers. This flag is set by CREATE/ALTER EVENT TRIGGER command, and unset at connection time when no active triggers found. Author: Konstantin Knizhnik, Mikhail Gribkov Discussion: https://postgr.es/m/0d46d29f-4558-3af9-9c85-7774e14a7709%40postgrespro.ru Reviewed-by: Pavel Stehule, Takayuki Tsunakawa, Greg Nancarrow, Ivan Panchenko Reviewed-by: Daniel Gustafsson, Teodor Sigaev, Robert Haas, Andres Freund Reviewed-by: Tom Lane, Andrey Sokolov, Zhihong Yu, Sergey Shinderuk Reviewed-by: Gregory Stark, Nikita Malakhov, Ted Yu
* Acquire ControlFileLock in relevant SQL functions.Thomas Munro2023-10-16
| | | | | | | | | | | | | | Commit dc7d70ea added functions that read the control file, but didn't acquire ControlFileLock. With unlucky timing, file systems that have weak interlocking like ext4 and ntfs could expose partially overwritten contents, and the checksum would fail. Back-patch to all supported releases. Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de
* Dissociate btequalimage() from interval_ops, ending its deduplication.Noah Misch2023-10-14
| | | | | | | | | | | | | | | | | | Under interval_ops, some equal values are distinguishable. One such pair is '24:00:00' and '1 day'. With that being so, btequalimage() breaches the documented contract for the "equalimage" btree support function. This can cause incorrect results from index-only scans. Users should REINDEX any btree indexes having interval-type columns. After updating, pg_amcheck will report an error for almost all such indexes. This fix makes interval_ops simply omit the support function, like numeric_ops does. Back-pack to v13, where btequalimage() first appeared. In back branches, for the benefit of old catalog content, btequalimage() code will return false for type "interval". Going forward, back-branch initdb will include the catalog change. Reviewed by Peter Geoghegan. Discussion: https://postgr.es/m/20231011013317.22.nmisch@google.com
* Don't spuriously report FD_SETSIZE exhaustion on Windows.Noah Misch2023-10-14
| | | | | | | | | | | | | | | | | Starting on 2023-08-03, this intermittently terminated a "pgbench -C" test in CI. It could affect a high-client-count "pgbench" without "-C". While parallel reindexdb and vacuumdb reach the same problematic check, sufficient client count and/or connection turnover is less plausible for them. Given the lack of examples from the buildfarm or from manual builds, reproducing this must entail rare operating system configurations. Also correct the associated error message, which was wrong for non-Windows. Back-patch to v12, where the pgbench check first appeared. While v11 vacuumdb has the problematic check, reaching it with typical vacuumdb usage is implausible. Reviewed by Thomas Munro. Discussion: https://postgr.es/m/CA+hUKG+JwvTNdcyJTriy9BbtzF1veSRQ=9M_ZKFn9_LqE7Kp7Q@mail.gmail.com
* Harden xxx_is_visible() functions against concurrent object drops.Tom Lane2023-10-14
| | | | | | | | | | | | | | | | | | | For the same reasons given in commit 403ac226d, adjust these functions to not assume that checking SearchSysCacheExists can guarantee success of a later fetch. This follows the same internal API choices made in the earlier commit: add a function XXXExt(oid, is_missing) and use that to eliminate the need for a separate existence check. The changes are very straightforward, though tedious. For the moment I just made the new functions static in namespace.c, but we could export them if a need emerges. Per bug #18014 from Alexander Lakhin. Given the lack of hard evidence that there's a bug in non-debug builds, I'm content to fix this only in HEAD. Discussion: https://postgr.es/m/18014-28c81cb79d44295d@postgresql.org
* Harden has_xxx_privilege() functions against concurrent object drops.Tom Lane2023-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The versions of these functions that accept object OIDs are supposed to return NULL, rather than failing, if the target object has been dropped. This makes it safe(r) to use them in queries that scan catalogs, since the functions will be applied to objects that are visible in the query's snapshot but might now be gone according to the catalog snapshot. In most cases we implemented this by doing a SearchSysCacheExists test and assuming that if that succeeds, we can safely invoke the appropriate aclchk.c function, which will immediately re-fetch the same syscache entry. It was argued that if the existence test succeeds then the followup fetch must succeed as well, for lack of any intervening AcceptInvalidationMessages call. Alexander Lakhin demonstrated that this is not so when CATCACHE_FORCE_RELEASE is enabled: the syscache entry will be forcibly dropped at the end of SearchSysCacheExists, and then it is possible for the catalog snapshot to get advanced while re-fetching the entry. Alexander's test case requires the operation to happen inside a parallel worker, but that seems incidental to the fundamental problem. What remains obscure is whether there is a way for this to happen in a non-debug build. Nonetheless, CATCACHE_FORCE_RELEASE is a very useful test methodology, so we'd better make the code safe for it. After some discussion we concluded that the most future-proof fix is to give up the assumption that checking SearchSysCacheExists can guarantee success of a later fetch. At best that assumption leads to fragile code --- for example, has_type_privilege appears broken for array types even if you believe the assumption holds. And it's not even particularly efficient. There had already been some work towards extending the aclchk.c APIs to include "is_missing" output flags, so this patch extends that work to cover all the aclchk.c functions that are used by the has_xxx_privilege() functions. (This allows getting rid of some ad-hoc decisions about not throwing errors in certain places in aclchk.c.) In passing, this fixes the has_sequence_privilege() functions to provide the same guarantees as their cousins: for some reason the SearchSysCacheExists tests never got added to those. There is more work to do to remove the unsafe coding pattern with SearchSysCacheExists in other places, but this is a pretty self-contained patch so I'll commit it separately. Per bug #18014 from Alexander Lakhin. Given the lack of hard evidence that there's a bug in non-debug builds, I'm content to fix this only in HEAD. (Perhaps we should clean up the has_sequence_privilege() oversight in the back branches, but in the absence of field complaints I'm not too excited about that either.) Discussion: https://postgr.es/m/18014-28c81cb79d44295d@postgresql.org
* Fix bulk table extension when copying into multiple partitionsAndres Freund2023-10-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When COPYing into a partitioned table that does now permit the use of table_multi_insert(), we could error out with ERROR: could not read block NN in file "base/...": read only 0 of 8192 bytes because BulkInsertState->next_free was not reset between partitions. This problem occurred only when not able to use table_multi_insert(), as a dedicated BulkInsertState for each partition is used in that case. The bug was introduced in 00d1e02be24, but it was hard to hit at that point, as commonly bulk relation extension is not used when not using table_multi_insert(). It became more likely after 82a4edabd27, which expanded the use of bulk extension. To fix the bug, reset the bulk relation extension state in BulkInsertState in ReleaseBulkInsertStatePin(). That was added (in b1ecb9b3fcf) to tackle a very similar issue. Obviously the name is not quite correct, but there might be external callers, and bulk insert state needs to be reset in precisely in the situations that ReleaseBulkInsertStatePin() already needed to be called. Medium term the better fix likely is to disallow reusing BulkInsertState across relations. Add a test that, without the fix, reproduces #18130 in most configurations. The test also catches the problem fixed in b1ecb9b3fcf when run with small shared_buffers. Reported-by: Ivan Kolombet <enderstd@gmail.com> Analyzed-by: Tom Lane <tgl@sss.pgh.pa.us> Analyzed-by: Andres Freund <andres@anarazel.de> Bug: #18130 Discussion: https://postgr.es/m/18130-7a86a7356a75209d%40postgresql.org Discussion: https://postgr.es/m/257696.1695670946%40sss.pgh.pa.us Backpatch: 16-
* Improve the naming in wal_sync_method code.Nathan Bossart2023-10-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | * sync_method is renamed to wal_sync_method. * sync_method_options[] is renamed to wal_sync_method_options[]. * assign_xlog_sync_method() is renamed to assign_wal_sync_method(). * The names of the available synchronization methods are now prefixed with "WAL_SYNC_METHOD_" and have been moved into a WalSyncMethod enum. * PLATFORM_DEFAULT_SYNC_METHOD is renamed to PLATFORM_DEFAULT_WAL_SYNC_METHOD, and DEFAULT_SYNC_METHOD is renamed to DEFAULT_WAL_SYNC_METHOD. These more descriptive names help distinguish the code for wal_sync_method from the code for DataDirSyncMethod (e.g., the recovery_init_sync_method configuration parameter and the --sync-method option provided by several frontend utilities). This change also prevents name collisions between the aforementioned sets of code. Since this only improves the naming of internal identifiers, there should be no behavior change. Author: Maxim Orlov Discussion: https://postgr.es/m/CACG%3DezbL1gwE7_K7sr9uqaCGkWhmvRTcTEnm3%2BX1xsRNwbXULQ%40mail.gmail.com
* psql: Add completion support for AT [ LOCAL | TIME ZONE ]Michael Paquier2023-10-13
| | | | | | | | | AT TIME ZONE is completed with a list of supported timezones, something not needed by AT LOCAL. Author: Dagfinn Ilmari Mannsåker Reviewed-by: Jim Jones Discussion: https://postgr.es/m/87jzyzsvgv.fsf@wibble.ilmari.org
* Add support for AT LOCALMichael Paquier2023-10-13
| | | | | | | | | | | | | | | When converting a timestamp to/from with/without time zone, the SQL Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the session's time zone. This includes three system functions able to do the work in the same way as the existing flavors for AT TIME ZONE, except that these need to be marked as stable as they depend on the session's TimeZone GUC. Bump catalog version. Author: Vik Fearing Reviewed-by: Laurenz Albe, Cary Huang, Michael Paquier Discussion: https://postgr.es/m/8e25dec4-5667-c1a5-6581-167d710c2182@postgresfriends.org
* Add wait events for checkpoint delay mechanism.Thomas Munro2023-10-13
| | | | | | | | | | | When MyProc->delayChkptFlags is set to temporarily block phase transitions in a concurrent checkpoint, the checkpointer enters a sleep-poll loop to wait for the flag to be cleared. We should show that as a wait event in the pg_stat_activity view. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKGL7Whi8iwKbzkbn_1fixH3Yy8aAPz7mfq6Hpj7FeJrKMg%40mail.gmail.com
* Unify two isLogSwitch tests in XLogInsertRecord.Robert Haas2023-10-12
| | | | | | | | | | | | | | | | | An upcoming patch wants to introduce an additional special case in this function. To keep that as cheap as possible, minimize the amount of branching that we do based on whether this is an XLOG_SWITCH record. Additionally, and also in the interest of keeping the overhead of special-case code paths as low as possible, apply likely() to the non-XLOG_SWITCH case, since only a very tiny fraction of WAL records will be XLOG_SWITCH records. Patch by me, reviewed by Dilip Kumar, Amit Kapila, Andres Freund, and Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoYy-Vc6G9QKcAKNksCa29cv__czr+N9X_QCxEfQVpp_8w@mail.gmail.com
* Fix runtime partition pruning for HASH partitioned tablesDavid Rowley2023-10-13
| | | | | | | | | | | | | | | | | | | | | | | This could only affect HASH partitioned tables with at least 2 partition key columns. If partition pruning was delayed until execution and the query contained an IS NULL qual on one of the partitioned keys, and some subsequent partitioned key was being compared to a non-Const, then this could result in a crash due to the incorrect keyno being used to calculate the stateidx for the expression evaluation code. Here we fix this by properly skipping partitioned keys which have a nullkey set. Effectively, this must be the same as what's going on inside perform_pruning_base_step(). Sergei Glukhov also provided a patch, but that's not what's being used here. Reported-by: Sergei Glukhov Reviewed-by: tender wang, Sergei Glukhov Discussion: https://postgr.es/m/d05b26fa-af54-27e1-f693-6c31590802fa@postgrespro.ru Backpatch-through: 11, where runtime partition pruning was added.
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-21 15:52:40 +0000