You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/18 18:46:19 UTC
[GitHub] [arrow-datafusion] WinkerDu opened a new pull request, #2267: trim quoted table name in `FROM` clause when creating relation
WinkerDu opened a new pull request, #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #2147.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
Suppose table `t1` has a int-typed column named `id_field`, sql like
```
select * from "t1" where "t1"."id_field" > 1
```
will throw error `thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Plan("No field named 't1.id_field'. Valid fields are '\"t1\".id_field'.")'`
This pr fix it.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Trim quoted table name in `FROM` clause when creating relation, like `"tb_name"` -> `tb_name`
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
No.
<!--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#discussion_r852417780
##########
datafusion/core/src/sql/planner.rs:
##########
@@ -638,7 +638,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
TableFactor::Table {
ref name, alias, ..
} => {
- let table_name = name.to_string();
+ // Trim quoted table name: "db_name" -> db_name
+ let table_name = name.to_string().trim_matches('\"').to_string();
Review Comment:
If we fix it there, I can handle bringing the new version of sqlparser into DataFusion, FYI
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu closed pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
WinkerDu closed pull request #2267: trim quoted table name in `FROM` clause when creating relation
URL: https://github.com/apache/arrow-datafusion/pull/2267
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan commented on a diff in pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
Dandandan commented on code in PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#discussion_r852351865
##########
datafusion/core/src/sql/planner.rs:
##########
@@ -638,7 +638,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
TableFactor::Table {
ref name, alias, ..
} => {
- let table_name = name.to_string();
+ // Trim quoted table name: "db_name" -> db_name
+ let table_name = name.to_string().trim_matches('\"').to_string();
Review Comment:
`trim_matches` also trims double, triple, etc. quotes. and doesn't care about symmetry (one start quote should be ended by one enrd quote).
I believe we should handle (to support quotes table name) the parsing at the sqlparser library:
https://github.com/sqlparser-rs/sqlparser-rs
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] yjshen commented on a diff in pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
yjshen commented on code in PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#discussion_r852547585
##########
datafusion/core/src/sql/planner.rs:
##########
@@ -638,7 +638,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
TableFactor::Table {
ref name, alias, ..
} => {
- let table_name = name.to_string();
+ // Trim quoted table name: "db_name" -> db_name
+ let table_name = name.to_string().trim_matches('\"').to_string();
Review Comment:
Agree we should do this during SQL parsing, returning a unified pattern of table names with qualifiers.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] jackwener commented on pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#issuecomment-1101939619
I also fix them in the draft PR. I find we will trim many time.
We should handle in `sqlparser`.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#issuecomment-1101670150
cc @andygrove @alamb @xudong963
Please have a review, thank you.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2267: trim quoted table name in `FROM` clause when creating relation
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on PR #2267:
URL: https://github.com/apache/arrow-datafusion/pull/2267#issuecomment-1101964957
@Dandandan @alamb @yjshen @jackwener
Thank you for the information, I will try to fix it in `sqlparser` at first,
I will close this pr and update follow up information in #2147.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org