iffyio commented on code in PR #1675:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1675#discussion_r1928898319
##########
src/parser/mod.rs:
##########
@@ -11225,6 +11283,11 @@ impl<'a> Parser<'a> {
let alias = self.maybe_parse_table_alias()?;
+ // maybe parse so we will still support queries like 'SELECT *
FROM T USE LIMIT 1' in BigQuery, for example
Review Comment:
```suggestion
```
Thinking we can probably skip this comment, I think the use of `maybe_parse`
on its own clarifies as much.
##########
tests/sqlparser_bigquery.rs:
##########
@@ -1601,6 +1602,34 @@ fn parse_join_constraint_unnest_alias() {
);
}
+#[test]
+fn parse_select_with_use() {
+ let sql = "SELECT * FROM T USE LIMIT 1";
Review Comment:
Regarding the test, it might be a bit odd to have this test in this file,
specific to bigquery since the behavior isn't bigquery specific. This
potentially relates to @yoavcloud's comment on gating this with a dialect flag,
thinking that might also make the test easier to setup
I think what we could do in that case would be e.g.
- introduce a `dialects.supports_table_hints()` flag to gate the feature (we
could still use the `maybe_parse` or if the grammar is unambiguous for the
dialects that support table hints, then we the flag could replace `maybe_parse`
entirely`
- We move the `parse_select_table_with_index_hints` from mysql to
`sqlparser_common.rs` and to use the
[all_dialects_where](https://github.com/apache/datafusion-sqlparser-rs/blob/ef072be9e1b1ecbf8032bd2040131a9d5b00de5d/tests/sqlparser_common.rs#L125)
format
- We move this test case into that `parse_select_table_with_index_hints`
test where it uses e.g.
```rust
let unsupported_dialects = all_dialects_where(|d|
!d.supports_table_index_hints());
unsupported_dialects.verified_stmt(sql);
...
```
##########
tests/sqlparser_mysql.rs:
##########
@@ -2898,6 +2900,21 @@ fn parse_lock_tables() {
mysql().verified_stmt("UNLOCK TABLES");
}
+#[test]
+fn parse_select_table_with_index_hints() {
+ mysql()
Review Comment:
for one of these scenarios can we assert that the `index_hints` property of
the returned statement was in fact populated with the expected result?
--
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
