# User-visible defects in commit 62c3b4c (postgres_fdw ArrayCoerceExpr pushdown), still present in master

Audit of 62c3b4c "Support for deparsing of ArrayCoerceExpr node in contrib/postgres_fdw"
(Alexander Korotkov, 2025-07-18). Code is functionally unchanged in master (only comment-typo
fix 4c5159a since) and is in REL_19_STABLE, so these ship in PostgreSQL 19 unless fixed.

Method: 23-agent workflow — 12 independent finder lenses (23 raw findings) → root-cause
dedup (3 clusters) → per-cluster adversarial refuter + independent end-to-end tracer + live
reproduction on a server built from this tree (master @ a8c2547, PG 19beta1; loopback FDW
with a second database as the "remote", so local-only catalog objects genuinely don't exist
remotely; ground truth = forced-local evaluation of the identical predicate) → completeness
critic. **All three defects: refuter confirmed, tracer confirmed, live-reproduced.**

---

## 1. (high, wrong results) `foreign_expr_walker` never vets the element-coercion pathway (`elemexpr`)

**Anchor:** `contrib/postgres_fdw/deparse.c:706-733` (walker case recurses only into `e->arg`, line 713)

The per-element conversion — a `FuncExpr` of the `pg_cast` function, a `CoerceViaIO`, or a
`RelabelType` — lives in `e->elemexpr` and is never examined. Unlike `T_FuncExpr` (:566),
`T_OpExpr` (:614), and `T_ScalarArrayOpExpr` (:654), no `is_shippable()` check touches the
function that gives the cast its semantics; the `:1050` exit vets only the result *array type*.
The would-be backstop `contain_mutable_functions` (`is_foreign_expr`, :287) reaches `elemexpr`
via `expression_tree_walker` but checks only volatility, and IMMUTABLE is the conventional
declaration for cast functions. `deparseArrayCoerceExpr` (:3544-3556) then ships bare
`arg::resulttype` (or nothing for implicit format), so the remote re-resolves the element
coercion against *its* `pg_cast` catalog. Pre-commit, all of this was evaluated locally
(walker default case rejected the node).

Violates the documented contract (postgres-fdw.sgml ~1168-1173: only built-in /
listed-extension types, operators, functions are shipped) and is inconsistent with the
scalar form of the identical cast, which the `T_FuncExpr` shippability check still keeps local.

Live-reproduced (all observed on PG 19beta1):

```sql
-- Remote database "remotedb" (stock, only tables):
CREATE TABLE public.t_text (id int, c text[]);
INSERT INTO t_text VALUES (1,'{12345}'), (2,'{999}');
CREATE TABLE public.t_s (id int, c text);
INSERT INTO t_s VALUES (1,'00005'), (2,'5');

-- Local database:
CREATE EXTENSION postgres_fdw;
CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname 'remotedb');
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
CREATE FOREIGN TABLE ft_text (id int, c text[]) SERVER loopback OPTIONS (table_name 't_text');
CREATE FOREIGN TABLE ft_s    (id int, c text)   SERVER loopback OPTIONS (table_name 't_s');

-- (A) Explicit-cast form: silent wrong results
CREATE FUNCTION text2int(text) RETURNS int LANGUAGE plpgsql IMMUTABLE STRICT
  AS 'BEGIN RETURN length($1); END';
CREATE CAST (text AS integer) WITH FUNCTION text2int(text);

EXPLAIN (VERBOSE, COSTS OFF) SELECT id, c FROM ft_text WHERE c::int[] = ARRAY[5];
--   Remote SQL: SELECT id, c FROM public.t_text WHERE ((c::integer[] = '{5}'::integer[]))
SELECT id, c FROM ft_text WHERE c::int[] = ARRAY[5];
--   Observed: 0 rows.  Correct (pre-commit/local semantics, cf.
--   SELECT * FROM (SELECT * FROM ft_text OFFSET 0) s WHERE c::int[] = ARRAY[5]):
--   (1,'{12345}') because length('12345') = 5.
SELECT id, c FROM ft_text WHERE c::int[] = ARRAY[12345];
--   Observed: spurious row (1,'{12345}'); locally: 0 rows.

-- (B) New hard error on data the local cast handles:
--   on remote: INSERT INTO t_text VALUES (3,'{abc}');
SELECT id FROM ft_text WHERE c::int[] = ARRAY[5];
--   Observed: ERROR: invalid input syntax for type integer: "abc"
--   CONTEXT: remote SQL command: SELECT id FROM public.t_text WHERE ((c::integer[] = '{5}'::integer[]))
--   Locally-evaluated control returns id=1.

-- (C) Implicit-format branch (cast omitted entirely; see also defect 3):
CREATE FUNCTION int2padded(int) RETURNS text LANGUAGE sql IMMUTABLE STRICT
  AS $$ SELECT lpad(CAST($1 AS varchar), 5, '0') $$;
CREATE CAST (integer AS text) WITH FUNCTION int2padded(int) AS IMPLICIT;
SET plan_cache_mode = force_generic_plan;
PREPARE s(int[]) AS SELECT id FROM ft_s WHERE c = ANY($1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE s(ARRAY[5]);
--   Remote SQL: SELECT id FROM public.t_s WHERE ((c = ANY ($1::integer[])))
EXECUTE s(ARRAY[5]);
--   Observed: ERROR: operator does not exist: text = integer  (remote SQL command)
--   Correct/pre-commit: id=1 (verified via OFFSET 0 control).

-- Control demonstrating the inconsistency: the identical cast in scalar form stays local
EXPLAIN (VERBOSE, COSTS OFF) SELECT id FROM ft_text WHERE c[1]::int = 5;
--   Filter: ((ft_text.c[1])::integer = 5)   (local; correct results)
```

