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
