alamb commented on code in PR #11423:
URL: https://github.com/apache/datafusion/pull/11423#discussion_r1675699247
##########
datafusion/sql/src/relation/join.rs:
##########
@@ -107,7 +110,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
JoinConstraint::On(sql_expr) => {
let join_schema = left.schema().join(right.schema())?;
// parse ON expression
- let expr = self.sql_to_expr(sql_expr, &join_schema,
planner_context)?;
+ let mut expr =
+ self.sql_to_expr(sql_expr, &join_schema, planner_context)?;
+ // ON expression must be a boolean expression
+ match expr.get_type(&join_schema)? {
+ DataType::Boolean => {}
+ DataType::Null => {
+ expr = Expr::Cast(Cast::new(expr.into(),
DataType::Boolean))
+ }
+ other => {
+ return plan_err!(
+ "ON expression must be boolean, but got {other:?}"
Review Comment:
I think the same rule applies to other APIs (not just SQL).
What would you think about moving the check to `LogicalPlanBuilder`
https://github.com/apache/datafusion/blob/6e637488188c6620ecd113bf47987bd98f6d7871/datafusion/expr/src/logical_plan/builder.rs#L671-L686
That way this check would be applied to the DataFrame API as well as any
custom plans that were created?
##########
datafusion/sqllogictest/test_files/join.slt:
##########
@@ -998,11 +997,22 @@ CREATE TABLE t2 (v0 DOUBLE) AS VALUES (-1.663563947387);
statement ok
CREATE TABLE t3 (v0 DOUBLE) AS VALUES (0.05112015193508901);
+# Test issue: https://github.com/apache/datafusion/issues/11269
query RR
SELECT t3.v0, t2.v0 FROM t1,t2,t3 WHERE t3.v0 >= t1.v0;
----
0.051120151935 -1.663563947387
+# Test issue: https://github.com/apache/datafusion/issues/11414
+query IRR
+SELECT * FROM t1 INNER JOIN t2 ON NULL RIGHT JOIN t3 ON TRUE;
+----
+NULL NULL 0.051120151935
+
+# ON expression must be boolean type
+query error DataFusion error: Error during planning: ON expression must be
boolean, but got Utf8
+SELECT * FROM t1 INNER JOIN t2 ON 'TRUE'
Review Comment:
I think the error is clear and is a reasonable behavior. If it is important
for someone's usecase, perhaps we can add support for it then
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