Because pushed-down quals also gate direct UPDATE/DELETE, the wrong-row selection extends to
wrong rows *modified*.

---

## 2. (high, wrong results) Builtin GUC-sensitive I/O coercions shipped via `elemexpr`'s `CoerceViaIO` — zero user-defined objects needed

**Anchor:** `contrib/postgres_fdw/deparse.c:706` (same unvetted-`elemexpr` hole, distinct fix implication)

postgres_fdw deliberately never ships scalar `T_CoerceViaIO` (absent from walker and
`deparseExpr` dispatch), so `f8col::text` stays a local Filter. But `f8arr::text[]` is an
ArrayCoerceExpr whose `elemexpr` is a `CoerceViaIO` (string-category I/O fallback,
parse_coerce.c:3275-3283) — and it's now judged shippable: `float8out`/`byteaout`/`textin`
are marked immutable in pg_proc.dat despite depending on `extra_float_digits` /
`bytea_output`. postgres_fdw itself forces `SET extra_float_digits = 3` on every remote
session (connection.c:840) and never syncs `bytea_output`, so local-vs-remote rendering
diverges deterministically.

Key fix implication: **an `is_shippable()` check on the `elemexpr` function OID would NOT
catch this** — the I/O functions are builtin. The walker must refuse non-RelabelType element
coercions (or the deparser must stop delegating them).

Live-reproduced:

```sql
-- Remote (stock): CREATE TABLE public.t1 (id int, f8 float8[], txt text[]);
--                 INSERT INTO t1 VALUES (1, ARRAY[(0.1::float8 + 0.2::float8)], ARRAY['0.3']);
-- Local: CREATE FOREIGN TABLE ft1 (id int, f8 float8[], txt text[]) SERVER loopback OPTIONS (table_name 't1');
SET extra_float_digits = 0;
SELECT id FROM ft1 WHERE f8::text[] = txt;
--   Observed on master: 0 rows.  Remote SQL: SELECT id FROM public.t1 WHERE ((f8::text[] = txt))
--   (remote session renders '{0.30000000000000004}' under forced extra_float_digits=3)
--   Correct (pre-commit local semantics, cf. WHERE (f8::text[] = txt OR random() < -1)): id=1.

-- bytea variant: remote ALTER DATABASE ... SET bytea_output='escape', local default 'hex':
--   WHERE barr::text[] = txt → 0 rows pushed down vs id=1 locally.

-- Control: scalar WHERE txt[1] = f8[1]::text stays a local Filter and is correct.
```

---

## 3. (medium, remote errors) `deparseArrayCoerceExpr` omits `COERCE_IMPLICIT_CAST` coercions entirely

**Anchor:** `contrib/postgres_fdw/deparse.c:3552`

Emitting no text for implicit-format coercions bets that the remote parser re-derives the
same coercion during operator resolution. That bet fails independently of any `elemexpr`
vetting fix:

- **(a)** local implicit pg_cast entry backed by a *builtin* function — every involved OID is
  shippable, yet the remote errors:

