[GitHub] [spark] peter-toth commented on a change in pull request #31682: [WIP][SPARK-34545][SQL] Fix issues with valueCompare feature of pyrolite

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org