iffyio commented on code in PR #2344:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2344#discussion_r3279776711
##########
src/parser/mod.rs:
##########
@@ -2050,14 +2052,9 @@ impl<'a> Parser<'a> {
})?;
match expr {
- // If we get back a compound field access or
identifier,
- // we flatten the nested expression.
- // For example if the current root is `foo`
- // and we get back a compound identifier
expression `bar.baz`
- // The full expression should be `foo.bar.baz`
(i.e.
- // a root with an access chain with 2 entries) and
not
- // `foo.(bar.baz)` (i.e. a root with an access
chain with
- // 1 entry`).
+ // `parse_prefix` does not itself follow compound
chains, but a
+ // dialect override could still return a compound
expression, so
+ // keep the flatten arms for safety.
Review Comment:
is the previous comment no longer valid? the new comment reads like it would
be confusing without context on the diff in this PR
##########
tests/sqlparser_postgres.rs:
##########
@@ -9243,3 +9243,27 @@ fn parse_lock_table() {
}
}
}
+
+/// `parse_compound_expr` used to do 2^N work on `IF a.b.c...x.#` because every
+/// `.` re-entered `parse_subexpr` over the rest of the chain.
+#[test]
+fn parse_compound_chain_no_exponential_blowup() {
Review Comment:
given the functionality isn't pg specific, can we maybe move this test to be
a benchmark test?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
