Repository: dathere/qsv (language: rust)

Issue:
Allow `--key <column name(s)>` in addition to `--key <column index(es)>` for `qsv diff`
**Is your feature request related to a problem? Please describe.**
It is sometimes easier and usually less error-prone to identify a column by its name (entry in header row) instead of its integer index among the columns. So it would be great to allow specification of the key column(s) by name instead of just by index when running `qsv diff`. This would be particularly helpful if the location of the key columns may be different between the two files (there is currently no way to match records using key columns that have the same name but different indexes).

**Describe the solution you'd like**
Allow `qsv diff --key <column name(s)>`, e.g., `qsv diff --key type,age` or `qsv diff --key "sales rep","sales year"` when both tables have header rows. Then use the specified columns for matching when comparing rows.

**Describe alternatives you've considered**

`--key-name <column name(s)>` would be possible, but it doesn't seem necessary to add a different flag for this

Currently I just make do by counting the index of the columns and making sure it is the same in both tables.

**Additional context**
(none)


Discussion / hints:
I agree that it'd be convenient to support column names with the `--key` option and can certainly add it.

However, the `--key` column names still need to be in the same place for both left and right CSVs, as that is a requirement of the underlying library - `csv-diff`, that we're using.

That is, if we add column name support, both CSVs will have to have the selected names in the same index locations.

Perhaps, use `qsv select` first to reorder the columns before `diff`ing?


Fix patch:
diff --git a/src/cmd/diff.rs b/src/cmd/diff.rs
index 20c535d9d..fce8d09fa 100644
--- a/src/cmd/diff.rs
+++ b/src/cmd/diff.rs
@@ -62,15 +62,25 @@ diff options:
     --delimiter-output <arg>    The field delimiter for writing the CSV diff result.
                                 Must be a single character. (default: ,)
     -k, --key <arg...>          The column indices that uniquely identify a record
-                                as a comma separated list of indices, e.g. 0,1,2.
+                                as a comma separated list of indices, e.g. 0,1,2
+                                or column names, e.g. name,age.
+                                Note that when selecting columns by name, only the 
+                                left CSV's headers are used to match the column names
+                                and it is assumed that the right CSV has the same
+                                selected column names in the same order as the left CSV.
                                 (default: 0)
     --sort-columns <arg...>     The column indices by which the diff result should be
-                                sorted as a comma separated list of indices, e.g. 0,1,2.
+                                sorted as a comma separated list of indices, e.g. 0,1,2
+                                or column names, e.g. name,age.
                                 Records in the diff result that are marked as "modified"
                                 ("delete" and "add" records that have the same key,
                                 but have different content) will always be kept together
                                 in the sorted diff result and so won't be sorted
                                 independently from each other.
+                                Note that when selecting columns by name, only the 
+                                left CSV's headers are used to match the column names
+                                and it is assumed that the right CSV has the same
+                                selected column names in the same order as the left CSV.
     -j, --jobs <arg>            The number of jobs to run in parallel.
                                 When not set, the number of jobs is set to the number
                                 of CPUs detected.
@@ -128,30 +138,99 @@ pub fn run(argv: &[&str]) -> CliResult<()> {
         );
     }
 
