sort: apply -n to the key, not the whole line - #14331
Open
Socialpranker wants to merge 1 commit into
Open
Conversation
`-n` parses a number from every line up front and compares those numbers
before any key is looked at. That shortcut is only the same comparison
when the key spans the whole line, but nothing checked that:
printf "19\\n21\\n" | sort -n -k1.2
sorts on "9" and "1", so GNU prints 21 first; uutils compared 19 against
21 and printed 19. The same went for a key picked out by field
(`-n -t. -k2`), and for a key with its own `r`, which the shortcut could
not honor because it only knows the global ordering.
Guard the shortcut with `can_use_whole_line_numeric`, in the shape the
neighboring `can_use_fast_lexicographic` already uses: a single key,
numeric, not reversed, starting at the first character of the first
field and running to the end of the line. Plain `-n` and `-n -k1` still
take it.
|
GNU testsuite comparison: |
Contributor
it is old, please try with master binaries |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-k1.2selects from the second character on, so the keys are9and1and21 sorts first. uutils compared 19 against 21 instead.
-nparses a number out of every line once, up front, andcompare_bycomparesthose numbers before it reaches the key loop. That is the same comparison only
when the key happens to span the whole line, and nothing checked that it did.
--debugshows the key was selected correctly all along -- it was simply notwhat got compared:
Two more shapes of the same defect:
The last one cannot work through the shortcut at all: it compares with the
global ordering, and a key's own
rnever reaches it.It also broke the last-resort comparison. When every key is equal, GNU falls
back to comparing the lines byte by byte:
The fix
can_use_whole_line_numericguards the shortcut, in the shape the neighboringcan_use_fast_lexicographicandcan_use_fast_ascii_insensitivealready use:one key, numeric, not reversed, starting at the first character of the first
field and running to the end of the line. Plain
-nand-n -k1still take it,so the optimization is kept where it is correct --
sort -nover 300k shuffledintegers is unchanged (0.051s patched vs 0.079s before, same ballpark, the
shortcut is still in play).
sort -n -k1.2goes the long way now and costsabout 18% more, which is the price of the right answer.
-h,-g,-Mand-Vnever had a whole-line shortcut, which is why theywere already correct here.
How the GNU behavior was established
By running the installed GNU binary as a black box and recording its output --
every transcript above is from
debian:stable-slimwith GNU coreutils 9.7. Idid not read GNU coreutils source.
Testing
cargo test --features sort --no-default-features -- test_sort: 188 passed,0 failed (187 pre-existing, 1 new). No existing test needed a change.
test_numeric_sort_uses_the_key_not_the_whole_linecovers the characteroffset, the field, the key-local
r, the last-resort fallback, and the twowhole-line cases that must keep working.
sort.rsandchunks.rsalone (test fileuntouched) makes the new test fail.
cargo fmt --all --checkandcargo clippy -p uu_sort --all-targets: clean.leading blanks, negatives,
nan/inf, comma- and dot-separated fields,non-numeric lines) x 21 flag combinations: 33 cases moved from differing to
byte-identical, 0 regressions.
Two cases still differ, identically before and after this change, so they are
pre-existing and unrelated:
sort -n -ctruncates the offending line in itsdiagnostic (
1.10reported as1.1,nanasNaN).Also noticed while sweeping, and left alone as a separate question: GNU treats a
key's
bas an ordering option, sosort -n -k1bsorts lexicographically,while uutils applies the global
-nto it. That is about which modifiers countas ordering options, not about which text gets compared.
Disclosure
Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI policy
in CONTRIBUTING.md. Every GNU behavior quoted above came from running the
installed binary, not from reading GPL source. All testing was run locally.