New Round: Zsolt Parragi's Crash/Correctness Report
Zsolt Parragi (Percona) contributes the first external reproduction testing of the v20 patch and surfaces four concrete bugs, two of which are outright server crashes. This moves the review from code-reading critique (Jim Jones) to runtime validation, and the findings are significantly more damaging than the stylistic/correctness issues flagged previously.
1. NULL-pointer crash on partitioned targets
A COPY into a range-partitioned root with a pre-existing conflicting row segfaults. This directly confirms the architectural question raised in the prior analysis ("Partitioned targets: what OID goes in copy_tbl?") — the patch doesn't just have an under-specified semantic for partitioning, it has no working code path at all. The likely cause is that the conflict-routing logic derefs resultRelInfo state that only the root-level ResultRelInfo carries, while tuple routing has swapped in a per-leaf ResultRelInfo that lacks the conflict-table wiring. Fixing this requires propagating the conflict-table ResultRelInfo through ExecFindPartition / ExecInitRoutingInfo, analogous to how ON CONFLICT DO NOTHING is threaded per-partition.
2. NULL-pointer crash under REPEATABLE READ
A single-row duplicate inside a REPEATABLE READ transaction crashes. This is consistent with the speculative-insertion machinery assuming READ COMMITTED semantics: under RR, the EvalPlanQual / HeapTupleSelfUpdated paths that ON CONFLICT relies on behave differently, and INSERT ... ON CONFLICT itself historically had to grow explicit handling for serializable/RR isolation. The COPY path apparently omits whatever ExecOnConflictUpdate does to cope with a visible-to-snapshot conflicting tuple under RR, and falls off the end with a NULL TupleTableSlot or similar.
3. Silent data loss with BEFORE ROW triggers on a no-index table
The most alarming report: COPY reports "3 copied" but SELECT count(*) returns 0 on a table with no indexes at all and a no-op BEFORE INSERT trigger. Since there are no unique constraints, no conflict path should ever engage — yet tuples vanish. This strongly suggests the patch's control flow incorrectly routes rows into the "skip main insert" branch whenever some condition tied to trigger presence or ExecOpenIndices(..., true) is met, independent of whether a conflict was actually detected. It may be directly caused by the unconditional speculative-index opening that Jim Jones flagged in the prior round — speculative insertion expects a finalization step (table_tuple_complete_speculative) that is apparently being skipped, leaving tuples in a never-confirmed state that gets pruned. If so, Jim's fix (gate ExecOpenIndices's second arg on on_conflict != ONCONFLICT_NONE) is not just a hygiene fix but load-bearing for data integrity.
4. COPY TO silently accepts the options
Zsolt independently rediscovers exactly the defect Jim Jones identified last round: COPY t TO ... (ON_CONFLICT TABLE, CONFLICT_TABLE ...) succeeds instead of raising. Two independent reviewers hitting the same bug confirms it's a first-impression failure that will continue to embarrass the patch until fixed.
Significance
- The patch has now been demonstrated to crash the backend in two common configurations (partitioned tables, RR isolation) and to silently lose data in a third. Any one of these would normally bounce a patch out of a commitfest; together they indicate the v20 code has not been exercised beyond the simplest single-table READ COMMITTED happy path.
- The data-loss scenario (no indexes, so no conflict possible) is the most important finding because it points at a structural flaw — the conflict-routing branch is engaging when it shouldn't — rather than a gap in coverage. It also validates the prior analysis's concern that unconditional speculative-insertion setup is dangerous.
- No response from Jian Universality (the patch author) yet in this round.
- Still no committer has engaged with the thread.