alamb commented on code in PR #18043:
URL: https://github.com/apache/datafusion/pull/18043#discussion_r2430426425
##########
datafusion/common/src/config.rs:
##########
@@ -292,6 +292,88 @@ config_namespace! {
}
}
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
Review Comment:
Can you please add documentation to this enum that explains:
1. What it represents
2. How it is related to
https://docs.rs/sqlparser/latest/sqlparser/dialect/trait.Dialect.html ?
Specifically it would help to understand if the intent is to mirror the code
in `sqlparser` (but is replicated to avoid adding a sqlparser dependency)
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -374,7 +373,7 @@ impl SessionState {
pub fn sql_to_statement(
Review Comment:
I think this is technically a breaking API change (now it requires a
&Dialect). Maybe can you add a note to the upgrade guide and a doc example of
how to use this API to help people upgrade?
This is explained a bit more in
https://datafusion.apache.org/contributor-guide/api-health.html#upgrade-guides
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
