LucaCappelletti94 commented on code in PR #2606:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2606#discussion_r4173632404


##########
tests/sqlparser_mssql.rs:
##########
@@ -2963,3 +2963,70 @@ fn parse_create_proc() {
         .expect_err("PROC should remain MSSQL-specific");
     ms_and_generic().verified_stmt("SELECT proc FROM jobs");
 }
+
+#[test]
+fn parse_mssql_deferred_join_conditions() {
+    let sql = "SELECT a.id FROM a LEFT JOIN b INNER JOIN c ON b.id = c.id ON 
a.id = b.id";
+    let statement = ms().one_statement_parses_to(
+        sql,
+        "SELECT a.id FROM a LEFT JOIN (b INNER JOIN c ON b.id = c.id) ON a.id 
= b.id",
+    );
+    let Statement::Query(query) = statement else {
+        unreachable!()
+    };
+    let select = query.body.as_select().unwrap();
+    let outer_join = only(&only(&select.from).joins);
+    assert!(matches!(&outer_join.join_operator,
+        JoinOperator::Left(JoinConstraint::On(expr)) | 
JoinOperator::LeftOuter(JoinConstraint::On(expr))
+            if expr.to_string() == "a.id = b.id"));
+    let TableFactor::NestedJoin {
+        table_with_joins,
+        alias,
+    } = &outer_join.relation
+    else {
+        panic!("expected right-nested join");
+    };
+    assert!(alias.is_none());
+    let inner_join = only(&table_with_joins.joins);
+    assert!(matches!(&inner_join.join_operator,
+        JoinOperator::Join(JoinConstraint::On(expr)) | 
JoinOperator::Inner(JoinConstraint::On(expr))
+            if expr.to_string() == "b.id = c.id"));
+
+    for (sql, expected) in [
+        (
+            "SELECT * FROM a JOIN b JOIN c ON b.id = c.id ON a.id = b.id",
+            "SELECT * FROM a JOIN (b JOIN c ON b.id = c.id) ON a.id = b.id",
+        ),
+        (
+            "SELECT * FROM a FULL JOIN b RIGHT JOIN c ON b.id = c.id ON a.id = 
b.id",
+            "SELECT * FROM a FULL JOIN (b RIGHT JOIN c ON b.id = c.id) ON a.id 
= b.id",
+        ),
+        (
+            "SELECT * FROM a JOIN b JOIN c JOIN d ON c.id = d.id ON b.id = 
c.id ON a.id = b.id",
+            "SELECT * FROM a JOIN (b JOIN (c JOIN d ON c.id = d.id) ON b.id = 
c.id) ON a.id = b.id",
+        ),
+    ] {
+        ms().one_statement_parses_to(sql, expected);
+    }
+}

Review Comment:
   Recursion red test
   
   ```suggestion
   }
   
   #[test]
   fn parse_mssql_deferred_join_enforces_recursion_limit() {
       let mut sql = String::from("SELECT * FROM t0");
       for index in 1..=100 {
           sql.push_str(&format!(" JOIN t{index}"));
       }
       for _ in 1..=100 {
           sql.push_str(" ON 1 = 1");
       }
       let result = Parser::new(&MsSqlDialect {})
           .with_recursion_limit(5)
           .try_with_sql(&sql)
           .and_then(|mut parser| parser.parse_statements());
       assert_eq!(result, Err(ParserError::RecursionLimitExceeded));
   }
   ```



##########
src/dialect/mssql.rs:
##########
@@ -65,6 +65,10 @@ impl Dialect for MsSqlDialect {
         true
     }
 
+    fn supports_left_associative_joins_without_parens(&self) -> bool {

Review Comment:
   I suspect there may still be recursion stack overflows relative to this 
feature. Please bound join-parser recursion before enabling this dialect and 
assert `RecursionLimitExceeded` on a long chain with a small configured limit
   
   The shared parser guard is tracked in 
[#2440](https://github.com/apache/datafusion-sqlparser-rs/pull/2440).



-- 
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]

Reply via email to