dongjoon-hyun edited a comment on pull request #29172: URL: https://github.com/apache/spark/pull/29172#issuecomment-661616433
For @HyukjinKwon comments, - `CaseInsensitiveMap` is already using modified `originalMap` at `-` operation. Technically, `originalMap` is still immutable, and `+` operation can use modified `originalMap` too in the same reason. In short, both `-` and `+` operation should return the desirable map object for case-insensitivity with this kind of `indeterministic` behavior. Also, this PR makes the `CaseInsensitiveMap` class more consistent. - 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
