Hi Ayush, Thank you for the patch. I tried v2 (the cfbot branch cf/7295, CF entry 7295) and would like to share what I saw. I can send the scripts and the output if they are useful.
Setup - Master at c1694b7b962c (11 Nov 2026) plus the patch as one diff (3 files: prepjointree.c and the generated_virtual test files). `git apply --check` was clean on that master, so no rebase is needed at the moment. - configure with --enable-cassert --enable-debug --enable-tap-tests --without-icu, on Linux x86_64. I built the unpatched tree first, then the patched tree. Both builds had 0 compiler warnings. Tests that passed - `make check` with the patch: all 243 tests passed. - With only the prepjointree.c change reverted and the new test kept, generated_virtual fails (1 of 243 tests failed). The diff is exactly the new test: the expected `ERROR: cannot use system column "tableoid" in EXCLUDED` becomes `ERROR: variable not found in subplan target lists`. So the new test does exercise the fix. Manual tests (temporary cluster, one run each on the unpatched and the patched build) - Plain table, virtual column `c regclass GENERATED ALWAYS AS (tableoid) VIRTUAL`, INSERT ... ON CONFLICT DO UPDATE: - `SET b = (excluded.c::oid)::int`: unpatched gives "variable not found in subplan target lists"; patched gives the new tableoid error. - `WHERE excluded.c IS NOT NULL`: unpatched did not fail and the update ran (b became 9); patched gives the new error and the row stays unchanged. - Partitioned table with two partitions (tableoid is different per partition): `WHERE excluded.c IS NOT NULL` on the unpatched build ran without error and updated both rows (in pt1 and pt2); I did not check which tableoid value it used. `SET b = (excluded.c::oid)::int` gave "variable not found in subplan target lists". With the patch, both give the new tableoid error and no row is changed. - Expression that combines tableoid with other columns (`a * 100 + (tableoid::oid)::int % 7`): with `SET b = excluded.c::int` and with `WHERE excluded.c > 0`, unpatched gives "variable not found in subplan target lists" for both; patched gives the new error for both. - Cases that still work with the patch, and gave the same output before and after: - a stored generated column (`b * 10`) referenced as excluded.c; - a virtual column without tableoid (`a * 10`, the example from the first message of the thread), where excluded.c in SET still works on both builds; - excluded.b (an ordinary column) on the plain and the partitioned table; - reading the target table's own virtual column (`m1.c`) in SET; - reading the tableoid virtual column with a plain SELECT. - The server did not crash during these runs (assert-enabled build); the only ERROR lines in the server log were the ones listed above. I did not keep the full server logs. Small question and comment - The message is `cannot use system column "tableoid" in EXCLUDED`, which seems clear to me. When the user wrote `excluded.c` (the virtual column), the message does not name `c`, so a hint or the column name might help. This is only a suggestion, and I think the message is fine as it is. - I did not check the placement of the check in expand_virtual_generated_columns() against other options, and I do not have an opinion on the back-patch to PG 18. Not tested - Back-patching, and PG 18 or any other branch. - Other system columns (ctid, xmin, and so on) in a generation expression; only tableoid was tried. - Domain, inheritance (non-partition) and multi-level partition cases; INSERT ... ON CONFLICT with RETURNING; MERGE; views and rules; prepared statements and plan caching. - make check-world, the isolation tests and the other TAP suites (I ran only make check). - Meson build, non-assert builds, Windows, 32-bit, other compilers, ICU builds. - The wording of the error message beyond the points above, the code style (pgindent), and a full code review. Thanks again, and please let me know if you want me to rerun anything with different settings. Best regards, João Rito
