01 split tablecmds.c Rejected 14 msgs Aug 21, 22:29
The proposer initiated a discussion to split the tablecmds.c file, which is over 22,000 lines long and too large for GitHub's code search. The file originated in the 80s as command.c and grew significantly, especially due to recent additions like partitioning features. Several participants agreed with the idea, noting it's the largest source file and that partitioning code likely contributed a large portion of its growth. Initial suggestions for splitting included moving catalog-related changes to catalog/, tablespace functions to commands/tablespace.c, and notably, partitioning and inheritance code to a new file. The proposer then created a preliminary patch to move partitioning-related code to a new file, tablecmds_partition.c, and created tablecmds_internal.h for shared internal structures. Commenters suggested naming conventions like tablecmds_partition.c and introducing tablecmds_internal.h for clearer API boundaries, which the proposer incorporated in subsequent patch versions. However, the patch faced rebasing issues due to tablecmds.c receiving other modifications in the interim. The proposer attempted to fix headerscheck but eventually indicated uncertainty about when they would be able to return to the patch, effectively stalling its progress.
02 Bypassing cursors in postgres_fdw to enable parallel plans Patch Review 48 msgs Aug 21, 10:29
This thread focuses on enhancing `postgres_fdw` to support parallel query execution on remote servers by addressing its current limitation due to cursor usage. The proposer introduced a patch that allows bypassing cursors via a new GUC, `postgres_fdw.use_cursor`, and employs chunked data fetching. A key challenge involves correctly handling simultaneous queries, for which the patch uses a Tuplestore to aggregate results. Reviewer 1 provided extensive feedback, highlighting issues with `active_scan` pointer management, consistency in EXPLAIN output, and the design of `drain_other_active_scan` and `save_to_tuplestore` functions. The discussion progressed through multiple patch revisions, with the proposer iteratively addressing the raised concerns. Architectural considerations regarding connection state management and potential similarities between existing and new data handling mechanisms were also discussed.
03 [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs Discussing 17 msgs Aug 21, 16:29
This thread proposed adding a new SQL function, `pg_current_vxact_id()`, to directly expose a backend's virtual transaction ID (VXID). The proposer argued that VXIDs are crucial for tracking all transactions, including read-only ones, and for log correlation, but currently require inefficient `pg_locks` queries or log parsing. The patch aimed to provide an O(1) direct memory read, improving performance and API consistency. Initial reviews questioned the compelling use cases, suggesting `pg_locks` or PID for correlation. The proposer responded by emphasizing performance gains, semantic clarity (avoiding the `pg_locks` subsystem for a transaction ID), and alignment with PostgreSQL's pattern of providing direct accessor functions. The discussion also involved suggestions for OID range, format string macro `VXID_FMT`, and consistent documentation terminology (`localXID`). A reviewer noted the performance argument was theoretical and requested empirical evidence, leading the proposer to provide `pgbench` results showing significant speed improvements.
04 Possible replace of strncpy on xactdesc.c Rejected 9 msgs Aug 21, 20:28
The proposer initiated a discussion to replace a `strncpy()` call with `strlcpy()` in `xactdesc.c`, arguing that other functions in the same file already use `strlcpy()` and it's a generally better practice. A reviewer initially supported the general idea, and another reviewer confirmed the patch worked without regressions after local testing. However, a third reviewer questioned whether `strlcpy()` was appropriate given that the length of the string (GID) was already known, suggesting `memcpy()` might be more direct. A core developer responded, noting that `memcpy()` would relax a type check that `strlcpy()` implicitly provides. Ultimately, the proposer discovered that a similar change had already been committed by another developer (`dd50eb9145e`) while his patch was under review. Due to this independent commitment, the proposer decided to close their commitfest entry for this patch.
05 [PATCH] Add support for SAOP in the optimizer for partial index paths Proposed 14 msgs Aug 21, 16:29
The proposer introduced a patch to enhance PostgreSQL's optimizer, enabling it to utilize ScalarArrayOpExpr (SAOP) clauses (like ANY() and IN()) with partial index paths, a feature previously limited to BitmapOr paths. Initial review from a reviewer suggested several improvements, including pre-filtering suitable indexes, optimizing loops, consolidating test cases into bitmapopts.sql, and benchmarking performance overheads. The proposer implemented some of these, incorporating a bitmap for candidate indexes and a significant refactor to improve index selection logic. Further optimizations were added to handle cases where only synthesized equality clauses were matched. A new reviewer provided detailed feedback, noting whitespace errors, suggesting patch consolidation, dead code removal, and stylistic improvements. The proposer acknowledged these comments, noted that the patch had missed its commitfest deadline, and expressed intent to rebase and address the feedback in a future version.
06 Credits For v19 Discussing 21 msgs Aug 21, 06:29
A participant describes a new system for automatically generating PostgreSQL release credits for v19, using previous release data and mailing list emails to identify contributors. The system aims to streamline the process but has gaps for first-time contributors or initial typos. To address this, the participant published three lists: unverified names, emails without associated names, and preferred names with variations, requesting community review and corrections. Multiple community members responded, providing feedback on the spelling and order of various names, particularly Chinese, Indian, and other non-Western names. There was also discussion about how to handle names that appear multiple times or have different spellings (e.g., with or without umlauts), and a specific question arose about whether AI tools should be included in the credits if they contribute. The proposer continued to refine the lists based on feedback, including a patch to fix canonical sources, and asked for suggestions on how to distribute larger commit log files for crowdsourced review.
07 hashjoins vs. Bloom filters (yet again) Patch Review 69 msgs Aug 21, 22:29
The proposer initiated a discussion on reintroducing Bloom filters to hash joins, aiming for performance improvements in selective joins and situations with large, spilling hash tables. Earlier attempts faced challenges with cost estimation and filter sizing. Commenter 1 highlighted an issue with the current selectivity estimation, which uses ordinary join cardinality, potentially leading to pessimistic outcomes with join fanout, and suggested semi-join selectivity. The proposer agreed on the need for exact build_relids matches for correct estimates but acknowledged a potential trade-off in snowflake schemas. Commenter 2 provided a detailed survey of Bloom filter implementations in other analytical databases, noting common filter types (Bloom, IN-list, min/max range) but differences in planning approaches. Reviewer 1 offered a comprehensive review of a patch version, identifying issues like probe cost charging and dead code paths, while confirming correctness in testing. The proposer incorporated several fixes into a new patch version. There was a brief side discussion, where Commenter 2 inquired about combining indexes, which the proposer clarified as largely orthogonal to the thread's main topic of dynamic filter pushdown. The ongoing discussion revolves around fine-tuning the selectivity estimates and the impact of strict build_relids matching on various query patterns.
08 heapam_relation_toast_am() returns the wrong AM for a wrapped heap AM Rejected 4 msgs Aug 21, 19:28
The thread discusses an issue where `heapam_relation_toast_am()` returned the OID of a wrapping table access method (AM) instead of the literal heap AM OID, causing problems for TOAST table creation and indexing when an AM reuses heap's `TableAmRoutine`. The proposer suggested a fix by explicitly returning `HEAP_TABLE_AM_OID`. However, two commenters argued against this, stating that the function is heap-specific and any wrapping AM should explicitly override the `relation_toast_am` callback if it wants heap-like TOAST behavior. They also raised concerns about testing and potential breakage of existing code. The proposer ultimately withdrew the proposal, acknowledging the validity of the objections.
09 toast table corrupted by vacuum - missing chunk number 0 for toast value Discussing 8 msgs Aug 21, 17:29
The original poster reported a rare corruption issue in PostgreSQL 14.20 where TOAST tables became corrupted, manifesting as 'missing chunk number 0 for toast value', immediately following a `VACUUM` operation. The corruption involved a TOAST page where pruning occurred, leaving only dead tuple pointers. Despite the corruption, the associated main table rows were not updated, and their `xmin`, `xmax` values matched those of the TOAST rows. The issue is highly infrequent and could not be reproduced on restored instances. Commenters suggested checking for fixes in later minor versions or investigating if an old row version was erroneously made visible after its TOAST data had been legitimately removed. However, further investigation is severely limited as the necessary historical WAL segments and backups are no longer available, making it challenging to pinpoint the root cause.
10 missing possibility to use alternative translated month names in to_char function Patch Review 9 msgs Aug 21, 22:29
The proposer identified an issue where to_char(current_date, 'tmmonth') often returns translated month names in the genitive case due to glibc behavior, which is problematic when the nominative case is needed. The proposer noted that glibc's strftime function offers an alternative format specifier (%OB) for nominative month names. A patch was proposed to add a new to_char modifier, TAMMONTH, to allow users to access these alternative nominative forms. Reviewer 1 provided feedback, confirming the patch's purpose, testing, and documentation. They identified an issue where the abbreviated form TAMMON fell back to English instead of being localized and questioned a redundant if condition and potential strftime_l compatibility issues. The proposer subsequently updated the patch to implement TAMMON (using %Ob) and removed the redundant if condition. Reviewer 1 gave a '+1 for Ready for Committer' after reviewing the updated patch, suggesting minor refactoring.
11 [PATCH] doc: clarify AS requirement when VALUES used in a FROM clause Patch Review 7 msgs Aug 21, 14:29
The thread discusses an outdated statement in the PostgreSQL documentation regarding the AS clause requirement when VALUES is used in a FROM clause. The original documentation incorrectly stated that an AS clause is required, which has not been true since PostgreSQL 16. The proposer submitted a patch to correct this. Commenter 1 suggested alternative wording to clarify that the AS clause itself is optional, not just the column names within it. The proposer refined the patch based on this feedback, and commenter 2 further suggested using wording similar to queries.sgml to distinguish between a table alias and column aliases. The proposer accepted this, and a revised patch was submitted. The discussion then moved to whether the patch should be backported to earlier versions (v14 and v15) and how to phrase the good practice recommendation for table aliases.
12 Tracking role modification timestamps in pg_authid / pg_roles Discussing 6 msgs Aug 21, 13:28
The proposer introduced a feature to add a `rollastupdated` `timestamptz` column to `pg_authid` (and its views). The goal is to provide a low-cost mechanism for declarative role management tools to detect changes in role definitions, thus enabling more efficient reconciliation by avoiding unnecessary `ALTER ROLE` commands. This timestamp would update upon role creation, alteration, renaming, and GUC setting changes. Initial feedback included a suggestion for a shorter column name. A reviewer questioned why this specific catalog was chosen, asking for clarification on the general applicability or specific need for `pg_authid` over other catalogs, prompting a discussion about the scope of such a feature.
13 Possible race condition in pg_basebackup Discussing 3 msgs Aug 21, 18:28
The proposer identified a race condition in `pg_basebackup` when multiple instances are run concurrently with `--wal-method=stream --create-slot`. The issue occurs because `BASE_BACKUP` (which triggers a checkpoint) is sent before `StartLogStreamer()` creates the slot. This timing gap allows a subsequent checkpoint (potentially from a concurrent backup) to recycle needed WAL segments before the slot can reserve them, leading to 'WAL segment already removed' errors. Reviewer 1 confirmed the analysis, noting a prior theoretical mention of this race, and suggested a design discussion, proposing the server itself should temporarily retain WAL from the REDO location until the WAL streamer connects. Reviewer 2 connected this problem to recent commits (PG14-19) that addressed similar race conditions in replication slot invalidation by acquiring exclusive locks during WAL reservation, providing relevant context for a comprehensive solution.
14 Reduce memory overheads for storing a Memoize tuple Patch Review 7 msgs Aug 21, 16:29
The proposer identified an optimization to reduce memory overheads for storing `MemoizeTuple`s in `nodeMemoize.c`. By using `ExecCopySlotMinimalTupleExtra()` to store the `next` pointer within extra bytes allocated alongside the `MinimalTuple`, the patch saves 16 bytes per cached tuple and reduces `palloc` calls. An example demonstrated significant memory reduction. A reviewer suggested defining a macro for `MAXALIGN(sizeof(MinimalTuple))`, which the proposer incorporated. The patch was subsequently committed. However, post-commit, buildfarm failures emerged on 32-bit builds because the test case's `Memoize` cache entries fit within the minimum `work_mem`, preventing evictions and thus not exercising the intended behavior. A reviewer proposed a fix to modify the test to use `COUNT(t1)`, which would force `Memoize` to cache the entire inner tuple and correctly expose the memory saving scenario.
15 Parallel Apply Patch Review 11 msgs Aug 21, 03:29
The thread discusses the "Parallel Apply" feature, which aims to improve logical replication performance by applying changes in parallel. The proposer initially submitted a patch set (v17). Early discussions revolved around the choice of hash table implementations, their naming conventions, and minor code suggestions. A reviewer provided detailed feedback on patch003, focusing on comments, scope/lifespan of hash table entries, and commit message clarity. The proposer addressed these comments, leading to new patch versions (v19). Another reviewer provided further comments on v19-0001 and v19-0002, concerning struct placement, macro usage, and code style, which the proposer largely addressed in a subsequent version. The proposer then re-split patches, rewrote commit messages, and introduced a change-size-based memory limit control for dependency tracking. Subsequent updates also addressed issues with partitioned tables, unique key/foreign key dependency tracking, and a DML operation bug. The latest update (V21) fixed additional dependency checks and improved safety checks for concurrent schema changes.
16 Many of psql's describe functions bloat cache / waste mem Patch Review 3 msgs Aug 21, 14:29
The thread originated from a report by an initial reporter about excessive memory usage (~7MB) by psql's \df command, even in databases without user-defined functions. This was attributed to the query populating catcaches for every system function. The proposer submitted a patch to optimize describeFunctions() by filtering functions in pg_catalog and information_schema by p.pronamespace *before* pg_function_is_visible() is evaluated, when system functions are not requested and no pattern is supplied. The patch significantly reduced CacheMemoryContext increase and execution time. A reviewer tested the patch, confirmed memory reduction, and analyzed the query plan. However, the reviewer identified a potential issue where the PostgreSQL planner could reorder the filter conditions, causing pg_function_is_visible() to still run for every catalog function, negating the optimization. The reviewer demonstrated this by altering the function cost.
17 Further cleanup related to statistics import support in postgres_fdw Patch Review 5 msgs Aug 21, 21:28
This thread focuses on a cleanup patch for the `postgres_fdw` extension, specifically related to statistics import support. The proposer introduced changes aimed at improving code readability and consistency, including reordering struct definitions and function arguments, renaming variables, fixing typos, and clarifying comments. A reviewer provided detailed feedback, generally approving the changes but raising minor concerns about the long-term maintainability of strict ordering and suggesting adjustments to comment wording. The proposer integrated most of the feedback, pushing the typo fixes separately and incorporating other suggestions into a revised patch. The latest review confirms the updated patch looks good, with only a minor `pgindent` formatting issue remaining. The discussion also included a recommendation for backporting the changes to PG19 to minimize version differences.
18 Support EXCEPT for TABLES IN SCHEMA publications Discussing 81 msgs Aug 21, 18:28
This lengthy thread discusses implementing `EXCEPT` clauses for `FOR TABLES IN SCHEMA` publications, primarily focusing on complex interactions with inheritance and cross-schema partitions. Debates centered on the level of protection against user misconfigurations versus simplifying rules and assuming user intent. Reviewer 1 consistently argued for erroring out on conflicting publication definitions. Commenter 1 (the original proposer) proposed a simpler 'fine-grained clauses take precedence' rule but later identified issues with it. The thread also explored whether existing `FOR TABLE` rules about implicit descendant inclusion apply to `FOR TABLES IN SCHEMA`. Multiple patch versions (v25-v29) were shared, with code complexity being a major concern. The latest message from a commenter proposes that DDL operations should not be restricted by publication metadata and that 'INCLUSION wins over EXCLUSION' in conflicting publication rules, drawing parallels with `pgoutput`'s behavior.
19 MERGE/SPLIT PARTITIONS issues/questions Discussing 31 msgs Aug 21, 14:29
This extensive thread discusses multiple issues and questions surrounding the MERGE PARTITIONS and SPLIT PARTITIONS commands. The initial proposer raised concerns about logical replication breaking due to plain heap inserts for moved rows, changes in generated columns, propagation to subscribers, and silent changes to table properties like RLS, replica identity, and publication membership. Early discussions revolved around how to handle divergences between partitions and the parent table, such as RLS policies and table access methods. A set of patches was proposed to address some of these issues, particularly by rejecting operations that would silently drop RLS policies. There's an ongoing debate about whether the feature should disallow merge/split operations if child partitions have *any* differences from the parent to ensure safety and simplify implementation for the upcoming release, or if more nuanced handling of various divergences is needed. One specific patch regarding table access methods was criticized for still allowing invisible changes.
20 Fix CPU cost of right-semi and right-anti hash joins Patch Review 12 msgs Aug 21, 17:29
The proposer identified and provided a fix for a costing bug in `final_cost_hashjoin()` affecting `JOIN_RIGHT_SEMI` and `JOIN_RIGHT_ANTI` hash joins. The `cpu_tuple_cost` was being overestimated because it was charged on `hashjointuples` derived from the outer side, rather than the inner rows actually emitted by these join types. This led to the optimizer choosing slower query plans, with an example showing a 3x performance improvement after the fix. Initial patches suggested charging `cpu_tuple_cost` on the path's own row estimate. However, a reviewer pointed out this could undercharge for tuples examined but filtered. The proposer then refined the patch to compute `hashjointuples` more accurately for `right-semi` and `right-anti` joins, mirroring how `JOIN_SEMI` and `JOIN_ANTI` already handle the outer side. The latest version of the patch (v4) specifically addresses `JOIN_RIGHT_ANTI` for joinqual costing. The discussion indicates a thorough review process to ensure costing accuracy across different qual types.
21 Introduce XID age based replication slot invalidation Patch Review 46 msgs Aug 21, 22:29
The proposer restarted a discussion about implementing XID-age based replication slot invalidation to address problems like WAL accumulation and xmin/catalog_xmin holding back vacuuming, which can lead to transaction ID wraparound. The existing idle_replication_slot_timeout addresses inactivity but not necessarily high XID churn, prompting the need for max_slot_xid_age. The proposed solution introduces a max_slot_xid_age GUC, invalidating slots with XID age exceeding this limit during CHECKPOINT and VACUUM operations, prioritizing database availability. Early patches addressed error handling and spinlock issues. Commenter 1 raised concerns about how synced slots on standbys would interact with this new mechanism, especially regarding their xmin values potentially holding back the primary via hot_standby_feedback. The proposer investigated this, concluding that synced slots should indeed be invalidated by max_slot_xid_age on the standby if their catalog_xmin is aged and blocking the primary, proposing a TAP test to confirm this behavior. Reviewer 1 agreed with invalidating aged synced slots but questioned a specific scenario related to long hot_standby_feedback intervals. Commenter 1 also suggested adding alerts or a new column to pg_replication_slots to warn users about impending invalidation, which the proposer acknowledged as useful but proposed to discuss separately. The latest patch (v14) incorporates support for invalidating XID-aged synced replication slots on standbys and includes a new TAP test for this case.
22 Allow a prosupport function to be attached to an aggregate Patch Review 10 msgs Aug 21, 15:28
The proposer identified a limitation where extensions cannot attach a planner support function to an aggregate, despite the existence of `SupportRequestSimplifyAggref` for the planner. This is due to `CREATE AGGREGATE` and `ALTER FUNCTION` not supporting the `SUPPORT` clause for aggregates. The proposer provided a patch to add `SUPPORT` to `CREATE AGGREGATE` and `ALTER AGGREGATE`. Commenter 1 initially suggested delaying this to v20, but commenter 2 insisted it was a "must-fix" for v19 due to the extensibility principle. Commenter 2 then submitted a revised patch focusing on adding `SUPPORT` to `CREATE AGGREGATE` and ensuring `pg_dump` compatibility, acknowledging that the initial approach of allowing `ALTER FUNCTION` on aggregates was non-orthogonal. The proposer appreciated this but pointed out that it didn't address the goal of optimizing built-in aggregates like `sum()` or `avg()` without redefining their entire structure, suggesting `ALTER AGGREGATE ... SUPPORT` for this use case. Commenter 2 firmly rejected this, citing concerns about modifying built-in aggregates via extensions and the complexity of such a change at this stage of the release cycle. The discussion indicates a path forward for user-defined aggregates via `CREATE AGGREGATE` but a clear rejection of modifying built-in aggregates through `ALTER AGGREGATE` for the current release.
23 problems with toast.* reloptions Committed 39 msgs Aug 21, 22:29
The proposer identified several bugs in how VACUUM and autovacuum handle toast.* reloptions, specifically concerning the inheritance of parameters from the main table to its TOAST table, which was not happening consistently as per documentation. Issues included vacuum_rel() not looking up main relation reloptions, autovacuum only inheriting if no TOAST reloptions were set, and autovacuum not resolving some parameters like vacuum_truncate independently. The proposer outlined a plan to fix these by teaching autovacuum and vacuum_rel() to properly combine reloptions (TOAST winning if set) and for autovacuum to resolve all parameters upfront. Through multiple patch versions, the solution involved introducing helper functions like merge_toast_reloptions() and get_effective_relopts() to consolidate the logic. Reviewer 1 suggested optimizations and encapsulation, which the proposer addressed, simplifying code and improving API contracts. Reviewer 2 provided further feedback. The proposer committed initial cleanup patches (0001-0003) and later committed the remaining patches, confirming the completion of the work.
24 [PATCH] Fix NULL dereference in subscription REFRESH on concurrent DROP Committed 8 msgs Aug 21, 18:28
This thread addresses a NULL dereference crash in `ALTER SUBSCRIPTION ... REFRESH PUBLICATION` when subscribed tables or sequences are concurrently dropped. The original issue was that `get_rel_name()` or `get_namespace_name()` could return NULL for dropped relations, which `quote_literal_cstr()` would then dereference, causing a segfault. The proposed fix, iterated through several versions, evolved from simple NULL checks to using `try_table_open()` and then back to simpler NULL checks, along with refactoring and adding a TAP test case. After much discussion and refinement, including feedback on logging policy (deciding to skip silently) and test placement, the patch reached version 4. The latest message confirms the patch is considered good and will be prepared for backbranching and pushed.
25 Allow table AMs to define their own reloptions Patch Review 16 msgs Aug 21, 19:28
The thread proposes allowing table access methods (AMs) to define their own relation options (reloptions) through a new `relation_options` routine, similar to index AMs. This would enable AMs to manage their specific options, such as `fillfactor` or `toast_tuple_target`, independently from heap's default options. An initial patch was provided by the original proposer. Later, a different proposer introduced a new, competing patch for the same feature. Reviewers identified several issues with the latest patch, including incorrect handling of `reloptions` during `ALTER TABLE`, and bugs in the accompanying `dummy_table_am` test module related to `fillfactor` and support for text columns. The latest response from the second proposer confirms these bugs and details fixes implemented in an upcoming patch version. The discussion involves addressing the technical challenges of integrating custom reloptions while maintaining compatibility and correct behavior during AM changes.
26 heapam_relation_toast_am() returns the wrong AM for a wrapped heap AM Rejected 1 msgs Aug 21, 19:28
This thread is a follow-up to an earlier discussion regarding `heapam_relation_toast_am()`. The proposer, in response to feedback from a previous exchange, explicitly withdraws the patch proposal. The proposer acknowledges that any table access method (AM) that wraps heap's `TableAmRoutine` must also explicitly override the `relation_toast_am` callback, rather than expecting `heapam_relation_toast_am()` to return `HEAP_TABLE_AM_OID` for it. The proposer plans to document this implementation detail to prevent future issues.
27 CREATE OR REPLACE MATERIALIZED VIEW Discussing 17 msgs Aug 21, 13:28
The thread focuses on implementing a `CREATE OR REPLACE MATERIALIZED VIEW` command. The proposer submitted multiple patch versions (v5-v8). Early discussions involved addressing issues with the `IF NOT EXISTS` clause (which was subsequently dropped) and ensuring the 'replace' semantics align with the idempotency principle observed in other `CREATE OR REPLACE` commands, particularly concerning tablespace and options. Reviewers noted limitations, such as inability to drop columns during replacement, and potential race conditions. Later, a core developer questioned the `CREATE OR REPLACE` approach for materialized views, suggesting an `ALTER MATERIALIZED VIEW` command might be more appropriate to modify only the query while preserving existing data and other properties. This shift in discussion indicates ongoing debate about the fundamental approach and syntax.
28 ProcArrayAdd/ProcArrayRemove in Prepared Transaction Discussing 3 msgs Aug 21, 11:28
The proposer identified a performance bottleneck in distributed PostgreSQL environments related to `ProcArrayAdd` and `ProcArrayRemove` operations during prepared transaction management. These functions acquire an exclusive `ProcArrayLock` and perform `memmove` operations, which can cause contention. The proposer suggested an optimization: caching `PGPROC` structures for prepared transactions within `ProcArray` to eliminate the need for frequent `Add`/`Remove` calls. This approach raises questions regarding the careful management of cached `PGPROC` states, particularly how `databaseId` and `roleId` are bound. The proposer clarified that these fields could potentially be rewritten for dummy `PGPROC`s. A commenter noted that this proposal relates to an existing thread on `ProcArrayAdd`/`Remove` performance. The commenter also cautioned that the proposed caching could trade off `GetSnapshotData`'s predictable linear memory access for improved `PROC` registration efficiency, recommending benchmarks to validate any such tradeoff and suggesting that any unordered section of `PGPROC`s should remain very small.
29 postgres_fdw: Fix flaky push down FUNCTION RTE test Patch Review 3 msgs Aug 21, 14:29
The thread discusses a flaky test in postgres_fdw related to unnest function pushdown, which exhibited different query plans on different machines (Mac vs. Amazon Linux 2). The issue arose because the costs of the nested loop's inner and outer sides were too similar, leading to inconsistent plan choices. The proposer suggested stabilizing the test by changing a range predicate WHERE ((c3 < '00010')) to an equality predicate WHERE ((c3 = '00010')). This change restricts one side to a single row, making it consistently the outer side of the loop and thus producing a stable plan across environments. A reviewer confirmed the problem and the effectiveness of the proposed patch.
30 Race conditions in logical decoding Patch Review 17 msgs Aug 21, 18:28
The proposer identified a race condition in logical decoding where a COMMIT record could be processed and a snapshot built before the commit is fully recorded in CLOG, leading to incorrect visibility checks and data corruption. An initial fix involving waiting for CLOG updates caused deadlocks in synchronous replication. Commenters debated adding `TransactionIdIsInProgress()` checks, but this also led to deadlocks. A reviewer explained the complex layering causing these deadlocks, related to WAL writing, CLOG updates, synchronous replica ACKs, and procarray removal. The reviewer proposed two patches: 0001 (waiting for CLOG consistency) and 0002 (omitting still-running transactions), with 0002 failing tests. The issue was listed for backpatching to v19. The latest message refines patch 0001, providing a more robust waiting mechanism and a process-local cache to optimize `TransactionIdDidCommit()` calls, addressing the underlying `latestCompletedXid` problem.
31 Thread-safe stringToNode() / pg_strtok() Patch Review 9 msgs Aug 21, 16:29
This thread focuses on making `stringToNode()` and `pg_strtok()` thread-safe by replacing global variables with a `ReadNodeContext` struct passed through the call stack. Initial feedback was positive. The proposer updated the patch to fix non-`DEBUG_NODE_TESTS_ENABLED` builds and to ensure `_readExtensibleNode()` and `ExtensibleNodeMethods->nodeRead()` also receive `ReadNodeContext` to support extensions using `pg_strtok()`. The latest review acknowledged the improvements but suggested further structural changes. These included removing `readBitmapset()` and similar helper functions in favor of `readNode()` to simplify the API and eliminate a forward declaration of `ReadNodeContext` from `nodes.h`. The proposer is currently considering the implications of these proposed structural changes, particularly regarding potential overhead for `Bitmapset` serialization.
32 [PATCH] Fix compilation of nodeMergejoin.c with EXEC_MERGEJOINDEBUG Committed 5 msgs Aug 21, 15:28
The thread began with a bug report and a simple patch from the proposer to fix a compilation error in `nodeMergejoin.c` when the `EXEC_MERGEJOINDEBUG` macro was defined. The error was due to a missing include for `debugtup()`. Commenter 1 observed that this debug code had been broken and unnoticed for at least ten years, raising the question of its relevance. Commenter 2 strongly agreed, suggesting that `execdebug.h` and its associated debug support might not be useful and could be removed entirely. Following this, commenter 1 provided a patch to clean up `execdebug.h` and its APIs by removing the unused debug code. Commenter 2 approved this cleanup patch, indicating consensus on removing the largely unused and broken debugging infrastructure.
33 Fix XLogFileReadAnyTLI silently applying divergent WAL from wrong timeline Patch Review 8 msgs Aug 21, 16:29
The proposer identified a critical bug in archive recovery where a standby could silently apply divergent WAL from an incorrect timeline during an HA switchover. This happens because `XLogFileReadAnyTLI` could fall back to an older timeline's WAL segment that diverged after the switch point. A patch (0001) was proposed to prevent this, ensuring WAL is only read from the *owning* timeline for the requested LSN. Initial discussions confirmed this was a known issue. Reviewers suggested prioritizing stalling over silent corruption, clarifying comments, requesting positive test assertions, and improving diagnostics. An optimization (patch 0002) for `walreceiver` restarts was also proposed but later decided to be discussed separately. The latest patch (v3) addresses the feedback for the core issue, stopping after attempting the newest eligible timeline, explaining why parent copies cannot be used, adding `DEBUG1` messages for clarity, and enhancing tests for both negative and positive outcomes.
34 walsummarizer can get stuck when switching timelines Patch Review 30 msgs Aug 21, 16:29
The proposer identified a scenario where the WAL summarizer could get stuck during a timeline switch on a standby using archive recovery. This occurs if the WAL file containing the switchpoint is only archived on the new timeline. A patch was provided and later committed to resolve this. However, the newly introduced test case for this feature began failing on certain buildfarm environments, indicating a timing-sensitive issue. Reviewers pinpointed that the test's failure was related to the order of CHECKPOINT and pg_switch_wal() operations, and potentially due to bgwriter's LogStandbySnapshot activity. A patch was proposed to swap the order of these SQL statements in the test, with detailed comments justifying the change. Further buildfarm failures led to a discussion about refining the LSN recording and catch-up logic within the test to make it more robust against varying timings.
35 Row pattern recognition Patch Review 83 msgs Aug 21, 06:29
The thread discusses various aspects of the Row Pattern Recognition (RPR) feature patch (v50). Early messages (before the provided snippet) highlighted a correctness problem where window function results could change based on other window functions in the query, suggesting the RPR match should happen upfront. Later messages focused on issues related to view definitions and `pg_dump`/`pg_restore` failures due to ambiguous column references in `DEFINE` clauses after schema changes. A proposer 1 added a warning to the documentation and explored a subquery workaround for ambiguity. A commenter then proposed refactoring parts of `nodeWindowAgg.c` and `execRPR.c` for readability and error handling. Reviewer 1 expressed concern about the rapid submission of new refactoring patches, urging a pause to allow the existing queue to be absorbed and verified, while still welcoming other forms of contributions.
36 pgstat: Flush some statistics within running transactions, take 2 Patch Review 18 msgs Aug 21, 16:29
This thread continues an earlier discussion on flushing statistics mid-transaction. The proposer introduced a patch providing SQL and C APIs to trigger on-demand stats flushing. It allows immediate flushing of non-transactional counters (scans, fetches, blocks) and all other pending stats (function, IO, WAL), while transactional write counters are deferred. Initial reviews focused on handling multi-level dependencies, preventing double-counting, and correcting interactions with `pgstat_report_analyze()` and truncate logic. The proposer revised the patch to re-queue partially flushed entries and improve time-based counter precision. Subsequent revisions addressed `pgStatFlushInProgress` safety during errors and refined lock acquisition efficiency for relation stats. This led to a proposed architectural change to split `PgStat_TableCounts` into transactional and non-transactional components. The latest patch (v7) incorporates these structural changes, re-factors table stats, and expands custom stats examples to reflect the new transactional split.
37 pg_dump: assert failure sorting casts/transforms Patch Review 5 msgs Aug 21, 10:29
This thread addresses an assertion failure in `pg_dump` that occurs when sorting casts or transforms whose types share identical unqualified names across different schemas. The issue stems from casts and transforms lacking their own namespace for sorting. The proposer submitted a patch that adds tie-breakers by comparing the full natural keys, including schema names, for the referenced types. Reviewers provided feedback on the test coverage, suggesting additional symmetric test cases for source-type ambiguity and recommending clearer naming conventions within the tests. Minor refinements to code comments were also suggested. The proposer has provided multiple patch revisions to incorporate this feedback, aiming for a robust and clear solution.
38 [PATCH v4] Add pg_current_vxact_id() function Patch Review 1 msgs Aug 21, 16:29
This thread proposes adding `pg_current_vxact_id()`, a new SQL function to expose the current backend's virtual transaction ID (VXID). VXIDs are consistently assigned to all backends, including read-only transactions, making them ideal for transaction tracking and log correlation. The function returns the VXID as "procNumber/localXID" text, aligning with PostgreSQL's internal formats. The proposer argues that a direct function is superior to the existing `pg_locks` workaround, which is inefficient (O(n) and involves lock acquisition). Performance tests using `pgbench` demonstrate significant speedups (2x to 7x) with the new O(1) function, especially under concurrency and high lock contention. The v4 patch, rebased to master, incorporates previous review feedback regarding a `VXID_FMT` macro and consistent documentation terminology ("localXID"), aiming for an efficient, semantically clear, and consistent API.
39 Proposal: Conflict log history table for Logical Replication Patch Review 88 msgs Aug 21, 10:29
This extensive thread focuses on the development and review of a proposal for a conflict log history table in Logical Replication. Initial discussions covered aspects like data type choices for tuple representation and documentation phrasing. As the patch evolved through many revisions, significant issues were discovered. These include race conditions leading to incorrect remote transaction IDs and commit timestamps in streaming mode, and a critical bug where large row conflicts cause string buffer overflows, thereby halting replication. Reviewers also proposed refactoring global flags for remote transaction information into a more consolidated context structure. The proposer has been actively integrating feedback and providing updated patch sets, while also clarifying design decisions regarding distinct transaction ID fields.
40 Apply extended statistics to join clause during parameterized path costing Discussing 1 msgs Aug 21, 15:28
The proposer has introduced a new patch aimed at leveraging extended statistics, specifically `dependencies` statistics, to improve row cardinality estimates for join clauses during parameterized path costing. The proposed mechanism allows a clause to be considered compatible if it involves exactly two `varnos`, one being the relation for which the estimate is being made and the other from a single external relation, treating the latter as a pseudoconstant. The initial `EXPLAIN` output provided by the proposer shows a significant improvement in accuracy for the scan's row estimate when the extended statistics are applied. However, a reviewer immediately pointed out an inconsistency in the current implementation, where the improved scan estimate (e.g., `rows=200`) doesn't propagate correctly to the parent Nested Loop join's row estimate (e.g., `rows=98` instead of `50*200`). The reviewer also questioned why the solution only considers `dependencies` statistics and not `ndistinct` or MCV lists.
41 Orphaned Files in PostgreSQL Discussing 3 msgs Aug 21, 10:29
This thread proposes a solution to the problem of orphaned relation files in PostgreSQL that can result from unclean shutdowns during transactions that create new relations. These orphaned files consume disk space and complicate backups. The proposer suggests implementing a durable 'relation-creation marker' stored in a new `pg_relcreate` directory. This marker would track each transactionally created permanent relation. A reviewer raised a critical concern regarding the proposed ordering of logging the `XLOG_SMGR_CREATE` record before the physical file creation, highlighting potential negative interactions with concurrent checkpoints. The proposer has acknowledged this concern and committed to exploring and incorporating a revised approach.
42 pg_upgrade --copy-file-range fails with EINVAL on Linux 4.19 Discussing 2 msgs Aug 21, 11:28
The proposer reported a failure in `pg_upgrade --copy-file-range` when run on Linux kernel 4.19, resulting in an `EINVAL` error. The issue stems from a specific behavior of the `copy_file_range` syscall on this kernel version: it returns an invalid argument error when `NULL` is passed for `off_in` and `off_out` and the `len` argument (`SSIZE_MAX`) causes an internal overflow. While `pg_upgrade`'s initial probe for `copy_file_range` succeeds, the subsequent copy loop repeatedly calls the syscall with the problematic `NULL` arguments, leading to the failure and aborting the upgrade. The proposer suggested tightening the probe or fixing the copy loop. A commenter responded by pointing out that Linux 4.19 is an End-of-Life kernel and proposed a pragmatic solution: to conditionally undefine `HAVE_COPY_FILE_RANGE` in the build configuration for kernels older than version 5.3, thereby preventing `pg_upgrade` from attempting to use the problematic syscall on these older, unsupported systems.
43 Proposal: Supporting URI SAN in Certificate Authentication Proposed 3 msgs Aug 21, 09:28
The proposer introduced a feature request to add support for URI Subject Alternative Names (URI SAN) in PostgreSQL certificate authentication. Currently, PostgreSQL only uses the certificate's Subject (Common Name or Distinguished Name) for identity, which limits interoperability with modern URI-based workload identity systems like SPIFFE/SPIRE. The proposal suggests enabling URI SAN entries to serve as the client identity, possibly through a new `clientname=uri` option in `pg_hba.conf`. Commenter 1 confirmed community interest, citing a similar request from the pgBackRest project. Commenter 2 expressed strong support, emphasizing the growing adoption of SPIFFE/SPIRE in cloud-native environments and the benefits of native PostgreSQL support for zero-trust identity integration.
44 [PATCH] Several refactorings for pg_dump Patch Review 6 msgs Aug 21, 08:28
The proposer submitted a patch for `pg_dump` to perform several refactorings. Initially, this involved removing a write-only field `ArchiveHandle.lookaheadSize` and replacing the magic constant `512` with `TAR_BLOCK_SIZE`. A reviewer suggested that using `TAR_BLOCK_SIZE` might not be appropriate for a generic lookahead buffer, as TAR is only one of the archive options. The reviewer recommended instead initializing `lookaheadSize` and using that value where appropriate. The proposer then provided a v2 patch incorporating this feedback, which received a quick approval from another reviewer.
45 index prefetching Patch Review 80 msgs Aug 19, 21:28
This extensive thread discusses and develops an index prefetching feature for PostgreSQL, primarily by introducing a new `amgetbatch` interface to replace `amgettuple`. The goal is to improve performance for index scans, particularly on higher-latency storage, by proactively fetching data blocks. Early discussions involved heuristics for when to start prefetching and how to measure "unconsumed I/O." The proposer has submitted numerous patch versions, evolving the design to simplify slot handling, integrate GiST and SP-GiST index types, and refine prefetching heuristics based on empirical testing. Recent work focuses on avoiding small regressions in workloads like pgbench SELECT, micro-optimizations for point lookups, and improving progress reporting for `EXPLAIN (ANALYZE, IO)`. The first patch, which revises the table AM to use a higher-level, slot-based interface, is intended for imminent commit, with subsequent patches addressing SP-GiST support and improved planning heuristics for bloated indexes.
46 Fix small psql slash option leaks Committed 12 msgs Aug 21, 05:28
This thread began with the proposer identifying and patching several minor memory leaks in `psql`'s meta-commands, such as `\getresults` and `\gset` in pipeline mode, where `malloc`'d strings were not being freed. While not severe, these leaks could accumulate in long-running sessions. A reviewer then discovered a more critical issue: `psql_scan_slash_option()` and `psql_scan_slash_command()` could return a static `oom_buffer` on out-of-memory, potentially leading to invalid `free()` calls. The proposer provided additional patches to address this by having these functions return `NULL` and log an error on OOM. All these patches were reviewed, approved, and subsequently committed. A follow-up patch was also committed to fix an issue where an invalid `\getresults` argument could leave `psql` in an incorrect state.
47 use of SPI by postgresImportForeignStatistics Discussing 57 msgs Aug 21, 07:29
This long-running thread began with concerns about the postgresImportForeignStatistics function's use of SPI, citing potential issues with search_path safety, incorrect relation identification during concurrent renames, and overall design complexity. The initial message questioned the necessity of using SQL statements via FunctionCallInfo instead of a direct C interface. A later part of the thread, included here, focuses on two main aspects. First, a patch to rename the restore_stats option to import_stats was proposed and subsequently committed for v19. Second, for v20, a more significant refactoring was discussed, aiming to eliminate FCINFO construction and standardize parameter passing for statistics functions. The proposer revised the strategy, but a reviewer expressed strong reservations about the complexity, bug-proneness, and the patch's current state, suggesting a significant rework and a new discussion thread.
48 doc: Reformat SELECT queries using GRAPH_TABLE Proposed 1 msgs Aug 21, 07:29
This thread proposes a documentation patch to improve the readability of SELECT queries that use GRAPH_TABLE in ddl.sgml and queries.sgml. The proposer noted that these queries are currently written on single long lines, necessitating horizontal scrolling. The patch suggests breaking these queries across multiple lines, aligning with the formatting style of CREATE PROPERTY GRAPH statements already in the documentation, and referencing examples from SQL:2023 overviews for consistent indentation and line break placement.
49 Report bytes and transactions actually sent downtream Patch Review 22 msgs Aug 20, 12:28
The thread discusses adding a new statistic, output_bytes, to logical replication slots to report the amount of decoded data produced for the consumer by the output plugin. Initial confusion existed regarding whether to include protocol messages like keepalives or only the plugin's direct output. Participants agreed to exclude connection-management messages and focus on the data produced by the output plugin, including transaction changes and their delimiters. The naming sent_bytes was debated, with plugin_total_bytes and output_bytes suggested, settling on output_bytes for clarity. Reviewers provided feedback on the calculation, ensuring it only counts plugin-produced data, and on refactoring the statistics update mechanism. The proposer initially suggested splitting refactoring, but then provided a combined patch for review.
50 Report index currently being vacuumed in pg_stat_progress_vacuum Discussing 20 msgs Aug 21, 00:28
This thread proposes enhancing `pg_stat_progress_vacuum` to report the OID of the index currently being vacuumed, aiding in debugging slow vacuum operations. The initial patch included arrays in the leader's row to track worker PIDs and index OIDs. Discussion evolved into a debate on the optimal design for parallel progress reporting. Reviewer 1 argued for a 'one row per worker' model for better granularity and extensibility, rather than arrays in the leader's row. Commenter 1 initially supported arrays but later suggested a separate `pg_stat_progress_vacuum_worker` view for per-worker details. The proposer then leaned towards a single 'one row per worker' view, but commenter 1 raised strong objections, citing a breach of existing design principles and potential ambiguity in interpreting merged statistics, advocating for separate views for aggregate and worker-level data.