Repository: apache/datafusion (language: rust)

Issue:
Incorrect backslash treatment in string literals in DataFusion CLI
In standard SQL, the `\` character has no special meaning in `'...'` varchar literals.

In PostgreSQL
```
postgres=# SELECT '\', '\\'
postgres-# ;
 ?column? | ?column?
----------+----------
 \        | \\
(1 row)
```

in DataFusion sqllogictest the behavior is the same:

```
query T
SELECT '\'
----
\

query T
SELECT '\\'
----
\\

query T
SELECT '\\\'
----
\\\

query T
SELECT '\\\\'
----
\\\\
```

However, DataFusion CLI behaves differently

```
$ cargo run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/datafusion-cli`
DataFusion CLI v43.0.0
> SELECT '\\';
+-----------+
| Utf8("\") |
+-----------+
| \         |
+-----------+
1 row(s) fetched.
Elapsed 0.049 seconds.

> SELECT '\';  🤔 Invalid statement: SQL error: TokenizerError("unsupported escape char: '\\''")
```

Given that DataFusion CLI is used a lot to test and verify DataFusion's behavior, it's super important for the CLI to behave correctly with respect to its input.



Discussion / hints:
BTW @pmcgleenon  hit this when updating ClickBench as well:

https://github.com/ClickHouse/ClickBench/pull/301
take

Fix patch:
diff --git a/datafusion-cli/src/exec.rs b/datafusion-cli/src/exec.rs
index d560dee987f5..552d438ca390 100644
--- a/datafusion-cli/src/exec.rs
+++ b/datafusion-cli/src/exec.rs
@@ -22,7 +22,7 @@ use crate::helper::split_from_semicolon;
 use crate::print_format::PrintFormat;
 use crate::{
     command::{Command, OutputFormat},
-    helper::{unescape_input, CliHelper},
+    helper::CliHelper,
     object_storage::get_object_store,
     print_options::{MaxRows, PrintOptions},
 };
