Jefffrey commented on code in PR #9268:
URL: https://github.com/apache/arrow-datafusion/pull/9268#discussion_r1501739991


##########
datafusion/sql/src/utils.rs:
##########
@@ -246,3 +246,168 @@ pub(crate) fn normalize_ident(id: Ident) -> String {
         None => id.value.to_ascii_lowercase(),
     }
 }
+
+pub(crate) fn unescape(s: &str) -> Option<String> {
+    let mut queue: VecDeque<_> = String::from(s).chars().collect();
+    let mut unescaped = String::new();
+
+    while let Some(c) = queue.pop_front() {
+        if c != '\\' {
+            unescaped.push(c);
+            continue;
+        }
+
+        let ch = match queue.pop_front() {
+            Some(c) => match c {
+                'b' => '\u{0008}',
+                'f' => '\u{000C}',
+                'n' => '\n',
+                'r' => '\r',
+                't' => '\t',
+                'u' => unescape_unicode_16(&mut queue)?,
+                'U' => unescape_unicode_32(&mut queue)?,
+                'x' => unescape_byte(&mut queue)?,
+                c if c.is_digit(8) => unescape_octal(c, &mut queue)?,
+                c => c,
+            },
+            None => return None,
+        };
+        unescaped.push(ch);
+    }
+
+    Some(unescaped)
+}
+
+// Hexadecimal byte value. \xh, \xhh (h = 0–9, A–F)
+fn unescape_byte(queue: &mut VecDeque<char>) -> Option<char> {
+    let mut s = String::new();
+
+    for _ in 0..2 {
+        match next_hex_digit(queue) {
+            Some(c) => s.push(c),
+            None => break,
+        }
+    }
+
+    if s.is_empty() {
+        return Some('x');
+    }
+
+    to_char::<16>(&s)
+}
+
+#[inline]
+fn next_hex_digit(queue: &mut VecDeque<char>) -> Option<char> {
+    match queue.front() {
+        Some(c) if c.is_ascii_hexdigit() => queue.pop_front(),
+        _ => None,
+    }
+}
+
+// 16-bit hexadecimal Unicode character value. \uxxxx (x = 0–9, A–F)
+fn unescape_unicode_16(queue: &mut VecDeque<char>) -> Option<char> {
+    unescape_unicode::<4>(queue)
+}
+
+// 32-bit hexadecimal Unicode character value. \Uxxxxxxxx (x = 0–9, A–F)
+fn unescape_unicode_32(queue: &mut VecDeque<char>) -> Option<char> {
+    unescape_unicode::<8>(queue)
+}
+
+fn unescape_unicode<const NUM: usize>(queue: &mut VecDeque<char>) -> 
Option<char> {
+    let mut s = String::new();
+    for _ in 0..NUM {
+        s.push(queue.pop_front()?);
+    }
+    to_char::<16>(&s)
+}
+
+// Octal byte value. \o, \oo, \ooo (o = 0–7)
+fn unescape_octal(c: char, queue: &mut VecDeque<char>) -> Option<char> {
+    let mut s = String::new();
+
+    match c {
+        '0' | '1' | '2' | '3' => {
+            s.push(c);
+            for _ in 0..2 {
+                match next_octal_digest(queue) {
+                    Some(c) => s.push(c),
+                    None => break,
+                }
+            }
+        }
+        '4' | '5' | '6' | '7' => {
+            s.push(c);
+            if let Some(c) = next_octal_digest(queue) {
+                s.push(c);
+            }
+        }
+        _ => return None,
+    }
+    to_char::<8>(&s)
+}
+
+#[inline]
+fn next_octal_digest(queue: &mut VecDeque<char>) -> Option<char> {
+    match queue.front() {
+        Some(c) if c.is_digit(8) => queue.pop_front(),
+        _ => None,
+    }
+}
+
+#[inline]
+fn to_char<const RADIX: u32>(s: &str) -> Option<char> {
+    match u32::from_str_radix(s, RADIX) {
+        Err(_) => None,
+        Ok(n) => char::from_u32(n),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::unescape;
+
+    fn check_unescape(s: &str, expected: Option<&str>) {
+        assert_eq!(unescape(s), expected.map(|s| s.to_string()));
+    }
+
+    #[test]
+    fn test_unescape() {
+        check_unescape(r"\b", Some("\u{0008}"));
+        check_unescape(r"\f", Some("\u{000C}"));
+        check_unescape(r"\t", Some("\t"));
+        check_unescape(r"\r\n", Some("\r\n"));
+        check_unescape(r"\/", Some("/"));
+        check_unescape(r"/", Some("/"));
+        check_unescape(r"\\", Some("\\"));
+
+        // 16 and 32-bit hexadecimal Unicode character value
+        check_unescape(r"\u4c91", Some("\u{4c91}"));
+        check_unescape(r"\u4c916", Some("\u{4c91}6"));
+        check_unescape(r"\U0010FFFF", Some("\u{10FFFF}"));
+        check_unescape(r"\u", None);
+        check_unescape(r"\U", None);
+        check_unescape(r"\U1010FFFF", None);
+
+        // hexadecimal byte value
+        check_unescape(r"\xCAD", Some("\u{00ca}D"));
+        check_unescape(r"\xA9", Some("\u{00a9}"));
+        check_unescape(r"\x4B", Some("\u{004b}"));
+        check_unescape(r"\x4", Some("\u{0004}"));
+        check_unescape(r"\x4L", Some("\u{0004}L"));
+        check_unescape(r"\x", Some("x"));
+        check_unescape(r"\xP", Some("xP"));
+
+        // octal byte value
+        check_unescape(r"\1", Some("\u{0001}"));
+        check_unescape(r"\12", Some("\u{000a}"));
+        check_unescape(r"\123", Some("\u{0053}"));
+        check_unescape(r"\1232", Some("\u{0053}2"));
+        check_unescape(r"\4", Some("\u{0004}"));
+        check_unescape(r"\45", Some("\u{0025}"));
+        check_unescape(r"\450", Some("\u{0025}0"));

Review Comment:
   Was checking against Postgres, got some interesting behaviour:
   
   ```
   postgres=# select e'\450' as a;
    a
   ---
    (
   (1 row)
   ```
   
   Also can you add these test cases as I'm curious as to how this 
implementation will handle it vs Postgres
   
   ```
   postgres=# select e'\077' as a;
    a
   ---
    ?
   (1 row)
   
   postgres=# select e'\078' as a;
      a
   -------
    \x078
   (1 row)
   
   postgres=# select e'\079' as a;
      a
   -------
    \x079
   (1 row)
   
   postgres=# select e'\080' as a;
   ERROR:  invalid byte sequence for encoding "UTF8": 0x00
   ```



##########
datafusion/sql/src/utils.rs:
##########
@@ -246,3 +246,168 @@ pub(crate) fn normalize_ident(id: Ident) -> String {
         None => id.value.to_ascii_lowercase(),
     }
 }
+
+pub(crate) fn unescape(s: &str) -> Option<String> {
+    let mut queue: VecDeque<_> = String::from(s).chars().collect();
+    let mut unescaped = String::new();
+
+    while let Some(c) = queue.pop_front() {
+        if c != '\\' {
+            unescaped.push(c);
+            continue;
+        }
+
+        let ch = match queue.pop_front() {
+            Some(c) => match c {
+                'b' => '\u{0008}',
+                'f' => '\u{000C}',
+                'n' => '\n',
+                'r' => '\r',
+                't' => '\t',
+                'u' => unescape_unicode_16(&mut queue)?,
+                'U' => unescape_unicode_32(&mut queue)?,
+                'x' => unescape_byte(&mut queue)?,
+                c if c.is_digit(8) => unescape_octal(c, &mut queue)?,
+                c => c,
+            },
+            None => return None,
+        };

Review Comment:
   ```suggestion
           let ch = match queue.pop_front()? {
               'b' => '\u{0008}',
               'f' => '\u{000C}',
               'n' => '\n',
               'r' => '\r',
               't' => '\t',
               'u' => unescape_unicode_16(&mut queue)?,
               'U' => unescape_unicode_32(&mut queue)?,
               'x' => unescape_byte(&mut queue)?,
               c if c.is_digit(8) => unescape_octal(c, &mut queue)?,
               c => c,
           };
   ```



##########
datafusion/sql/src/utils.rs:
##########
@@ -246,3 +246,168 @@ pub(crate) fn normalize_ident(id: Ident) -> String {
         None => id.value.to_ascii_lowercase(),
     }
 }
+
+pub(crate) fn unescape(s: &str) -> Option<String> {
+    let mut queue: VecDeque<_> = String::from(s).chars().collect();

Review Comment:
   I *think* you might be able to just get away with 
[`Chars`](https://doc.rust-lang.org/std/str/struct.Chars.html) instead of a 
`VecDeque`, as you're only popping from the front anyway, and `Chars` allows 
you to iterate from the front and also peek. Might be worth exploring towards 
:thinking: 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to