01 [BUG?] check_exclusion_or_unique_constraint false negative Patch Review 29 msgs Oct 5, 23:28
This extensive thread investigates a potential bug related to `check_exclusion_or_unique_constraint` leading to false negatives, particularly concerning `SnapshotDirty` scans missing logical tuples during concurrent updates. The proposer iterated on a solution, rebasing patches multiple times and fixing test races. A reviewer provided a significant rebase of the patch, addressing conflicts due to upstream changes. The reviewer also identified a key behavioral change introduced by the patch: the MVCC scan approach would no longer wait for in-flight `INSERT` transactions, potentially leading to earlier `update_missing` reports compared to the existing `SnapshotDirty` behavior. The proposer acknowledged this change, noting that the current master's behavior in this specific scenario is considered a race rather than a guarantee, and updated the commit message accordingly.
02 Remove invalid SS2/SS3 handling from EUC-KR routines Patch Review 10 msgs Oct 6, 02:29
This thread addresses inconsistencies in PostgreSQL's handling of EUC-KR encoding, specifically concerning the single-shift codes SS2 (0x8E) and SS3 (0x8F). The proposer pointed out that the KS X 2901 standard for EUC-KR only defines 1-byte (ASCII) and 2-byte (KS X 1001) sequences, making 3-byte sequences (involving SS2/SS3) invalid. However, PostgreSQL's documentation and some internal functions (pg_euckr_mblen()) incorrectly supported 3-byte sequences via shared helpers, despite pg_euckr_verifychar() already rejecting them. The proposed patch aims to correct this by implementing EUC-KR-specific multibyte functions, updating the maxmblen from 3 to 2, and fixing the documentation. Reviewer 1 provided a positive assessment. Reviewer 2 identified that the change impacts query planning (width calculations), TOAST table creation, and function behavior (e.g., lpad()). The proposer has since provided updated patches (v4) with regression tests and detailed explanations of the observed behavior changes, recommending that this change should be applied to master only.
03 PSQL schema "describe" \dn is not escaping quotes Patch Review 17 msgs Oct 5, 18:29
The thread identifies and addresses a bug where the psql `\dn` command fails for schema names containing quotes due to incorrect escaping, and the publication footer is missing for double-quoted patterns. The proposer submits an initial patch (v1), which receives feedback on indentation and function usage from reviewer 1, and consistency from reviewer 2, leading to v2. After v3 includes test cases as requested by reviewer 3, reviewer 5 points out that the patch introduces unintended pattern matching behavior, creating misleading output for multiple schemas. The proposer rewrites the logic (v4) to align with `\d table` behavior. Reviewer 6 then discovers a regression in v4 concerning case-sensitive quoted schema names, which is fixed in v5. Further memory leak and initialization issues found by reviewer 6 and reviewer 1 are addressed in v6 and v7. The latest review for v7 raises significant concerns about user-visible output changes, backporting implications, inconsistent no-match behavior compared to `\dt`, breaking CSV output with multiple headers, and minor coding nits.
04 aio: worker: Free SMGR objects when idle Discussing 14 msgs Oct 5, 19:30
The proposer identified a resource leak in I/O worker processes where `SMgrRelation` objects accumulate without being destroyed because `smgropen()` is called outside of a transaction. The initial patch proposed cleanup when the worker is idle. Commenter 1 noted a related bug and suggested an invalidation message-based approach, but reviewer 1 raised concerns about spinlock contention. Commenter 2 proposed an alternative LRU-based solution with I/O stamps and checkpoint-triggered cleanup to avoid shared locks. The proposer refined their patch to trigger cleanup based on the number of I/Os or unpinned entries, and to handle idle workers. The discussion continues to explore the best mechanism to ensure timely cleanup without introducing performance bottlenecks.
05 [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically Discussing 19 msgs Oct 5, 13:29
This thread is about adding an `--initdb` option to `pg_upgrade` to automate the new cluster's initialization, aiming to reduce manual errors. Initial discussions covered handling `--check` with `--initdb`, documentation needs, security concerns regarding command injection, and cleanup of orphan directories. Over several versions, the proposer and a collaborating developer refined the patch, making `--check --initdb` a dry run, implementing automatic cleanup for failed upgrades, using secure command assembly, and splitting documentation into a separate patch. However, the latest review of v6 identifies new critical issues: `--check --initdb` still skips most compatibility checks, the utility fails to find its binary without the `-B` option, and it incorrectly rejects the `-O` option when `--initdb` is used, indicating significant unresolved design problems.
06 PG19: two RI fast-path issues found while testing the batching revert Discussing 18 msgs Oct 5, 13:29
This thread began with reports of two pre-existing bugs in PostgreSQL 19's Referential Integrity (RI) fast-path, found during testing of an RI batching revert: incorrect column-level SELECT permission checks and improper invalidation of the RI cast cache. The proposer provided initial fixes, which a reviewer refined. Later, additional issues were discovered, including a race condition in `ri_HashCompareOp()` and problems with metadata invalidation on `pg_amop` changes. Some of these issues have already been committed. However, the latest messages indicate further problems with the RI fast path directly calling cast functions, leading to incorrect error handling for NULL returns and stale cast definitions. New patches are proposed to address these and similar issues in older RI paths.
07 UNDO with constant time recovery (CTR) Patch Review 10 msgs Oct 6, 02:29
This thread discusses the implementation of UNDO logs in PostgreSQL to enable constant-time recovery (CTR) for file system operations. The proposer based this work on earlier efforts like ZHeap and Zedstore but diverged by initially focusing on FILEOPS without changing the HEAP access method. The initial patch set (v2) received significant review. Commenter 1 pointed out issues such as test dependencies, incomplete cleanup after `CREATE DATABASE` crashes, and server panics due to filesystem errors within critical sections. Reviewer 2 questioned the "constant-time recovery" claim, as measurements indicated linear time complexity for rollback and crash recovery, and raised concerns about UNDO operations outside transactional boundaries and their implications for unlogged relations. The proposer has acknowledged the feedback and is in the process of rebasing and reorganizing the patch series.
08 [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c) Patch Review 12 msgs Oct 5, 15:29
The thread addresses a segmentation fault in `RI_Fkey_cascade_del` caused by a use-after-free issue when `BEFORE DELETE` triggers on a self-referential table invalidate the plan cache during a reentrant RI trigger execution. The proposer provided a two-part patch: a test case to reproduce the bug and the fix. Initial discussion by a reviewer acknowledged the crash but questioned whether `BEFORE DELETE` triggers should perform such actions (deleting other rows). The proposer argued for legitimate use cases or for explicit error messages if such actions are forbidden. The proposed fix involves introducing reentrancy guards and reference counting for plans to prevent premature freeing. A detailed review identified several issues with the reference counting, including leaks on error paths, incorrect entry removal, and potential hash key reuse. The proposer addressed these points in subsequent patch versions (v4), incorporating a ResourceOwner for robust error handling and a delete-later list for invalidated plans. The latest review of v4 was positive, confirming that the bookkeeping issues were resolved. The proposer then rebased the patches to v5 to apply cleanly on master, with a minor change in the deferred plan freeing logic due to upstream changes. The reviewer of v5 confirms the new placement is safe and the fix still looks correct.
09 [PG19]pg_verifybackup never finishes on a gzip-compressed tar backup Patch Review 3 msgs Oct 6, 02:29
The proposer reported a bug in PostgreSQL 19 where pg_verifybackup fails to terminate (hangs indefinitely) when verifying a gzip-compressed tar-format backup generated by pg_basebackup with WAL streaming. This occurs because pg_basebackup produces a pg_wal.tar.gz file that is a concatenation of multiple gzip members, one for each tar header. The astreamer_gzip_decompressor_content() function within pg_waldump (used by pg_verifybackup) incorrectly loops infinitely after decompressing the first gzip member, as inflate() continuously returns Z_STREAM_END without processing subsequent members. The proposer provided a patch to resolve this by calling inflateReset() when Z_STREAM_END is encountered, allowing the decompression to continue to the next gzip member. A test case was also included. Commenter 1 inquired about a potential duplicate issue, but the proposer clarified it is a distinct problem.
10 Parallel autovacuum: DROP DATABASE WITH (FORCE) fails on the parallel workers Patch Review 7 msgs Oct 6, 02:29
The proposer discovered a bug in parallel autovacuum (PG19+) causing DROP DATABASE WITH (FORCE) to fail with a permission denied error when parallel autovacuum workers are active in the target database. The problem arises because the autovacuum leader worker reports an InvalidOid for its role, while its parallel workers correctly report BOOTSTRAP_SUPERUSERID. This inconsistency causes TerminateOtherDBBackends() to incorrectly deny termination. Reviewer 1 suggested a broader fix to cover general parallel query workers spawned by background workers, advocating for the leader's roleId to be used for all lock group members. Subsequent patch versions (v2, v3) adopted this generalization. Reviewer 1 later identified a concurrency issue in v3 related to dereferencing `lockGroupLeader` without adequate locking. The latest patch (v4) from the proposer addresses this by acquiring the leader's partition lock and re-validating the leader before accessing its roleId, simplifying the check and making the fix robust. The fix is intended for backpatching to PG14.
11 Logical replication: lost updates/deletes and invalid log messages caused by SnapshotDirty + concurrent updates Discussing 4 msgs Oct 5, 23:28
This thread was initiated to focus on the logical replication implications of a bug where `SnapshotDirty`/`SnapshotSelf` scans can miss logical tuples during concurrent updates. The proposer detailed how these missed tuples lead to "lost DELETEs", "lost UPDATEs", and invalid log messages in logical replication, providing concrete scenarios. A developer confirmed this behavior is a bug. The proposer then proposed documenting the issue as a temporary measure. In the latest exchange, the proposer addressed a reviewer's concern (from a related thread) about a behavioral change, explaining that the proposed fix would cause `update_missing` to be reported sooner for in-flight `INSERT`s than on master. The proposer justified this by stating that master's current behavior is racy rather than a guarantee and that the commit message for the fix has been updated to clarify this.
12 pg_resetwal: refuse to run when backup_label exists Discussing 10 msgs Oct 6, 00:29
This thread discusses a proposal to enhance `pg_resetwal` by making it refuse to run when a `backup_label` file exists in the data directory. The goal is to prevent users from accidentally corrupting valid backups. An initial reviewer raised concerns about restricting a powerful salvaging tool, suggesting an alternative `--cluster-state` option. The proposer clarified that the change only enforces removing `backup_label` *before* `pg_resetwal`, which is already implicitly required for server startup. Subsequent reviewers supported this, noting it forces deliberate action and provides clearer warnings, preferring manual removal over an `-f` override. The `--cluster-state` option was deemed out of scope. Despite some initial reservations about the patch's value from one reviewer, the patch was ultimately moved to "Ready for Committer", with the final decision on the `-f` override left to the committer.
13 [PG19] COPY (query) TO ... (FORMAT json) uses the table's column names Patch Review 4 msgs Oct 6, 02:29
The proposer identified a bug in PostgreSQL 19's COPY (query) TO ... (FORMAT json) functionality, where the JSON keys were incorrectly derived from the underlying table's column names instead of the query's projected or aliased column names. This led to incorrect JSON output, particularly with UNION ALL queries or explicit column aliases. The root cause was traced to CopyToJsonOneRow() using the table's descriptor when a scan node did not project. The proposer suggested an initial fix, but a reviewer provided an optimized v2 patch. This v2 patch efficiently reuses the copied tuple and stamps it with the query's descriptor using heap_copy_tuple_as_datum(), avoiding a performance hit. Commenter 1 thoroughly tested v2, confirming it resolved the reported issues across various query patterns and even fixed an unmentioned edge case involving views and CTEs, with no performance regressions. The proposer also confirmed the efficacy and efficiency of v2.
14 Parallel Apply Patch Review 12 msgs Oct 5, 13:29
This thread focuses on developing a "Parallel Apply" feature for logical replication. Early discussions covered the rationale for using different hash table implementations (`dshash`, `simplehash`, `HTAB`) based on specific requirements. Reviewers provided feedback on comments, naming conventions, and the scope of hash table entries. The proposer iteratively updated the patches (v17-v21), incorporating fixes for dependency detection (including toasted columns and partitioned tables), refining memory limit controls for dependency tracking, and improving code readability. A significant update involved collecting unique indexes from leaf partitions for partitioned tables. The latest message indicates that the current patch set requires rebasing to continue the review process.
15 pgstat: allow a stats kind to use its own dedicated dsa/dshash Patch Review 6 msgs Oct 6, 02:29
This thread proposes a new feature for PostgreSQL statistics, allowing certain statistics kinds (e.g., pg_stat_statements) to use their own dedicated DSA area and dshash. This aims to improve performance by reducing lock contention, enabling independent sizing of shared memory, and optimizing iteration for kinds with numerous entries. The initial patch (v1) introduced an 'own_hash' option and new APIs to retrieve dedicated dshash and DSA pointers. Reviewer 1 provided comprehensive feedback, suggesting improvements such as better handling of out-of-memory scenarios during insertion, reconsidering startup failure semantics for persistent statistics, and exploring whether a dedicated hash necessarily implies a dedicated DSA. The proposer released v2, addressing several of these points, including using DSHASH_INSERT_NO_OOM and refining DSA retrieval. The decision to keep 'own_hash' tied to a dedicated DSA was maintained for isolation. Reviewer 2 provided further feedback on v2, raising concerns about potential duplication in the global stats structure and suggesting simplifications for hash table management.
16 [PATCH v1] Fix races in Windows pthread emulation Discussing 6 msgs Oct 5, 20:28
This thread addresses a race condition in the Windows pthread mutex emulation, specifically impacting ARM64 builds where the ECPG thread/alloc test could hang. The proposer identified that the `InterlockedExchange` function, used in mutex initialization, could overwrite a fully initialized state, causing subsequent `pthread_mutex_unlock` calls to fail and leave critical sections locked. The initial patch proposed using `InterlockedCompareExchange` and interlocked operations for mutex state and `pthread_once_t` to ensure correct memory ordering. Commenters confirmed the issue's occurrence on both ARM64 and AMD64. The discussion also covered potential performance overhead of the interlocked operations and the backpatch compatibility of changing `pthread_once_t` from `bool` to `LONG`. The proposer is considering revising the patch to potentially separate the mutex and `pthread_once` fixes.
17 NOT NULL NOT ENFORCED Patch Review 18 msgs Oct 5, 07:29
The thread discusses the implementation of `NOT NULL NOT ENFORCED` constraints in PostgreSQL to complete SQL feature F492. The proposer outlines rules for single-column constraints, partitioned tables, and inheritance. An early discussion point concerned `pg_dump` behavior: whether `NOT NULL NOT ENFORCED` should be dumped with column definitions or as separate `ALTER TABLE` statements. A reviewer argued for inline dumping for user-friendliness, and the proposer later updated the patch, finding the change less complex than initially thought. Further review included a suggestion for tab-completion and comprehensive testing of a later patch version (v5) across various scenarios, which yielded positive results. The commit message was also refined for clarity and a rebased v6 patch was provided.
18 stale comment in struct AlteredTableInfo Committed 2 msgs Oct 6, 02:29
A commenter identified a stale and incomplete comment within the struct AlteredTableInfo definition. The existing comment, "/* Objects to rebuild after completing ALTER TYPE operations */", did not fully reflect the scope of the objects, as they also apply to "ALTER COLUMN SET EXPRESSION" operations. The commenter proposed an updated comment to accurately include both types of operations, enhancing code clarity.
19 enhancing pg_basebackup speeds up to ~23Gbps (small fixes + io_uring/Direct I/O) Patch Review 6 msgs Oct 5, 16:28
This thread focuses on significantly enhancing `pg_basebackup` performance, aiming for speeds up to ~23Gbps by optimizing various aspects without altering its single-threaded, single-connection design. The proposer identified several areas for improvement: reducing syscalls, avoiding redundant memory copies, and leveraging Direct I/O with asynchronous submission using `io_uring`. Initial benchmarks show nearly a 2x speedup. The proposed patches include new blackhole targets for better bottleneck identification, a transfer rate timing message, and increased buffer lengths to reduce `pread()` calls. Another contributor joined the thread with an improvement for incremental backups, using `posix_fadvise()` for prefetching scattered blocks, showing significant speedups. This incremental backup optimization was merged into the main patch set. The current discussion includes reviewing specific patch details, like buffer alignment and checksum verification logic.
20 amcheck: add index-all-keys-match verification for B-Tree Patch Review 13 msgs Oct 5, 09:29
This thread proposes a new `amcheck` feature, `indexallkeysmatch`, to detect B-tree corruptions where index keys do not match their corresponding heap tuple keys. The initial patch utilized a Bloom filter for efficiency. Early reviews identified issues with duplicate visibility checks and potential memory leaks, which the proposer subsequently addressed. Further discussions uncovered false positive corruption reports under concurrent `VACUUM` operations and unhandled errors when heap segments were missing. The proposer delivered a revised patchset (v6) that resolved the `VACUUM` issue by mirroring MVCC index scan logic for heap lookups and introduced dedicated checks for dangling entries and lost heap segments. The latest development involves a compilation issue caused by recent core changes, for which a reviewer provided a fixup patch.
21 WAL segment file descriptor leak on read errors can PANIC the server Patch Review 14 msgs Oct 5, 22:29
This thread discusses a critical file descriptor (FD) leak in WAL read paths, which can lead to server PANICs or 'Too many open files' errors. The proposer identified that WAL segment FDs, opened via BasicOpenFile(), are not properly closed on error paths, particularly in logical decoding, 2PC WAL read code, and the WAL summarizer. The initial proposed fix involved registering a memory context reset callback in XLogReaderAllocate(). Commenter 1 confirmed the issue and noted that even the walsender leaks FDs on normal exits. Commenter 2 suggested a more flexible approach: lazy registration of the callback only when a segment is actually opened, and anchoring it to the reader's memory context. The proposer agreed and submitted a v2 patch implementing lazy registration. However, Commenter 2 then raised concerns about a layer violation in v2's approach and proposed a different patch with a helper routine XLogReaderRegisterReset() to define the callback within segment open callbacks, aiming for less invasiveness and better alignment with BasicOpenFile(). Commenter 3 provided minor review comments on the latest patch, and Commenter 2 is awaiting further feedback from the proposer.
22 Vacuum statistics Patch Review 92 msgs Oct 4, 14:28
This extensive thread discusses an effort to enhance PostgreSQL's VACUUM statistics. Initially, the proposer aimed to integrate a comprehensive set of new statistics directly into core, including various tuple and page counts, WAL metrics, and timing information. Early in the discussion, a reviewer raised concerns about the significant increase in statistics volume and suggested a more granular approach, potentially involving a GUC. The proposer addressed initial review comments, fixing issues like commit message clarity and distinguishing between heap and index statistics. A major turning point occurred when a new reviewer questioned the approach, advocating for a design similar to pg_stat_statements, where core provides hooks and an external module handles the collection and storage of most detailed statistics. The proposer agreed to this hybrid model, restructuring the patch set to include a small core component and a new ext_vacuum_statistics extension. Subsequent reviews focused on addressing concerns with the extension-based approach, specifically regarding garbage accumulation for dropped objects and safe handling of statistics updates during error callbacks. The proposer revised the patches to use object_access_hook for cleanup and refactored error reporting to avoid locks in error handlers. The latest iteration of patches also included fixes for CI issues, rebase efforts, and minor function naming adjustments. However, the most recent feedback from a reviewer has identified new issues concerning parallel VACUUM double-counting resource usage and improper privilege handling for extension reset functions, indicating ongoing development and refinement.
23 Proposal: SELECT * EXCLUDE (...) command Discussing 9 msgs Oct 5, 10:29
This thread proposes implementing SELECT * EXCLUDE (...) functionality, which allows excluding specific columns from a SELECT * expansion. The feature has been discussed previously and is also being considered for the SQL standard, with a paper outlining proposed semantics shared by a SQL standard proposer. Initial patch review revealed several deviations from the proposed standard, including incorrect error handling for non-existent or ambiguous columns, and issues with dropped columns, exclusion leakage, and qualified name resolution. There's an ongoing debate regarding the ambiguity rules and column privilege checks, with some arguing for a more lenient implementation than the SQL standard proposal. The latest messages confirm an updated SQL standard syntax (including REPLACE and RENAME clauses) and the proposer's intent to release a v3 patch incorporating these changes and fixing the reported bugs.
24 pg_dump/restore failure (dependency?) on BF serinus Discussing 7 msgs Oct 5, 13:29
The thread started with a `pg_dump`/`pg_restore` failure on a build farm member, specifically a foreign key constraint creation failing due to a missing unique constraint. Initially suspected as a dependency issue, further investigation by a commenter revealed a deeper "catalog check race" as the root cause. This race occurs during concurrent `ATTACH` statements for 2-level partitioned tables, where the primary key index of the parent table is left permanently `indisvalid=false`. The issue stems from concurrent `ATTACH` statements incompletely counting valid indexes before all transactions commit. A patch was previously suggested to address this, but the latest message indicates its removal, signifying ongoing work to find a resolution.
25 Let an ordering index scan hand its ORDER BY value to the target list Discussing 5 msgs Oct 5, 18:29
This thread proposes an optimization for `ORDER BY` clauses utilizing index ordering values. The proposer observes that the current system re-computes ranking values even when an index provides an exact value, leading to inefficiency and preventing direct exposure of scores in the result set. The proposed solution is to allow the executor to substitute the index's exact `ORDER BY` value into the target list, mirroring `Index Only Scan` behavior. An initial patch (v1, then v2) is provided. A reviewer challenges the motivation, questioning the claim of "incorrect results," and reiterates a long-standing core developer's principle that an index's plan choice should not alter the query's row-data output. The proposer clarifies that the primary goal is optimization and exposing the score, not changing the operator's meaning.
26 issues with eager aggregation Committed 8 msgs Oct 6, 00:29
This thread addresses several issues found with the eager aggregation feature. The proposer reported three specific bugs leading to wrong answers or internal errors, alongside a more debatable behavioral change: a division-by-zero error occurring with eager aggregation that wouldn't happen otherwise. This occurs because eager aggregation can evaluate expressions on rows that are later filtered by joins. The primary developer confirmed the three bugs and provided fixes for them. For the behavioral change, the developer argued it aligns with existing query transformations (e.g., pushed-down WHERE/HAVING clauses, inlined CTEs) where expressions are evaluated on rows not necessarily present in the final result. Remedies for users, such as using `CASE` or specific GUCs, were discussed. The three bugs were eventually fixed and committed, and it was agreed that the documentation should be updated to clarify the expression evaluation behavior.
27 pg_*_advice: tsv load failure, etc. Committed 21 msgs Oct 5, 21:28
This thread discusses various issues and potential improvements for the pg_plan_advice and pg_stash_advice modules, identified through an Opus review. Initial concerns included an empty stashed advice string persisting, pg_start_stash_advice_worker() silently losing persisted advice, and stash-supplied advice overriding GUC settings. Secondary issues involved cached generic plans ignoring advice changes and lack of syntax validation for stashed advice. A set of patches was proposed to address some of these, including documentation for GEQO vs. pg_plan_advice, fixes for JOIN_ORDER advice feedback, disallowing empty sublists, and handling partition names without schemas. Two of these patches (disallowing empty sublists and partition names) were committed. Later, a commenter identified new cases where pg_plan_advice generated unenforceable GATHER advice for set operations and where PARTITIONWISE advice on plain tables disabled all scan paths. A doc patch was also submitted to clarify that advice is consulted only at plan time and how to invalidate cached plans. The main developer provided a new patch set fixing brown-paper-bag bugs related to child_append_relid_sets and the JOIN_ORDER feedback, acknowledging the complexity and that not all reported issues were resolved. They indicated these fixes would be committed and back-patched to v19.
28 Limiting WAL retained for archiving, like max_slot_wal_keep_size Rejected 3 msgs Oct 5, 19:30
The proposer suggested adding a configuration setting similar to `max_slot_wal_keep_size` but for WAL archiving. This would allow PostgreSQL to automatically remove older unarchived WAL segments if the archiving process falls behind or breaks, preventing disk space exhaustion. The server would record this action, alerting tools about gaps in Point-in-Time Recovery (PITR). A commenter expressed strong reservations, arguing that such a feature would compromise the guarantee of PITR by allowing critical WAL segments to be lost. They emphasized that the decision to discard WAL should reside with the archiving tool, not the core PostgreSQL server.
29 Progress reporting: a debug trace and a test framework Patch Review 1 msgs Oct 6, 02:29
The proposer submitted version 4 of a patch series focused on progress reporting, which includes a debug trace and a new test framework. During the rebase process, the test framework's completeness check successfully identified a deviation: a commit (1378aa13430) had introduced PROGRESS_VACUUM_CURRENT_INDEX_RELID without a corresponding description in the progress.h specification. The proposer clarified this counter represents the OID of the index currently being processed by a worker and updated the patch to correctly classify it, demonstrating the efficacy of the completeness check in maintaining specification integrity. Aside from this update during rebase, the four bug fixes (0001-0004) and the PROGRESS_DEBUG trace (0005) from the previous version remain unchanged.
30 [PG19] eager aggregation gives wrong results because of bpchar_ops Patch Review 1 msgs Oct 6, 02:29
The proposer identified a bug in PostgreSQL 19 where eager aggregation produces incorrect results for bpchar columns due to inconsistencies in how bpchar equality and the equalimage functions in bpchar_ops opclasses handle trailing spaces. This discrepancy leads to miscounts in queries involving bpchar joins and eager aggregation. The issue also affects nbtree deduplication. The proposer demonstrated the bug with a clear test case, showing a regression from PG18. A fix is proposed involving dropping a specific support function from the affected opclasses, similar to a prior fix for interval_ops. This solution, however, would also inadvertently end deduplication for char(n), which was previously considered safe. The proposer suggests treating this as an open item given its novelty in PG19.
31 [PATCH v1] amcheck: Allow interrupting the child-level rightlink walk Committed 5 msgs Oct 5, 12:29
This thread discusses a patch to improve the robustness of `amcheck` by allowing interruptions during the child-level rightlink walk in `bt_child_highkey_check()`. The proposer identified that this specific loop in `verify_nbtree.c` lacked `CHECK_FOR_INTERRUPTS()`, making it uninterruptible, which is a significant concern when checking potentially corrupt indexes with long or circular rightlink chains. The patch, a single-line addition, was quickly reviewed and approved, being recognized as a consistency fix and a safety improvement.
32 doc: Document Linux cgroup memory limits Patch Review 11 msgs Oct 5, 15:29
The thread proposes adding documentation about Linux cgroup memory limits to the PostgreSQL manual, as many installations run Postgres in containers where cgroup limits dictate OOM killer behavior. The initial patch focused on `memory.max`, how backends are killed, and huge page caveats. Reviewer 1 provided detailed testing feedback, suggesting clarifications on `memory_hugetlb_accounting` (huge pages counting) and the impact of `memory.oom.group` on postmaster termination and crash recovery. The proposer incorporated these changes in v2, adding a paragraph on `memory.oom.group`. Further refinement in v3 corrected the wording regarding automatic restart versus recovery after a group kill. In v4, additional points were integrated, covering systemd's `OOMPolicy` when running as a service, and the `oom_score_adj -1000` exemption for the postmaster from group kills. Commenter 1 provided broader context on PostgreSQL's memory management, suggesting it's risky to run on cgroups2 without more aggressive self-limiting or connection rotation. The proposer responded by softening the wording in the final paragraph of v5, recommending a safety margin, but deferring broader memory management discussions as outside the scope of this documentation patch. The patch has been deemed "Ready for Committer" by reviewer 1.
33 COPY FROM ... WHERE fails for negated operators Patch Review 2 msgs Oct 5, 20:28
This thread identifies a bug in `COPY FROM stdin WHERE NOT (a > 0)` which results in a "cache lookup failed for function" error. The proposer pinpoints the problem to `DoCopy()` neglecting to call `fix_opfuncids()`, a necessary step to properly fill in operator function IDs, similar to how `RelationGetIndexPredicate()` operates. This omission causes `negate_clause()` to be called with incorrect arguments. A patch was provided to include the `fix_opfuncids()` call within `DoCopy()`. A core committer reviewed the patch, acknowledging the immediate necessity of the fix while also noting a broader desire to refactor and remove `fix_opfuncids()` entirely in the future.
34 Reducing relcache memory usage 2: shrink sizeof(RelationData) Patch Review 4 msgs Oct 5, 12:29
This thread details an effort to significantly reduce PostgreSQL's `relcache` memory usage by shrinking the `sizeof(RelationData)` structure. The proposer introduced a patch set aimed at reducing the size from 488 bytes to 256 bytes, potentially saving approximately 53 MiB of `CacheMemoryContext` per backend for large schemas. The patch set includes changes like introducing unions for mutually exclusive states, reordering members for better packing, lazy allocation of partition information, and removing redundant members. A reviewer provided feedback on individual patches, raising concerns about lazy computation overheads and the use of sentinel values. The proposer addressed these concerns, confirming the safety of lazy computations and running `pgbench` tests on partitioned tables, which showed no measurable performance regressions.
35 Planning time quadratic in the IN-list length for "c = X AND (a, b) IN (...)" with BitmapOr Patch Review 6 msgs Oct 5, 19:30
The proposer reported a quadratic increase in query planning time for `c = X AND (a, b) IN (...)` queries with long `IN` lists, specifically when `BitmapOr` scans are involved. Planning times reached several seconds for 4000 entries across recent PostgreSQL versions. A reviewer quickly provided a patch that reorders the predicate testing logic in `predtest.c`, prioritizing the arm at the same position. This change dramatically reduced planning times from seconds to milliseconds for thousands of entries. The proposer thoroughly tested the patch on multiple PostgreSQL versions, confirming its effectiveness and correctness. The patch has been registered in the commitfest, and the proposer volunteered as its reviewer, confirming its readiness for committing.
36 Limiting WAL retained for archiving, like max_slot_wal_keep_size Discussing 2 msgs Oct 5, 22:29
This thread introduces a proposal to limit WAL retention for archiving, similar to 'max_slot_wal_keep_size', to prevent 'pg_wal' from filling up if archiving fails or cannot keep pace. The proposer suggested an opt-in setting that would automatically remove the oldest unarchived WAL segments once a limit is exceeded, and record this event in 'pg_stat_archiver' to signal a missing WAL gap and the need for a new base backup. Commenter 1, while acknowledging the problem, expressed significant concerns about the proposed solution. These concerns include the risk of silently broken recoveries, incorrect success reports for backups whose WAL ranges include dropped segments, the non-durable nature of 'pg_stat_archiver' for recording critical gaps, and potential confusion for the archiver process. Instead of automatically dropping WAL, Commenter 1 suggested a less risky alternative: enhancing 'pg_stat_archiver' to expose metrics on archiving lag, such as the oldest unarchived segment or bytes pending archiving, to allow proactive alerting.
37 Reduce SyncRepLock contention on the commit path Patch Review 3 msgs Oct 5, 13:29
This thread proposes a patch series to reduce `SyncRepLock` contention in synchronous replication, a critical bottleneck for high commit rates. The proposer's four patches aim to move non-critical work out of the lock's critical section: waking waiters after lock release, computing synced positions before acquiring the lock, releasing waiters once per batch of replies, and skipping the lock if acknowledgement has already arrived. After initial feedback and rebase by a reviewer, the proposer submitted an updated version (v3) addressing minor issues and fixing a test case. The reviewer then confirmed that the patch series is ready for commit-fest, indicating its maturity.
38 Serverside SNI support in libpq Patch Review 75 msgs Oct 5, 15:29
The thread focuses on serverside SNI support in libpq, particularly addressing issues related to SSL configuration reloads and potential crashes. An early bug involved a NULL pointer SIGSEGV when `ssl_sni` was changed during a failed reload, leading to access of non-existent SSL configuration. The proposer developed a series of patches, iteratively refining the solution based on feedback from reviewers. Initial proposed fixes for `ssl_sni` handling were discussed, with the goal of aligning `ssl_sni` behavior with other SSL configuration parameters on reload failures. Subsequent patches addressed memory leaks, dangling pointers, and `SSL_CTX` cleanup. Reviewers identified issues such as missing null checks and the need for better test coverage. The latest versions of the patches have been reviewed positively, with additional comprehensive tests proposed to cover more edge cases, including encrypted per-host keys and turning SNI off with missing files. The proposer indicated plans to commit the fixes soon, after addressing the test format.
39 Fix "unexpected logical decoding status change" error; from concurrent logical decoding activation Patch Review 41 msgs Oct 5, 22:29
This thread addresses a race condition that causes "unexpected logical decoding status change" errors, particularly exposed by concurrent REPACK operations. The proposer explained that simultaneous creation of logical slots can lead to multiple XLOG_LOGICAL_DECODING_STATUS_CHANGE records being written due to a window between LWLock acquisitions and a barrier wait in EnableLogicalDecoding(). The second record, if found by a decoding slot, triggers an error. The discussion expanded to include challenges with logical slot synchronization on standbys, specifically when the standby lags in replay. An earlier solution could lead to repeated creation and dropping of valid slots. To mitigate this, the proposed patch introduced a check that allows a slot to be retained if its restart_lsn is ahead of the standby's replay LSN, preventing unnecessary drops when the standby is simply behind. This new logic ensures that the slot waits for replay to catch up rather than being discarded. The latest exchanges involved refining the comments for this behavior and suggesting minor improvements for the accompanying TAP test.
40 REPACK (CONCURRENTLY) might keep dropped-column data Patch Review 10 msgs Oct 5, 16:28
This thread identifies a bug in `REPACK (CONCURRENTLY)` where dropped-column data might be retained under specific scenarios, leading to bloat. The proposer provided two main replication cases: one involving logical replication on a subscriber when a column is dropped and the publisher updates rows, and another involving a `BEFORE UPDATE` trigger that returns `OLD` or a modified `NEW` tuple after a column is dropped. In both cases, the `REPACK (CONCURRENTLY)` operation, intended to remove bloat, failed to clear the dropped column data, resulting in larger-than-expected table sizes. An initial fix was proposed and committed, which explicitly sets dropped columns to NULL in the tuple before writing it out during the repack process. Further adjustments to the test cases and the fix's implementation (e.g., handling `tts_isnull`) were also committed. The latest message introduces new cases (replayed INSERT, UPDATE moving rows between partitions, COPY, MERGE, INSERT ON CONFLICT) where dropped column data might still be carried and stored, potentially causing `REPACK` failures due to oversized rows.
41 Fix reindexdb with parallel index-level conrurrent run Discussing 4 msgs Oct 5, 20:28
The thread discusses a bug in `reindexdb` where using parallel jobs (`-j N`) with `REINDEX CONCURRENTLY` for multiple indexes on the *same* table fails. The issue arises because `reindexdb` batches DDL commands for a single table, implicitly creating a transaction which conflicts with `REINDEX CONCURRENTLY`. A patch was proposed to send commands individually. However, subsequent review revealed that this fix introduced performance regressions by eliminating parallel execution for single-index tables and did not properly handle cancels or failures. A reviewer also questioned the practical benefits of parallelizing concurrent reindexing at the index level due to inherent wait phases. The latest suggestion is to have `reindexdb` return an explicit error for this specific problematic scenario, rather than attempting to fix it, as a fix wouldn't necessarily improve speed.
42 two small tab-completion patches (ALTER CONSTRAINT, CHECK modifiers) Proposed 1 msgs Oct 5, 21:28
The proposer introduced two small patches to enhance psql's tab-completion. The first patch aims to add completion for DEFERRABLE, ENFORCED, INHERIT, and their negating forms (e.g., NOT DEFERRABLE) after ALTER TABLE t ALTER CONSTRAINT c. The second patch proposes adding NOT ENFORCED, NOT VALID, and NO INHERIT as completion options after ADD [CONSTRAINT c] CHECK (...). These changes are intended to improve the user experience when defining or modifying constraints using psql.
43 Direct TOAST v2, faster, smaller and no migration needed Discussing 28 msgs Oct 5, 16:28
This thread introduces "Direct TOAST," an alternative storage format for out-of-line (TOASTed) variable-length attributes, aiming for faster performance, smaller footprint, and zero-migration. The core idea is to bypass the traditional TOAST B-tree index by embedding physical TIDs directly in the `varlena` pointer and within hierarchical chunk trees. This is claimed to reduce index insertion/WAL overhead, eliminate B-tree root-to-leaf traversals for reads, and improve vacuuming by enabling self-pruning without index cleanup. Initial benchmarks are presented with significant TPS improvements. However, a committer and other reviewers raise serious concerns about the design tradeoffs, questioning the universality of the performance claims, the impact on existing TOAST features like independent TOAST-only VACUUMs, and the complexity introduced. The proposer clarifies that the feature is optional, controlled by a storage parameter, and aims to address some of the raised issues (e.g., self-pruning for direct TOAST chunks, dynamic schema adaptation). The debate continues over the need for new infrastructure for this approach versus optimizing existing B-tree storage, and the implications for tools like VACUUM FULL, CLUSTER, and REPACK, which the proposer states would be forbidden directly on the TOAST table but would work fine on the main table.
44 Report relation extension blockers within parallel lock groups Committed 12 msgs Oct 5, 20:28
The thread discusses a fix for `pg_blocking_pids()` which previously failed to report processes blocking relation extension requests within the same parallel lock group. This oversight stemmed from a prior commit that introduced conflicts for relation extension locks among parallel group members without updating the blocking process detection logic. The proposer submitted a patch to add a specific exception for relation extension locks, ensuring `pg_blocking_pids()` correctly identifies blockers. The patch also included documentation updates to clarify that a process might appear to block itself when another member of its parallel group is the actual blocker. After a brief clarification on an earlier reverted change related to page locks, and a suggestion for documentation restructuring which was incorporated, the patch was committed.
45 examine_variable ignored CollateExpr Patch Review 6 msgs Oct 5, 05:29
This thread addresses an issue in `examine_variable` where `CollateExpr` nodes are implicitly stripped through `RelabelType` transformations, potentially causing inaccurate row estimates for queries using `COLLATE` clauses in `GROUP BY` or `WHERE` conditions. The proposer submitted a patch showing improved estimates for a specific test case. A commenter proposed a more general solution involving re-normalizing expressions, which was initially criticized for being too expensive. The commenter then refined their approach to an in-place function to avoid performance overhead. The discussion further progressed with a new reviewer identifying a scenario where the proposed patch might lead to worse estimates when no extended statistics object is present, suggesting a more conditional application of the stripping logic.
46 hashjoins vs. Bloom filters (yet again) Discussing 78 msgs Oct 5, 16:28
This thread discusses the reintroduction of a patch to integrate Bloom filters into hash joins to improve performance, particularly for selective joins or when batching (spilling to disk) is necessary. The proposer has previously worked on this twice. The core challenge is accurately estimating when the Bloom filter provides a benefit, as it introduces overhead if not selective enough. Previous attempts stalled due to this estimation difficulty and sizing concerns. A new version of the patch is being discussed, with a proof-of-concept for a semi-join + false-positive model for selectivity estimation. There's a debate between a "bottom-up, cost-based" approach (which requires better selectivity estimation for unmatched rows) and an "opportunistic" approach (which is simpler but might miss some optimizations). Specific issues raised include the need for exact matching of build relids for correct estimates, and potential trade-offs in snowflake schemas. A reviewer also provided a PoC patch for a semi-join + false-positive model. The thread also discusses challenges in estimating unmatched rows with current statistics infrastructure.
47 Session in aborted transaction misses effective_wal_level change Patch Review 9 msgs Oct 5, 18:29
This thread addresses a bug where an `effective_wal_level` change can be missed in a session with an aborted transaction, preventing subsequent transactions from being logically decoded. The proposer provides a reproducer script and an initial patch (v1) to move the `AtEOXact_LogicalCtl` call later. Reviewer 1 confirms the issue, provides an updated patch (v2) with regression tests, and suggests wider application of the fix. After additional reviews, a discussion ensues regarding the optimal placement of the `AtEOXact_LogicalCtl` call to avoid potential race conditions during memory cleanup callbacks. This leads to an updated patch (v4) that places the call after resetting the top-level transaction ID for a more robust solution.
48 Fix out-of-bounds array indexing in JsonValueList Patch Review 4 msgs Oct 5, 15:29
The thread discusses an out-of-bounds array indexing error in `JsonValueList` within `jsonpath_exec.c`, triggered when compiling PostgreSQL with `-fsanitize=bounds` or `-fstrict-flex-arrays=1`. The proposer identified that the `JsonValueList` structure, intended to act as a chunked list with its first chunk on the stack, incorrectly indexed past the declared `BASE_JVL_ITEMS` size. Although no memory overruns were reported in standard builds, the issue represents undefined behavior under C11. The proposer suggested a fix involving separating `JsonValueList` into two distinct structs: `JsonValueList` for the initial items and `JsonValueListChunk` for subsequent items. A committer acknowledged the problem, stating they had also encountered it, and later provided an alternative, simpler solution. This alternative solution uses a union within the existing `JsonValueList` struct, which effectively addresses the bounds issue while keeping the code simpler, with the caveat that it requires not exceeding `-fstrict-flex-arrays=1` (which is already a limitation for other reasons). A reviewer provided a positive assessment of the committer's proposed solution.
49 Fix apply worker crash when subscriber table has only a deferrable primary key Patch Review 22 msgs Oct 5, 09:29
This thread began by reporting an apply worker crash (or silent data loss in non-debug builds) when a subscriber table had only a `DEFERRABLE` primary key and the published table lacked `REPLICA IDENTITY FULL`. The issue stemmed from a discrepancy in how deferrable primary keys were handled by different functions within the apply worker. The initial patch rectified this by aligning the logic, resulting in the correct error message instead of a crash, and was subsequently committed after several rounds of review. The thread continued with a 'remaining issue' to further enhance the consistency and safety of the index lookup logic within the apply worker, specifically by utilizing an already computed local index OID rather than re-fetching it. A new patch addressing this refinement has been submitted and is currently under review.
50 Per-thread leak in ECPG's memory.c Committed 1 msgs Oct 5, 19:30
This thread addresses a per-thread memory leak in ECPG's `memory.c`, specifically within the `auto_mem_destructor()` function. The initial reporter identified two main problems: incorrect retrieval of thread-specific data at thread exit and an erroneous attempt to free user-owned heap objects by `ECPGfree_auto_mem()`. A patch was proposed to correct these issues by properly utilizing the destructor's argument and changing the freeing mechanism to `ecpg_clear_auto_mem()`, which correctly frees only the internal list structure, not the user's data. A reviewer confirmed the patch's effectiveness in preventing a double-free and suggested a minor cleanup for the destructor.