-    let primary_key_cols = match args.flag_key {
+    let mut csv_rdr_left = rconfig_left.reader()?;
+    let mut csv_rdr_right = rconfig_right.reader()?;
+
+    let headers_left = csv_rdr_left.byte_headers()?;
+    let headers_right = csv_rdr_right.byte_headers()?;
+
+    let primary_key_cols: Vec<usize> = match args.flag_key {
         None => vec![0],
-        Some(s) => s
-            .split(',')
-            .map(str::parse::<usize>)
-            .collect::<Result<Vec<_>, _>>()
-            .map_err(|err| CliError::Other(err.to_string()))?,
+        Some(s) => {
+            // check if the key is a comma separated list of numbers
+            if s.chars().all(|c: char| c.is_numeric() || c == ',') {
+                s.split(',')
+                    .map(str::parse::<usize>)
+                    .collect::<Result<Vec<_>, _>>()
+                    .map_err(|err| CliError::Other(err.to_string()))?
+            } else {
+                // check if the key is a comma separated list of column names
+                let left_key_indices = s
+                    .split(',')
+                    .enumerate()
+                    .map(|(index, col_name)| {
+                        headers_left
+                            .iter()
+                            .position(|h| h == col_name.as_bytes())
+                            .ok_or_else(|| {
+                                CliError::Other(format!(
+                                    "Column name '{col_name}' not found on left CSV"
+                                ))
+                            })
+                            .map(|pos| pos + index + 1)
+                    })
+                    .collect::<Result<Vec<usize>, _>>()?;
+
+                // now check if the right CSV has the same selected column names in the same order
+                let right_key_indices = s
+                    .split(',')
+                    .enumerate()
+                    .map(|(index, col_name)| {
+                        headers_right
+                            .iter()
+                            .position(|h| h == col_name.as_bytes())
+                            .ok_or_else(|| {
+                                CliError::Other(format!(
+                                    "Column name '{col_name}' not found on right CSV"
+                                ))
+                            })
+                            .map(|pos| pos + index + 1)
+                    })
+                    .collect::<Result<Vec<usize>, _>>()?;
+
+                if left_key_indices != right_key_indices {
+                    return fail_clierror!("Column names on left and right CSVs do not match");
+                }
+                left_key_indices
+            }
+        },
     };
 
     let sort_cols = args
         .flag_sort_columns
         .map(|s| {
-            s.split(',')
-                .map(str::parse::<usize>)
-                .collect::<Result<Vec<_>, _>>()
-                .map_err(|err| CliError::Other(err.to_string()))
+            // check if the sort columns are a comma separated list of numbers
+            if s.chars().all(|c: char| c.is_numeric() || c == ',') {
+                s.split(',')
+                    .map(str::parse::<usize>)
+                    .collect::<Result<Vec<_>, _>>()
+                    .map_err(|err| CliError::Other(err.to_string()))
+            } else {
+                // check if the sort columns is a comma separated list of column names
+                let left_sort_indices = s
+                    .split(',')
+                    .enumerate()
+                    .map(|(index, col_name)| {
+                        headers_left
+                            .iter()
+                            .position(|h| h == col_name.as_bytes())
+                            .ok_or_else(|| {
+                                CliError::Other(format!(
+                                    "Column name '{col_name}' not found on left CSV"
+                                ))
+                            })
+                            .map(|pos| pos + index + 1)
+                    })
+                    .collect::<Result<Vec<usize>, _>>()?;
+
+                Ok(left_sort_indices)
+            }
         })
         .transpose()?;
 
     let wtr = Config::new(&args.flag_output)
         .delimiter(args.flag_delimiter_output)
         .writer()?;
-    let csv_rdr_left = rconfig_left.reader()?;
-    let csv_rdr_right = rconfig_right.reader()?;
 
     // set RAYON_NUM_THREADS
     util::njobs(args.flag_jobs);


Test patch:
diff --git a/tests/test_diff.rs b/tests/test_diff.rs
index a6c3dd1ba..4b4cdf1e7 100644
--- a/tests/test_diff.rs
+++ b/tests/test_diff.rs
@@ -165,6 +165,48 @@ diffresult,case_enquiry_id,open_dt,target_dt,closed_dt,ontime,case_status,closur
     }
 }
 
