Repository: dathere/qsv (language: rust)

Issue:
BUG extsort CSV MODE issues
---
**Describe the bug**
I want to sort a CSV file with **extsort** in CSV MODE but sometimes get either a message
"`io error: invalid record index 18446744073709551615 (there are 16 records`)" or the file is sorted wrongly.

**To Reproduce**
Steps to reproduce the behavior:
Input file `test_ids.csv`:
```
pnm,tc_id,pc_id
405,139280,9730000630075
405,139281,9730000630075
131,139282862,9730065908379
138,139282863,9730065908379
138,139282864,9730065908379
405,139282865,9730065908379
138,139282866,9730065908379
138,139282867,9730065908379
138,139282868,9730065908379
138,139282869,9730065908379
138,139282870,9730065908379
138,139282871,9730065908379
252,139282,9730000630075
241,139283,9730000630075
272,139284,9730000630075
273,139285,9730000630075
```

_Commands:_
```
qsv index test_ids.csv
qsv extsort --select tc_id test_ids.csv sorted.csv
```

=> `io error: invalid record index 18446744073709551615 (there are 16 records)`

**Expected behavior**
No error.

**Desktop (please complete the following information):**
 - OS: Windows 11 64bit
 - qsv Version : qsv 1.0.0-mimalloc-apply;fetch;foreach;geocode;Luau 0.653;prompt;to;polars-0.44.2-31b7bb9;self_update-8-8;4.75 GiB-1.74   GiB-1.44 GiB-5.94 GiB (x86_64-pc-windows-msvc compiled with Rust 1.83) prebuilt

**Additional context**
In other cases with big files the extsort command works, but 
`qsv dedup --select tc_id --sorted sorted.csv | qsv select tc_id -o out.csv`
shows an error:
`Aborting! Input not sorted! ByteRecord(["138" ... is greater than ByteRecord([" ...`
=> extsort seems to sort wrongly in these cases.



Discussion / hints:
Happy New Year and thanks for the detailed report @datatraveller1 .

However, I can't seem to reproduce your issue given the commands above:

```
qsv extsort --select tc_id test_ids.csv sorted.csv
qsv table sorted.csv                              
pnm  tc_id      pc_id
405  139280     9730000630075
405  139281     9730000630075
131  139282862  9730065908379
138  139282863  9730065908379
138  139282864  9730065908379
405  139282865  9730065908379
138  139282866  9730065908379
138  139282867  9730065908379
138  139282868  9730065908379
138  139282869  9730065908379
138  139282870  9730065908379
138  139282871  9730065908379
252  139282     9730000630075
241  139283     9730000630075
272  139284     9730000630075
273  139285     9730000630075
```

As to the error in your Additional context:

```
qsv dedup --select tc_id --sorted sorted.csv | qsv select tc_id -o out.csv
Aborting! Input not sorted! ByteRecord(["138", "139282871", "9730065908379"]) is greater than ByteRecord(["252", "139282", "9730000630075"])
```

That Is because you sorted on the second column `tc_id` and `pnm` - the first column is not sorted:
```
qsv sort check sorted.csv
not sorted
```
Hi @jqnatividad Thank you very much and a happy new year, too!
I think my additional context was a bit misleading, because it doesn't apply to the example (I'm sorry about that).

However, don't you get the io error: invalid record index message?
I get it if I copy the content above (Input file test_ids.csv) into test_ids.csv and call:
```
C:\test\qsv>test.bat

C:\test\qsv>qsv index test_ids.csv

C:\test\qsv>qsv extsort --select tc_id test_ids.csv sorted.csv
io error: invalid record index 18446744073709551615 (there are 16 records)

C:\test\qsv>
```
If you don't get the error, maybe it is a MS Windows issue with the `qsv index` command?
Hi @jqnatividad, I have now found out:
The `qsv index` error happens with  test_ids.csv if `EOL` is `CRLF`, with `LF` it works correctly.
Can you please adjust the index command so it also works for CRLF (which is default on MS Windows)?
Hi @datatraveller1 ,
Can you send me a sample file I can test with? I don't have ready access to Windows.

Also, I'd be interested to know what generates the CSV that's causing `qsv index` to fail... What does `qsv validate` report?

The csv crate is supposed to handle this transparently - https://docs.rs/csv/latest/csv/enum.Terminator.html
Hi @jqnatividad,