@@ -172,7 +172,7 @@ pub async fn exec_from_repl(
                 }
             }
             Ok(line) => {
-                let lines = split_from_semicolon(line);
+                let lines = split_from_semicolon(&line);
                 for line in lines {
                     rl.add_history_entry(line.trim_end())?;
                     tokio::select! {
@@ -215,7 +215,6 @@ pub(super) async fn exec_and_print(
     sql: String,
 ) -> Result<()> {
     let now = Instant::now();
-    let sql = unescape_input(&sql)?;
     let task_ctx = ctx.task_ctx();
     let dialect = &task_ctx.session_config().options().sql_parser.dialect;
     let dialect = dialect_from_str(dialect).ok_or_else(|| {
diff --git a/datafusion-cli/src/helper.rs b/datafusion-cli/src/helper.rs
index 871e6de9d31f..64c34c473736 100644
--- a/datafusion-cli/src/helper.rs
+++ b/datafusion-cli/src/helper.rs
@@ -22,11 +22,8 @@ use std::borrow::Cow;
 
 use crate::highlighter::{NoSyntaxHighlighter, SyntaxHighlighter};
 
-use datafusion::common::sql_datafusion_err;
-use datafusion::error::DataFusionError;
 use datafusion::sql::parser::{DFParser, Statement};
 use datafusion::sql::sqlparser::dialect::dialect_from_str;
-use datafusion::sql::sqlparser::parser::ParserError;
 
 use rustyline::completion::{Completer, FilenameCompleter, Pair};
 use rustyline::error::ReadlineError;
@@ -63,15 +60,6 @@ impl CliHelper {
 
     fn validate_input(&self, input: &str) -> Result<ValidationResult> {
         if let Some(sql) = input.strip_suffix(';') {
-            let sql = match unescape_input(sql) {
-                Ok(sql) => sql,
-                Err(err) => {
-                    return Ok(ValidationResult::Invalid(Some(format!(
-                        "  🤔 Invalid statement: {err}",
-                    ))))
-                }
-            };
-
             let dialect = match dialect_from_str(&self.dialect) {
                 Some(dialect) => dialect,
                 None => {
@@ -166,40 +154,8 @@ impl Validator for CliHelper {
 
 impl Helper for CliHelper {}
 
-/// Unescape input string from readline.
-///
-/// The data read from stdio will be escaped, so we need to unescape the input before executing the input
-pub fn unescape_input(input: &str) -> datafusion::error::Result<String> {
-    let mut chars = input.chars();
-
-    let mut result = String::with_capacity(input.len());
-    while let Some(char) = chars.next() {
-        if char == '\\' {
-            if let Some(next_char) = chars.next() {
-                // https://static.rust-lang.org/doc/master/reference.html#literals
-                result.push(match next_char {
-                    '0' => '\0',
-                    'n' => '\n',
-                    'r' => '\r',
-                    't' => '\t',
-                    '\\' => '\\',
-                    _ => {
-                        return Err(sql_datafusion_err!(ParserError::TokenizerError(
-                            format!("unsupported escape char: '\\{}'", next_char)
-                        )))
-                    }
-                });
-            }
-        } else {
-            result.push(char);
-        }
-    }
-
-    Ok(result)
-}
-
 /// Splits a string which consists of multiple queries.
-pub(crate) fn split_from_semicolon(sql: String) -> Vec<String> {
+pub(crate) fn split_from_semicolon(sql: &str) -> Vec<String> {
     let mut commands = Vec::new();
     let mut current_command = String::new();
     let mut in_single_quote = false;
@@ -310,14 +266,13 @@ mod tests {
          )?;
         assert!(matches!(result, ValidationResult::Valid(None)));
 
-        // should be invalid
         let result = readline_direct(
-             Cursor::new(
-                 r"create external table test stored as csv location 'data.csv' options ('format.delimiter' '\u{07}');"
-                     .as_bytes()),
-             &validator,
-         )?;
-        assert!(matches!(result, ValidationResult::Invalid(Some(_))));
+            Cursor::new(
+                r"select '\', '\\', '\\\\\', 'dsdsds\\\\', '\t', '\0', '\n';".as_bytes(),
+            ),
+            &validator,
+        )?;
+        assert!(matches!(result, ValidationResult::Valid(None)));
 
         Ok(())
     }
@@ -346,15 +301,15 @@ mod tests {
     fn test_split_from_semicolon() {
         let sql = "SELECT 1; SELECT 2;";
         let expected = vec!["SELECT 1;", "SELECT 2;"];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = r#"SELECT ";";"#;
         let expected = vec![r#"SELECT ";";"#];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = "SELECT ';';";
         let expected = vec!["SELECT ';';"];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = r#"SELECT 1; SELECT 'value;value'; SELECT 1 as "text;text";"#;
         let expected = vec![
@@ -362,18 +317,18 @@ mod tests {
             "SELECT 'value;value';",
             r#"SELECT 1 as "text;text";"#,
         ];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = "";
         let expected: Vec<String> = Vec::new();
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = "SELECT 1";
         let expected = vec!["SELECT 1;"];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
 
         let sql = "SELECT 1;   ";
         let expected = vec!["SELECT 1;"];
-        assert_eq!(split_from_semicolon(sql.to_string()), expected);
+        assert_eq!(split_from_semicolon(sql), expected);
     }
 }


Test patch:
diff --git a/datafusion-cli/tests/cli_integration.rs b/datafusion-cli/tests/cli_integration.rs
index 27cabf15afec..fa170ae19259 100644
--- a/datafusion-cli/tests/cli_integration.rs
+++ b/datafusion-cli/tests/cli_integration.rs
@@ -39,6 +39,10 @@ fn init() {
     ["--command", "select 1; select 2;", "--format", "json", "-q"],
     "[{\"Int64(1)\":1}]\n[{\"Int64(2)\":2}]\n"
 )]
+#[case::exec_backslash(
+    ["--file", "tests/data/backslash.txt", "--format", "json", "-q"],
+    "[{\"Utf8(\\\"\\\\\\\")\":\"\\\\\",\"Utf8(\\\"\\\\\\\\\\\")\":\"\\\\\\\\\",\"Utf8(\\\"\\\\\\\\\\\\\\\\\\\\\\\")\":\"\\\\\\\\\\\\\\\\\\\\\",\"Utf8(\\\"dsdsds\\\\\\\\\\\\\\\\\\\")\":\"dsdsds\\\\\\\\\\\\\\\\\",\"Utf8(\\\"\\\\t\\\")\":\"\\\\t\",\"Utf8(\\\"\\\\0\\\")\":\"\\\\0\",\"Utf8(\\\"\\\\n\\\")\":\"\\\\n\"}]\n"
+)]
 #[case::exec_from_files(
     ["--file", "tests/data/sql.txt", "--format", "json", "-q"],
     "[{\"Int64(1)\":1}]\n"
diff --git a/datafusion-cli/tests/data/backslash.txt b/datafusion-cli/tests/data/backslash.txt
new file mode 100644
index 000000000000..f2fe2b03746e
--- /dev/null
+++ b/datafusion-cli/tests/data/backslash.txt
@@ -0,0 +1,1 @@
+select '\', '\\', '\\\\\', 'dsdsds\\\\', '\t', '\0', '\n';
\ No newline at end of file