You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "xitep (via GitHub)" <gi...@apache.org> on 2026/01/30 13:19:55 UTC
[PR] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
xitep opened a new pull request, #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185
* this is merely building on top of already existing "connect by" functionality for snowflake :)
* and making the `start with` clause optional [as per docs](https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/Hierarchical-Queries.html) and encountered in the wild
* the AST doesn't expose, however, the information which form was parse, i.e. either `START WITH .. CONNECT BY ..` or the other way around
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747034447
##########
tests/sqlparser_common.rs:
##########
@@ -12785,6 +12785,61 @@ fn parse_connect_by() {
"prior"
)))]
);
+
+ // no START WITH and NOCYCLE
+ let connect_by_5 = "SELECT child, parent FROM t CONNECT BY NOCYCLE parent = PRIOR child";
+ assert_eq!(
+ dialects.verified_only_select(connect_by_5),
+ Select {
+ select_token: AttachedToken::empty(),
+ optimizer_hint: None,
+ distinct: None,
+ top: None,
+ top_before_distinct: false,
+ projection: vec![
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("child"))),
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("parent"))),
+ ],
+ exclude: None,
+ from: vec![TableWithJoins {
+ relation: table_from_name(ObjectName::from(vec![Ident::new("t")])),
+ joins: vec![],
+ }],
+ into: None,
+ lateral_views: vec![],
+ prewhere: None,
+ selection: None,
+ group_by: GroupByExpr::Expressions(vec![], vec![]),
+ cluster_by: vec![],
+ distribute_by: vec![],
+ sort_by: vec![],
+ having: None,
+ named_window: vec![],
+ qualify: None,
+ window_before_qualify: false,
+ value_table_mode: None,
+ connect_by: Some(ConnectBy {
+ condition: None,
+ relationships: vec![Expr::BinaryOp {
+ left: Expr::Identifier(Ident::new("parent")).into(),
+ op: BinaryOperator::Eq,
+ right: Expr::Prior(Expr::Identifier(Ident::new("child")).into()).into(),
+ }],
+ nocycle: true,
+ }),
+ flavor: SelectFlavor::Standard,
+ }
+ );
+
+ // ~ CONNECT BY after WHERE and before GROUP BY
Review Comment:
:D old habits die hard, obviously! :facepalm: sorry for the troubles.
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2759217198
##########
src/parser/mod.rs:
##########
@@ -14184,27 +14188,27 @@ impl<'a> Parser<'a> {
}
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
- pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- (condition, relationships)
- } else {
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- self.expect_keywords(&[Keyword::CONNECT, Keyword::BY])?;
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- (condition, relationships)
- };
- Ok(ConnectBy {
- condition,
- relationships,
- })
+ pub fn maybe_parse_connect_by(&mut self) -> Result<Vec<ConnectByKind>, ParserError> {
+ let mut clauses = Vec::with_capacity(2);
+ loop {
+ if let Some(idx) = self.parse_keywords_(&[Keyword::START, Keyword::WITH]) {
Review Comment:
unfortunately that won't work. (i tried.)
the underlying method calls `parse_keyword -> peek_keyword -> peek_token_ref -> peek_nth_token_ref` will skip leading whitespace. therefore, an index remembered before invoking this call chain might point to a whitespace instead of the identified keyword.
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747050374
##########
src/ast/query.rs:
##########
@@ -374,6 +374,8 @@ pub struct Select {
pub prewhere: Option<Expr>,
/// WHERE
pub selection: Option<Expr>,
+ /// [START WITH ..] CONNECT BY ..
+ pub connect_by: Option<ConnectBy>,
Review Comment:
i've added the links, but it kind of feels like duplication; after all the two relevant links are just on top of the struct.
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#issuecomment-3854592785
@xitep could you take a look at the conflicts when you have some time?
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2746608179
##########
tests/sqlparser_common.rs:
##########
@@ -12785,6 +12785,61 @@ fn parse_connect_by() {
"prior"
)))]
);
+
+ // no START WITH and NOCYCLE
+ let connect_by_5 = "SELECT child, parent FROM t CONNECT BY NOCYCLE parent = PRIOR child";
+ assert_eq!(
+ dialects.verified_only_select(connect_by_5),
+ Select {
+ select_token: AttachedToken::empty(),
+ optimizer_hint: None,
+ distinct: None,
+ top: None,
+ top_before_distinct: false,
+ projection: vec![
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("child"))),
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("parent"))),
+ ],
+ exclude: None,
+ from: vec![TableWithJoins {
+ relation: table_from_name(ObjectName::from(vec![Ident::new("t")])),
+ joins: vec![],
+ }],
+ into: None,
+ lateral_views: vec![],
+ prewhere: None,
+ selection: None,
+ group_by: GroupByExpr::Expressions(vec![], vec![]),
+ cluster_by: vec![],
+ distribute_by: vec![],
+ sort_by: vec![],
+ having: None,
+ named_window: vec![],
+ qualify: None,
+ window_before_qualify: false,
+ value_table_mode: None,
+ connect_by: Some(ConnectBy {
+ condition: None,
+ relationships: vec![Expr::BinaryOp {
+ left: Expr::Identifier(Ident::new("parent")).into(),
+ op: BinaryOperator::Eq,
+ right: Expr::Prior(Expr::Identifier(Ident::new("child")).into()).into(),
+ }],
+ nocycle: true,
+ }),
+ flavor: SelectFlavor::Standard,
+ }
+ );
+
+ // ~ CONNECT BY after WHERE and before GROUP BY
Review Comment:
```suggestion
// CONNECT BY after WHERE and before GROUP BY
```
##########
src/parser/mod.rs:
##########
@@ -14185,25 +14185,35 @@ impl<'a> Parser<'a> {
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
+ let (condition, relationships, nocycle) = if self
+ .parse_keywords(&[Keyword::CONNECT, Keyword::BY])
+ {
+ let (relationships, nocycle) = self.with_state(ParserState::ConnectBy, |parser| {
Review Comment:
wondering a couple things around the current function impl
The current impl essentially duplicates the code in the if and else blocks. Since the goal is solely to parse in arbitrary order, can we rewrite it to use a loop instead? e.g.
```rust
loop {
if self.parse_keywords(STARTS, WITH) {
} else if self.parse_keywords(CONNECT, BY) {
} else {
break
}
}
```
I think the function would be much easier to follow as a result.
Also, can we instead of having three separate but fields `relationships` and `nocycle` and `condition`, can we use a representation like e.g.
```rust
enum ConnectByKind {
ConnectBy{
relationships: Vec<Expr>,
nocycle: bool,
},
StartsWith(Expr)
}
connect_by: Vec<ConnectByKind>
```
and the order in which the clauses show up in the query will be preserved as a result too (and we skip the consideration of whether snowflake supports specific ordering)
##########
src/parser/mod.rs:
##########
@@ -14000,6 +14000,17 @@ impl<'a> Parser<'a> {
None
};
+ let connect_by = if self.dialect.supports_connect_by()
+ && self
+ .parse_one_of_keywords(&[Keyword::START, Keyword::CONNECT])
+ .is_some()
+ {
+ self.prev_token();
Review Comment:
```suggestion
&& self
.peek_one_of_keywords(&[Keyword::START, Keyword::CONNECT])
.is_some()
{
```
would this be equivalent?
##########
src/ast/query.rs:
##########
@@ -374,6 +374,8 @@ pub struct Select {
pub prewhere: Option<Expr>,
/// WHERE
pub selection: Option<Expr>,
+ /// [START WITH ..] CONNECT BY ..
+ pub connect_by: Option<ConnectBy>,
Review Comment:
Can we add a link to the docs where this syntax comes from?
##########
src/ast/query.rs:
##########
@@ -1094,23 +1094,34 @@ impl fmt::Display for TableWithJoins {
/// Joins a table to itself to process hierarchical data in the table.
///
/// See <https://docs.snowflake.com/en/sql-reference/constructs/connect-by>.
+/// See <https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/Hierarchical-Queries.html>
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub struct ConnectBy {
/// START WITH
- pub condition: Expr,
+ pub condition: Option<Expr>,
/// CONNECT BY
pub relationships: Vec<Expr>,
+ /// [CONNECT BY] NOCYCLE
+ pub nocycle: bool,
Review Comment:
Similarly, can we add a link to the docs for this 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
Re: [PR] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2759217198
##########
src/parser/mod.rs:
##########
@@ -14184,27 +14188,27 @@ impl<'a> Parser<'a> {
}
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
- pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- (condition, relationships)
- } else {
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- self.expect_keywords(&[Keyword::CONNECT, Keyword::BY])?;
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- (condition, relationships)
- };
- Ok(ConnectBy {
- condition,
- relationships,
- })
+ pub fn maybe_parse_connect_by(&mut self) -> Result<Vec<ConnectByKind>, ParserError> {
+ let mut clauses = Vec::with_capacity(2);
+ loop {
+ if let Some(idx) = self.parse_keywords_(&[Keyword::START, Keyword::WITH]) {
Review Comment:
unfortunately that won't work. (i tried.)
the underlying method calls `parse_keyword -> peek_keyword -> peek_token_ref -> peek_nth_token_ref` will skip leading whitespace. therefore, remembering the index before invoking this call chain might point to a whitespace instead of the identified keyword.
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747100412
##########
src/parser/mod.rs:
##########
@@ -14185,25 +14185,35 @@ impl<'a> Parser<'a> {
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
+ let (condition, relationships, nocycle) = if self
+ .parse_keywords(&[Keyword::CONNECT, Keyword::BY])
+ {
+ let (relationships, nocycle) = self.with_state(ParserState::ConnectBy, |parser| {
Review Comment:
yes, that's good idea! and i'll go for it, since `sqlparser-rs` wants to be generic and cover many dialects.
however, there's a subtle difference between the two branches:
at least for `Oracle`, an initial `START WITH ...` _must be followed_ by a `CONNECT BY ...`; whereas an initial `CONNECT BY` _may have_ a `START WITH ...` follower.
with the `Vec<ConnectByKind>`, one can validate this afterwards; similarly the `loop` implies that a `CONNECT BY ... START WITH ... CONNECT BY ... CONNECT BY` would be parsed successfully, which the client would have to validate (and very likely reject.)
i guess it boils down to what `sqlparser-rs` wants be, either "strictly-typed" or "duck-typed", figuratively speaking. personally i'm rather inclined towards the former, since it takes away the burden from the
clients and centralises know-how into the (shared) library. (just like most of the time, it's simply a trade-off.)
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747010806
##########
src/parser/mod.rs:
##########
@@ -14000,6 +14000,17 @@ impl<'a> Parser<'a> {
None
};
+ let connect_by = if self.dialect.supports_connect_by()
+ && self
+ .parse_one_of_keywords(&[Keyword::START, Keyword::CONNECT])
+ .is_some()
+ {
+ self.prev_token();
Review Comment:
:ok_hand: yes! thanks
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2759852359
##########
src/parser/mod.rs:
##########
@@ -4521,16 +4521,29 @@ impl<'a> Parser<'a> {
/// consumed and returns false
#[must_use]
pub fn parse_keywords(&mut self, keywords: &[Keyword]) -> bool {
- let index = self.index;
- for &keyword in keywords {
- if !self.parse_keyword(keyword) {
- // println!("parse_keywords aborting .. did not find {:?}", keyword);
- // reset index and return immediately
- self.index = index;
- return false;
+ self.parse_keywords_(keywords).is_some()
+ }
+
+ /// Just like [Self::parse_keywords], but - upon success - returns the
+ /// token index of the first keyword.
+ #[must_use]
+ fn parse_keywords_(&mut self, keywords: &[Keyword]) -> Option<usize> {
Review Comment:
```suggestion
fn parse_keywords_indexed(&mut self, keywords: &[Keyword]) -> Option<usize> {
```
##########
src/parser/mod.rs:
##########
@@ -14184,27 +14188,27 @@ impl<'a> Parser<'a> {
}
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
- pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- (condition, relationships)
- } else {
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- self.expect_keywords(&[Keyword::CONNECT, Keyword::BY])?;
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- (condition, relationships)
- };
- Ok(ConnectBy {
- condition,
- relationships,
- })
+ pub fn maybe_parse_connect_by(&mut self) -> Result<Vec<ConnectByKind>, ParserError> {
+ let mut clauses = Vec::with_capacity(2);
+ loop {
+ if let Some(idx) = self.parse_keywords_(&[Keyword::START, Keyword::WITH]) {
Review Comment:
I see, that makes sense!
##########
src/parser/mod.rs:
##########
@@ -4521,16 +4521,29 @@ impl<'a> Parser<'a> {
/// consumed and returns false
#[must_use]
pub fn parse_keywords(&mut self, keywords: &[Keyword]) -> bool {
- let index = self.index;
- for &keyword in keywords {
- if !self.parse_keyword(keyword) {
- // println!("parse_keywords aborting .. did not find {:?}", keyword);
- // reset index and return immediately
- self.index = index;
- return false;
+ self.parse_keywords_(keywords).is_some()
+ }
+
+ /// Just like [Self::parse_keywords], but - upon success - returns the
+ /// token index of the first keyword.
+ #[must_use]
+ fn parse_keywords_(&mut self, keywords: &[Keyword]) -> Option<usize> {
+ let start_index = self.index;
+ let mut first_keyword_index = None;
+ match keywords {
+ [keyword, keywords @ ..] if self.parse_keyword(*keyword) => {
+ first_keyword_index = Some(self.index.saturating_sub(1));
+ for &keyword in keywords {
+ if !self.parse_keyword(keyword) {
+ self.index = start_index;
+ first_keyword_index = None;
+ break;
+ }
+ }
}
+ _ => {}
}
- true
+ first_keyword_index
Review Comment:
can we rewrite to something like this to follow the previous pattern? (with the slice matching and calling parse_keyword multiple times, makes the new version slightly harder to follow)
```rust
let start_index = self.index;
let mut first_keyword_index = None;
for keyword in keywords {
if !self.parse_keyword(*keyword) {
self.index = start_index;
return None
}
if first_keyword_index.is_none() {
first_keyword_index = Some(self.index.saturating_sub(1));
}
}
first_keyword_index
```
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2746255644
##########
src/ast/query.rs:
##########
@@ -475,6 +475,10 @@ impl fmt::Display for Select {
SpaceOrNewline.fmt(f)?;
Indent(selection).fmt(f)?;
}
+ if let Some(ref connect_by) = self.connect_by {
+ SpaceOrNewline.fmt(f)?;
+ connect_by.fmt(f)?;
+ }
Review Comment:
i can't verify this change for snowflake, but [this doc](https://docs.snowflake.com/en/sql-reference/constructs) indicates to me that it's ok
##########
src/ast/query.rs:
##########
@@ -374,6 +374,8 @@ pub struct Select {
pub prewhere: Option<Expr>,
/// WHERE
pub selection: Option<Expr>,
+ /// START WITH .. CONNECT BY
Review Comment:
```suggestion
/// [START WITH ..] CONNECT BY ..
```
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747034447
##########
tests/sqlparser_common.rs:
##########
@@ -12785,6 +12785,61 @@ fn parse_connect_by() {
"prior"
)))]
);
+
+ // no START WITH and NOCYCLE
+ let connect_by_5 = "SELECT child, parent FROM t CONNECT BY NOCYCLE parent = PRIOR child";
+ assert_eq!(
+ dialects.verified_only_select(connect_by_5),
+ Select {
+ select_token: AttachedToken::empty(),
+ optimizer_hint: None,
+ distinct: None,
+ top: None,
+ top_before_distinct: false,
+ projection: vec![
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("child"))),
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("parent"))),
+ ],
+ exclude: None,
+ from: vec![TableWithJoins {
+ relation: table_from_name(ObjectName::from(vec![Ident::new("t")])),
+ joins: vec![],
+ }],
+ into: None,
+ lateral_views: vec![],
+ prewhere: None,
+ selection: None,
+ group_by: GroupByExpr::Expressions(vec![], vec![]),
+ cluster_by: vec![],
+ distribute_by: vec![],
+ sort_by: vec![],
+ having: None,
+ named_window: vec![],
+ qualify: None,
+ window_before_qualify: false,
+ value_table_mode: None,
+ connect_by: Some(ConnectBy {
+ condition: None,
+ relationships: vec![Expr::BinaryOp {
+ left: Expr::Identifier(Ident::new("parent")).into(),
+ op: BinaryOperator::Eq,
+ right: Expr::Prior(Expr::Identifier(Ident::new("child")).into()).into(),
+ }],
+ nocycle: true,
+ }),
+ flavor: SelectFlavor::Standard,
+ }
+ );
+
+ // ~ CONNECT BY after WHERE and before GROUP BY
Review Comment:
:D old habits die hard, obviously! (i'm sorry for the troubles.)
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2749282720
##########
src/parser/mod.rs:
##########
@@ -14185,25 +14185,35 @@ impl<'a> Parser<'a> {
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
+ let (condition, relationships, nocycle) = if self
+ .parse_keywords(&[Keyword::CONNECT, Keyword::BY])
+ {
+ let (relationships, nocycle) = self.with_state(ParserState::ConnectBy, |parser| {
Review Comment:
fyi; i just realised that for validation and error reporting we'll need at least to have the start tokens of the corresponding "kind" clauses; i'll extend the `ConnectByKind` to this:
```rust
enum ConnectByKind {
ConnectBy{
connect_token: AttachedToken,
relationships: Vec<Expr>,
nocycle: bool,
},
StartsWith {
start_token: AttachedToken,
condition: Expr
}
}
```
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2759042694
##########
src/parser/mod.rs:
##########
@@ -14184,27 +14188,27 @@ impl<'a> Parser<'a> {
}
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
- pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- (condition, relationships)
- } else {
- self.expect_keywords(&[Keyword::START, Keyword::WITH])?;
- let condition = self.parse_expr()?;
- self.expect_keywords(&[Keyword::CONNECT, Keyword::BY])?;
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
- })?;
- (condition, relationships)
- };
- Ok(ConnectBy {
- condition,
- relationships,
- })
+ pub fn maybe_parse_connect_by(&mut self) -> Result<Vec<ConnectByKind>, ParserError> {
+ let mut clauses = Vec::with_capacity(2);
+ loop {
+ if let Some(idx) = self.parse_keywords_(&[Keyword::START, Keyword::WITH]) {
Review Comment:
can we instead save `let idx = self.index` in the loop before calling the parse method? looks like that would let us avoid introducing the `parse_keywords_` method?
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#issuecomment-3854976182
should be good now, i rebased. please squash merge the whole pile of small commits. last but not least, many thanks for your support @iffyio
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2760575410
##########
src/parser/mod.rs:
##########
@@ -4521,16 +4521,29 @@ impl<'a> Parser<'a> {
/// consumed and returns false
#[must_use]
pub fn parse_keywords(&mut self, keywords: &[Keyword]) -> bool {
- let index = self.index;
- for &keyword in keywords {
- if !self.parse_keyword(keyword) {
- // println!("parse_keywords aborting .. did not find {:?}", keyword);
- // reset index and return immediately
- self.index = index;
- return false;
+ self.parse_keywords_(keywords).is_some()
+ }
+
+ /// Just like [Self::parse_keywords], but - upon success - returns the
+ /// token index of the first keyword.
+ #[must_use]
+ fn parse_keywords_(&mut self, keywords: &[Keyword]) -> Option<usize> {
Review Comment:
i struggled with a good name; thanks for the suggestion
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2746244546
##########
src/ast/query.rs:
##########
@@ -374,6 +374,8 @@ pub struct Select {
pub prewhere: Option<Expr>,
/// WHERE
pub selection: Option<Expr>,
+ /// STARTING WITH .. CONNECT BY
Review Comment:
```suggestion
/// START WITH .. CONNECT BY
```
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2747034447
##########
tests/sqlparser_common.rs:
##########
@@ -12785,6 +12785,61 @@ fn parse_connect_by() {
"prior"
)))]
);
+
+ // no START WITH and NOCYCLE
+ let connect_by_5 = "SELECT child, parent FROM t CONNECT BY NOCYCLE parent = PRIOR child";
+ assert_eq!(
+ dialects.verified_only_select(connect_by_5),
+ Select {
+ select_token: AttachedToken::empty(),
+ optimizer_hint: None,
+ distinct: None,
+ top: None,
+ top_before_distinct: false,
+ projection: vec![
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("child"))),
+ SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("parent"))),
+ ],
+ exclude: None,
+ from: vec![TableWithJoins {
+ relation: table_from_name(ObjectName::from(vec![Ident::new("t")])),
+ joins: vec![],
+ }],
+ into: None,
+ lateral_views: vec![],
+ prewhere: None,
+ selection: None,
+ group_by: GroupByExpr::Expressions(vec![], vec![]),
+ cluster_by: vec![],
+ distribute_by: vec![],
+ sort_by: vec![],
+ having: None,
+ named_window: vec![],
+ qualify: None,
+ window_before_qualify: false,
+ value_table_mode: None,
+ connect_by: Some(ConnectBy {
+ condition: None,
+ relationships: vec![Expr::BinaryOp {
+ left: Expr::Identifier(Ident::new("parent")).into(),
+ op: BinaryOperator::Eq,
+ right: Expr::Prior(Expr::Identifier(Ident::new("child")).into()).into(),
+ }],
+ nocycle: true,
+ }),
+ flavor: SelectFlavor::Standard,
+ }
+ );
+
+ // ~ CONNECT BY after WHERE and before GROUP BY
Review Comment:
:D getting rid of old habits is - obviously - quite difficult :D
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2749282720
##########
src/parser/mod.rs:
##########
@@ -14185,25 +14185,35 @@ impl<'a> Parser<'a> {
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
+ let (condition, relationships, nocycle) = if self
+ .parse_keywords(&[Keyword::CONNECT, Keyword::BY])
+ {
+ let (relationships, nocycle) = self.with_state(ParserState::ConnectBy, |parser| {
Review Comment:
fyi; i just realised that for validation and error reporting we'll need at least to have the start tokens of the corresponding "kind" clauses; i'll extend `ConnectByKind` to this:
```rust
enum ConnectByKind {
ConnectBy{
connect_token: AttachedToken,
relationships: Vec<Expr>,
nocycle: bool,
},
StartsWith {
start_token: AttachedToken,
condition: Expr
}
}
```
--
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] [Oracle] Support hierarchical queries [datafusion-sqlparser-rs]
Posted by "xitep (via GitHub)" <gi...@apache.org>.
xitep commented on code in PR #2185:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2185#discussion_r2749282720
##########
src/parser/mod.rs:
##########
@@ -14185,25 +14185,35 @@ impl<'a> Parser<'a> {
/// Parse a `CONNECT BY` clause (Oracle-style hierarchical query support).
pub fn parse_connect_by(&mut self) -> Result<ConnectBy, ParserError> {
- let (condition, relationships) = if self.parse_keywords(&[Keyword::CONNECT, Keyword::BY]) {
- let relationships = self.with_state(ParserState::ConnectBy, |parser| {
- parser.parse_comma_separated(Parser::parse_expr)
+ let (condition, relationships, nocycle) = if self
+ .parse_keywords(&[Keyword::CONNECT, Keyword::BY])
+ {
+ let (relationships, nocycle) = self.with_state(ParserState::ConnectBy, |parser| {
Review Comment:
fyi; i just realised that for validation and error reporting we'll need at least to have the start tokens of the corresponding "kind" clauses; i'll extend `ConnectByKind` to this:
```rust
enum ConnectByKind {
ConnectBy{
connect_token: AttachedToken,
nocycle: bool,
relationships: Vec<Expr>,
},
StartsWith {
start_token: AttachedToken,
condition: Expr
}
}
```
--
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