Re: [PATCH] Add target-column context for assignment coercion errors

Hi Midhush,

Thanks for the patch. I built it (base vs patched at 82d31451606) and
ran a differential review. One blocker; the rest holds up well.

Blocker -- memory corruption on any coercion, under --enable-cassert
--------------------------------------------------------------------
On a cassert build the patched tree writes past an allocated chunk on
essentially any cast (MEMORY_CONTEXT_CHECKING catches it). Base is
clean; the patch is the only difference:

SELECT 1::bigint;
WARNING: problem in alloc set MessageContext: detected write past
chunk end in block ... chunk ...

- Triggers on 1::bigint, 'ok'::varchar(2), 1.5::numeric(2,1),
'a'::char(1). Plain funcs/operators (abs, length, ||) do not, so it
is specific to the coercion FuncExprs.
- It fires at parse/plan with no execution (EXPLAIN (COSTS OFF) and
PREPARE both warn), and on valid values too, so it is independent of
the "value too long" path.
- The coercion path runs across most of the regression suite, so the
"all 239 tests pass" run was very likely without --enable-cassert;
with cassert the warnings are immediate.

My guess is the new FuncExpr fields or their handling on the parse/plan
coercion path, rather than the runtime callback. I stopped the design
pass here until that is root-caused; the rest of what I checked:

Checks out
----------
- CONTEXT lands exactly where a destination column is known:
varchar(n)/char(n)/numeric(p,s), UPDATE, DEFAULT, GENERATED STORED,
PREPARE/const-fold, INSERT ... SELECT.
- Renames resolve lazily to the new name (col_old -> col_new). Nice.
- Source errors are not misattributed: INSERT ... VALUES (1/0) still
reports "division by zero", with no column context.
- equal_ignore / query_jumble_ignore on the new fields are correct --
equal()/CSE and pg_stat_statements stay unaffected.

Your design questions
---------------------
- Growing FuncExpr for every node in the system to carry Oid +
AttrNumber, for a niche feature, is the main axis worth hashing out
on-list -- it is what stalled the 2015 threads. A dedicated node or
a side channel may be worth weighing against the per-node size.
- Annotated coercions always route through the fusage helper, even with
track_functions off. I measured the bulk path (2M INSERTs into a
varchar(4) column, -O2, 10 samples each): base 1861 ms vs patched
1827 ms median, -1.8% [95% CI -9.7 .. +4.0] -- within noise. No
measurable overhead, so the routing is fine on cost grounds.

LLVM
----
Built and installed --with-llvm (LLVM 23): it compiles, links, and
llvmjit.so loads, with JIT operational (a heavy query reports
Functions: 6). Under jit=on the coercion error still prints the
correct CONTEXT, and the corruption WARNING still fires too -- which
reconfirms the fault is at parse/plan, i.e. JIT-independent.

I could not get EXPLAIN to show the assignment-coercion projection
itself being JIT-compiled (INSERT/UPDATE ModifyTable target
projections were not JIT-compiled in the shapes I tried, which looks
like a PG JIT behaviour rather than the patch). So the
EEOP_FUNCEXPR_COERCION dispatch is confirmed by code review
(referenced_functions[] in llvmjit_types.c, dispatched in
llvmjit_expr.c to the same helper as the interpreter) plus the module
building and loading, rather than by a captured JIT-of-the-coercion
run.

Happy to re-check once the cassert issue is sorted.

Regards,
Manu