[SPARK-12275][SQL] No plan for BroadcastHint in some condition #10265
Conversation
|
Hi, @yucai , my code is executed successfully with this pr. It's great. |
|
@zzcclp Good to know :). |
|
LGTM |
|
ok to test. @yhuai can you verify? |
|
Test build #47592 has finished for PR 10265 at commit
|
|
The failure seems not related to this change. |
| @@ -364,7 +364,7 @@ private[sql] abstract class SparkStrategies extends QueryPlanner[SparkPlan] { | |||
| case e @ EvaluatePython(udf, child, _) => | |||
| BatchPythonEvaluation(udf, e.output, planLater(child)) :: Nil | |||
| case LogicalRDD(output, rdd) => PhysicalRDD(output, rdd, "ExistingRDD") :: Nil | |||
| case BroadcastHint(child) => apply(child) | |||
| case BroadcastHint(child) => planLater(child) :: Nil | |||
There was a problem hiding this comment.
This is quite obvious, apply method will only make the rules within the object BasicOperators applied, but the data source(Parquet) is an extended strategy defined in another object, planLater means to search the all of the strategies(including the extensions), which is make more sense. That's why it caused exception says No plan for BroadcastHint as apply(child) returns Nil.
|
Weird, I find no difference for org.apache.spark.sql.sources.JsonHadoopFsRelationSuite#"test all data types - TimestampType" in Jekins's test report. And also, I tried both org.apache.spark.sql.sources.JsonHadoopFsRelationSuite and org.apache.spark.sql.DataFrameJoinSuite with the latest Spark upstream + my PR locally, no failure also. What's wrong :( ? Jekins, can you retest this please? |
|
retest this please |
|
@andrewor14 , @yhuai could you kindly help re-trigger testing? Much thanks! |
|
ok to test |
|
test this please |
|
LGTM pending jenkins |
|
When merge this, let's also merge it to branch 1.5, 1.6, and master. |
|
Test build #47642 has finished for PR 10265 at commit
|
|
Thanks! Merging to 1.5, 1.6, and master. |
When SparkStrategies.BasicOperators's "case BroadcastHint(child) => apply(child)" is hit, it only recursively invokes BasicOperators.apply with this "child". It makes many strategies have no change to process this plan, which probably leads to "No plan" issue, so we use planLater to go through all strategies. https://issues.apache.org/jira/browse/SPARK-12275 Author: yucai <yucai.yu@intel.com> Closes #10265 from yucai/broadcast_hint. (cherry picked from commit ed87f6d) Signed-off-by: Yin Huai <yhuai@databricks.com>
|
@yucai can you create a 1.5 backport for this one? There was a conflict when I tried to merge it to branch 1.5. It will be good to have a pr for the backport and make sure all tests are good. Thanks! |
|
@yhuai sure, I will do that ASAP. |
…backport backport apache#10265 to branch 1.5 When SparkStrategies.BasicOperators's "case BroadcastHint(child) => apply(child)" is hit, it only recursively invokes BasicOperators.apply with this "child". It makes many strategies have no change to process this plan, which probably leads to "No plan" issue, so we use planLater to go through all strategies. https://issues.apache.org/jira/browse/SPARK-12275
… backport backport #10265 to branch 1.5. When SparkStrategies.BasicOperators's "case BroadcastHint(child) => apply(child)" is hit, it only recursively invokes BasicOperators.apply with this "child". It makes many strategies have no change to process this plan, which probably leads to "No plan" issue, so we use planLater to go through all strategies. https://issues.apache.org/jira/browse/SPARK-12275 Author: yucai <yucai.yu@intel.com> Closes #10291 from yucai/backport_1.5_no_plan_for_broadcasthint and squashes the following commits: b09715c [yucai] [SPARK-12275][SQL] No plan for BroadcastHint in some condition - 1.5 backport
When SparkStrategies.BasicOperators's "case BroadcastHint(child) => apply(child)" is hit, it only recursively invokes BasicOperators.apply with this "child". It makes many strategies have no change to process this plan, which probably leads to "No plan" issue, so we use planLater to go through all strategies.
https://issues.apache.org/jira/browse/SPARK-12275