```sql
-- Remote (stock): CREATE TABLE public.t (c2 text, c3 int[]); INSERT INTO t VALUES ('3','{3}');
-- Local:
CREATE FOREIGN TABLE ft (c2 text, c3 int[]) SERVER loopback OPTIONS (table_name 't');
CREATE CAST (integer AS text) WITH FUNCTION pg_catalog.to_hex(integer) AS IMPLICIT;
SELECT count(*) FROM ft WHERE c2 = ANY (c3);
--   Remote SQL: SELECT count(*) FROM public.t WHERE ((c2 = ANY (c3)))   -- coercion vanished
--   Observed: ERROR: operator does not exist: text = integer
--   Correct/pre-commit (to_hex(3)='3'): count = 1.
-- Same via generic plan: PREPARE p(int[]) AS ... WHERE c2 = ANY($1)
--   → Remote SQL ((c2 = ANY ($1::integer[]))), identical error.
```

- **(b)** declared-type mismatch, *no user objects at all*: remote column is an enum, local
  foreign table declares `char(1)`; `PREPARE s(varchar[]) ... WHERE d = ANY($1)` ships
  `d = ANY ($1::character varying[])` and fails
  `operator does not exist: public.enum_of_int_like = character varying`, where pre-commit
  it returned 2 rows via a local Filter. This mode was demonstrated in-thread by the patch
  author and flagged by a reviewer pre-commit, then consciously accepted.

Rated medium: loud error rather than silent corruption, and the scalar implicit-cast FuncExpr
path has long had the same omission policy (this commit extends the exposure to array
coercions). Unlike `RelabelType`, an ArrayCoerceExpr is not pure relabeling, so the omission
drops real semantics; a remote can also bind a semantically different operator (e.g. texteq
vs bpchareq trailing-space handling) rather than erroring.

---

## Follow-up lead from the completeness critic (traced in code, not live-run)

**Array-over-domain casts push a hidden `CoerceToDomain` to the remote** (medium confidence).
When the target element type is a domain, `elemexpr` contains `CoerceToDomain` — the one
shape that evades even the volatility backstop: `check_functions_in_node` has no
`T_CoerceToDomain` case and domain CHECK expressions live in the catalog, not the node tree.
With the domain in a server-listed extension, `is_shippable(resulttype)` passes (implicit
array types are extension members, pg_type.c:628-635), so `carr::dom[]` ships while scalar
`c::dom` never does, and the *remote's* domain constraints (remote catalog state, remote
session GUCs) replace local enforcement — contradicting clauses.c:3428-3440's rationale for
not const-folding through CoerceToDomain. Traced failure modes: `unrecognized configuration
parameter` remote errors for GUC-reading constraints; silently weaker constraint enforcement
under local ALTER DOMAIN / extension version skew. Mitigating: requires the `extensions`
option, whose docs shift identical-behavior responsibility to the user.

## Checked and found clean / derivative

- **Collation logic** (deparse.c:719-731): exact parity with `T_RelabelType`; `elemexpr`
  cannot introduce a Var-derived collation (`CaseTestExpr` is built with InvalidOid collation,
  parse_coerce.c:947). Affirmatively clean.
- **ORDER BY / GROUP BY / aggregate pushdown, direct UPDATE/DELETE, EPQ rechecks**: traced;
  only derivative manifestations of defects 1-2 (wrong sort/grouping keys, wrong rows
  modified, nondeterministic row drops when a diverging pushed qual is re-evaluated locally
  in an EPQ recheck), not new defect classes.
- **Fix shape implied by the evidence**: restrict pushdown to element coercions that are pure
  relabelings (elemexpr is RelabelType/bare CaseTestExpr) — and if kept, always emit the cast
  rather than omitting implicit format; vetting only the elemexpr function OID is insufficient
  (defect 2), and omission is unsound even when everything is shippable (defect 3).

## Provenance

- Workflow `wf_16d96442-0cf`: 23 agents (cap 30), 12/12 finder lenses returned, 23 raw
  findings → 3 root causes, 0 dropped, 0 refuted. Each root cause: adversarial refuter
  **confirmed** + independent end-to-end tracer **confirmed** + empirical reproducer
  **reproduced=yes**. Archives lens independently matched defect 3(b) to the pre-commit
  review thread.
- Empirical setup: reused `tmp_install` of this tree (PG 19beta1, master a8c2547); fresh
  cluster in session scratchpad, unix-socket only; "remote" = second database over a loopback
  postgres_fdw server. Servers stopped afterward; socket dirs removed; data dirs left in
  scratchpad; git tree untouched (`tmp_install/` is gitignored).
- Full machine-readable results: `/tmp/claude-1000/-home-nm-src-pg-postgresql/afe6d0ac-a2cb-4fa6-9526-676825c85665/tasks/wlbjtr33p.output`