Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 29.4k
[SPARK-25314][SQL] Fix Python UDF accessing attributes from both side of join in join conditions#22326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-25314][SQL] Fix Python UDF accessing attributes from both side of join in join conditions #22326
Changes from all commits
9c579bd9ea1cf6b6b0aa6b626fa753dd0284ca7fd11109eb3c6345fe83660d5fdc86cafbf32f4292b09c6749a96a598a4ea2c8ddd005bb3fb0dfab3306fcb998cd3ccd1db33a87f0f50d2739af7f669542b6977dFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -552,6 +552,70 @@ def test_udf_in_filter_on_top_of_join(self): | ||
| df = left.crossJoin(right).filter(f("a", "b")) | ||
| self.assertEqual(df.collect(), [Row(a=1, b=1)]) | ||
| def test_udf_in_join_condition(self): | ||
| # regression test for SPARK-25314 | ||
| from pyspark.sql.functions import udf | ||
| left = self.spark.createDataFrame([Row(a=1)]) | ||
| right = self.spark.createDataFrame([Row(b=1)]) | ||
| f = udf(lambda a, b: a == b, BooleanType()) | ||
| df = left.join(right, f("a", "b")) | ||
| with self.assertRaisesRegexp(AnalysisException, 'Detected implicit cartesian product'): | ||
| df.collect() | ||
| with self.sql_conf({"spark.sql.crossJoin.enabled": True}): | ||
| self.assertEqual(df.collect(), [Row(a=1, b=1)]) | ||
| def test_udf_in_left_semi_join_condition(self): | ||
| # regression test for SPARK-25314 | ||
| from pyspark.sql.functions import udf | ||
| left = self.spark.createDataFrame([Row(a=1, a1=1, a2=1), Row(a=2, a1=2, a2=2)]) | ||
| right = self.spark.createDataFrame([Row(b=1, b1=1, b2=1)]) | ||
| f = udf(lambda a, b: a == b, BooleanType()) | ||
| df = left.join(right, f("a", "b"), "leftsemi") | ||
| with self.assertRaisesRegexp(AnalysisException, 'Detected implicit cartesian product'): | ||
| df.collect() | ||
| with self.sql_conf({"spark.sql.crossJoin.enabled": True}): | ||
| self.assertEqual(df.collect(), [Row(a=1, a1=1, a2=1)]) | ||
| def test_udf_and_common_filter_in_join_condition(self): | ||
xuanyuanking marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| # regression test for SPARK-25314 | ||
| # test the complex scenario with both udf and common filter | ||
| from pyspark.sql.functions import udf | ||
| left = self.spark.createDataFrame([Row(a=1, a1=1, a2=1), Row(a=2, a1=2, a2=2)]) | ||
| right = self.spark.createDataFrame([Row(b=1, b1=1, b2=1), Row(b=1, b1=3, b2=1)]) | ||
| f = udf(lambda a, b: a == b, BooleanType()) | ||
xuanyuanking marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| df = left.join(right, [f("a", "b"), left.a1 == right.b1]) | ||
| # do not need spark.sql.crossJoin.enabled=true for udf is not the only join condition. | ||
| self.assertEqual(df.collect(), [Row(a=1, a1=1, a2=1, b=1, b1=1, b2=1)]) | ||
| def test_udf_and_common_filter_in_left_semi_join_condition(self): | ||
| # regression test for SPARK-25314 | ||
| # test the complex scenario with both udf and common filter | ||
| from pyspark.sql.functions import udf | ||
| left = self.spark.createDataFrame([Row(a=1, a1=1, a2=1), Row(a=2, a1=2, a2=2)]) | ||
| right = self.spark.createDataFrame([Row(b=1, b1=1, b2=1), Row(b=1, b1=3, b2=1)]) | ||
| f = udf(lambda a, b: a == b, BooleanType()) | ||
| df = left.join(right, [f("a", "b"), left.a1 == right.b1], "left_semi") | ||
| # do not need spark.sql.crossJoin.enabled=true for udf is not the only join condition. | ||
| self.assertEqual(df.collect(), [Row(a=1, a1=1, a2=1)]) | ||
| def test_udf_not_supported_in_join_condition(self): | ||
xuanyuanking marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| # regression test for SPARK-25314 | ||
| # test python udf is not supported in join type besides left_semi and inner join. | ||
| from pyspark.sql.functions import udf | ||
| left = self.spark.createDataFrame([Row(a=1, a1=1, a2=1), Row(a=2, a1=2, a2=2)]) | ||
| right = self.spark.createDataFrame([Row(b=1, b1=1, b2=1), Row(b=1, b1=3, b2=1)]) | ||
| f = udf(lambda a, b: a == b, BooleanType()) | ||
| def runWithJoinType(join_type, type_string): | ||
| with self.assertRaisesRegexp( | ||
| AnalysisException, | ||
| 'Using PythonUDF.*%s is not supported.' % type_string): | ||
| left.join(right, [f("a", "b"), left.a1 == right.b1], join_type).collect() | ||
| runWithJoinType("full", "FullOuter") | ||
| runWithJoinType("left", "LeftOuter") | ||
| runWithJoinType("right", "RightOuter") | ||
| runWithJoinType("leftanti", "LeftAnti") | ||
| def test_udf_without_arguments(self): | ||
| self.spark.catalog.registerFunction("foo", lambda: "bar") | ||
| [row] = self.spark.sql("SELECT foo()").collect() | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -19,6 +19,7 @@ package org.apache.spark.sql.catalyst.optimizer | ||||||||
| import scala.annotation.tailrec | ||||||||
| import org.apache.spark.sql.AnalysisException | ||||||||
| import org.apache.spark.sql.catalyst.expressions._ | ||||||||
| import org.apache.spark.sql.catalyst.planning.ExtractFiltersAndInnerJoins | ||||||||
| import org.apache.spark.sql.catalyst.plans._ | ||||||||
| @@ -152,3 +153,51 @@ object EliminateOuterJoin extends Rule[LogicalPlan] with PredicateHelper { | ||||||||
| if (j.joinType == newJoinType) f else Filter(condition, j.copy(joinType = newJoinType)) | ||||||||
| } | ||||||||
| } | ||||||||
| /** | ||||||||
| * PythonUDF in join condition can not be evaluated, this rule will detect the PythonUDF | ||||||||
| * and pull them out from join condition. For python udf accessing attributes from only one side, | ||||||||
| * they are pushed down by operation push down rules. If not (e.g. user disables filter push | ||||||||
| * down rules), we need to pull them out in this rule too. | ||||||||
| */ | ||||||||
| object PullOutPythonUDFInJoinCondition extends Rule[LogicalPlan] with PredicateHelper { | ||||||||
| def hasPythonUDF(expression: Expression): Boolean = { | ||||||||
| expression.collectFirst { case udf: PythonUDF => udf }.isDefined | ||||||||
xuanyuanking marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||||||||
| } | ||||||||
| override def apply(plan: LogicalPlan): LogicalPlan = plan transformUp { | ||||||||
| case j @ Join(_, _, joinType, condition) | ||||||||
| if condition.isDefined && hasPythonUDF(condition.get) => | ||||||||
| if (!joinType.isInstanceOf[InnerLike] && joinType != LeftSemi) { | ||||||||
xuanyuanking marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||||||||
| // The current strategy only support InnerLike and LeftSemi join because for other type, | ||||||||
| // it breaks SQL semantic if we run the join condition as a filter after join. If we pass | ||||||||
| // the plan here, it'll still get a an invalid PythonUDF RuntimeException with message | ||||||||
| // `requires attributes from more than one child`, we throw firstly here for better | ||||||||
| // readable information. | ||||||||
| throw new AnalysisException("Using PythonUDF in join condition of join type" + | ||||||||
| s" $joinType is not supported.") | ||||||||
| } | ||||||||
| // If condition expression contains python udf, it will be moved out from | ||||||||
| // the new join conditions. | ||||||||
| val (udf, rest) = | ||||||||
| splitConjunctivePredicates(condition.get).partition(hasPythonUDF) | ||||||||
| val newCondition = if (rest.isEmpty) { | ||||||||
| logWarning(s"The join condition:$condition of the join plan contains PythonUDF only," + | ||||||||
| s" it will be moved out and the join plan will be turned to cross join.") | ||||||||
| None | ||||||||
| } else { | ||||||||
| Some(rest.reduceLeft(And)) | ||||||||
| } | ||||||||
| val newJoin = j.copy(condition = newCondition) | ||||||||
| joinType match { | ||||||||
| case _: InnerLike => Filter(udf.reduceLeft(And), newJoin) | ||||||||
| case LeftSemi => | ||||||||
| Project( | ||||||||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we are simulating a left semi join here. Seems we can do the same thing for left anti join. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me try. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried two ways to implement LeftAnti here:
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, let's leave left anti join then, thanks for trying! MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks :) | ||||||||
| j.left.output.map(_.toAttribute), | ||||||||
| Filter(udf.reduceLeft(And), newJoin.copy(joinType = Inner))) | ||||||||
| case _ => | ||||||||
| throw new AnalysisException("Using PythonUDF in join condition of join type" + | ||||||||
| s" $joinType is not supported.") | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
Uh oh!
There was an error while loading. Please reload this page.