Re: [PR] Fix: JOIN should require ON condition [datafusion-sqlparser-rs]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #1552:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1552#discussion_r1866317958


##########
src/dialect/mod.rs:
##########
@@ -687,6 +687,41 @@ pub trait Dialect: Debug + Any {
     fn is_reserved_for_identifier(&self, kw: Keyword) -> bool {
         keywords::RESERVED_FOR_IDENTIFIER.contains(&kw)
     }
+
+    /// Verifies whether the provided `JoinOperator` is supported by this SQL dialect.
+    /// Returns `true` if the `JoinOperator` is supported, otherwise `false`.
+    fn verify_join_operator(&self, _join_operator: &JoinOperator) -> bool {
+        true
+    }
+
+    /// Verifies if the given `JoinOperator`'s constraint is valid for this SQL dialect.
+    /// Returns `true` if the join constraint is valid, otherwise `false`.
+    fn verify_join_constraint(&self, join_operator: &JoinOperator) -> bool {

Review Comment:
   > * Should this behavior be considered a bug?
   
   I don't have a strong opinion -- is it causing anyone problems? It seems like the ramification of allowing `JOIN`... as `CROSS JOIN` 's largest implication is that now DataFusion has some new dialect. 
   
   If it isn't causing problems, my suggestion is do nothing until someone has a concrete usecase
   
   
   



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