Incremental Analysis: Review Discussion on Backpatching and Test Placement
Summary of New Activity
Fujii Masao reviewed the patch set and provided feedback on two key areas: (1) test placement for the dblink FDW-level validation fix, and (2) backpatching strategy for v18. Matheus Alcantara responded with a revised patch version addressing the test feedback.
Key Technical Discussions
Test Placement (Patch 0003)
Fujii argued that adding a TAP test for the ALTER FOREIGN DATA WRAPPER ... (use_scram_passthrough ...) validation is overkill, since neither dblink nor postgres_fdw currently has tests verifying option-level restrictions. He suggested that if such a test is added, it belongs in sql/dblink.sql (a standard SQL regression test) rather than as a TAP test. Matheus agreed and moved the test to sql/dblink.sql in a new patch version. An open question remains: whether a similar test should be added for postgres_fdw.
Backpatching to v18
Fujii raised the backpatching question explicitly: since v18 shipped with the current (buggy) behavior and no documentation stating user-mapping should take precedence, changing the behavior in a minor release could affect users relying on the current semantics. Despite this concern, Fujii expressed a preference for backpatching to v18, reasoning that having only v18 behave differently from later versions "seems odd and potentially confusing."
Matheus supported backpatching by citing existing precedent in the postgres_fdw documentation for sslkey and sslcert, which explicitly states: "If both are present, the user mapping setting overrides the connection setting." This establishes an existing documented pattern that use_scram_passthrough should follow.
Patch 0003 Backpatching
Both participants agreed that the FDW-level validation fix (patch 0003) is safe to backpatch to v18 regardless, since use_scram_passthrough set at the FDW level never had any effect — so even if users set it there, it wasn't working, making the restriction a clarification rather than a behavior change.