Technical Analysis: Column Name Escaping in postgres_fdw Statistics Import

Core Problem

Commit 28972b6fc3d introduced a statistics import feature in postgres_fdw that allows a foreign table's ANALYZE to fetch pre-computed statistics from the remote side (via pg_stats) instead of sampling rows over the wire. This is a significant performance win for large foreign tables and is controlled by the analyze_sampling/restore_stats foreign table options.

The implementation constructs a remote SQL query that filters pg_stats rows by column name. The offending construction looks like:

AND attname = ANY('{col1, col2, ...}'::text[])

The bug is a classic quoting-level confusion: the column-name list is built by applying quote_identifier() to each name and then embedding the whole thing inside a single-quoted string literal that will be parsed as a text[] array literal. quote_identifier() only handles the identifier quoting level (doubling embedded double quotes, wrapping in "..." when needed). It does nothing about single quotes, which are the delimiter of the enclosing string literal.

Consequently, any column name containing a literal single quote (e.g. "col'quote") terminates the outer string literal prematurely on the remote server, producing a syntax error during ANALYZE. The reproducer in the thread demonstrates this cleanly: a foreign table with a column named col'quote causes ANALYZE ft to fail with syntax error at or near "quote" inside the remote SQL.

Architecturally this matters because:

1. It silently breaks ANALYZE on any foreign table whose column names contain single quotes — a legal PostgreSQL identifier character when double-quoted.
2. Stats import is a new-in-v18 feature, so this is an open item for v19 (the thread explicitly mentions adding it to the v19 open items list), meaning it needs to be resolved before release to avoid shipping a broken feature.
3. It reflects a general class of bug in deparse code where the author conflated two different escaping contexts (identifier vs. string literal).

Proposed Fix

The patch abandons the '{...}'::text[] literal form in favor of an ARRAY[...]::text[] constructor:

AND attname = ANY(ARRAY['col1','col''quote',...]::text[])

Each element is emitted via deparseStringLiteral(), which correctly doubles embedded single quotes and handles the E'' prefix when backslashes are present. This is the same escaping primitive already used elsewhere in the same function for schemaname and tablename, so the fix also improves internal consistency — the function was already doing the right thing for the table identifier but the wrong thing for column names.

The semantic distinction the patch enforces is important: on the remote side, pg_stats.attname is compared as a string value, not parsed as an identifier. Therefore string-literal escaping (deparseStringLiteral) is the correct tool; quote_identifier was a category error.

Relationship to Prior Thread

Fujita points out this is essentially the same root cause as a prior thread ([1] — "Fix array-element quoting in postgres_fdw import statistics"). That earlier patch apparently addressed array-element quoting issues but, per Ayush, did not actually resolve the single-quote-in-column-name case. Fujita's guidance is procedural: keep discussion of the same root cause in a single thread rather than forking, though he does not ask Ayush to abandon this thread outright.

Test Coverage Discussion

A minor review point from Alex Guo suggested dropping VERBOSE from ANALYZE VERBOSE simport_ft_quote. Fujita overrides this: VERBOSE output is the only observable signal that stats import actually occurred (vs. silently falling back to row sampling). Without it the test would pass even if the code path were broken in a different way. This is a small but instructive point — committer-level review here recognizes that regression tests for this feature must assert the mechanism, not just the absence of errors.

Status at End of Thread

After Fujita's procedural comments, Ayush registered the patch in the commitfest (entry 6695) and, after ~12 days of silence, pinged asking whether further edits were needed or whether the item should be added to the v19 open items list. The thread shows no committer response to that ping within the captured messages. Given the simplicity of the fix, the clear root cause, and Fujita's engagement, this looks like a straightforward commit candidate — the main open question is purely organizational (which thread to commit under).

Key Technical Takeaways

• Deparse code must track escaping context explicitly. quote_identifier and deparseStringLiteral are not interchangeable; they operate at different lexical levels.
• The ARRAY[...] constructor is preferable to '{...}'::text[] literals when elements are untrusted strings, because it shifts each element into its own string-literal context with well-understood escaping rules, rather than nesting array-literal syntax (with its own "/,/\ rules) inside a SQL string literal (with its own ' rules) — a doubly-nested escaping problem that is easy to get wrong.
• Feature introduction commits deserve scrutiny of their deparse paths for all character classes of identifiers, especially column and relation names which users can create with arbitrary Unicode and punctuation.