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 2021/11/04 08:52:22 UTC
[GitHub] [arrow-datafusion] lichuan6 opened a new pull request #1237: ignore case of `with header row` in sql when creating external table
lichuan6 opened a new pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237
Currently, creating an external table using
```SQL
CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV with header row LOCATION 'foo.csv'
```
will fail when `with header row` tokens are in lower case. This pr is a fix.
--
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 pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961365013
--
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] woodsmur commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
woodsmur commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-962451866
@Dandandan Could you please review this PR?
--
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] lichuan6 commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
--
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 change in pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#discussion_r742916293
##########
File path: datafusion/src/sql/parser.rs
##########
@@ -294,7 +294,7 @@ impl<'a> DFParser<'a> {
}
fn consume_token(&mut self, expected: &str) -> bool {
Review comment:
This method could take a token instead of a `&str` removing the need to convert it to a string again?
--
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] lichuan6 commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961710959
@Dandandan Could you please approve and make the tests run?
--
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] lichuan6 edited a comment on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 edited a comment on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
--
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] woodsmur removed a comment on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
woodsmur removed a comment on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-962451866
@Dandandan Could you please review this PR?
--
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] lichuan6 commented on a change in pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on a change in pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#discussion_r743037901
##########
File path: datafusion/src/sql/parser.rs
##########
@@ -294,7 +294,7 @@ impl<'a> DFParser<'a> {
}
fn consume_token(&mut self, expected: &str) -> bool {
Review comment:
fixed :)
--
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] lichuan6 commented on a change in pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on a change in pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#discussion_r743037901
##########
File path: datafusion/src/sql/parser.rs
##########
@@ -294,7 +294,7 @@ impl<'a> DFParser<'a> {
}
fn consume_token(&mut self, expected: &str) -> bool {
Review comment:
fixed :)
##########
File path: datafusion/src/sql/parser.rs
##########
@@ -294,7 +294,7 @@ impl<'a> DFParser<'a> {
}
fn consume_token(&mut self, expected: &str) -> bool {
Review comment:
@Dandandan Thanks for review, I just fixed 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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] lichuan6 edited a comment on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 edited a comment on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
> @lichuan6 there appears to be some failures on the CI checks
OK, thanks @alamb for pointing it out. It seems using `self.consume_token(&Token::make_keyword("WITH"))` will only recognize upper case keyword. What's the best way to do case-insensitive token parsing?
--
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] lichuan6 edited a comment on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 edited a comment on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
> @lichuan6 there appears to be some failures on the CI checks
OK, thanks @alamb for pointing it out. It seems using `self.consume_token(&Token::make_keyword("WITH"))` will only recognize upper case keyword. What's the best way to do case-insensitive token parsing?
Currently, after changing signature from
```rust
fn consume_token(&mut self, expected: &str) -> bool
```
to
```rust
fn consume_token(&mut self, expected: &Token) -> bool
```
to do case-insensitive parsing, I created a new `Token::Word` enum with upper case of next token. Don't know if this is the best practice.
--
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 pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-962506298
If @Dandandan doesn't have anything to add I'll plan to merge this PR tomorrow
--
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] lichuan6 commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
--
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] lichuan6 commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
> @lichuan6 there appears to be some failures on the CI checks
OK, thanks for pointing it out. It seems using `self.consume_token(&Token::make_keyword("WITH"))` will only recognize upper case keyword. What's the best way to do case-insensitive token parsing?
--
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] lichuan6 commented on a change in pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on a change in pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#discussion_r743037901
##########
File path: datafusion/src/sql/parser.rs
##########
@@ -294,7 +294,7 @@ impl<'a> DFParser<'a> {
}
fn consume_token(&mut self, expected: &str) -> bool {
Review comment:
@Dandandan Thanks for review, I just fixed 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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961365013
@lichuan6 there appears to be some failures on the CI checks
--
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] lichuan6 edited a comment on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 edited a comment on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-961631017
--
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] lichuan6 commented on pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
lichuan6 commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-962452247
Hi, @Dandandan Could you please review this PR?
--
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 pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237#issuecomment-962586196
Thanks again @lichuan6
--
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 merged pull request #1237: ignore case of `with header row` in sql when creating external table
Posted by GitBox <gi...@apache.org>.
alamb merged pull request #1237:
URL: https://github.com/apache/arrow-datafusion/pull/1237
--
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