Fix SPLIT PARTITION bound-overlap bug and other improvements

Incremental Update: Patch 0004 Pushed, Reverted Due to Buildfarm Failures, Then Re-pushed with Different Test Strategy

Key Events

1. Patch 0004 (reject degenerate no-op split) was pushed by Korotkov on May 20, confirming v5 was the final version after Chao's approval.

2. Immediate revert due to buildfarm failures: The regression tests included ICU locale-dependent test cases (using non-deterministic collations like und-u-ks-level2 to test the collation-aware LIST bound comparison). Most buildfarm animals lack the required ICU locale support, causing widespread test failures.

3. New test strategy (v6): Korotkov replaced the locale-dependent test approach with a clever locale-independent technique: using -0.0 and 0.0 as the test values. In IEEE 754 floating point, these are bitwise different but logically equivalent (they compare as equal). This exercises the same code path — detecting whether bounds that are semantically identical but represented differently are correctly identified as a degenerate no-op split — without requiring any specific locale/collation support.

4. CI validation passed: Chao submitted v6 to the commitfest and confirmed the CI run succeeded, indicating the buildfarm should now be happy with the float-based test approach.

Technical Significance

The -0.0 vs 0.0 trick is an elegant solution to a real testing problem in PostgreSQL: how to test "values that are logically equal but physically different" without depending on optional locale infrastructure. This is the same semantic property that non-deterministic collations provide for strings, but floating-point signed zeros provide it portably across all platforms.

Lesson Learned

The thread reveals a practical pitfall for contributors: the existence of ICU tests in collate.icu.utf8.sql does NOT mean ICU-dependent tests will pass across the full buildfarm. Those specific test files are conditionally executed only on systems with ICU support. Regular regression tests (like those for partition splitting) must work on all buildfarm configurations.

Final Status

Patch 0004 is ready to be re-pushed with the v6 test approach. All four patches in the series are now either committed (0001-0003) or validated and awaiting final push (0004).

Incremental Update: Patches 0001-0003 Committed, Collation Bug Found in 0004

Major Development: Patches Committed

Alexander Korotkov pushed patches 0001 through 0003 (the core bound-overlap bug fix, the HINT message fix, and the documentation improvement). These are now in the tree. Only patch 0004 (reject degenerate no-op split) remains under discussion.

Critical Technical Finding: Collation-Sensitive Bound Comparison in LIST Path

Chao identified a subtle correctness issue in Korotkov's v3 implementation of check_split_partition_not_same_bound() for LIST partitions:

The Problem: The RANGE path in the function honors collation when comparing bounds (using the appropriate comparison functions), but the LIST path did not. Chao constructed a concrete counterexample using a case-insensitive ICU collation (und-u-ks-level2):

• Split partition bound: ('a', 'b')
• New partition bound: ('a', 'A')

Under the case-insensitive collation, 'b' and 'A' are NOT equal, so these are genuinely different bounds — the split is not degenerate. However, a naive length-based comparison (both lists have 2 elements) would incorrectly flag this as a no-op split.

The Fix (v4): Since check_partition_bounds_for_split_list() already validates that the new partition's bound is contained within the split partition's bound, detecting the degenerate case requires checking the reverse direction — whether the split partition's bound is also contained in the new partition's bound. If containment is bidirectional, the bounds are semantically identical (accounting for collation), and the split is a no-op.

Additional Insight: Chao noted that the original approach from v2 (using partition_bounds_equal() with datumIsEqual()) would have also failed to handle non-deterministic collations correctly, since datumIsEqual() does bitwise comparison. So the collation issue existed in all prior versions — Korotkov's v3 refactoring just made it more visible by separating the LIST and RANGE paths.

Current Status

Korotkov produced a v5 revision with an improved function header comment and claims to have added regression tests, but Chao reports that no test changes are visible between v4 and v5 — suggesting Korotkov may have forgotten to save/include his test additions. Awaiting a corrected v5 or v6.

Incremental Update: Alexander Korotkov Takes Over Patch Revision, Signals Intent to Commit

Key Development

Alexander Korotkov (Supabase) has reviewed and revised the 4-patch series, making substantive changes to patches 0002 and 0004, and signaled intent to commit by asking "Any objections if I commit this?"

Technical Changes in Korotkov's Revision

Patch 0002 (HINT message fix): Beyond fixing the DEFAULT-case hint that Chao identified, Korotkov also fixed grammar in hint messages across other code branches in the same function. This broadens the scope of the hint message improvements.

Patch 0004 (Reject degenerate no-op split): Korotkov made two architectural simplifications:

1. Replaced heavyweight bound comparison with lightweight check: Chao's original implementation used partition_bounds_create() and partition_bounds_equal() in check_split_partition_not_same_bound() to detect the degenerate case. Korotkov considered this too heavyweight — these functions build full partition bound data structures from scratch, which is unnecessary at that late stage of validation where sufficient information is already available. He replaced this with a more direct/lightweight comparison.

2. Removed memory context wrapping: Chao's implementation wrapped allocations in a dedicated memory context. Korotkov removed this, reasoning that the allocations are small, non-repetitive, and only live during the DDL operation. He notes that other similar code paths in PostgreSQL don't bother with memory context isolation for this pattern of allocation.

Process Note

Korotkov apparently forgot to attach the revised patches to his email, which Chao flagged. The actual patch contents are not yet available for review.

Additional Technical Observation from Chao

Chao pointed out that an existing code comment in the source explicitly acknowledges that with a DEFAULT partition, the new partition's lower bound may be greater than the split partition's lower bound — which further confirms that the original "exactly match" hint message was incorrect for the DEFAULT case. This strengthens the justification for patch 0002.