join: report the field number the user gave - #13995
Conversation
Two ways the 'incompatible join fields' message diverged from its input. parse_field_number clamps an out-of-range field to usize::MAX, and get_field_number rendered it one-based with k + 1: an abort under overflow-checks, a wrap to 0 otherwise. Saturate instead. Separately, translate! stringifies a value and re-parses it as i64 or f64 before handing it to Fluent, whose number type is f64-backed. Every integer above 2^53 was silently rounded. Route those through as exact decimal strings, keeping the number path for values f64 holds exactly so plural rules and number formatting are unaffected. Fixes uutils#13376
|
GNU testsuite comparison: |
Merging this PR will improve performance by 13.22%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | shuf_lines[100000] |
72.1 ms | 54.7 ms | +31.81% |
| ⚡ | Simulation | shuf_repeat_sampling[50000] |
14 ms | 12.3 ms | +14.2% |
| ⚡ | Simulation | expand_many_lines[100000] |
116.4 ms | 107.3 ms | +8.55% |
| ⚡ | Simulation | expand_custom_tabstops[50000] |
31.5 ms | 29.2 ms | +7.93% |
| ⚡ | Simulation | cksum_crc32b |
42.4 ms | 40.2 ms | +5.49% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing MsfPablo:join-exact-field-numbers (bfd55ff) with main (b2a617e)2
Footnotes
-
171 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(e275cd5) during the generation of this report, so b2a617e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
parse_field_number uses usize (matching GNU join's size_t), so a field value at or above usize::MAX saturates to it. The previous test hard-coded 64-bit saturation ceilings (u64::MAX and 2^53+1), which failed on 32-bit targets (i686, i686-musl, i686-windows) where usize::MAX is 4294967295. Build the expected text from usize::MAX and gate the sub-usize::MAX above-f64-precision case on target_pointer_width.
1 participant
Fixes #13376.
Both halves of the issue come down to the reported field number not matching the input.
The
k + 1overflow.parse_field_numberdeliberately clamps an out-of-range field tousize::MAX, andget_field_numberthen renders it one-based withk + 1. That aborts under overflow-checks and wraps to0in a normal build. Now saturates.The rounding. This one is not join-specific, so I fixed it where it lives.
translate!stringifies a value, re-parses it asi64orf64, and sets it on Fluent as a number — butFluentNumberis f64-backed, so anything past 2^53 rounds. Note that taking thei64branch is not enough to stay exact;-j 9007199254740993parses fine asi64and still comes out as…992. So the check is whether f64 can hold the value exactly, not whetheri64can:That also fixes the
csplitcase you mentioned:All the values from the issue now match GNU, including
18446744073709551615for the clamped case.Tests: unit tests in
locale.rsfor the representability boundary, and an integration test intest_join.rscovering the four field values above.Since this touches
uucore, I ran the suites for csplit, nl, split, head, expr, seq, tail, wc, fold and sort as well as join — 982 tests, all passing.