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


##########
datafusion/sql/src/unparser/dialect.rs:
##########
@@ -15,41 +15,54 @@
 // specific language governing permissions and limitations
 // under the License.
 
-/// Dialect is used to capture dialect specific syntax.
+use regex::Regex;
+use sqlparser::keywords::ALL_KEYWORDS;
+
+/// `Dialect` to use 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
 /// Note: this trait will eventually be replaced by the Dialect in the 
SQLparser package
 ///
 /// See <https://github.com/sqlparser-rs/sqlparser-rs/pull/1170>
 pub trait Dialect {
-    fn identifier_quote_style(&self) -> Option<char>;
+    fn identifier_quote_style(&self, _identifier: &str) -> Option<char>;
 }
 pub struct DefaultDialect {}
 
 impl Dialect for DefaultDialect {
-    fn identifier_quote_style(&self) -> Option<char> {
-        Some('"')
+    fn identifier_quote_style(&self, _identifier: &str) -> Option<char> {

Review Comment:
   @backkem I want to check if I should also change the signature (L29) in the 
Dialect trait. I'm not familiar with naming conventions in Rust. I guess 
_identifier means this parameter is an identifier, but we ignore it in this 
method, right?



-- 
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