Repository: dathere/qsv (language: rust)

Issue:
`joinp`: align `--ignore-leading-zeros` handling to be the same as `join`
### Discussed in https://github.com/dathere/qsv/discussions/2424

<div type='discussions-op-text'>

<sup>Originally posted by **datatraveller1** January  8, 2025</sup>
Hi @jqnatividad, first of all, thank you very much for the new `--ignore-leading-zero` (shortcut `-z`) option.

It works great, but I have a question. e.g. I use:
`qsv joinp -i -z --left-semi --cache-schema=-1 company_id,art_no file1.csv company_id,art_no file2.csv -o left-semi.csv`

This works correctly except that the leading zeros are stripped from the output column `art_no` in` left-semi.csv`. I had hoped that stripping leadings zeros would only happen for the keys, but if the original value of art_no is `01`, the value in the output file gets `1`.

Reading the documentation, I'm not sure what is the expected behaviour: 
"`When set, joins are done ignoring leading zeros. Note that this is only applied to the join keys for both numeric and string columns. The output columns will not have leading zeros.`"
If "`will not have leading zeros`" isn't a typo, the behaviour is as expected. If this should read "`will not have omitted zeros`", it would be a problem. Thank you very much!</div>


Discussion / hints:

Fix patch:
diff --git a/src/cmd/joinp.rs b/src/cmd/joinp.rs
index c7ff2873e..36e91dde0 100644
--- a/src/cmd/joinp.rs
+++ b/src/cmd/joinp.rs
@@ -29,11 +29,6 @@ joinp arguments:
     Note that <input1> is the left CSV data set and <input2> is the right CSV data set.
 
 joinp options:
-    -i, --ignore-case      When set, joins are done case insensitively.
--z, --ignore-leading-zeros  When set, joins are done ignoring leading zeros.
-                           Note that this is only applied to the join keys for
-                           both numeric and string columns. The output columns
-                           will not have leading zeros.
     --left                 Do a 'left outer' join. This returns all rows in
                            first CSV data set, including rows with no
                            corresponding row in the second data set. When no
@@ -220,6 +215,15 @@ joinp options:
    --null-value <arg>        The string to use when writing null values.
                              (default: <empty string>)
 
+    -i, --ignore-case        When set, joins are done case insensitively.
+-z, --ignore-leading-zeros   When set, joins are done ignoring leading zeros.
+                             Note that this is only applied to the join keys for
+                             both numeric and string columns. Also note that
+                             Polars will automatically remove leading zeros from
+                             numeric columns when it infers the schema.
+                             To force the schema to be all String types,
+                             set --cache-schema to -1 or -2.
+
 Common options:
     -h, --help             Display this message
     -o, --output <file>    Write output to <file> instead of stdout.
@@ -237,7 +241,7 @@ use std::{
     str,
 };
 
-use polars::{datatypes::AnyValue, prelude as pl, prelude::*, sql::SQLContext};
+use polars::prelude::*;
 use serde::Deserialize;
 use tempfile::tempdir;
 
