peter-toth commented on a change in pull request #31848:
URL: https://github.com/apache/spark/pull/31848#discussion_r595472475
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileScan.scala
##########
@@ -86,7 +86,7 @@ trait FileScan extends Scan
override def equals(obj: Any): Boolean = obj match {
case f: FileScan =>
- fileIndex == f.fileIndex && readSchema == f.readSchema
+ fileIndex == f.fileIndex && readSchema == f.readSchema &&
ExpressionSet(partitionFilters) == ExpressionSet(f.partitionFilters) &&
ExpressionSet(dataFilters) == ExpressionSet(f.dataFilters)
Review comment:
I was thinking about a change like in
https://github.com/apache/spark/pull/31848/commits/f65ebe3f397053e103d482c50d7f250f798bb825.
That way we don't need to duplicate some parts of the canonicalization logic
defined in `QueryPlan.normalizeExpressions` and in `Expression.canonicalized`
(`Canonicalize.expressionReorder`, `Canonicalize.ignoreTimeZone`)...
If this looks ok to you then I can add UTs for `.equals()` into a new
`FileScanSuite` as @dongjoon-hyun suggested and move the e2e UT from
https://github.com/apache/spark/pull/31820 to here as we don't need the other
change in `BatchScanExec`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
