01 Report oldest xmin source when autovacuum cannot remove tuples Discussing 44 msgs Jul 7, 18:28
The proposer initiated a discussion to enhance VACUUM logs by including the reason for the oldest xmin, which prevents dead tuples from being removed. This aims to help diagnose and prevent table bloat by providing actionable information that is currently hard to capture retrospectively from volatile views. The initial patch proposed logging the reason and, if applicable, the backend PID. This sparked extensive discussion, including concerns about the overhead of an additional ProcArray scan, the timing of the report (after vacuum completion vs. earlier), and the accuracy of identifying the *highest-priority* blocker. Through several iterations, the patch evolved to address these concerns, adopting a priority scan to correctly identify the blocker, refining the information captured (e.g., GID for prepared transactions, application_name for standbys), and improving memory allocation. The latest versions (v7, v8, v9) also introduced significant correctness fixes, such as scoping the blocker scan to the relation's horizon kind, comparing against effective xmin for replication slots, covering all slots, and classifying backend states more accurately. A helper function was factored out to prevent logic drift between ComputeXidHorizons() and the new function. The recovery case was explicitly removed from GetXidHorizonBlockers()'s scope.
02 Fix gistkillitems & add regression test to microvacuum Committed 12 msgs Jul 7, 18:28
The proposer identified a bug in GiST where XLOG_GIST_DELETE records, related to the microvacuum feature, were not properly covered, specifically when all dead tuples reside on the root page. This meant gistkillitems was not called for the root page, making small GiST indexes with deleted items problematic for microvacuum. The proposer provided a patch with a regression test to address this. Initial feedback pointed out an existing, failing isolation test, which the proposer then adapted to. Reviewer 1 confirmed the bug, suggested the patch was suitable for master but not necessarily for backpatching, and proposed minor comment improvements. Reviewer 2 validated the fix by reproducing the issue and confirming correct handling of the root page. Although XLOG_GIST_DELETE records were not observed in one test case, the fix for curBlkno initialization was deemed correct. The patch was ultimately committed by a committer.
03 Bump soft open file limit (RLIMIT_NOFILE) to hard limit on startup Patch Review 31 msgs Jul 7, 08:29
This thread proposes to increase PostgreSQL's soft open file limit (`RLIMIT_NOFILE`) to the hard limit during startup, addressing frequent 'too many open files' errors and aligning with modern system configurations. This change is particularly beneficial for I/O-intensive workloads and common user issues. The discussion involved several patch iterations. Initial feedback led to refinements, including renaming helper functions and relocating code. A significant update introduced custom `pg_system()` and `pg_popen()` functions designed to restore the original, lower file limits in child processes immediately after forking but before `exec`, while maintaining the higher limit for the main PostgreSQL process. This ensures compatibility with older applications using `select(2)` and prepares for future multi-threading. A part of the proposed change for `pgbench` was already committed in PostgreSQL 18.
04 Refactoring postmaster's code to cleanup after child exit Discussing 22 msgs Jul 7, 09:29
This thread, initially about refactoring postmaster's cleanup code, has largely shifted to addressing persistent test failures, particularly in `001_connection_limits.pl` under Valgrind and `002_connection_limits.pl` on Hurd systems. Early discussions focused on improving error reporting in TAP tests by capturing `stderr` and `stdout` for better debuggability. Patches were proposed and well-received to fix a race condition and enhance error messages in connection limit tests. Later, a specific issue on Hurd was identified, where `IO::Socket::UNIX`'s `getpeername()` call failed due to an unimplemented system call. A temporary `SKIP` patch for this test on Hurd was suggested and discussed, with the latest messages confirming its successful implementation as a workaround in Debian packages.
05 Bug: mdunlinkfiletag unlinks mainfork seg.0 instead of indicated fork+segment Committed 8 msgs Jul 7, 18:28
The proposer identified a bug in PostgreSQL's mdunlinkfiletag function within the MD smgr internals. This bug caused unlink requests for specific forks or segments to incorrectly target segment 0 of the main fork, rather than the requested fork and segment. This behavior, present since PG16 for forks and PG12 for segments other than 0, could be problematic during recovery or for extensions expecting precise unlinking. Initially, the proposer offered a patch to fix the incorrect unlinking. However, a reviewer suggested an alternative direction: since the mechanism is primarily used for "tombstone" files (main fork, segment 0) and is not intended for general-purpose unreliable segment unlinking by external code, it would be better to rename register_unlink_segment() to register_unlink_tombstone() and add Assert() statements to ensure it's only used for tombstone files. The proposer agreed with this revised approach, submitting a new patch. Subsequent reviewers confirmed the correctness and clarity of this new direction. The committer agreed with the renaming and assertion, and committed the changes to master.
06 injection_points: Switch wait/wakeup to use atomics rather than latches Patch Review 18 msgs Jul 7, 10:29
This thread discusses a proposal to switch the `injection_points` facility from using latches and condition variables to atomic counters for wait/wakeup mechanisms. The core committer (proposer 1) identified limitations with the current latch-based approach, especially for scenarios where processes lack assigned latches (e.g., in postmaster context or during backend termination), which hinders testing. The proposed patch uses atomic counters and a periodic polling loop with escalating delays (10us to 100ms), arguing it provides a good balance of responsiveness and portability. A reviewer initially expressed concern about reduced responsiveness compared to instant wakeups, but another committer (commenter 1) clarified that latches only work after a `PGPROC` slot is available, which is not always the case (e.g., authentication, postmaster). A third committer (commenter 2) confirmed the need for this change for corruption-seeking tests and suggested improving how wait points are identified. Further discussion explored methods for tests without `PGPROC` access, including printing LOG entries or using memory-mapped files. Proposer 1 rejected the file-mapping approach as too invasive, suggesting a simpler file-based marker system. Commenter 2 then submitted a new patch based on the simpler file-based approach, rebased on the atomics commit.
07 BF mamba failure Discussing 14 msgs Jul 7, 10:29
This thread reports a recurring FATAL error "trying to drop stats entry already dropped" leading to replica shutdowns, specifically on cascading streaming replicas. The issue has been observed across multiple PostgreSQL versions (15.4, 17.3, 17.6) and persists despite upgrades. The reporter notes a heavy workload with frequent creation and dropping of tables (high OID consumption). A core committer suggests the problem might be related to transaction ID wraparound and race conditions in standbys, particularly with how `pgstats` entries are dropped during WAL redo. Initial investigations included adding "generation" to the error log for better debugging (committed to 17.6). The committer is attempting to reproduce the issue with specific test scenarios but has so far been unsuccessful. The latest update confirms the issue still occurs in 17.6, highlighting the difficulty in tracking down this elusive bug.
08 Add enable_groupagg GUC parameter to control GroupAggregate usage Patch Review 14 msgs Jul 7, 07:29
This thread proposes adding a new GUC parameter, enable_groupagg, to control the use of GroupAggregate plans. The proposer demonstrated a significant performance improvement (35x faster in a specific parallel query scenario) by disabling GroupAggregate and forcing HashAggregate. Reviewers supported the idea, noting that existing GUCs like enable_sort are too broad for specific aggregate control. The discussion expanded the scope of enable_groupagg to cover other sort-based operations like SetOp and Unique. A committer took over the patch, incorporating these broader changes (v4) and refining the cost model for AGG_MIXED strategies. The latest discussions focused on documentation clarity, with the committer preferring to maintain conceptual consistency with existing GUCs rather than enumerating specific plan node types. The committer expressed intent to push the patch soon.
09 Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server Committed 25 msgs Jul 7, 01:29
The proposer reported a bug where `ALTER SUBSCRIPTION ... SERVER` or `... CONNECTION` fails if the *old* foreign server or its user mapping is broken, contradicting documentation that suggests ACL checks should be skipped. The root cause was `GetSubscription()` calling `ForeignServerConnectionString()` even when ACL checks were explicitly set to `false`, attempting to connect to a potentially broken server. The proposed fix involved storing the server OID in the `Subscription` object and lazily calling `ForeignServerConnectionString()` only when `sub->conninfo` is actually needed. Discussions also covered related issues, such as `DROP SUBSCRIPTION` behavior when `SLOT_NAME=NONE` is used but tablesync slots still exist. Parts of the initial patch (0001 and 0002) have been committed to address the primary bug. Further discussion on a third patch (0003) involved using subtransactions to capture errors during connection attempts for `DROP SUBSCRIPTION`, but the proposer ultimately opted against this complexity, favoring a simpler "escape hatch" for users to force `DROP SUBSCRIPTION` without remote publisher contact, acknowledging a pre-existing bug related to documentation deviation.
10 Do not lock tables in get_tables_to_repack Patch Review 3 msgs Jul 7, 17:28
The proposer raised a concern regarding the `REPACK` process, specifically how tables are locked during the initial list building. The existing mechanism is flawed because concurrent table drops can still occur after the list is built (due to per-table transactions), and `ConditionalLockRelationOid()` can lead to unexpected `SKIP_LOCKED` behavior. The proposer suggests removing these locks and ensuring `repack_is_permitted_for_relation()` robustly handles concurrent drops. A commenter initially questioned the urgency for `REPACK` compared to `VACUUM` but another core developer supported the proposal, highlighting inconsistencies in `REPACK`'s current table list acquisition. This core developer agreed with the proposed direction and attached an alternative patch for review, aiming for a more defensive implementation.
11 Add malloc attribute to memory allocation functions Discussing 7 msgs Jul 7, 16:29
The proposer suggested adding the `malloc` attribute to memory allocation functions to leverage compiler features for error detection, code coverage, and optimization. While initial testing showed no regressions, a core reviewer raised significant concerns that GCC's static analyzer, lacking an understanding of PostgreSQL's memory contexts, would generate a large volume of false-positive memory leak warnings. The proposer confirmed that such warnings increased dramatically from 62 to 598. Commenters then discussed the broader benefits of the attribute beyond leak detection, particularly for frontend tools without memory contexts. The proposer proposed suppressing the leak warnings for backend code, leaving the path open for continued discussion.
12 plpython: NULL pointer dereference on broken sequence objects Patch Review 6 msgs Jul 7, 10:29
This thread discusses a NULL pointer dereference vulnerability in PL/Python, similar to a recently fixed issue in PL/Perl. The proposer identified that `PySequence_GetItem()` and related functions in PL/Python (like `PyMapping_Items()` and `PyList_GetItem()`) did not check for NULL return values, allowing a maliciously crafted or broken Python object (e.g., one that claims a length but fails to deliver items) to crash the backend. A patch was submitted to add NULL checks and raise proper errors. A reviewer confirmed the fix and suggested extending it to mapping-related functions, which the proposer promptly addressed in a v2 patch. The v2 patch was reviewed, approved, and then committed and back-patched to v14. After the commit, another commenter identified a similar remaining issue in `PLySequence_ToJsonbValue()` where `PySequence_Size()` returns -1 for unresolvable lengths, leading to silent empty array output and a poisoned Python error indicator, proposing a further patch.
13 Mark class_descr strings for translation Discussing 5 msgs Jul 7, 18:28
The proposer identified an issue with message translation in pg_depend.c, where get_object_class_descr() returns a string (intended for internal messages) that is embedded in user-facing error messages. This results in unnatural translations. The proposer proposed a patch to mark these class_descr strings for translation using gettext_noop() at their definition and _() at their usage. A reviewer approved this approach, confirming that both halves of the patch (marking at array definition and translating at consumption) are necessary and correctly implemented. However, a committer raised a concern, suggesting that class_descr strings are *meant* for internal messages and their use in user-facing messages is a bug at the caller's site. The committer expressed hesitation about adding many new translation strings that only differ by object type, suggesting alternative solutions might be preferred.
14 Implement waiting for wal lsn replay: reloaded Patch Review 50 msgs Jul 7, 15:29
This thread continues the discussion and refinement of a patchset aimed at improving the waiting for WAL LSN replay mechanism. Initially, the proposer submitted a patch to fix a misplaced wake-up call in PerformWalRecovery to prevent missed wake-ups during recovery pauses or promotions. Commenter 1 (an early reviewer) had pushed an earlier version of the patchset but then discovered new test failures related to the wait_for_catchup() implementation, which required adjustments to test scripts. The proposer provided a revised patch to simplify the fix by directly calling wait_for_catchup() and accepting a return to polling in some cases. Commenter 1 approved this, noting the clarity gained. Later, commenter 3 provided a detailed review of the current implementation, raising concerns about reading XLogRecoveryCtl->lastReplayedEndRecPtr without locks, the performance impact of three extra function calls on a hot path for every WAL record, and suggesting moving the WaitLSNWakeup() calls inside ApplyWalRecord() for better code structure. The proposer agreed to test the performance impact and supported moving the calls.
15 Proposal: new file format for hba/ident/hosts configuration? Proposed 2 msgs Jul 7, 17:28
The proposer highlights several issues with PostgreSQL's current hba/ident/hosts configuration file formats, including their PostgreSQL-specific nature, limited extensibility, and inconsistencies in order-sensitivity. The proposer suggests introducing a new, standard configuration format while retaining the existing formats for backward compatibility, with new features exclusively implemented in the new format. After evaluating INI, XML, JSON, YAML, and TOML, the proposer expresses a preference for TOML, finding JSON too verbose despite having an existing parser. The proposer notes that TOML was suggested by others and appears to offer a good balance of precise specification and human readability. A commenter agrees with the assessment of existing formats and the potential benefits of TOML or JSON5, also emphasizing the value of JSON Schema for validation.
16 truncating casts of pgoff_t Committed 4 msgs Jul 7, 10:29
The proposer identified instances where `pgoff_t` values, typically 64-bit integers, were cast to smaller types for output, potentially leading to truncation in error messages. This was deemed mostly cosmetic but could have practical implications in `basebackup_server.c`. The initial patch aimed to fix this by consistently using the `%lld` print format and casting to `long long int`, aligning with existing practices and improving robustness against negative values. Reviewers agreed with the general approach and suggested a more robust error handling (`elog(ERROR)`) instead of an `Assert` for `histfilelen` in `SendTimeLineHistory()`. They also noted a related issue of reading beyond the original file length if the file grows during reading, which is being addressed in a separate thread. The proposer committed the patch with the suggested `elog` change.
17 wait_event_type for WAIT FOR LSN Discussing 5 msgs Jul 7, 16:29
The proposer suggested reclassifying several `WAIT_FOR_WAL_*` events from the `WaitEventClient` category to `WaitEventIPC` in `wait_event_names.txt`. The rationale is that these waits are not solely related to client socket I/O but can also stem from internal delays like local fsync or WAL replay, making them more akin to inter-process communication. Commenters generally agreed with this move. A discussion also ensued regarding `WAIT_FOR_STANDBY_CONFIRMATION`, with the proposer explaining the arguments for and against reclassifying it. Ultimately, the proposer decided against moving `WAIT_FOR_STANDBY_CONFIRMATION` due to the higher standard for reclassifying events that have already been released in a previous version, and this decision was accepted by other participants.
18 Don't use the deprecated and insecure PQcancel in our frontend tools anymore Patch Review 22 msgs Jul 7, 15:29
This thread focuses on a significant refactoring effort to remove the insecure PQcancel function from PostgreSQL's frontend tools (like psql) in favor of newer, more secure cancellation APIs introduced in a recent commit. The initial proposer outlined the changes, which involve moving cancellation logic to a dedicated thread on non-Windows platforms to align with existing Windows behavior and avoid signal-safety issues. The proposer also considered backporting the change to recent PostgreSQL versions. Discussions included addressing complexities in psql's cancel handling, identifying redundant global variables, and potential follow-up work like using threads for pg_dump on Unix. One reviewer committed a documentation-only patch related to current cancel handling. Later in the thread, a race condition with select() and signal handling was identified, leading to a new patch proposal that uses a 1-second poll as a temporary fix. The reviewer then explored more robust long-term solutions, suggesting various alternatives like pselect(), ppoll(), or custom helper functions, indicating ongoing deeper architectural discussions.
19 Propagate stadistinct through GROUP BY/DISTINCT in subqueries and CTEs Patch Review 5 msgs Jul 7, 07:29
This thread addresses a critical performance issue in PostgreSQL's query planner, where cardinality estimates for GROUP BY or DISTINCT clauses within subqueries and CTEs were often inaccurate, leading to inefficient execution plans. The proposer demonstrated cases where queries ran hundreds or thousands of times slower due to the planner falling back on 1/DEFAULT_NUM_DISTINCT for selectivity. The proposed solution is a patch that propagates stadistinct (number of distinct values) from base tables to GROUP BY or DISTINCT key columns, allowing for more accurate 1/ndistinct estimates. The latest version of the patch (v2) also accounts for how stanullfrac (null fraction) should be adjusted when grouping collapses NULL values.
20 Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM Proposed 4 msgs Jul 7, 14:28
This thread addresses a long-standing issue where views using NULLIF or IS [NOT] DISTINCT FROM with equality operators not present in the search_path (e.g., hstore) fail to dump and restore correctly due to the deparser emitting unqualified operator names. The proposer initially suggested rewriting these constructs into CASE expressions with schema-qualified operators (v1), but a reviewer raised concerns about performance and the need for a more comprehensive solution for similar cases like JOIN USING. The proposer acknowledged these concerns and withdrew v1. A revised approach (v2) was then proposed, which involves modifying the deparser to add explicit OPERATOR() decoration to all constructs that resolve operators by unqualified names, covering a broader range of affected statements. This new approach aims to preserve the original expression tree, maintaining performance and optimizations. The reviewer found this direction promising.
21 Add missing CustomPathMethods on typedefs.list Rejected 2 msgs Jul 7, 14:28
This thread concerns a cosmetic issue where `CustomPathMethods` was not correctly indented in `src/include/nodes/extensible.h` because its declaration was missing from `typedefs.list`. The proposer submitted a patch to add it. However, a reviewer explained that `typedefs.list` is a mechanically generated file. Changes to it will not persist if the typedef is not actively used in the code to declare objects (variables, fields, parameters, etc.). Since `CustomPath` references it using `const struct CustomPathMethods *methods;` rather than the typedef, the proposed fix was deemed non-persistent. The reviewer suggested that a persistent fix would require modifying the usage or duplicating the typedef declaration in `pathnodes.h`.
22 generic plans and "initial" pruning Patch Review 59 msgs Jul 6, 09:29
This thread focuses on optimizing lock acquisition for generic plans, particularly in the context of partition pruning. The proposer redesigned the executor's startup process, moving execution lock acquisition out of GetCachedPlan() to callers, allowing for more precise, pruning-aware locking. The patch series (v13) introduced functions to handle validity checks, retries, and to lock only the relevant, unpruned partitions. Reviewer 2 confirmed that SELECT queries with reused generic plans successfully acquired locks only on active partitions. However, UPDATE statements still locked all partitions, which the proposer clarified is outside the scope of this specific optimization for now. A critical resource ownership bug in the PortalLockCachedPlan() retry path was identified by Reviewer 2 on a Windows build and confirmed by the proposer, indicating the need for further refinement before commitment.
23 postgres_fdw: fix cumulative stats after imported foreign-table stats Committed 12 msgs Jul 7, 14:28
The thread identifies and resolves a bug in `postgres_fdw` where cumulative statistics were not updated for foreign tables when `ANALYZE` imported remote statistics using the `restore_stats = true` option. The proposer demonstrated that `pg_stat_get_analyze_count` remained zero and `pg_stat_get_last_analyze_time` was null for such tables. The root cause was identified as `do_analyze_rel()` being skipped during stats import, which is the only function that calls `pgstat_report_analyze()`. Initially, the proposer suggested modifying the `ImportForeignStatistics` API. However, a reviewer proposed an alternative: calling `pgstat_report_analyze()` directly within the `postgresImportForeignStatistics()` FDW callback. This alternative was adopted, refined, and subsequently committed and back-patched, resolving the issue.
24 Support EXCEPT for TABLES IN SCHEMA publications Patch Review 93 msgs Jul 7, 11:28
This thread discusses extending the `EXCEPT` clause functionality to `TABLES IN SCHEMA` publications, building on previous work for `FOR ALL TABLES`. The goal is to allow users to exclude specific tables from schema-level publications, which currently publish all tables without exclusion. The proposer outlined a new syntax for `CREATE PUBLICATION`, `ALTER PUBLICATION ADD`, and `ALTER PUBLICATION SET` with the `EXCEPT` clause, requiring schema-qualified table names. Key rules for combining `TABLES IN SCHEMA` with `FOR TABLE` and handling conflicting definitions were detailed, along with consistency requirements for partitioned tables (e.g., excluding a root table also excludes its partitions). The discussion has involved multiple patches and detailed review, focusing on refining error messages, clarifying the behavior of `ADD/DROP TABLE` for excluded tables, and ensuring consistent handling of partition hierarchies. A recent bug related to conflicting exclusion lists was also reported and is under discussion.
25 implement CAST(expr AS type FORMAT 'template') Discussing 53 msgs Jul 7, 12:28
The thread discusses the implementation of `CAST(expr AS type FORMAT 'template')` to align with the SQL standard. The initial patch introduced this syntax, primarily transforming it into existing `to_` formatting functions. A reviewer questioned the design's coherence, particularly how it relates to standard casts and suggested the existing `to_char` functions might suffice. Another commenter, who also contributed patches, expanded on potential use cases beyond date/time, envisioning a generic mechanism for conversions between types and external representations (text/bytea), including support for extension types. However, the reviewer maintained skepticism about the feature's fundamental value and its broadness. In the latest messages, the commenter who proposed the broader scope acknowledged that their previous patch was too expansive, realizing that the SQL standard's T839 feature is specifically for formatted casts between datetime types and character strings. This commenter suggests narrowing the scope to this specific standard feature and proposes renaming the thread to reflect this focus, while the reviewer continues to question the overall necessity of the feature.
26 ci: namespace ccache by PostgreSQL major version Rejected 4 msgs Jul 7, 18:28
The proposer observed that CI jobs fail with "incompatible library ... version mismatch" errors when the PostgreSQL major version is updated. This occurs because GitHub Actions ccache reuses old object files, leading to libraries compiled against a previous version. To address this, the proposer suggested namespacing the ccache key by the PostgreSQL major version, by adding a PG_MAJOR_VERSION variable to the CI file, updated by version_stamp.pl. A reviewer expressed suspicion, arguing that major version is only a partial indicator of ABI compatibility, and that ccache should inherently detect such changes if used correctly (e.g., in "depend mode" or "direct mode"). The proposer acknowledged this feedback, realizing their initial approach was flawed. Another commenter explicitly stated this was not the right answer and pointed to a different thread and a more suitable technical solution.
27 pg_stat_io_histogram Patch Review 33 msgs Jul 6, 12:29
The proposer introduces `pg_stat_io_histogram` to track and display I/O latency profiles, aiming to help identify I/O outliers and "stuck" PostgreSQL instances. The initial proposal focuses on a global histogram for various backend types, objects, contexts, and I/O types. Early discussions center on the overhead and memory footprint, particularly for `pgStatLocal` and `PendingIOStats` in each backend. A core committer expresses concern about storing "lots of values that are never anything but zero." The proposer explores several strategies to reduce memory usage, including dynamically allocating memory and using indirect offsets for histograms in both backend-local and shared memory. Multiple patch versions are shared, with `v9b` and `v10` significantly reducing shared memory footprint. Discussions also cover the histogram's range and bucket granularity, with suggestions to prioritize higher latencies for outlier detection.
28 MinGW ccache snafu on CI Discussing 2 msgs Jul 7, 18:28
The proposer reported a recurring issue in the MinGW CI job where ccache gets confused, leading to "incorrect version" errors (e.g., vacuumdb (PostgreSQL) 19beta1 found, 20devel expected). This suggests ccache fails to recompile correctly, producing stale object files. The proposer noted that the MinGW job uses precompiled headers and wondered if incorrect ccache switches might be a factor, contrasting it with the MSVC job which also uses PCH but doesn't exhibit the same problems. A commenter pointed to a related thread, suggesting that adding -fpch-deps for gcc (until ccache or meson handles it automatically) might be the solution.
29 Adding a stored generated column without long-lived locks Discussing 9 msgs Jul 7, 18:28
The proposer introduced a feature to add a stored generated column to a large table without requiring long-lived AccessExclusiveLock for a full table rewrite. The initial proposal involved an ALTER TABLE ... ALTER COLUMN ... ADD GENERATED ALWAYS AS (expr) STORED command, which would skip the rewrite if a CHECK (c = expr) constraint already existed and was valid. Discussion ensued about standard SQL syntax compliance, whether changing the expression should also avoid a rewrite, and existing "tricks" for online migrations. A subsequent design iteration (v2) refined the approach to explicitly use a constraint: ALTER TABLE ... ALTER COLUMN ... ADD GENERATED ALWAYS AS (expr) STORED USING CONSTRAINT check_name, failing if no suitable constraint was found, and changing (c = expr) to (c IS NOT DISTINCT FROM expr) to handle NULLs. Further refinement (v3) suggested dropping the explicit (expr) from the syntax, deriving it directly from the constraint, and dropping the constraint automatically. However, a reviewer preferred not dropping the constraint, aligning with existing behavior. The current patch (v6) adheres to iteration 3, does not drop the constraint, and includes documentation and tab completion.
30 Function scan FDW pushdown Patch Review 16 msgs Jul 7, 10:29
This thread discusses the development and refinement of a patch to enable Foreign Data Wrappers (FDWs) to push down function scans, allowing functions like `generate_series` to be executed remotely. The initial patch faced issues with `rte->functions` handling and unfinished design aspects, leading to it being returned for feedback. Subsequent versions addressed these concerns, including stricter checks for function types (disallowing set-returning complex types or functions with parameters in arguments) and moving function information to foreign scan private data. A key challenge was correctly handling `rti` mappings after `setrefs` and ensuring proper deparsing of `ColumnRef` for whole-row variables, especially with nullable outer sides. Discussions also revolved around the cost model, explaining why local joins might still be preferred over pushed-down ones due to cost estimations. The latest iterations focus on refining the handling of whole-row variables in update/returning clauses and classifying `baserestrictinfo` correctly by passing `fpinfo` down to `foreign_expr_walker`. The thread is actively addressing review comments and identified issues, with a focus on robustness and correct behavior across various query types.
31 [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements Patch Review 51 msgs Jul 7, 17:28
The proposer introduced a patch to add `pg_get_table_ddl()`, a function to reconstruct `CREATE TABLE` statements and related DDL for indexes, constraints, rules, and statistics. The function aims to support various table and column features, offering options to fine-tune the output. Initial feedback included concerns about overlap with `pg_dump` and suggestions for option handling. Several issues were identified through review, such as incorrect clause order for `TABLESPACE` and `ON COMMIT`, missing `ONLY` for inherited default overrides, and erroneous duplicate unique indexes on partitioned tables, which the proposer addressed in subsequent patch versions. The latest review of v17 confirmed previous fixes but highlighted new problems: `serial` columns produce non-replayable DDL because sequences are not created, and partition/inheritance children lose specific properties like dropped defaults or custom constraint names upon DDL replay.
32 Increased SECURITY RISCS from omitting some compikler options when building PG with meson; e.g. -fcf-protection=full Discussing 2 msgs Jul 7, 12:28
A proposer highlighted that self-compiled PostgreSQL instances using Meson on Fedora 44 lack default security compiler options, such as Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT), unlike distribution or PGDG provided packages. This discrepancy could lead to reduced security. The proposer suggested enabling these compiler options by default in both autoconfigure and Meson build systems to ensure consistent security across different operating systems, PostgreSQL versions, architectures, and compilers, and to improve build reproducibility. A commenter disagreed, arguing that distributions are better equipped to decide on specific security protections. The commenter stated it would be unreasonable and overly complex for PostgreSQL to track and align with the diverse and evolving compiler flags adopted by various operating systems.
33 Use HostsFileName everywhere Patch Review 4 msgs Jul 7, 08:29
The proposer identified a bug where three specific PostgreSQL error messages hardcoded 'pg_hosts.conf' instead of referencing the configurable `hosts_file` GUC. This inconsistency could lead to confusion when a custom hosts file is in use. A patch was submitted to update these error messages to dynamically use the `HostsFileName` variable. Reviewers confirmed the bug's validity and the patch's correctness, considering it a useful cosmetic improvement. A minor typo in the commit message was noted. A potential NULL-safety edge case, if `hosts_file` were unset, was investigated but deemed non-fatal as configuration loading at startup prevents a NULL value in typical operation.
34 remove unnecessary volatile qualifiers Committed 4 msgs Jul 7, 16:29
The proposer initiated a thread to remove `volatile` qualifiers deemed unnecessary from the PostgreSQL codebase, aiming to improve code clarity and potentially enable compiler optimizations. A reviewer provided feedback, identifying one instance where `volatile` was still necessary due to usage within `PG_TRY/CATCH` blocks and suggesting a minor refactoring. The proposer promptly incorporated the feedback, reverting the problematic change and implementing the suggested refactoring. The revised patch was then committed to the repository.
35 [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks Patch Review 32 msgs Jul 7, 07:29
This thread discusses a patch aimed at fixing a long-standing design issue in PostgreSQL where recovery target GUC assign hooks incorrectly raised errors, causing guc.c's internal state to become inconsistent. The proposer's solution involves removing conflict checks from these assign hooks and centralizing conflict detection in validateRecoveryParameters(), which runs safely after all GUCs are loaded. This changes when the error occurs (from postmaster startup to the startup process) and results in conflicting settings being silently ignored if recovery is not requested. Discussions covered the wording of error messages, the utility of hinting pg_settings during a failed startup, and the clarity of regression test comments. The patch has undergone several revisions, with the latest focusing on refining error messages and test descriptions.
36 Row pattern recognition Discussing 50 msgs Jul 7, 07:29
This thread is an extensive discussion on a patch for Row Pattern Recognition (RPR). Initially, a correctness problem was identified where window function results could change based on unrelated window functions, leading to incorrect pattern matching advancement. This highlighted a need for the RPR window to perform matching upfront. Subsequent discussions addressed changes in the planner's cost model, specifically how it charges for DEFINE expressions, with a reviewer clarifying that the change was a simplification and did not alter the cost. The thread also delved into advanced design considerations for CLASSIFIER and row pattern variable storage, and a bug was found related to context-absorption optimization causing incorrect match counts in specific queries. The participants discussed whether to integrate major refactoring changes immediately or defer them to follow-up patches to keep the current commit small.
37 ri_Fast* crash w/ nullable UNIQUE constraint Discussing 3 msgs Jul 7, 16:29
The proposer initiated a review of the `ri_Fast*` family of commits, identifying three significant issues. The first is a crash occurring with foreign key constraints that reference nullable unique constraints, due to an incorrect assertion. The second is a session-lifespan memory leak originating from functions called within a `TopMemoryContext` that should use a shorter-lived context. The third raises concerns about whether newly queued deferred triggers might be lost if they are created during the firing of fast-path batch callbacks. The implementer acknowledged these findings and committed to addressing them after resolving a related issue.
38 libpq: Process buffered SSL read bytes to support records >8kB on async API Committed 26 msgs Jul 7, 16:29
The thread discusses a critical issue in libpq's asynchronous API when using SSL, where records larger than 8KB could cause deadlocks or hangs due to the library failing to fully drain OpenSSL's internal buffer. The proposer provided patches to address this by ensuring pqReadData() processes all buffered SSL read bytes. Early discussions involved concerns about the "tight coupling" between libpq and OpenSSL's buffering mechanisms and potential impacts on future architectural changes, which the proposer clarified were not being precluded. Multiple reviewers provided feedback, identifying a layering violation and a bug where buffer pointers were not correctly advanced. These issues were resolved, a reproducer script was shared, and the patch was successfully tested by a third party. The final version was committed and backpatched to all supported PostgreSQL versions.
39 RFC: Logging plan of the running query Discussing 62 msgs Jul 7, 15:29
This "Request for Comments" thread discusses a proposed feature to log the execution plan of a running query, accessible via a pg_log_query_plan() function. Early discussions focused on the naming of the function and the asynchronous nature of the operation, with an agreement to document potential delays. The proposer addressed initial feedback by fixing specific code issues (e.g., extern keywords, duplicate definitions, boolean parameters, injection points) and updating documentation regarding asynchronicity and innermost query plan logging. A reviewer provided detailed architectural feedback, suggesting the use of a ExecProcNode() hook for better extensibility and safety instead of the current ExecSetExecProcNodeRecurse() approach, and raising concerns about QueryDesc pointer safety and predictability for automation. The proposer responded by reiterating performance concerns about hooks and clarifying the behavior with nested queries, explaining that the current mechanism would log the plan for deeper queries. The discussion continues regarding implementation robustness and guarantees for users.
40 pg_dump: use threads for parallel workers on all platforms Discussing 2 msgs Jul 7, 15:29
This thread discusses a proposal to standardize pg_dump's parallel worker implementation across all platforms by using threads instead of processes, coordinated via an in-process work queue. The proposer has already developed the Windows half of this change. A reviewer provided feedback, agreeing with the direction but raising concerns about testing Windows-only code and inquiring about thread-safety hazards for setFmtEncoding() and quote_all_identifiers. The reviewer also noted the introduction of _Thread_local and suggested removing an unused hook. A specific bug fix related to an unchecked _beginthreadex return (patch 0001) was committed and backpatched by the reviewer, with the understanding that the larger thread-based model unification remains an ongoing discussion.
41 Global temporary tables Patch Review 32 msgs Jul 7, 10:29
This extensive thread details the development of a patch series implementing global temporary tables (GTTs), where definitions are permanent and shared, but data is session-local and temporary. The proposer aims to provide a new design addressing issues in older attempts. The patch series introduces a `RELPERSISTENCE_GLOBAL_TEMP` type, manages local storage and buffers for each session, and tracks GTTs across backends using a shared hash table to prevent concurrent DDL issues. Key aspects include handling indexes, sequences, and introducing `pg_temp_class` for per-session overrides. Recent discussions focused on `relfrozenxid` handling for GTTs, which was revised to set `pg_class.relfrozenxid` to 0 and update only `pg_temp_class` values. However, a reviewer uncovered a critical bug where `pg_temp_class` tuples could be lost during savepoint rollbacks, leading to unvacuumable and undroppable tables. The proposer submitted v7 to address this by implementing sub-XID tracking for `pg_temp_class` tuples. Another commenter reported that sequences attached to GTTs with `ON COMMIT DELETE ROWS` were not reset, which the proposer indicated was the expected behavior consistent with local temp tables. A different commenter reported issues with `pg_dump` not preserving the `ON COMMIT` clause, aborted transactions pinning `frozenxid`, and potential NULL pointer crashes with OOM in `dshash_find`. The latest comment identifies an infinite loop in `gtr_process_invalidated_gtrs()` when `debug_discard_caches` is enabled due to whole-cache invalidation.
42 Fix duplicate detection for null-treatment window functions Proposed 1 msgs Jul 7, 15:29
This thread identifies a bug in ExecInitWindowAgg() where duplicate detection for window functions with IGNORE NULLS or explicit RESPECT NULLS options fails. The proposer explains that the ignore_nulls flag is not copied when function information is appended to perfuncstate data, leading to wfunc->ignore_nulls == perfunc[i].ignore_nulls always evaluating to false for these cases. A minimal one-line fix is proposed to copy the ignore_nulls value, along with a clear reproduction case and debug logs illustrating the "cache miss" before the fix and "cache hit" after. The proposer included temporary debug logs in the patch to aid reviewers, noting they should be removed before commitment.
43 support create index on virtual generated column. Patch Review 21 msgs Jul 7, 14:28
This thread focuses on implementing support for creating indexes on virtual generated columns (VGCs). Early proposals involved transforming VGC indexes into expression indexes, with various restrictions and challenges related to dependency tracking and `ALTER TABLE` operations. An early patch (v1) was found to cause a bootstrap crash, which was subsequently fixed. A reviewer suggested a simpler approach by only supporting VGCs in expression indexes and limiting `SET EXPRESSION` functionality. The original proposer debated the implications of not adding new catalog columns for VGC indexes. A new proposer (proposer 2) took over, outlining a v2 patch that expands VGCs into expressions during index creation, storing them in `pg_index.indexprs`, and adding a new `indisvirtual` catalog column for clarity. The latest update, v4, revises `generated_virtual.sql` and related output, covering index constraints, regular indexes, and foreign keys, with all tests passing.
44 Reject ill-formed range bounds histograms in pg_restore_attribute_stats() Patch Review 2 msgs Jul 7, 08:29
The proposer identified a bug in `pg_restore_attribute_stats()` and `pg_set_attribute_stats()` where these functions accept malformed range bounds histograms. These histograms, if not properly sorted or containing empty ranges, cause internal assertions to fail (crashing the backend) or lead to incorrect selectivity estimates in the planner. The issue is analogous to a previous bug with oversized MCV lists. The proposer submitted a patch that adds validation to the bounds histogram during import, rejecting ill-formed input with a `WARNING` to prevent such runtime failures, aligning with existing error handling practices for other inconsistent statistics inputs.
45 CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions Discussing 82 msgs Jul 7, 08:29
This thread details the implementation of `CAST(... ON CONVERSION ERROR)` for error-safe type conversions. The proposer identified several consistency issues, such as differing error propagation for operand errors (e.g., division by zero versus numeric overflow) and inconsistent behavior between plan-time constant folding and run-time evaluation. Debate also centered on how to handle user-defined cast functions—either disallowing them, permitting them with strict caveats, or limiting them to C/Internal language functions. A reviewer suggested that error-safeness should be a function property (via `CREATE FUNCTION`) rather than a cast property. The reviewer also detailed 'clear defects' in operand error swallowing and problematic interactions with nested `DEFAULT` clauses. The proposer has responded to some of these points and, in the latest messages, proposed a new `ERROR { SAFE | UNSAFE }` attribute for `CREATE FUNCTION` to explicitly mark error-safe functions, to be stored in `pg_proc`.
46 Proposal: Conflict log history table for Logical Replication Discussing 54 msgs Jul 7, 08:29
This thread concerns the design and implementation of a conflict log history table for Logical Replication. Following an initial commit of a preparatory part of the feature, discussions ensued regarding documentation, code comments, and minor stylistic improvements. However, a reviewer raised significant architectural concerns about the error handling strategy in the committed code. The reviewer criticized the approach of temporarily setting aside an error, invoking `AbortOutOfAnyTransaction()`, and then attempting to insert a conflict tuple before re-throwing the original error, citing potential unsafety for nested transaction blocks and risks of data inconsistency if the conflict logging itself fails. The original proposer defended the design, explaining that `AbortOutOfAnyTransaction()` operates at the outermost catch block and that `replorigin_xact_clear()` prevents replication origin advancement if conflict logging fails, ensuring consistency.
47 [RFC PATCH v0 0/7] Add EXPLAIN ANALYZE wait event reporting Discussing 8 msgs Jul 6, 18:29
This RFC thread proposes adding `EXPLAIN (ANALYZE, WAITS)` to report completed wait events, both at the statement and plan-node levels. The proposer outlined an inclusive attribution semantic for plan nodes, fixed-size accumulators with overflow handling, and allocation-free wait-end accounting. Key review questions included the option name (`WAITS` vs. `WAIT_EVENTS`), the inclusive per-node attribution, the fixed accumulator limit, and the hot-path overhead. A core reviewer expressed significant skepticism regarding the proposed direct measurement approach, particularly concerns about adding cycles to the `pgstat_report_wait_start/end()` functions and the potential for measurement overhead to heavily distort results. The reviewer and another commenter strongly advocated for a sampling-based approach instead, noting that event counts are less useful than durations. The discussion is now exploring the complexities of implementing either self-sampling with timer interrupts or external sampling, focusing on how to minimize overhead and effectively attribute wait events to plan nodes.
48 [PATCH] Fix for bug #19474: LIKE fails to match literal backslashes with nondeterministic collations Committed 11 msgs Jul 7, 09:29
This thread addresses Bug #19474, concerning `LIKE` operator failures to match literal backslashes with non-deterministic collations. The proposer identified an incorrect escaping mechanism as the root cause and submitted an initial patch. A reviewer suggested improving code clarity and robustness, leading the proposer to revise the patch to use an 'afterescape' flag and handle multibyte characters. A second reviewer confirmed the fix for both false-negative and false-positive cases. However, a core developer interjected, deeming the multibyte handling unnecessary and over-engineered for backend-safe encodings. Consequently, the core developer simplified and committed a patch that solely addressed the consecutive backslash issue, eliminating the extraneous multibyte logic.
49 Add per-backend AIO statistics Proposed 1 msgs Jul 7, 11:28
The proposer introduces a patch to enhance AIO (Asynchronous I/O) monitoring by adding per-backend statistics. Currently, AIO insights are limited to real-time handle usage or total I/O volume, lacking details on synchronous fallbacks, handle exhaustion, or how I/O completions are distributed across backends. The proposed patch introduces cumulative counters such as `started`, `executed_sync`, `executed_async`, `completed_self`, `completed_other`, `handle_waits`, and `submitted`. These statistics are designed to help users understand and tune AIO behavior by revealing metrics like the ratio of synchronous execution, instances of handle exhaustion, and patterns of cross-backend I/O completion. The data will be accessible through a new system function, `pg_stat_get_backend_aio()`, and integrates with existing backend statistics infrastructure with minimal performance overhead. The proposer suggests starting with a per-backend view before considering a global one.
50 [PATCH] Limit PL/Perl scalar copies to work_mem Rejected 2 msgs Jul 7, 07:29
This thread discusses a proposed patch to limit the size of PL/Perl scalar copies to prevent out-of-memory (OOM) killer invocations when large text values are returned. The proposer aimed to reject Perl strings larger than work_mem * 1024 bytes. However, a committer quickly rejected the patch, pointing out that memory overcommitment is a system-wide issue in PostgreSQL, work_mem is intended for memory-to-disk spilling rather than hard query failures, and PL/Perl can bypass such limits by allocating memory internally. The proposer acknowledged and accepted the feedback, concluding that the approach was inappropriate.