I have attached the file with "Attach files":
[test_ids.csv](https://github.com/user-attachments/files/18303446/test_ids.csv)

If this doesn't work, I think you can also simply use `sed` to replace the LF with CRLF:
`sed 's/$/\r/' filename > newfile`

`qsv validate test_ids.csv`
shows Valid: 3 Columns: ("pnm", "tc_id", "pc_id"); Records: 16; Delimiter: ,

I'm not sure about where what fails.
With `qsv index` no error is thrown, so maybe `qsv extsort` could evaluate the index wrongly.

All that said, now I use for what I wanted to achieve (without the need of index and extsort):
`qsv dedup --select tc_id test_ids.csv | qsv select tc_id -o out.csv`
This command works properly even for big files.
Thanks for the sample file @datatraveller1 .

I can now reproduce it and confirm its an underflow bug.  The large number should have tipped me off...

Fix patch:
diff --git a/src/cmd/extsort.rs b/src/cmd/extsort.rs
index 829889788..7f508585f 100644
--- a/src/cmd/extsort.rs
+++ b/src/cmd/extsort.rs
@@ -237,7 +237,7 @@ fn sort_csv(
             return fail!("Failed to retrieve position: invalid integer");
         };
 
-        idxfile.seek(position - position_delta)?;
+        idxfile.seek(position.saturating_sub(position_delta))?;
         idxfile.read_byte_record(&mut record_wrk)?;
         sorted_csv_wtr.write_byte_record(&record_wrk)?;
     }


Test patch:
diff --git a/resources/test/issue2391-test_ids.csv b/resources/test/issue2391-test_ids.csv
new file mode 100644
index 000000000..9d3e60dce
--- /dev/null
+++ b/resources/test/issue2391-test_ids.csv
@@ -0,0 +1,17 @@
+pnm,tc_id,pc_id
+405,139280,9730000630075
+405,139281,9730000630075
+131,139282862,9730065908379
+138,139282863,9730065908379
+138,139282864,9730065908379
+405,139282865,9730065908379
+138,139282866,9730065908379
+138,139282867,9730065908379
+138,139282868,9730065908379
+138,139282869,9730065908379
+138,139282870,9730065908379
+138,139282871,9730065908379
+252,139282,9730000630075
+241,139283,9730000630075
+272,139284,9730000630075
+273,139285,9730000630075
diff --git a/tests/test_extsort.rs b/tests/test_extsort.rs
index d91bb3788..0f7236e75 100644
--- a/tests/test_extsort.rs
+++ b/tests/test_extsort.rs
@@ -53,3 +53,68 @@ fn extsort_csvmode() {
 
     assert_eq!(dos2unix(&sorted_output), dos2unix(&expected_csv));
 }
+
+#[test]
+fn extsort_issue_2391() {
+    let wrk = Workdir::new("extsort_issue_2391").flexible(true);
+    wrk.clear_contents().unwrap();
+
+    let unsorted_csv = wrk.load_test_resource("issue2391-test_ids.csv");
+    wrk.create_from_string("issue2391-test_ids.csv", &unsorted_csv);
+    // create index
+    let mut cmd_wrk = wrk.command("index");
+    cmd_wrk.arg("issue2391-test_ids.csv");
+
+    wrk.assert_success(&mut cmd_wrk);
+
+    // as git mangles line endings, we need to convert manually to CRLF as per issue 2391
+    // see https://github.com/dathere/qsv/issues/2391
+    // convert LF to CRLF in test file to ensure consistent line endings
+    #[cfg(target_os = "windows")]
+    {
+        let mut cmd = wrk.command("cmd");
+        cmd.args([
+            "/C",
+            "type issue2391-test_ids.csv > issue2391-test_ids.tmp.csv && move /Y \
+             issue2391-test_ids.tmp.csv issue2391-test_ids.csv",
+        ]);
+        wrk.output(&mut cmd);
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        let mut cmd = wrk.command("sh");
+        cmd.args([
+            "-c",
+            "sed 's/$/\r/' issue2391-test_ids.csv > issue2391-test_ids.tmp.csv && mv \
+             issue2391-test_ids.tmp.csv issue2391-test_ids.csv",
+        ]);
+        wrk.output(&mut cmd);
+    }
+
+    let mut cmd = wrk.command("extsort");
+    cmd.arg("issue2391-test_ids.csv")
+        .args(["--select", "tc_id,pnm,pc_id"]);
+
+    wrk.assert_success(&mut cmd);
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    let expected = vec![
+        svec!["pnm", "tc_id", "pc_id"],
+        svec!["405", "139280", "9730000630075"],
+        svec!["405", "139281", "9730000630075"],
+        svec!["252", "139282", "9730000630075"],
+        svec!["131", "139282862", "9730065908379"],
+        svec!["138", "139282863", "9730065908379"],
+        svec!["138", "139282864", "9730065908379"],
+        svec!["405", "139282865", "9730065908379"],
+        svec!["138", "139282866", "9730065908379"],
+        svec!["138", "139282867", "9730065908379"],
+        svec!["138", "139282868", "9730065908379"],
+        svec!["138", "139282869", "9730065908379"],
+        svec!["138", "139282870", "9730065908379"],
+        svec!["138", "139282871", "9730065908379"],
+        svec!["241", "139283", "9730000630075"],
+        svec!["272", "139284", "9730000630075"],
+        svec!["273", "139285", "9730000630075"],
+    ];
+    assert_eq!(got, expected);
+}