12661 Commits (4cfce4e62c8f09f5b1f6a7f69760ca46a74406e2)

Author	SHA1	Message	Date
Peter Eisentraut	4cfce4e62c	Change copyObject() to use typeof_unqual ... Currently, when the argument of copyObject() is const-qualified, the return type is also, because the use of typeof carries over all the qualifiers. This is incorrect, since the point of copyObject() is to make a copy to mutate. But apparently no code ran into it. The new implementation uses typeof_unqual, which drops the qualifiers, making this work correctly. typeof_unqual is standardized in C23, but all recent versions of all the usual compilers support it even in non-C23 mode, at least as __typeof_unqual__. We add a configure/meson test for typeof_unqual and __typeof_unqual__ and use it if it's available, else we use the existing fallback of just returning void *. Reviewed-by: David Geier <geidav.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/92f9750f-c7f6-42d8-9a4a-85a3cbe808f3%40eisentraut.org	1 week ago
Heikki Linnakangas	cd375d5b6d	Remove useless errdetail_abort() ... I don't understand how to reach errdetail_abort() with MyProc->recoveryConflictPending set. If a recovery conflict signal is received, ProcessRecoveryConflictInterrupt() raises an ERROR or FATAL error to cancel the query or connection, and abort processing clears the flag. The error message from ProcessRecoveryConflictInterrupt() is very clear that the query or connection was terminated because of recovery conflict. The only way to reach it AFAICS is with a race condition, if the startup process sends a recovery conflict signal when the transaction has just entered aborted state for some other reason. And in that case the detail would be misleading, as the transaction was already aborted for some other reason, not because of the recovery conflict. errdetail_abort() was the only user of the recoveryConflictPending flag in PGPROC, so we can remove that and all the related code too. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/4cc13ba1-4248-4884-b6ba-4805349e7f39@iki.fi	2 weeks ago
Álvaro Herrera	96e2af6050	Reject ADD CONSTRAINT NOT NULL if name mismatches existing constraint ... When using ALTER TABLE ... ADD CONSTRAINT to add a not-null constraint with an explicit name, we have to ensure that if the column is already marked NOT NULL, the provided name matches the existing constraint name. Failing to do so could lead to confusion regarding which constraint object actually enforces the rule. This patch adds a check to throw an error if the user tries to add a named not-null constraint to a column that already has one with a different name. Reported-by: yanliang lei <msdnchina@163.com> Co-authored-by: Álvaro Herrera <alvherre@kurilemu.de> Co-authored-bu: Srinath Reddy Sadipiralla <srinath2133@gmail.com> Backpatch-through: 18 Discussion: https://postgr.es/m/19351-8f1c523ead498545%40postgresql.org	2 weeks ago
Peter Eisentraut	955e507668	Change StaticAssertVariableIsOfType to be a declaration ... This allows moving the uses to more natural and useful positions. Also, a declaration is the more native use of static assertions in C. Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/2273bc2a-045d-4a75-8584-7cd9396e5534%40eisentraut.org	2 weeks ago
Peter Eisentraut	137d05df2f	Rename AssertVariableIsOfType to StaticAssertVariableIsOfType ... This keeps run-time assertions and static assertions clearly separate. Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/2273bc2a-045d-4a75-8584-7cd9396e5534%40eisentraut.org	2 weeks ago
Tom Lane	da7a1dc0d6	Refactor att_align_nominal() to improve performance. ... Separate att_align_nominal() into two macros, similarly to what was already done with att_align_datum() and att_align_pointer(). The inner macro att_nominal_alignby() is really just TYPEALIGN(), while att_align_nominal() retains its previous API by mapping TYPALIGN_xxx values to numbers of bytes to align to and then calling att_nominal_alignby(). In support of this, split out tupdesc.c's logic to do that mapping into a publicly visible function typalign_to_alignby(). Having done that, we can replace performance-critical uses of att_align_nominal() with att_nominal_alignby(), where the typalign_to_alignby() mapping is done just once outside the loop. In most places I settled for doing typalign_to_alignby() once per function. We could in many places pass the alignby value in from the caller if we wanted to change function APIs for this purpose; but I'm a bit loath to do that, especially for exported APIs that extensions might call. Replacing a char typalign argument by a uint8 typalignby argument would be an API change that compilers would fail to warn about, thus silently breaking code in hard-to-debug ways. I did revise the APIs of array_iter_setup and array_iter_next, moving the element type attribute arguments to the former; if any external code uses those, the argument-count change will cause visible compile failures. Performance testing shows that ExecEvalScalarArrayOp is sped up by about 10% by this change, when using a simple per-element function such as int8eq. I did not check any of the other loops optimized here, but it's reasonable to expect similar gains. Although the motivation for creating this patch was to avoid a performance loss if we add some more typalign values, it evidently is worth doing whether that patch lands or not. Discussion: https://postgr.es/m/1127261.1769649624@sss.pgh.pa.us	2 weeks ago
Tom Lane	0c9f46c428	In s_lock.h, use regular labels with %= instead of local labels. ... Up to now we've used GNU-style local labels for branch targets in s_lock.h's assembly blocks. But there's an alternative style, which I for one didn't know about till recently: use regular assembler labels, and insert a per-asm-block number in them using %= to ensure they are distinct across multiple TAS calls within one source file. gcc has had %= since gcc 2.0, and I've verified that clang knows it too. While the immediate motivation for changing this is that AIX's assembler doesn't do local labels, it seems to me that this is a superior solution anyway. There is nothing mnemonic about "1:", while a regular label can convey something useful, and at least to me it feels less error-prone. Therefore let's standardize on this approach, also converting the one other usage in s_lock.h. Discussion: https://postgr.es/m/399291.1769998688@sss.pgh.pa.us	2 weeks ago
Michael Paquier	d46aa32ea5	Fix build inconsistency due to the generation of wait-event code ... The build generates four files based on the wait event contents stored in wait_event_names.txt: - wait_event_types.h - pgstat_wait_event.c - wait_event_funcs_data.c - wait_event_types.sgml The SGML file is generated as part of a documentation build, with its data stored in doc/src/sgml/ for meson and configure. The three others are handled differently for meson and configure: - In configure, all the files are created in src/backend/utils/activity/. A link to wait_event_types.h is created in src/include/utils/. - In meson, all the files are created in src/include/utils/. The two C files, pgstat_wait_event.c and wait_event_funcs_data.c, are then included in respectively wait_event.c and wait_event_funcs.c, without the "utils/" path. For configure, this does not present a problem. For meson, this has to be combined with a trick in src/backend/utils/activity/meson.build, where include_directories needs to point to include/utils/ to make the inclusion of the C files work properly, causing builds to pull in PostgreSQL headers rather than system headers in some build paths, as src/include/utils/ would take priority. In order to fix this issue, this commit reworks the way the C/H files are generated, becoming consistent with guc_tables.inc.c: - For meson, basically nothing changes. The files are still generated in src/include/utils/. The trick with include_directories is removed. - For configure, the files are now generated in src/backend/utils/, with links in src/include/utils/ pointing to the ones in src/backend/. This requires extra rules in src/backend/utils/activity/Makefile so as a make command in this sub-directory is able to work. - The three files now fall under header-stamp, which is actually simpler as guc_tables.inc.c does the same. - wait_event_funcs_data.c and pgstat_wait_event.c are now included with "utils/" in their path. This problem has not been an issue in the buildfarm; it has been noted with AIX and a conflict with float.h. This issue could, however, create conflicts in the buildfarm depending on the environment with unexpected headers pulled in, so this fix is backpatched down to where the generation of the wait-event files has been introduced. While on it, this commit simplifies wait_event_names.txt regarding the paths of the files generated, to mention just the names of the files generated. The paths where the files are generated became incorrect. The path of the SGML path was wrong. This change has been tested in the CI, down to v17. Locally, I have run tests with configure (with and without VPATH), as well as meson, on the three branches. Combo oversight in fa88928470 and 1e68e43d3f. Reported-by: Aditya Kamath <aditya.kamath1@ibm.com> Discussion: https://postgr.es/m/LV8PR15MB64888765A43D229EA5D1CFE6D691A@LV8PR15MB6488.namprd15.prod.outlook.com Backpatch-through: 17	2 weeks ago
Heikki Linnakangas	e2362eb2bd	Move shmem allocator's fields from PGShmemHeader to its own struct ... For readability. It was a slight modularity violation to have fields in PGShmemHeader that were only used by the allocator code in shmem.c. And it was inconsistent that ShmemLock was nevertheless not stored there. Moving all the allocator-related fields to a separate struct makes it more consistent and modular, and removes the need to allocate and pass ShmemLock separately via BackendParameters. Merge InitShmemAccess() and InitShmemAllocation() into a single function that initializes the struct when called from postmaster, and when called from backends in EXEC_BACKEND mode, re-establishes the global variables. That's similar to all the *ShmemInit() functions that we have. Co-authored-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/CAExHW5uNRB9oT4pdo54qAo025MXFX4MfYrD9K15OCqe-ExnNvg@mail.gmail.com	2 weeks ago
Andres Freund	87f7b824f2	tableam: Perform CheckXidAlive check once per scan ... Previously, the CheckXidAlive check was performed within the table_scan*next* functions. This caused the check to be executed for every fetched tuple, an unnecessary overhead. To fix, move the check to table_beginscan* so it is performed once per scan rather than once per row. Note: table_tuple_fetch_row_version() does not use a scan descriptor; therefore, the CheckXidAlive check is retained in that function. The overhead is unlikely to be relevant for the existing callers. Reported-by: Andres Freund <andres@anarazel.de> Author: Dilip Kumar <dilipbalaut@gmail.com> Suggested-by: Andres Freund <andres@anarazel.de> Suggested-by: Amit Kapila <akapila@postgresql.org> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/tlpltqm5jjwj7mp66dtebwwhppe4ri36vdypux2zoczrc2i3mp%40dhv4v4nikyfg	2 weeks ago
Tom Lane	bd9dfac8b1	Further fix extended alignment for older g++. ... Commit 6ceef9408 was still one brick shy of a load, because it caused any usage at all of PGIOAlignedBlock or PGAlignedXLogBlock to fail under older g++. Notably, this broke "headerscheck --cplusplus". We can permit references to these structs as abstract structs though; only actual declaration of such a variable needs to be forbidden. Discussion: https://www.postgresql.org/message-id/3119480.1769189606@sss.pgh.pa.us	2 weeks ago
Michael Paquier	efbebb4e85	Add support for "mcv" in pg_restore_extended_stats() ... This commit adds support for the restore of extended statistics of the kind "mcv", aka most-common values. This format is different from n_distinct and dependencies stat types in that it is the combination of three of the four different arrays from the pg_stats_ext view which in turn require three different input parameters on pg_restore_extended_statistics(). These are translated into three input arguments for the function: - "most_common_vals", acting as a leader of the others. It is a 2-dimension array, that includes the common values. - "most_common_freqs", 1-dimension array of float8[], with a number of elements that has to match with "most_common_vals". - "most_common_base_freqs", 1-dimension array of float8[], with a number of elements that has to match with "most_common_vals". All three arrays are required to achieve the restore of this type of extended statistics (if "most_common_vals" happens to be NULL in the catalogs, the rest is NULL by design). Note that "most_common_val_nulls" is not required in input, its data is rebuilt from the decomposition of the "most_common_vals" array based on its text[] representation. The initial versions of the patch provided this option in input, but we do not require it and it simplifies a lot the result. Support in pg_dump is added down to v13 which is where the support for this type of extended statistics has been added, when --statistics is used. This means that upgrade and dumps can restore extended statistics data transparently, like "dependencies", "ndistinct", attribute and relation statistics. For MCV, the values are directly queried from the relevant catalogs. Author: Corey Huinker <corey.huinker@gmail.com> Co-authored-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com	2 weeks ago
Masahiko Sawada	8f1e2dfe03	Consolidate replication origin session globals into a single struct. ... This commit moves the separate global variables for replication origin state into a single ReplOriginXactState struct. This groups logically related variables, which improves code readability and simplifies state management (e.g., resetting the state) by handling them as a unit. Author: Chao Li <lic@highgo.com> Suggested-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/CAEoWx2=pYvfRthXHTzSrOsf5_FfyY4zJyK4zV2v4W=yjUij1cA@mail.gmail.com	2 weeks ago
Masahiko Sawada	227eb4eea2	Refactor replication origin state reset helpers. ... Factor out common logic for clearing replorigin_session_* variables into a dedicated helper function, replorigin_xact_clear(). This removes duplicated assignments of these variables across multiple call sites, and makes the intended scope of each reset explicit. Author: Chao Li <lic@highgo.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/CAEoWx2=pYvfRthXHTzSrOsf5_FfyY4zJyK4zV2v4W=yjUij1cA@mail.gmail.com	2 weeks ago
Masahiko Sawada	1fdbca159e	Standardize replication origin naming to use "ReplOrigin". ... The replication origin code was using inconsistent naming conventions. Functions were typically prefixed with 'replorigin', while typedefs and constants used "RepOrigin". This commit unifies the naming convention by renaming RepOriginId to ReplOriginId. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CAD21AoBDgm3hDqUZ+nqu=ViHmkCnJBuJyaxG_yvv27BAi2zBmQ@mail.gmail.com	2 weeks ago
Robert Haas	4020b370f2	Allow for plugin control over path generation strategies. ... Each RelOptInfo now has a pgs_mask member which is a mask of acceptable strategies. For most rels, this is populated from PlannerGlobal's default_pgs_mask, which is computed from the values of the enable_* GUCs at the start of planning. For baserels, get_relation_info_hook can be used to adjust pgs_mask for each new RelOptInfo, at least for rels of type RTE_RELATION. Adjusting pgs_mask is less useful for other types of rels, but if it proves to be necessary, we can revisit the way this hook works or add a new one. For joinrels, two new hooks are added. joinrel_setup_hook is called each time a joinrel is created, and one thing that can be done from that hook is to manipulate pgs_mask for the new joinrel. join_path_setup_hook is called each time we're about to add paths to a joinrel by considering some particular combination of an outer rel, an inner rel, and a join type. It can modify the pgs_mask propagated into JoinPathExtraData to restrict strategy choice for that particular combination of rels. To make joinrel_setup_hook work as intended, the existing calls to build_joinrel_partition_info are moved later in the calling functions; this is because that function checks whether the rel's pgs_mask includes PGS_CONSIDER_PARTITIONWISE, so we want it to only be called after plugins have had a chance to alter pgs_mask. Upper rels currently inherit pgs_mask from the input relation. It's unclear that this is the most useful behavior, but at the moment there are no hooks to allow the mask to be set in any other way. Reviewed-by: Lukas Fittl <lukas@fittl.com> Reviewed-by: Jakub Wartak <jakub.wartak@enterprisedb.com> Reviewed-by: Greg Burd <greg@burd.me> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Haibo Yan <tristan.yim@gmail.com> Discussion: http://postgr.es/m/CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com	2 weeks ago
Amit Kapila	851f6649cc	Prevent invalidation of newly synced replication slots. ... A race condition could cause a newly synced replication slot to become invalidated between its initial sync and the checkpoint. When syncing a replication slot to a standby, the slot's initial restart_lsn is taken from the publisher's remote_restart_lsn. Because slot sync happens asynchronously, this value can lag behind the standby's current redo pointer. Without any interlocking between WAL reservation and checkpoints, a checkpoint may remove WAL required by the newly synced slot, causing the slot to be invalidated. To fix this, we acquire ReplicationSlotAllocationLock before reserving WAL for a newly synced slot, similar to commit 006dd4b2e5. This ensures that if WAL reservation happens first, the checkpoint process must wait for slotsync to update the slot's restart_lsn before it computes the minimum required LSN. However, unlike in ReplicationSlotReserveWal(), this lock alone cannot protect a newly synced slot if a checkpoint has already run CheckPointReplicationSlots() before slotsync updates the slot. In such cases, the remote restart_lsn may be stale and earlier than the current redo pointer. To prevent relying on an outdated LSN, we use the oldest WAL location available if it is greater than the remote restart_lsn. This ensures that newly synced slots always start with a safe, non-stale restart_lsn and are not invalidated by concurrent checkpoints. Author: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Vitaly Davydov <v.davydov@postgrespro.ru> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Backpatch-through: 17 Discussion: https://postgr.es/m/TY4PR01MB16907E744589B1AB2EE89A31F94D7A%40TY4PR01MB16907.jpnprd01.prod.outlook.com	3 weeks ago
Melanie Plageman	648a7e28d7	Eliminate use of cached VM value in lazy_scan_prune() ... lazy_scan_prune() takes a parameter from lazy_scan_heap() indicating whether the page was marked all-visible in the VM at the time it was last checked in find_next_unskippable_block(). This behavior is historical, dating back to commit 608195a3a3, when we did not pin the VM page until deciding we must read it. Now that the VM page is already pinned, there is no meaningful benefit to relying on a cached VM status. Removing this cached value simplifies the logic in both lazy_scan_heap() and lazy_scan_prune(). It also clarifies future work that will set the visibility map on-access: such paths will not have a cached value available, which would make the logic harder to reason about. And eliminating it enables us to detect and repair VM corruption on-access. Along with removing the cached value and unconditionally checking the visibility status of the heap page, this commit also moves the VM corruption handling to occur first. This reordering should have no performance impact, since the checks are inexpensive and performed only once per page. It does, however, make the control flow easier to understand. The new restructuring also makes it possible to set the VM after fixing corruption (if pruning found the page all-visible). Now that no callers of visibilitymap_set() use its return value, change its (and visibilitymap_set_vmbits()) return type to void. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com> Discussion: https://postgr.es/m/5CEAA162-67B1-44DA-B60D-8B65717E8B05%40gmail.com	3 weeks ago
Peter Eisentraut	5ca5f12c2c	Fix accidentally cast away qualifiers ... This fixes cases where a qualifier (const, in all cases here) was dropped by a cast, but the cast was otherwise necessary or desirable, so the straightforward fix is to add the qualifier into the cast. Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/b04f4d3a-5e70-4e73-9ef2-87f777ca4aac%40eisentraut.org	3 weeks ago
Peter Eisentraut	6ceef9408c	Disable extended alignment uses on older g++ ... Fix for commit a9bdb63bba. The previous plan of redefining alignas didn't work, because it interfered with other C++ header files (e.g., LLVM). So now the new workaround is to just disable the affected typedefs under the affected compilers. These are not typically used in extensions anyway. Discussion: https://www.postgresql.org/message-id/3119480.1769189606%40sss.pgh.pa.us	3 weeks ago
Michael Paquier	0e80f3f88d	Add pg_restore_extended_stats() ... This function closely mirror its relation and attribute counterparts, but for extended statistics (i.e. CREATE STATISTICS) objects, being able to restore extended statistics for an extended stats object. Like the other functions, the goal of this feature is to ease the dump or upgrade of clusters so as ANALYZE would not be required anymore after these operations, stats being directly loaded into the target cluster without any post-dump/upgrade computation. The caller of this function needs the following arguments for the extended stats to restore: - The name of the relation. - The schema name of the relation. - The name of the extended stats object. - The schema name of the extended stats object. - If the stats are inherited or not. - One or more extended stats kind with its data. This commit adds only support for the restore of the extended statistics kind "n_distinct", building the basic infrastructure for the restore of more extended statistics kinds in follow-up commits, including MVC and dependencies. The support for "n_distinct" is eased in this commit thanks to the previous work done particularly in commits 1f927cce44 and 44eba8f06e, that have added the input function for the type pg_ndistinct, used as data type in input of this new restore function. Bump catalog version. Author: Corey Huinker <corey.huinker@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com	3 weeks ago
Michael Paquier	c100340729	Remove PG_MMAP_FLAGS from mem.h ... Based on name of the macro, it was implied that it could be used for all mmap() calls on portability grounds. However, its use is limited to sysv_shmem.c, for CreateAnonymousSegment(). This commit removes the declaration, reducing the confusion around it as a portability tweak, being limited to SysV-style shared memory. This macro has been introduced in b0fc0df936 for sysv_shmem.c, originally. It has been moved to mem.h in 0ac5e5a7e1 a bit later. Suggested by: Peter Eisentraut <peter@eisentraut.org> Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/CAExHW5vTWABxuM5fbQcFkGuTLwaxuZDEE2vtx2WuMUWk6JnF4g@mail.gmail.com Discussion: https://postgr.es/m/12add41a-7625-4639-a394-a5563e349322@eisentraut.org	3 weeks ago
Peter Eisentraut	a9bdb63bba	Work around buggy alignas in older g++ ... Older g++ (<9.3) mishandle the alignas specifier (raise warnings that the alignment is too large), but the more or less equivalent attribute works. So as a workaround, #define alignas to that attribute for those versions. see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89357> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/3119480.1769189606%40sss.pgh.pa.us	3 weeks ago
Dean Rasheed	b4307ae2e5	Fix trigger transition table capture for MERGE in CTE queries. ... When executing a data-modifying CTE query containing MERGE and some other DML operation on a table with statement-level AFTER triggers, the transition tables passed to the triggers would fail to include the rows affected by the MERGE. The reason is that, when initializing a ModifyTable node for MERGE, MakeTransitionCaptureState() would create a TransitionCaptureState structure with a single "tcs_private" field pointing to an AfterTriggersTableData structure with cmdType == CMD_MERGE. Tuples captured there would then not be included in the sets of tuples captured when executing INSERT/UPDATE/DELETE ModifyTable nodes in the same query. Since there are no MERGE triggers, we should only create AfterTriggersTableData structures for INSERT/UPDATE/DELETE. Individual MERGE actions should then use those, thereby sharing the same capture tuplestores as any other DML commands executed in the same query. This requires changing the TransitionCaptureState structure, replacing "tcs_private" with 3 separate pointers to AfterTriggersTableData structures, one for each of INSERT, UPDATE, and DELETE. Nominally, this is an ABI break to a public structure in commands/trigger.h. However, since this is a private field pointing to an opaque data structure, the only way to create a valid TransitionCaptureState is by calling MakeTransitionCaptureState(), and no extensions appear to be doing that anyway, so it seems safe for back-patching. Backpatch to v15, where MERGE was introduced. Bug: #19380 Reported-by: Daniel Woelfel <dwwoelfel@gmail.com> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19380-4e293be2b4007248%40postgresql.org Backpatch-through: 15	3 weeks ago
Jacob Champion	f7521bf721	pqcomm.h: Explicitly reserve protocol v3.1 ... Document this unused version alongside the other special protocol numbers. Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAOYmi%2BkKyw%3Dh-5NKqqpc7HC5M30_QmzFx3kgq2AdipyNj47nUw%40mail.gmail.com	3 weeks ago
Michael Paquier	a36164e746	Add WALRCV_CONNECTING state to the WAL receiver ... Previously, a WAL receiver freshly started would set its state to WALRCV_STREAMING immediately at startup, before actually establishing a replication connection. This commit introduces a new state called WALRCV_CONNECTING, which is the state used when the WAL receiver freshly starts, or when a restart is requested, with a switch to WALRCV_STREAMING once the connection to the upstream server has been established with COPY_BOTH, meaning that the WAL receiver is ready to stream changes. This change is useful for monitoring purposes, especially in environments with a high latency where a connection could take some time to be established, giving some room between the [re]start phase and the streaming activity. From the point of view of the startup process, that flips the shared memory state of the WAL receiver when it needs to be stopped, the existing WALRCV_STREAMING and the new WALRCV_CONNECTING states have the same semantics: the WAL receiver has started and it can be stopped. Based on an initial suggestion from Noah Misch, with some input from me about the design. Author: Xuneng Zhou <xunengzhou@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Rahila Syed <rahilasyed90@gmail.com> Discussion: https://postgr.es/m/CABPTF7VQ5tGOSG5TS-Cg+Fb8gLCGFzxJ_eX4qg+WZ3ZPt=FtwQ@mail.gmail.com	3 weeks ago
Álvaro Herrera	69f98fce5b	Make some use of anonymous unions [reloptions] ... In the spirit of commit 4b7e6c73b0 and following, which see for more details; it appears to have been quite an uncontroversial C11 feature to use and it makes the code nicer to read. This commit changes the relopt_value struct. Author: Peter Eisentraut <peter@eisentraut.org> Author: Álvaro Herrera <alvherre@kurilemu.de> Note: Yes, this was written twice independently. Discussion: https://postgr.es/m/202601192106.zcdi3yu2gzti@alvherre.pgsql	3 weeks ago
Peter Eisentraut	c257ba8397	Record range constructor functions in pg_range ... When a range type is created, several construction functions are also created, two for the range type and three for the multirange type. These have an internal dependency, so they "belong" to the range type. But there was no way to identify those functions when given a range type. An upcoming patch needs access to the two- or possibly the three-argument range constructor function for a given range type. The only way to do that would be with fragile workarounds like matching names and argument types. The correct way to do that kind of thing is to record to the links in the system catalogs. This is what this patch does, it records the OIDs of these five constructor functions in the pg_range catalog. (Currently, there is no code that makes use of this.) Reviewed-by: Paul A Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://www.postgresql.org/message-id/7d63ddfa-c735-4dfe-8c7a-4f1e2a621058%40eisentraut.org	3 weeks ago
Nathan Bossart	25dc485074	Refactor some SIMD and popcount macros. ... This commit does the following: * Removes TRY_POPCNT_X86_64. We now assume that the required CPUID intrinsics are available when HAVE_X86_64_POPCNTQ is defined, as we have done since v16 for meson builds when USE_SSE42_CRC32C_WITH_RUNTIME_CHECK is defined and since v17 when USE_AVX512_POPCNT_WITH_RUNTIME_CHECK is defined. * Moves the MSVC check for HAVE_X86_64_POPCNTQ to configure-time. This way, we set it for all relevant platforms in one place. * Moves the #defines for USE_SSE2 and USE_NEON to c.h so that they can be used elsewhere without including simd.h. Consequently, we can remove the POPCNT_AARCH64 macro. * Moves the #includes for pg_bitutils.h to below the system headers in pg_popcount_{aarch64,x86}.c, since we no longer depend on macros from pg_bitutils.h to decide which system headers to use. Reviewed-by: John Naylor <johncnaylorls@gmail.com> Discussion: https://postgr.es/m/aWf_InS1VrbeXAfP%40nathan	3 weeks ago
Nathan Bossart	8c6653516c	Rename "fast" and "slow" popcount functions. ... Since we now have several implementations of the popcount functions, let's give them more descriptive names. This commit replaces "slow" with "portable" and "fast" with "sse42". While the POPCNT instruction is technically not part of SSE4.2, this naming scheme is close enough in practice and is arguably easier to understand than using "popcnt" instead. Reviewed-by: John Naylor <johncnaylorls@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/aWf_InS1VrbeXAfP%40nathan	3 weeks ago
Nathan Bossart	79e232ca01	Move x86-64-specific popcount code to pg_popcount_x86.c. ... This moves the remaining x86-64-specific popcount implementations in pg_bitutils.c to pg_popcount_x86.c. Reviewed-by: John Naylor <johncnaylorls@gmail.com> Discussion: https://postgr.es/m/aWf_InS1VrbeXAfP%40nathan	3 weeks ago
Tom Lane	4576208454	Force standard_conforming_strings to always be ON. ... Continuing to support this backwards-compatibility feature has nontrivial costs; in particular it is potentially a security hazard if an application somehow gets confused about which setting the server is using. We changed the default to ON fifteen years ago, which seems like enough time for applications to have adapted. Let's remove support for the legacy string syntax. We should not remove the GUC altogether, since client-side code will still test it, pg_dump scripts will attempt to set it to ON, etc. Instead, just prevent it from being set to OFF. There is precedent for this approach (see commit de66987ad). This patch does remove the related GUC escape_string_warning, however. That setting does nothing when standard_conforming_strings is on, so it's now useless. We could leave it in place as a do-nothing setting to avoid breaking clients that still set it, if there are any. But it seems likely that any such client is also trying to turn off standard_conforming_strings, so it'll need work anyway. The client-side changes in this patch are pretty minimal, because even though we are dropping the server's support, most of our clients still need to be able to talk to older server versions. We could remove dead client code only once we disclaim compatibility with pre-v19 servers, which is surely years away. One change of note is that pg_dump/pg_dumpall now set standard_conforming_strings = on in their source session, rather than accepting the source server's default. This ensures that literals in view definitions and such will be printed in a way that's acceptable to v19+. In particular, pg_upgrade will work transparently even if the source installation has standard_conforming_strings = off. (However, pg_restore will behave the same as before if given an archive file containing standard_conforming_strings = off. Such an archive will not be safely restorable into v19+, but we shouldn't break the ability to extract valid data from it for use with an older server.) Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3279216.1767072538@sss.pgh.pa.us	3 weeks ago
Álvaro Herrera	4d6a66f675	Allow Boolean reloptions to have ternary values ... From the user's point of view these are just Boolean values; from the implementation side we can now distinguish an option that hasn't been set. Reimplement the vacuum_truncate reloption using this type. This could also be used for reloptions vacuum_index_cleanup and buffering, but those additionally need a per-option "alias" for the state where the variable is unset (currently the value "auto"). Author: Nikolay Shaplov <dhyan@nataraj.su> Reviewed-by: Timur Magomedov <t.magomedov@postgrespro.ru> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/3474141.usfYGdeWWP@thinkpad-pgpro	3 weeks ago
Tom Lane	cec5fe0d1e	Remove useless flag PVC_INCLUDE_CONVERTROWTYPES. ... This was introduced in the SJE patch (fc069a3a6), but it doesn't do anything because pull_var_clause() never tests it. Apparently it snuck in from somebody's private fork. Remove it again, but only in HEAD -- seems best to let it be in v18. Author: Alexander Pyhalov <a.pyhalov@postgrespro.ru> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/70008c19d22e3dd1565ca57f8436c0ba@postgrespro.ru	3 weeks ago
Peter Eisentraut	b4555cb070	Fix for C++ compatibility ... After commit 476b35d4e3, some buildfarm members are complaining about not recognizing _Noreturn when building the new C++ module test_cplusplusext. This is not a C++ feature, but it was gated like #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L #define pg_noreturn _Noreturn But apparently that was not sufficient. Some platforms define __STDC_VERSION__ even in C++ mode. (In this particular case, it was g++ on Solaris, but apparently this is also done by some other platforms, and it is allowed by the C++ standard.) To fix, add a ... && !defined(__cplusplus) Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com	3 weeks ago
John Naylor	7892e25924	Update some comments for fasthash ... - Add advice about hashing multiple inputs with the incremental API - Generalize statements that were specific to C strings to include all variable length inputs, where applicable. - Update comments about the standalone functions and make it easy to find them. Reported-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: zengman <zengman@halodbtech.com> Discussion: https://postgr.es/m/CANWCAZZgKnf8dNOd_w03n88NqOfmMnMv2=D8_Oy6ADGyiMq+cg@mail.gmail.com Discussion: https://postgr.es/m/CANWCAZa-2mEUY27xBw2TpsybpvVu3Ez4ABrHCBqZpAs_UDTj2Q@mail.gmail.com	3 weeks ago
Amit Kapila	48efefa6ca	Improve errdetail for logical replication conflict messages. ... This change enhances the clarity and usefulness of error detail messages generated during logical replication conflicts. The following improvements have been made: 1. Eliminate redundant output: Avoid printing duplicate remote row and replica identity values for the multiple_unique_conflicts conflict type. 2. Improve message structure: Append tuple values directly to the main error message, separated by a colon (:), for better readability. 3. Simplify local row terminology: Remove the word 'existing' when referring to the local row, as this is already implied by context. 4. General code refinements: Apply miscellaneous code cleanups to improve how conflict detail messages are constructed and formatted. Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Shveta Malik <shveta.malik@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/CAHut+Psgkwy5-yGRJC15izecySGGysrbCszv_z93ess8XtCDOQ@mail.gmail.com	3 weeks ago
Michael Paquier	7ebb64c557	Add routine to free MCVList ... This addition is in the same spirit as 32e27bd320 for MVNDistinct and MVDependencies, except that we were missing a free routine for the third type of extended statistics, MCVList. I was not sure if we needed an equivalent for MCVList, but after more review of the main patch set for the import of extended statistics, it has become clear that we do. This is introduced as its own change as this routine can be useful on its own. This one is a piece that has not been written by Corey Huinker, I have just noticed it by myself on the way. Author: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com	4 weeks ago
Michael Paquier	d756fa1019	Add pg_clear_extended_stats() ... This function is able to clear the data associated to an extended statistics object, making things so as the object looks as newly-created. The caller of this function needs the following arguments for the extended stats to clear: - The name of the relation. - The schema name of the relation. - The name of the extended stats object. - The schema name of the extended stats object. - If the stats are inherited or not. The first two parameters are especially important to ensure a consistent lookup and ACL checks for the relation on which is based the extended stats object that will be cleared, relying first on a RangeVar lookup where permissions are checked without locking a relation, critical to prevent denial-of-service attacks when using this kind of function (see also 688dc6299a for a similar concern). The third to fifth arguments give a way to target the extended stats records to clear. This has been extracted from a larger patch by the same author, for a piece which is again useful on its own. I have rewritten large portions of it. The tests have been extended while discussing this piece, resulting on what this commit includes. The intention behind this feature is to add support for the import of extended statistics across dumps and upgrades, this change building one piece that we will be able to rely on for the rest of the changes. Bump catalog version. Author: Corey Huinker <corey.huinker@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com	4 weeks ago
Andres Freund	d40fd85187	lwlock: Remove support for disowned lwlwocks ... This reverts commit f8d7f29b3e, plus parts of subsequent commits fixing a typo in a parameter name. Support for disowned lwlocks was added for the benefit of AIO, to be able to have content locks "owned" by the AIO subsystem. But as of commit fcb9c977aa, content locks do not use lwlocks anymore. It does not seem particularly likely that we need this facility outside of the AIO use-case, therefore remove the now unused functions. I did choose to keep the comment added in the aforementioned commit about lock->owner intentionally being left pointing to the last owner. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/cj5mcjdpucvw4a54hehslr3ctukavrbnxltvuzzhqnimvpju5e@cy3g3mnsefwz	4 weeks ago
Andres Freund	55fbfb738b	lwlock: Remove ForEachLWLockHeldByMe ... As of commit fcb9c977aa, ForEachLWLockHeldByMe(), introduced in f4ece891fc, is not used anymore, as content locks are now implemented in bufmgr.c. It doesn't seem that likely that a new user of the functionality will appear all that soon, making removal of the function seem like the most sensible path. It can easily be added back if necessary. Discussion: https://postgr.es/m/lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg%40noioxkquezdw	4 weeks ago
Andres Freund	fcb9c977aa	bufmgr: Implement buffer content locks independently of lwlocks ... Until now buffer content locks were implemented using lwlocks. That has the obvious advantage of not needing a separate efficient implementation of locks. However, the time for a dedicated buffer content lock implementation has come: 1) Hint bits are currently set while holding only a share lock. This leads to having to copy pages while they are being written out if checksums are enabled, which is not cheap. We would like to add AIO writes, however once many buffers can be written out at the same time, it gets a lot more expensive to copy them, particularly because that copy needs to reside in shared buffers (for worker mode to have access to the buffer). In addition, modifying buffers while they are being written out can cause issues with unbuffered/direct-IO, as some filesystems (like btrfs) do not like that, due to filesystem internal checksums getting corrupted. The solution to this is to require a new share-exclusive lock-level to set hint bits and to write out buffers, making those operations mutually exclusive. We could introduce such a lock-level into the generic lwlock implementation, however it does not look like there would be other users, and it does add some overhead into important code paths. 2) For AIO writes we need to be able to race-freely check whether a buffer is undergoing IO and whether an exclusive lock on the page can be acquired. That is rather hard to do efficiently when the buffer state and the lock state are separate atomic variables. This is a major hindrance to allowing writes to be done asynchronously. 3) Buffer locks are by far the most frequently taken locks. Optimizing them specifically for their use case is worth the effort. E.g. by merging content locks into buffer locks we will be able to release a buffer lock and pin in one atomic operation. 4) There are more complicated optimizations, like long-lived "super pinned & locked" pages, that cannot realistically be implemented with the generic lwlock implementation. Therefore implement content locks inside bufmgr.c. The lockstate is stored as part of BufferDesc.state. The implementation of buffer content locks is fairly similar to lwlocks, with a few important differences: 1) An additional lock-level share-exclusive has been added. This lock-level conflicts with exclusive locks and itself, but not share locks. 2) Error recovery for content locks is implemented as part of the already existing private-refcount tracking mechanism in combination with resowners, instead of a bespoke mechanism as the case for lwlocks. This means we do not need to add dedicated error-recovery code paths to release all content locks (like done with LWLockReleaseAll() for lwlocks). 3) The lock state is embedded in BufferDesc.state instead of having its own struct. 4) The wakeup logic is a tad more complicated due to needing to support the additional lock-level This commit unfortunately introduces some code that is very similar to the code in lwlock.c, however the code is not equivalent enough to easily merge it. The future wins that this commit makes possible seem worth the cost. As of this commit nothing uses the new share-exclusive lock mode. It will be used in a future commit. It seemed too complicated to introduce the lock-level in a separate commit. It's worth calling out one wart in this commit: Despite content locks not being lwlocks anymore, they continue to use PGPROC->lw* - that seemed better than duplicating the relevant infrastructure. Another thing worth pointing out is that, after this change, content locks are not reported as LWLock wait events anymore, but as new wait events in the "Buffer" wait event class (see also 6c5c393b74). The old BufferContent lwlock tranche has been removed. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Heikki Linnakangas <heikki.linnakangas@iki.fi> Reviewed-by: Greg Burd <greg@burd.me> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff	4 weeks ago
Andres Freund	dac328c8a6	bufmgr: Change BufferDesc.state to be a 64-bit atomic ... This is motivated by wanting to merge buffer content locks into BufferDesc.state in a future commit, rather than having a separate lwlock (see commit c75ebc657f for more details). As this change is rather mechanical, it seems to make sense to split it out into a separate commit, for easier review. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff	4 weeks ago
Tom Lane	282b1cde9d	Optimize LISTEN/NOTIFY via shared channel map and direct advancement. ... This patch reworks LISTEN/NOTIFY to avoid waking backends that have no need to process the notification messages we just sent. The primary change is to create a shared hash table that tracks which processes are listening to which channels (where a "channel" is defined by a database OID and channel name). This allows a notifying process to accurately determine which listeners are interested, replacing the previous weak approximation that listeners in other databases couldn't be interested. Secondly, if a listener is known not to be interested and is currently stopped at the old queue head, we avoid waking it at all and just directly advance its queue pointer past the notifications we inserted. These changes permit very significant improvements (integer multiples) in NOTIFY throughput, as well as a noticeable reduction in latency, when there are many listeners but only a few are interested in any specific message. There is no improvement for the simplest case where every listener reads every message, but any loss seems below the noise level. Author: Joel Jacobson <joel@compiler.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/6899c044-4a82-49be-8117-e6f669765f7e@app.fastmail.com	4 weeks ago
Álvaro Herrera	35e3fae738	Remove #include <math.h> where not needed ... Liujinyang reported the one in binaryheap.c, I then found and analyzed the rest. For future patches, we require git archaelogical analysis before we accept patches of this nature. Co-authored-by: liujinyang <21043272@qq.com> Co-authored-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/tencent_6B302BFCAF6F010E00AB5C2C0ECB7AA3F205@qq.com	4 weeks ago
Heikki Linnakangas	d9c3c94365	Wake up autovacuum launcher from postmaster when a worker exits ... When an autovacuum worker exits, the launcher needs to be notified with SIGUSR2, so that it can rebalance and possibly launch a new worker. The launcher must be notified only after the worker has finished ProcKill(), so that the worker slot is available for a new worker. Before this commit, the autovacuum worker was responsible for that, which required a slightly complicated dance to pass the launcher's PID from FreeWorkerInfo() to ProcKill() in a global variable. Simplify that by moving the responsibility of the signaling to the postmaster. The postmaster was already doing it when it failed to fork a worker process, so it seems logical to make it responsible for notifying the launcher on worker exit too. That's also how the notification on background worker exit is done. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: li carol <carol.li2025@outlook.com> Discussion: https://www.postgresql.org/message-id/a5e27d25-c7e7-45d5-9bac-a17c8f462def@iki.fi	4 weeks ago
Heikki Linnakangas	c4b71e6f60	Remove some unnecessary code from multixact truncation ... With 64-bit multixact offsets, PerformMembersTruncation() doesn't need the starting offset anymore. The 'oldestOffset' value that TruncateMultiXact() calculates is no longer used for anything. Remove it, and the code to calculate it. 'oldestOffset' was included in the WAL record as 'startTruncMemb', which sounds nice if you e.g. look at the WAL with pg_waldump, but it was also confusing because we didn't actually use the value for determining what to truncate. Replaying the WAL would remove all segments older than 'endTruncMemb', regardless of 'startTruncMemb'. The 'startTruncOff' stored in the WAL record was similarly unnecessary even before 64-bit multixid offsets, it was stored just for the sake of symmetry with 'startTruncMemb'. Remove both from the WAL record, and rename the remaining 'endTruncOff' to 'oldestMulti' and 'endTruncMemb' to 'oldestOffset', for consistency with the variable names used for them in other places. Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://www.postgresql.org/message-id/000301b2-5b81-4938-bdac-90f6eb660843@iki.fi	4 weeks ago
Michael Paquier	32e27bd320	Introduce routines to validate and free MVNDistinct and MVDependencies ... These routines are useful to perform some basic validation checks on each object structure, working currently on attribute numbers for non-expression and expression attnums. These checks could be extended in the future. Note that this code is not used yet in the tree, and that these functions will become handy for an upcoming patch for the import of extended statistics data. However, they are worth their own independent change as they are actually useful by themselves, with at least the extension code argument in mind (or perhaps I am just feeling more pedantic today). Extracted from a larger patch by the same author, with many adjustments and fixes by me. Author: Corey Huinker <corey.huinker@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com	1 month ago
Peter Eisentraut	fa16e7fd84	Revert "Replace pg_restrict by standard restrict" ... This reverts commit f0f2c0c1ae. The original problem that led to the use of pg_restrict was that MSVC couldn't handle plain restrict, and defining it to something else would conflict with its __declspec(restrict) that is used in system header files. In C11 mode, this is no longer a problem, as MSVC handles plain restrict. This led to the commit to replace pg_restrict with restrict. But this did not take C++ into account. Standard C++ does not have restrict, so we defined it as something else (for example, MSVC supports __restrict). But this then again conflicts with __declspec(restrict) in system header files. So we have to revert this attempt. The comments are updated to clarify that the reason for this is now C++ only. Reported-by: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/CAGECzQRoD7chJP1-dneSrhxUJv%2BBRcigoGOO4UwGzaShLot2Yw%40mail.gmail.com	1 month ago
Andres Freund	ff219c1987	bufmgr: Make definitions related to buffer descriptor easier to modify ... This is in preparation to widening the buffer state to 64 bits, which in turn is preparation for implementing content locks in bufmgr. This commit aims to make the subsequent commits a bit easier to review, by separating out reformatting etc from the actual changes. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf	1 month ago