alamb commented on code in PR #10573:
URL: https://github.com/apache/datafusion/pull/10573#discussion_r1608928216


##########
datafusion-examples/examples/plan_to_sql.rs:
##########
@@ -52,7 +52,7 @@ fn simple_expr_to_sql_demo() -> Result<()> {
     let expr = col("a").lt(lit(5)).or(col("a").eq(lit(8)));
     let ast = expr_to_sql(&expr)?;
     let sql = format!("{}", ast);
-    assert_eq!(sql, r#"(("a" < 5) OR ("a" = 8))"#);
+    assert_eq!(sql, r#"((a < 5) OR (a = 8))"#);

Review Comment:
   ❤️ 



##########
datafusion/core/Cargo.toml:
##########
@@ -145,7 +145,7 @@ postgres-protocol = "0.6.4"
 postgres-types = { version = "0.2.4", features = ["derive", "with-chrono-0_4"] 
}
 rand = { workspace = true, features = ["small_rng"] }
 rand_distr = "0.4.3"
-regex = "1.5.4"
+regex = { workspace = true }

Review Comment:
   that is certainly nice to use the same version of regex everywhere 👍 



##########
datafusion/sql/src/unparser/dialect.rs:
##########
@@ -15,19 +15,30 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use regex::Regex;
+use sqlparser::keywords::ALL_KEYWORDS;
+
 /// Dialect is used to capture dialect specific syntax.

Review Comment:
   ```suggestion
   /// `Dialect` to usse for Unparsing
   ///
   /// The default dialect tries to avoid quoting identifiers unless necessary 
(e.g. `a` instead of `"a"`)
   /// but this behavior can be overridden as needed
   ```



##########
datafusion-examples/examples/plan_to_sql.rs:
##########
@@ -52,7 +52,7 @@ fn simple_expr_to_sql_demo() -> Result<()> {
     let expr = col("a").lt(lit(5)).or(col("a").eq(lit(8)));
     let ast = expr_to_sql(&expr)?;
     let sql = format!("{}", ast);
-    assert_eq!(sql, r#"(("a" < 5) OR ("a" = 8))"#);
+    assert_eq!(sql, r#"((a < 5) OR (a = 8))"#);

Review Comment:
   Given this change, perhaps we can remove the next example in the file 
`simple_expr_to_sql_demo_no_escape` as I don't think it serves any purpose



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to