+#[test]
+fn diff_sort_diff_result_by_first_column_name() {
+    let wrk = Workdir::new("diff");
+    let test_file = wrk.load_test_file("boston311-100.csv");
+    let test_file2 = wrk.load_test_file("boston311-100-diff.csv");
+
+    let mut cmd = wrk.command("diff");
+    cmd.arg("--sort-columns")
+        .arg("case_enquiry_id")
+        .arg(test_file)
+        .arg(test_file2);
+
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+
+    let diff_result_file_name = "diff_result_original_left_diff_right_sort_columns.csv";
+
+    wrk.create(diff_result_file_name, got);
+
+    let mut cmd = wrk.command("select");
+    // select all columns
+    cmd.arg("1-").arg(diff_result_file_name);
+
+    let actual: String = wrk.stdout(&mut cmd);
+    let expected = create_expected_diff_result_when_sorting_by_first_column();
+
+    assert_eq!(dos2unix(&actual), dos2unix(&expected));
+
+    fn create_expected_diff_result_when_sorting_by_first_column() -> String {
+        r#"
+diffresult,case_enquiry_id,open_dt,target_dt,closed_dt,ontime,case_status,closure_reason,case_title,subject,reason,type,queue,department,submittedphoto,closedphoto,location,fire_district,pwd_district,city_council_district,police_district,neighborhood,neighborhood_services_district,ward,precinct,location_street_name,location_zipcode,latitude,longitude,source
+-,101004113747,2022-01-01 23:46:09,2022-01-17 08:30:00,2022-01-02 11:03:10,ONTIME,Closed,Case Closed. Closed date : Sun Jan 02 11:03:10 EST 2022 Noted Case noted. Duplicate case. Posts already marked for contractor to repair.  ,Street Light Outages,Public Works Department,Street Lights,Street Light Outages,PWDx_Street Light Outages,PWDx,https://311.boston.gov/media/boston/report/photos/61d12e0705bbcf180c29cfc2/report.jpg,,103 N Beacon St  Brighton  MA  02135,11,04,9,D14,Brighton,15,22,2205,103 N Beacon St,02135,42.3549,-71.143,Citizens Connect App
++,101004113747,2022-01-01 23:46:09,2022-01-17 08:30:00,2022-01-02 11:04:10,ONTIME,Closed,Case Closed. Closed date : Sun Jan 02 11:03:10 EST 2022 Noted Case noted. Duplicate case. Posts already marked for contractor to repair.  ,Street Light Outages,Public Works Department,Street Lights,Street Light Outages,PWDx_Street Light Outages,PWDx,https://311.boston.gov/media/boston/report/photos/61d12e0705bbcf180c29cfc2/report.jpg,,103 N Beacon St  Brighton  MA  02135,11,04,9,D14,Brighton,15,22,2205,103 N Beacon St,02135,42.3549,-71.143,Citizens Connect App
+-,101004114069,2022-01-02 14:11:49,2022-01-05 08:30:00,2022-01-03 06:52:40,ONTIME,Closed,Case Closed. Closed date : Mon Jan 03 06:52:40 EST 2022 Resolved No violation found at this time  today is trash day.  ,Improper Storage of Trash (Barrels),Public Works Department,Code Enforcement,Improper Storage of Trash (Barrels),PWDx_Code Enforcement,PWDx,https://311.boston.gov/media/boston/report/photos/61d1f8e905bbcf180c2a3d7f/report.jpg,,22 Henchman St  Boston  MA  02109,3,1B,1,A1,Downtown / Financial District,3,Ward 3,0302,22 Henchman St,02109,42.3674,-71.0537,Citizens Connect App
++,101004114069,2022-01-02 14:11:49,2022-01-05 08:30:00,2022-01-03 06:52:40,ONTIME,Closed,Case Closed. Closed date : Mon Jan 03 06:52:40 EST 2022 Resolved No violation found at this time today is trash day.  ,Improper Storage of Trash (Barrels),Public Works Department,Code Enforcement,Improper Storage of Trash (Barrels),PWDx_Code Enforcement,PWDx,https://311.boston.gov/media/boston/report/photos/61d1f8e905bbcf180c2a3d7f/report.jpg,,22 Henchman St  Boston  MA  02109,3,1B,1,A1,Downtown / Financial District,3,Ward 3,0302,22 Henchman St,02109,42.3674,-71.0537,Citizens Connect App
+-,101004114152,2022-01-02 16:18:30,2022-01-10 08:30:00,2022-01-02 16:32:54,ONTIME,Closed,Case Closed. Closed date : Sun Jan 02 16:32:54 EST 2022 Noted This not not a city park  ,Litter / Ground Maintenance - Wellington Green (BPRD),Parks & Recreation Department,Park Maintenance & Safety,Ground Maintenance,PARK_Maintenance_Ground Maintenance,PARK,https://311.boston.gov/media/boston/report/photos/61d2169605bbcf180c2a4d65/photo_20220102_161627.jpg,,563 Columbus Ave  Roxbury  MA  02118,4,1C,7,D4,South End,6,Ward 4,0404,563 Columbus Ave,02118,42.3412,-71.0815,Citizens Connect App
++,101004114152,2022-01-02 16:18:30,2022-01-10 08:30:00,2022-01-02 16:32:54,ONTIME,Closed,Case Closed. Closed date : Sun Jan 02 16:32:54 EST 2022 Noted This not not a city park  ,Litter/Ground Maintenance - Wellington Green (BPRD),Parks & Recreation Department,Park Maintenance & Safety,Ground Maintenance,PARK_Maintenance_Ground Maintenance,PARK,https://311.boston.gov/media/boston/report/photos/61d2169605bbcf180c2a4d65/photo_20220102_161627.jpg,,563 Columbus Ave  Roxbury  MA  02118,4,1C,7,D4,South End,6,Ward 4,0404,563 Columbus Ave,02118,42.3412,-71.0815,Citizens Connect App
+-,101004114377,2022-01-03 07:50:09,2022-01-04 08:30:00,2022-01-03 10:35:57,ONTIME,Closed,Case Closed. Closed date : 2022-01-03 10:35:57.797 Case Resolved Vehicles mere moved will check again  ,Parking Enforcement,Transportation - Traffic Division,Enforcement & Abandoned Vehicles,Parking Enforcement,BTDT_Parking Enforcement,BTDT,,,618 E Sixth St  South Boston  MA  02127,6,05,2,C6,South Boston / South Boston Waterfront,5,Ward 6,0606,618 E Sixth St,02127,42.3332,-71.0357,Citizens Connect App
++,101004114377,2022-01-03 07:50:09,2022-01-04 08:30:00,2022-01-03 10:35:57,ONTIME,Closed,Case Closed. Closed date : 2022-01-03 10:35:57.797 Case Resolved Vehicles mere moved will check again sir ,Parking Enforcement,Transportation - Traffic Division,Enforcement & Abandoned Vehicles,Parking Enforcement,BTDT_Parking Enforcement,BTDT,,,618 E Sixth St  South Boston  MA  02127,6,05,2,C6,South Boston / South Boston Waterfront,5,Ward 6,0606,618 E Sixth St,02127,42.3332,-71.0357,Citizens Connect App
+        "#.trim().to_string()
+    }
+}
+
 #[test]
 fn diff_different_delimiters_sort_diff_result_by_first_column() {
     let wrk = Workdir::new("diff");
@@ -257,6 +299,44 @@ fn diff_no_diff_with_no_headers_in_result() {
     assert_eq!(got, expected);
 }
 
+#[test]
+fn diff_key_sort() {
+    let wrk = Workdir::new("diff_key_sort");
+
+    let left = vec![
+        svec!["h1", "h2", "h3"],
+        svec!["1", "foo", "bar"],
+        svec!["2", "fooz", "bart"],
+    ];
+    wrk.create("left.csv", left);
+
+    let right = vec![
+        svec!["h1", "h2", "h3"],
+        svec!["1", "foo", "bar"],
+        svec!["2", "booz", "fart"],
+    ];
+    wrk.create("right.csv", right);
+
+    let mut cmd = wrk.command("diff");
+    cmd.args([
+        "left.csv",
+        "right.csv",
+        "--key",
+        "h3,h1",
+        "--sort-columns",
+        "h1,h3,h2",
+    ]);
+
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    let expected: Vec<Vec<String>> = vec![
+        svec!["diffresult", "h1", "h2", "h3"],
+        svec!["+", "2", "booz", "fart"],
+        svec!["-", "2", "fooz", "bart"],
+    ];
+
+    assert_eq!(got, expected);
+}
+
 #[test]
 fn diff_only_left_has_headers_headers_in_result() {
     let wrk = Workdir::new("diff_only_left_has_headers_headers_in_result");