Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import scala.collection.JavaConverters._
import org.apache.spark.rdd.RDD
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.plans.QueryPlan
import org.apache.spark.sql.catalyst.plans.physical
import org.apache.spark.sql.catalyst.plans.physical.SinglePartition
import org.apache.spark.sql.execution.{ColumnarBatchScan, LeafExecNode, WholeStageCodegenExec}
Expand Down Expand Up @@ -52,6 +53,17 @@ case class DataSourceV2ScanExec(
case _ => false
}

override def doCanonicalize(): DataSourceV2ScanExec = {
DataSourceV2ScanExec(
output.map(QueryPlan.normalizeExprId(_, output)),
source,
options,
QueryPlan.normalizePredicates(
pushedFilters,
AttributeSeq(pushedFilters.flatMap(_.references).distinct)),
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use output here?

Copy link
Contributor Author

@mingjialiu mingjialiu Sep 10, 2020

Choose a reason for hiding this comment

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

reader)
}

override def hashCode(): Int = {
Seq(output, source, options).hashCode()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,29 @@ class DataSourceV2Suite extends QueryTest with SharedSQLContext {
}
}
}

test("SPARK-32708: same columns with different ExprIds should be equal after canonicalization ") {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have an end-to-end test, how about a low-level UT? Create two DataSourceV2ScanExec instances and check scan1.sameResult(scan2).

Copy link
Member

Choose a reason for hiding this comment

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

@cloud-fan I think this test case creates two DataSourceV2ScanExec and do the check. It looks ok to me.

def getV2ScanExec(query: DataFrame): DataSourceV2ScanExec = {
query.queryExecution.executedPlan.collect {
case d: DataSourceV2ScanExec => d
}.head
}

val df1 = spark.read.format(classOf[AdvancedDataSourceV2].getName).load()
val q1 = df1.select('i).filter('i > 6)
val df2 = spark.read.format(classOf[AdvancedDataSourceV2].getName).load()
val q2 = df2.select('i).filter('i > 6)
val scan1 = getV2ScanExec(q1)
val scan2 = getV2ScanExec(q2)
assert(scan1.sameResult(scan2))
assert(scan1.doCanonicalize().equals(scan2.doCanonicalize()))

val q3 = df2.select('i).filter('i > 5)
val scan3 = getV2ScanExec(q3)
assert(!scan1.sameResult(scan3))
assert(!scan1.doCanonicalize().equals(scan3.doCanonicalize()))
}

}

class SimpleSinglePartitionSource extends DataSourceV2 with ReadSupport {
Expand Down