@@ -544,40 +548,70 @@ impl JoinStruct {
             .map(polars::lazy::dsl::col)
             .collect();
 
-        // If ignore_case is enabled, create lowercase versions of the join columns
-        // but only if ignore_leading_zeros is not enabled
-        // because leading zeros handling does its own lowercase conversion
-        if self.ignore_case && !self.ignore_leading_zeros {
-            // Create temporary lowercase versions of join columns in left dataframe
+        // Handle ignore_case and ignore_leading_zeros transformations
+        if self.ignore_case || self.ignore_leading_zeros {
+            // Create temporary versions of join columns in left dataframe
             for col in &left_selcols {
-                self.left_lf = self
-                    .left_lf
-                    .with_column(col.clone().str().to_lowercase().alias(format!(
-                        "_qsv-{}-lower",
-                        col.to_string()
-                            .trim_start_matches(r#"col(""#)
-                            .trim_end_matches(r#"")"#)
-                    )));
+                let col_str = col.to_string();
+                let col_name = col_str
+                    .trim_start_matches(r#"col(""#)
+                    .trim_end_matches(r#"")"#)
+                    .to_string();
+                let temp_col_name = format!("_qsv-{col_name}-transformed");
+
+                // Create a temporary transformed column for joining
+                // we need to cast to string as the original column may be of a different type
+                // and we need a string type for the transformations
+                let transformed = col.clone().cast(DataType::String);
+                let transformed = if self.ignore_leading_zeros {
+                    transformed
+                        .str()
+                        .replace_all(lit(r"^0+"), lit(""), false)
+                        .str()
+                        .to_lowercase()
+                } else if self.ignore_case {
+                    transformed.str().to_lowercase()
+                } else {
+                    transformed
+                };
+
+                // Add transformed column for joining - keep original untouched
+                self.left_lf = self.left_lf.with_column(transformed.alias(&temp_col_name));
             }
 
-            // Create temporary lowercase versions of join columns in right dataframe
+            // Create temporary versions of join columns in right dataframe
             for col in &right_selcols {
-                self.right_lf = self
-                    .right_lf
-                    .with_column(col.clone().str().to_lowercase().alias(format!(
-                        "_qsv-{}-lower",
-                        col.to_string()
-                            .trim_start_matches(r#"col(""#)
-                            .trim_end_matches(r#"")"#)
-                    )));
+                let col_str = col.to_string();
+                let col_name = col_str
+                    .trim_start_matches(r#"col(""#)
+                    .trim_end_matches(r#"")"#)
+                    .to_string();
+                let temp_col_name = format!("_qsv-{col_name}-transformed");
+
+                // Create a temporary transformed column for joining
+                let transformed = col.clone().cast(DataType::String);
+                let transformed = if self.ignore_leading_zeros {
+                    transformed
+                        .str()
+                        .replace_all(lit(r"^0+"), lit(""), false)
+                        .str()
+                        .to_lowercase()
+                } else if self.ignore_case {
+                    transformed.str().to_lowercase()
+                } else {
+                    transformed
+                };
+
+                // Add transformed column for joining - keep original untouched
+                self.right_lf = self.right_lf.with_column(transformed.alias(&temp_col_name));
             }
 
-            // Create new vectors for the lowercase column names
+            // Update selcols to use the transformed columns for joining
             let left_selcols_w: Vec<_> = left_selcols
                 .iter()
                 .map(|col| {
                     polars::lazy::dsl::col(format!(
-                        "_qsv-{}-lower",
+                        "_qsv-{}-transformed",
                         col.to_string()
                             .trim_start_matches(r#"col(""#)
                             .trim_end_matches(r#"")"#)
@@ -585,11 +619,12 @@ impl JoinStruct {
                 })
                 .collect();
             left_selcols = left_selcols_w;
+
             let right_selcols_w: Vec<_> = right_selcols
                 .iter()
                 .map(|col| {
                     polars::lazy::dsl::col(format!(
-                        "_qsv-{}-lower",
+                        "_qsv-{}-transformed",
                         col.to_string()
                             .trim_start_matches(r#"col(""#)
                             .trim_end_matches(r#"")"#)
@@ -705,7 +740,7 @@ impl JoinStruct {
         };
 
         let mut results_df = if let Some(sql_filter) = &self.sql_filter {
-            let mut ctx = SQLContext::new();
+            let mut ctx = polars::sql::SQLContext::new();
             ctx.register("join_result", join_results.lazy());
             ctx.execute(sql_filter)
                 .and_then(polars::prelude::LazyFrame::collect)?
@@ -713,17 +748,34 @@ impl JoinStruct {
             join_results
         };
 
-        // if self.ignore_case, remove the temporary lowercase columns from the dataframe
-        if self.ignore_case && !self.ignore_leading_zeros {
-            // Get all column names
+        if self.ignore_case || self.ignore_leading_zeros {
+            // Remove temporary transformed columns and
+            // duplicate right-side join columns if coalesce is true
             let cols = results_df.get_column_names();
-            // Filter out the lowercase columns (those with "_qsv-*-lower" pattern)
-            let keep_cols: Vec<String> = cols
-                .iter()
-                .filter(|&col| !(col.starts_with("_qsv-") && col.ends_with("-lower")))
-                .map(|&s| s.to_string())
+            let mut keep_cols: Vec<String> = Vec::new();
+
+            let left_join_cols: Vec<String> = self
+                .left_sel
+                .split(',')
+                .map(std::string::ToString::to_string)
                 .collect();
-            // Select only the non-lowercase columns
+
+            for col in cols {
+                if col.contains("-transformed") {
+                    continue;
+                }
+
+                // For join columns, only keep the left version if coalesce is true
+                if self.coalesce && col.ends_with("_right") {
+                    let base_col = col.trim_end_matches("_right");
+                    if left_join_cols.contains(&base_col.to_string()) {
+                        continue;
+                    }
+                }
+
+                keep_cols.push(col.to_string());
+            }
+
             results_df = results_df.select(keep_cols)?;
         }
 
@@ -1060,98 +1112,6 @@ impl Args {
             right_lf = right_lf.filter(filter_right_expr);
         }
 
-        if self.flag_ignore_leading_zeros {
-            // Transform the join keys in the left and right dataframe to handle leading zeros
-            // For each join key column:
-            //   1. Cast to string type
-            //   2. If ignore_case is true, convert to lowercase
-            //   3. Trim leading zeros, regardless of the original type
-            // This allows joins to match on values like "001" and "1", "0001ABZ" and "1ABZ".
-            // Note that the output columns will not have leading zeros for the join keys.
-            //
-            // We had to add ignore_case handling to ignore_leading_zeros in the join keys because
-            // the existing code for ignore_case was not working for leading zeros if they are both
-            // used at the same time.
-            // The ignore_case code was creating temporary join key columns while
-            // ignore_leading_zeros was using the existing join key columns.
-            let ignore_case = self.flag_ignore_case;
-            left_lf = left_lf.with_columns(
-                self.arg_columns1
-                    .split(',')
-                    .map(|col| {
-                        let col_name = col.to_string();
-                        pl::col(&col_name).cast(pl::DataType::String).map(
-                            move |s| {
-                                Ok(Some(
-                                    pl::Series::new(
-                                        col_name.clone().into(),
-                                        s.str()?
-                                            .into_iter()
-                                            .map(|x| {
-                                                x.map(|val| {
-                                                    let v = if ignore_case {
-                                                        val.to_lowercase()
-                                                    } else {
-                                                        val.to_string()
-                                                    };
-                                                    if v.starts_with('0') {
-                                                        v.trim_start_matches('0').to_string()
-                                                    } else {
-                                                        v
-                                                    }
-                                                })
-                                                .unwrap_or_default()
-                                            })
-                                            .collect::<Vec<String>>(),
-                                    )
-                                    .into(),
-                                ))
-                            },
-                            pl::GetOutput::from_type(pl::DataType::String),
-                        )
-                    })
-                    .collect::<Vec<_>>(),
-            );
-
-            right_lf = right_lf.with_columns(
-                self.arg_columns2
-                    .split(',')
-                    .map(|col| {
-                        let col_name = col.to_string();
-                        pl::col(&col_name).cast(pl::DataType::String).map(
-                            move |s| {
-                                Ok(Some(
-                                    pl::Series::new(
-                                        col_name.clone().into(),
-                                        s.str()?
-                                            .into_iter()
-                                            .map(|x| {
-                                                x.map(|val| {
-                                                    let v = if ignore_case {
-                                                        val.to_lowercase()
-                                                    } else {
-                                                        val.to_string()
-                                                    };
-                                                    if v.starts_with('0') {
-                                                        v.trim_start_matches('0').to_string()
-                                                    } else {
-                                                        v
-                                                    }
-                                                })
-                                                .unwrap_or_default()
-                                            })
-                                            .collect::<Vec<String>>(),
-                                    )
-                                    .into(),
-                                ))
-                            },
-                            pl::GetOutput::from_type(pl::DataType::String),
-                        )
-                    })
-                    .collect::<Vec<_>>(),
-            );
-        }
-
         Ok(JoinStruct {
             left_lf,
             left_sel: self.arg_columns1.clone(),


Test patch:
diff --git a/tests/test_joinp.rs b/tests/test_joinp.rs
index 14ec399f2..124c16b44 100644
--- a/tests/test_joinp.rs
+++ b/tests/test_joinp.rs
@@ -1844,11 +1844,58 @@ fn joinp_ignore_leading_zero() {
     wrk.assert_success(&mut cmd);
 
     let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    // note that id and id_right have been stripped of leading zeros
+    // this is because Polars inferred the schema as all integers
+    // and automatically converted the values to integers
     let expected = vec![
-        svec!["id", "value", "desc"],
-        svec!["1", "a", "one"],
-        svec!["2", "b", "two"],
-        svec!["3", "c", "three"],
+        svec!["id", "value", "id_right", "desc"],
+        svec!["1", "a", "1", "one"],
+        svec!["2", "b", "2", "two"],
+        svec!["3", "c", "3", "three"],
+    ];
+    assert_eq!(got, expected);
+}
+
+#[test]
+fn joinp_ignore_leading_zero_string_schema() {
+    let wrk = Workdir::new("joinp_ignore_leading_zero_string_schema");
+
+    wrk.create(
+        "left.csv",
+        vec![
+            svec!["id", "value"],
+            svec!["001", "a"],
+            svec!["02", "b"],
+            svec!["3", "c"],
+        ],
+    );
+
+    wrk.create(
+        "right.csv",
+        vec![
+            svec!["id", "desc"],
+            svec!["1", "one"],
+            svec!["02", "two"],
+            svec!["003", "three"],
+        ],
+    );
+
+    let mut cmd = wrk.command("joinp");
+    cmd.args(&["id", "left.csv", "id", "right.csv"])
+        .arg("-z")
+        .args(["--cache-schema", "-2"]); // force schema to all String types
+
+    wrk.assert_success(&mut cmd);
+
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    // note that id and id_right have been stripped of leading zeros
+    // this is because Polars inferred the schema as all integers
+    // and automatically converted the values to integers
+    let expected = vec![
+        svec!["id", "value", "id_right", "desc"],
+        svec!["001", "a", "1", "one"],
+        svec!["02", "b", "02", "two"],
+        svec!["3", "c", "003", "three"],
     ];
     assert_eq!(got, expected);
 }
@@ -1885,10 +1932,10 @@ fn joinp_ignore_leading_zero_with_non_numeric() {
 
     let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
     let expected = vec![
-        svec!["code", "value", "desc"],
-        svec!["1A", "a", "one"],
-        svec!["2B", "b", "two"],
-        svec!["ABC", "c", "three"],
+        svec!["code", "value", "code_right", "desc"],
+        svec!["001A", "a", "1A", "one"],
+        svec!["02B", "b", "0002B", "two"],
+        svec!["ABC", "c", "ABC", "three"],
     ];
     assert_eq!(got, expected);
 }
@@ -1919,16 +1966,17 @@ fn joinp_ignore_leading_zero_multiple_columns() {
 
     let mut cmd = wrk.command("joinp");
     cmd.args(&["id,code", "left.csv", "id,code", "right.csv"])
-        .arg("--ignore-leading-zeros");
+        .arg("--ignore-leading-zeros")
+        .args(["--cache-schema", "-2"]); // force schema to all String types
 
     wrk.assert_success(&mut cmd);
 
     let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
     let expected = vec![
-        svec!["id", "code", "value", "desc"],
-        svec!["1", "1A", "a", "one"],
-        svec!["2", "2B", "b", "two"],
-        svec!["3", "ABC", "c", "three"],
+        svec!["id", "code", "value", "id_right", "code_right", "desc"],
+        svec!["001", "001A", "a", "1", "1A", "one"],
+        svec!["02", "02B", "b", "002", "0002B", "two"],
+        svec!["3", "ABC", "c", "03", "ABC", "three"],
     ];
     assert_eq!(got, expected);
 }
@@ -1960,16 +2008,59 @@ fn joinp_ignore_case_and_leading_zeros() {
     let mut cmd = wrk.command("joinp");
     cmd.args(&["id,code", "left.csv", "id,code", "right.csv"])
         .arg("--ignore-leading-zeros")
-        .arg("--ignore-case");
+        .arg("--ignore-case")
+        .args(["--cache-schema", "-2"]); // force schema to all String types
+
+    wrk.assert_success(&mut cmd);
+
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    let expected = vec![
+        svec!["id", "code", "value", "id_right", "code_right", "desc"],
+        svec!["001", "001abc", "a", "1", "00001ABC", "one"],
+        svec!["02", "02DEF", "b", "002", "0002def", "two"],
+        svec!["3", "XYZ", "c", "03", "xyz", "three"],
+    ];
+    assert_eq!(got, expected);
+}
+#[test]
+fn joinp_ignore_case_and_leading_zeros_coalesce() {
+    let wrk = Workdir::new("joinp_ignore_case_and_leading_zeros_coalesce");
+
+    wrk.create(
+        "left.csv",
+        vec![
+            svec!["id", "code", "value"],
+            svec!["001", "001abc", "a"],
+            svec!["02", "02DEF", "b"],
+            svec!["3", "XYZ", "c"],
+        ],
+    );
+
+    wrk.create(
+        "right.csv",
+        vec![
+            svec!["id", "code", "desc"],
+            svec!["1", "00001ABC", "one"],
+            svec!["002", "0002def", "two"],
+            svec!["03", "xyz", "three"],
+        ],
+    );
+
+    let mut cmd = wrk.command("joinp");
+    cmd.args(&["id,code", "left.csv", "id,code", "right.csv"])
+        .arg("--ignore-leading-zeros")
+        .arg("--ignore-case")
+        .arg("--coalesce")
+        .args(["--cache-schema", "-2"]); // force schema to all String types
 
     wrk.assert_success(&mut cmd);
 
     let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
     let expected = vec![
         svec!["id", "code", "value", "desc"],
-        svec!["1", "1abc", "a", "one"],
-        svec!["2", "2def", "b", "two"],
-        svec!["3", "xyz", "c", "three"],
+        svec!["001", "001abc", "a", "one"],
+        svec!["02", "02DEF", "b", "two"],
+        svec!["3", "XYZ", "c", "three"],
     ];
     assert_eq!(got, expected);
 }
@@ -2214,3 +2305,61 @@ fn joinp_non_equi_compound() {
     ];
     assert_eq!(got, expected);
 }
+
+#[test]
+fn joinp_ignore_leading_zeros_issue_2424() {
+    let wrk = Workdir::new("joinp_ignore_leading_zeros_issue_2424");
+
+    wrk.create(
+        "file1.csv",
+        vec![
+            svec!["id", "company_id", "art_no"],
+            svec!["1", "COM1", "0724"],
+            svec!["2", "cm2", "002"],
+            svec!["3", "com3", "78"],
+            svec!["4", "CM2", "01"],
+            svec!["5", "Cp5", "1"],
+            svec!["6", "CPA", "000"],
+        ],
+    );
+
+    wrk.create(
+        "file2.csv",
+        vec![
+            svec!["id", "company_id", "art_no"],
+            svec!["1", "com1", "724"],
+            svec!["2", "CM2", "02"],
+            svec!["3", "Com3", "078"],
+            svec!["4", "cm2", "1"],
+            svec!["5", "CP5", "01"],
+            svec!["6", "cpa", "0"],
+        ],
+    );
+
+    let mut cmd = wrk.command("joinp");
+    cmd.args(&[
+        "-i",
+        "-z",
+        "--left-semi",
+        "--cache-schema",
+        "-1",
+        "company_id,art_no",
+        "file1.csv",
+        "company_id,art_no",
+        "file2.csv",
+    ]);
+
+    wrk.assert_success(&mut cmd);
+
+    let got: Vec<Vec<String>> = wrk.read_stdout(&mut cmd);
+    let expected = vec![
+        svec!["id", "company_id", "art_no"],
+        svec!["1", "COM1", "0724"],
+        svec!["2", "cm2", "002"],
+        svec!["3", "com3", "78"],
+        svec!["4", "CM2", "01"],
+        svec!["5", "Cp5", "1"],
+        svec!["6", "CPA", "000"],
+    ];
+    assert_eq!(got, expected);
+}