peter-toth commented on a change in pull request #31682:
URL: https://github.com/apache/spark/pull/31682#discussion_r584847686
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/RowTest.scala
##########
@@ -108,6 +108,17 @@ class RowTest extends AnyFunSpec with Matchers {
}
}
+ describe("SPARK-34545: rows with different schema are not equal") {
Review comment:
I've updated the PR description to focus on the correctness issue. I
think it now clearly describes what is the issue with comparing
`GenericRowWithSchema` objects.
First I thought that this is a generic issue with `Row` that's why I tried
to fix `.equals()` there. But that raised other compatibility issues
(https://github.com/apache/spark/pull/31682#discussion_r584488733).
Then the current PR fixes `.equals()` in `GenericRowWithSchema` only, but
you can see that this also breaks a lots of UTs as many times in UTs we compare
`GenericRowWithSchema` to `GenericRow` just to verify its content.
Are those UTs incorrect and shall we fix them? Or shall I give up this idea
of fixing `.equals()` and we should just disable `valueCompare` in pyrolite:
https://github.com/apache/spark/pull/31682#issuecomment-787904759 I've checked
it, it also fixes the issue.
I will add an end to end test definitely. (Just need to figure out where we
have python e2e test. Sorry I'm not familiar with them.)
----------------------------------------------------------------
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]
