You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2026/05/10 11:38:14 UTC
[PR] Consolidate and document SQL AST shims [datafusion]
alamb opened a new pull request, #22094:
URL: https://github.com/apache/datafusion/pull/22094
## Which issue does this PR close?
- Related to https://github.com/apache/datafusion/pull/22069
## Rationale for this change
While upgrading to sqlparser, it was not clear why there were feature gates for the `sql` feature: https://github.com/apache/datafusion/pull/22069/changes#r3204705488
What is happening is that there is a mode to avoid the sqlparser dependency that was added by @timsaucer in
- #17332
I think this feature makes sense but it is a little hard to understand what is going on because it is implemented with a bunch of ` #[cfg(not(feature = "sql"))] ` and there is no central place that explains what is going on
## What changes are included in this PR?
1. Consolidate the sql ast shim structures in separate module
2. Document what is going on so it is easier to understand
## Are these changes tested?
Yes by CI
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22094:
URL: https://github.com/apache/datafusion/pull/22094#issuecomment-4421057946
Sice I think this will conflict with @andygrove 's upgrade of sqlparser and I don't think it will be potentially controversial, I am mergeing now
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #22094:
URL: https://github.com/apache/datafusion/pull/22094#discussion_r3219184605
##########
datafusion/expr/src/sql.rs:
##########
@@ -0,0 +1,143 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Local copies of [`sqlparser::ast`] structures
Review Comment:
this is the documentation that explains what is going on
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #22094:
URL: https://github.com/apache/datafusion/pull/22094#discussion_r3219020631
##########
datafusion/expr/src/expr.rs:
##########
@@ -1416,63 +1422,6 @@ impl Lambda {
}
}
-#[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
Review Comment:
this stuff is all moved to `sql.rs` and I put a module level comment on it
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22094:
URL: https://github.com/apache/datafusion/pull/22094#issuecomment-4425256515
Merge was failing due to
- https://github.com/apache/datafusion/pull/22110
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 merged PR #22094:
URL: https://github.com/apache/datafusion/pull/22094
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Consolidate and document SQL AST shims [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22094:
URL: https://github.com/apache/datafusion/pull/22094#issuecomment-4429034078
Thanks @xudong963
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org