[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661639927 Merged to master/3.0. I'll make a backporting PR for `branch-2.4`. 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661639361 Thank you all, @cloud-fan , @HyukjinKwon , @viirya , @maropu . I'll merge this PR because the first PR already passed GitHub Action. The second commit is only about `scala-2.13` which is not complied in Jenkins/GitHub Action currently. Its compilation is verified locally. 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661632212 Thank you, @HyukjinKwon . @cloud-fan and @HyukjinKwon . I updated Scala-2.13 code consistently with the same code logically. 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661628411 Thank you, @cloud-fan . BTW, hold on one second. I found that I need to update Scala-2.13 code path~ 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661616433 For @HyukjinKwon comments, - `CaseInsensitiveMap` is already modifying `originalMap` at `-` operation. `originalMap` is not immutable already. I believe `+` operation can modify it too in the same reason. Both `-` and `+` operation returns the desirable `CaseInsensitiveMap`. - In general, `DataFrameReader` [example](https://github.com/apache/spark/pull/29172#issuecomment-661604611) is orthogonal to this PR because this PR is focusing on `CaseInsensitiveMap` only. - For the example, I believe it's not a good practice. For that alternative approach, we need to add an implicit assumption into the Apache Spark code-base. And, it'll be fragile when we add a new code. - We should warn that `.toMap` should not be used always. - All concatenation should start with `collection.immutable.ListMap.empty[String, String]` always. 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661609307 Thank you for the review with the alternative. Also, cc @gatorsmile 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29172: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
dongjoon-hyun commented on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661504976 We need to fix this fundamental bug first. 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
