You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adamchainz (via GitHub)" <gi...@apache.org> on 2025/04/02 12:01:39 UTC
[PR] Add support for PostgreSQL 'IN ' [datafusion-sqlparser-rs]
adamchainz opened a new pull request, #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793
Fixes #1792.
This is my attempt to hack in support with my minimal Rust skills. I stopped when I realized that my change of `InSubquery` to use `SetExpr` for its subquery broke many tests.
--
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] Add support for 'IN ' [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#issuecomment-2791589818
Marking as draft in the meantime as this is no longer pending review, @adamchainz please feel free to undraft and ping when ready!
--
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] Add support for PostgreSQL 'IN ' [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2025399668
##########
src/parser/mod.rs:
##########
@@ -3744,10 +3744,18 @@ impl<'a> Parser<'a> {
self.expect_token(&Token::LParen)?;
let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
self.prev_token();
- Expr::InSubquery {
- expr: Box::new(expr),
- subquery: self.parse_query()?,
- negated,
+ if self.dialect.supports_in_query_body() {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: self.parse_query_body(self.dialect.prec_unknown())?,
+ negated,
+ }
+ } else {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: Box::new(SetExpr::Query(self.parse_query()?)),
+ negated,
+ }
}
Review Comment:
would it make sense to skip the `supports_in_query_body` flag entirely? thinking since the new representation `SetExpr` is a superset of the old query
--
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] Add support for parenthesized subquery as `IN` predicate [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#issuecomment-2807691385
choo choo!
--
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] Add support for 'IN ' [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2029711949
##########
src/parser/mod.rs:
##########
@@ -3742,24 +3742,23 @@ impl<'a> Parser<'a> {
});
}
self.expect_token(&Token::LParen)?;
- let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
- self.prev_token();
- Expr::InSubquery {
+ let in_op = match self.try_parse(|p| p.parse_query_body(p.dialect.prec_unknown())) {
Review Comment:
Yeah this makes sense!
##########
src/parser/mod.rs:
##########
@@ -3742,24 +3742,23 @@ impl<'a> Parser<'a> {
});
}
self.expect_token(&Token::LParen)?;
- let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
- self.prev_token();
- Expr::InSubquery {
+ let in_op = match self.try_parse(|p| p.parse_query_body(p.dialect.prec_unknown())) {
Review Comment:
Can we use `self.maybe_parse()` instead of `try_parse()`? that would automatically handle the recursion exceeded case for example
--
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] Add support for PostgreSQL 'IN ' [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2025622523
##########
src/parser/mod.rs:
##########
@@ -3744,10 +3744,18 @@ impl<'a> Parser<'a> {
self.expect_token(&Token::LParen)?;
let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
self.prev_token();
- Expr::InSubquery {
- expr: Box::new(expr),
- subquery: self.parse_query()?,
- negated,
+ if self.dialect.supports_in_query_body() {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: self.parse_query_body(self.dialect.prec_unknown())?,
+ negated,
+ }
+ } else {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: Box::new(SetExpr::Query(self.parse_query()?)),
+ negated,
+ }
}
Review Comment:
I am not sure since that would allow parsing this syntax on dialects that don't actually support it, right?
--
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] Add support for 'IN ' [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2044112735
##########
src/parser/mod.rs:
##########
@@ -3742,24 +3742,23 @@ impl<'a> Parser<'a> {
});
}
self.expect_token(&Token::LParen)?;
- let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
- self.prev_token();
- Expr::InSubquery {
+ let in_op = match self.try_parse(|p| p.parse_query_body(p.dialect.prec_unknown())) {
Review Comment:
Done! Thank you, that is nicer.
--
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] Add support for 'IN ' [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2029486915
##########
src/parser/mod.rs:
##########
@@ -3742,24 +3742,23 @@ impl<'a> Parser<'a> {
});
}
self.expect_token(&Token::LParen)?;
- let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
- self.prev_token();
- Expr::InSubquery {
+ let in_op = match self.try_parse(|p| p.parse_query_body(p.dialect.prec_unknown())) {
Review Comment:
The previous approach peeked for a `SELECT` or `WITH`, but we cannot rely on that because there may be more than one parenthesis, as in `IN ((SELECT ...) UNION ())`.
The new approach tries to parse a subquery first, then an in list second.
##########
tests/sqlparser_common.rs:
##########
@@ -2224,7 +2224,21 @@ fn parse_in_subquery() {
assert_eq!(
Expr::InSubquery {
expr: Box::new(Expr::Identifier(Ident::new("segment"))),
- subquery: Box::new(verified_query("SELECT segm FROM bar")),
+ subquery: verified_query("SELECT segm FROM bar").body,
+ negated: false,
+ },
+ select.selection.unwrap()
+ );
+}
+
+#[test]
+fn parse_in_union() {
Review Comment:
I only added one test - happy to add more if deemed necessary.
--
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] Add support for 'IN ' [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2029485879
##########
src/parser/mod.rs:
##########
@@ -3744,10 +3744,18 @@ impl<'a> Parser<'a> {
self.expect_token(&Token::LParen)?;
let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
self.prev_token();
- Expr::InSubquery {
- expr: Box::new(expr),
- subquery: self.parse_query()?,
- negated,
+ if self.dialect.supports_in_query_body() {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: self.parse_query_body(self.dialect.prec_unknown())?,
+ negated,
+ }
+ } else {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: Box::new(SetExpr::Query(self.parse_query()?)),
+ negated,
+ }
}
Review Comment:
Okay, done, and fixed to work in all cases!
--
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] Add support for parenthesized subquery as `IN` predicate [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#issuecomment-2807686630
The code train keeps on moving in this repo. very impressive
--
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] Add support for parenthesized subquery as `IN` predicate [datafusion-sqlparser-rs]
Posted by "adamchainz (via GitHub)" <gi...@apache.org>.
adamchainz commented on PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#issuecomment-2807571169
Yay, 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@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] Add support for parenthesized subquery as `IN` predicate [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793
--
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] Add support for PostgreSQL 'IN ' [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1793:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1793#discussion_r2027491945
##########
src/parser/mod.rs:
##########
@@ -3744,10 +3744,18 @@ impl<'a> Parser<'a> {
self.expect_token(&Token::LParen)?;
let in_op = if self.parse_keyword(Keyword::SELECT) || self.parse_keyword(Keyword::WITH) {
self.prev_token();
- Expr::InSubquery {
- expr: Box::new(expr),
- subquery: self.parse_query()?,
- negated,
+ if self.dialect.supports_in_query_body() {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: self.parse_query_body(self.dialect.prec_unknown())?,
+ negated,
+ }
+ } else {
+ Expr::InSubquery {
+ expr: Box::new(expr),
+ subquery: Box::new(SetExpr::Query(self.parse_query()?)),
+ negated,
+ }
}
Review Comment:
Ah yeah I think that would be fine to have the parser always accept the syntax
--
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