Skip to content
Snippets Groups Projects
Commit c5b73558 authored by Takuya UESHIN's avatar Takuya UESHIN Committed by Michael Armbrust
Browse files

[SPARK-15915][SQL] Logical plans should use canonicalized plan when override sameResult.

## What changes were proposed in this pull request?

`DataFrame` with plan overriding `sameResult` but not using canonicalized plan to compare can't cacheTable.

The example is like:

```
    val localRelation = Seq(1, 2, 3).toDF()
    localRelation.createOrReplaceTempView("localRelation")

    spark.catalog.cacheTable("localRelation")
    assert(
      localRelation.queryExecution.withCachedData.collect {
        case i: InMemoryRelation => i
      }.size == 1)
```

and this will fail as:

```
ArrayBuffer() had size 0 instead of expected size 1
```

The reason is that when do `spark.catalog.cacheTable("localRelation")`, `CacheManager` tries to cache for the plan wrapped by `SubqueryAlias` but when planning for the DataFrame `localRelation`, `CacheManager` tries to find cached table for the not-wrapped plan because the plan for DataFrame `localRelation` is not wrapped.
Some plans like `LocalRelation`, `LogicalRDD`, etc. override `sameResult` method, but not use canonicalized plan to compare so the `CacheManager` can't detect the plans are the same.

This pr modifies them to use canonicalized plan when override `sameResult` method.

## How was this patch tested?

Added a test to check if DataFrame with plan overriding sameResult but not using canonicalized plan to compare can cacheTable.

Author: Takuya UESHIN <ueshin@happy-camper.st>

Closes #13638 from ueshin/issues/SPARK-15915.
parent bc02d011
No related branches found
No related tags found
No related merge requests found
......@@ -65,10 +65,12 @@ case class LocalRelation(output: Seq[Attribute], data: Seq[InternalRow] = Nil)
}
}
override def sameResult(plan: LogicalPlan): Boolean = plan match {
case LocalRelation(otherOutput, otherData) =>
otherOutput.map(_.dataType) == output.map(_.dataType) && otherData == data
case _ => false
override def sameResult(plan: LogicalPlan): Boolean = {
plan.canonicalized match {
case LocalRelation(otherOutput, otherData) =>
otherOutput.map(_.dataType) == output.map(_.dataType) && otherData == data
case _ => false
}
}
override lazy val statistics =
......
......@@ -87,9 +87,11 @@ private[sql] case class LogicalRDD(
override def newInstance(): LogicalRDD.this.type =
LogicalRDD(output.map(_.newInstance()), rdd)(session).asInstanceOf[this.type]
override def sameResult(plan: LogicalPlan): Boolean = plan match {
case LogicalRDD(_, otherRDD) => rdd.id == otherRDD.id
case _ => false
override def sameResult(plan: LogicalPlan): Boolean = {
plan.canonicalized match {
case LogicalRDD(_, otherRDD) => rdd.id == otherRDD.id
case _ => false
}
}
override protected def stringArgs: Iterator[Any] = Iterator(output)
......
......@@ -60,9 +60,11 @@ case class LogicalRelation(
com.google.common.base.Objects.hashCode(relation, output)
}
override def sameResult(otherPlan: LogicalPlan): Boolean = otherPlan match {
case LogicalRelation(otherRelation, _, _) => relation == otherRelation
case _ => false
override def sameResult(otherPlan: LogicalPlan): Boolean = {
otherPlan.canonicalized match {
case LogicalRelation(otherRelation, _, _) => relation == otherRelation
case _ => false
}
}
// When comparing two LogicalRelations from within LogicalPlan.sameResult, we only need
......
......@@ -552,4 +552,15 @@ class CachedTableSuite extends QueryTest with SQLTestUtils with SharedSQLContext
selectStar,
Seq(Row(1, "1")))
}
test("SPARK-15915 Logical plans should use canonicalized plan when override sameResult") {
val localRelation = Seq(1, 2, 3).toDF()
localRelation.createOrReplaceTempView("localRelation")
spark.catalog.cacheTable("localRelation")
assert(
localRelation.queryExecution.withCachedData.collect {
case i: InMemoryRelation => i
}.size == 1)
}
}
......@@ -185,7 +185,7 @@ private[hive] case class MetastoreRelation(
/** Only compare database and tablename, not alias. */
override def sameResult(plan: LogicalPlan): Boolean = {
plan match {
plan.canonicalized match {
case mr: MetastoreRelation =>
mr.databaseName == databaseName && mr.tableName == tableName
case _ => false
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment