Skip to content

Conversation

@gatorsmile
Copy link
Member

The filter Filter (False) generates an empty LocalRelation in SimplifyFilters. In the current Explain, the optimized and physical plans look confusing.

For example,

val df = Seq(1 -> "a").toDF("a", "b")
df.where("1 = 0").explain(true)
== Analyzed Logical Plan ==
a: int, b: string
Filter (1 = 0)
+- Project [_1#0 AS a#2,_2#1 AS b#3]
   +- LocalRelation [_1#0,_2#1], [[1,a]]

== Optimized Logical Plan ==
LocalRelation [a#2,b#3] 

== Physical Plan ==
LocalTableScan [a#2,b#3] 

After the fix, the Optimized Logical Plan and Physical Plan are changed to

== Optimized Logical Plan ==
LocalRelation [a#2,b#3] []

== Physical Plan ==
LocalTableScan [a#2,b#3] [] 

The original PR is: #10494

@gatorsmile gatorsmile changed the title [SPARK-12536] [SQL] Added "Empty Seq" in Explain Outputs For Empty LocalRelation and LocalTableScan [SPARK-12536] [SQL] Added "[]" in Explain Outputs For Empty LocalRelation and LocalTableScan Jan 7, 2016
@gatorsmile
Copy link
Member Author

@cloud-fan @marmbrus Hopefully, my understanding is right. Feel free to let me know if the fix needs any change. Thank you!

@SparkQA
Copy link

SparkQA commented Jan 7, 2016

Test build #48946 has finished for PR 10646 at commit 3ac9735.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better way is to change this if to if seq.nonEmpty && seq.toSet.subsetOf(children.toSet)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Will do!

@SparkQA
Copy link

SparkQA commented Jan 11, 2016

Test build #49091 has finished for PR 10646 at commit 4ae1ce7.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 11, 2016

Test build #49092 has finished for PR 10646 at commit 18091f7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member Author

retest this please.

@SparkQA
Copy link

SparkQA commented Jan 11, 2016

Test build #49152 has finished for PR 10646 at commit 18091f7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these import changes are irrelevant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just saw these useless imports in this file. Do you want me to keep them unchanged?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, it's fine to remove useless imports.

@cloud-fan
Copy link
Contributor

LGTM

@gatorsmile
Copy link
Member Author

Thank you for your help! Sorry for wasting you a lot of time on it! @cloud-fan

@rxin
Copy link
Contributor

rxin commented Jan 12, 2016

@gatorsmile I'm actually not sure whether the new format is more clear. Do you mind closing this? Thanks.

@gatorsmile
Copy link
Member Author

Sure, let me close it.

@gatorsmile gatorsmile closed this Jan 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants