FrankChen021 commented on PR #5281:
URL: https://github.com/apache/calcite/pull/5281#issuecomment-5852617833
@vlsi Thanks again for your detailed feedback.
I have addressed the review comments:
- Rewrote the commit message with the correct CALCITE-7807 issue and
rationale.
- Documented why collation must be compared by identity.
- Split the test into focused cases covering unchanged attributes, a changed
charset, and changed coercibility.
- Added assertions for the resulting charset and collation.
- The canonicalization concern remains outside this PR.
- For `SqlCollation#isSameAs`, if you agree that we add a method in the
`SqlCollation` to replace current reference comparision, I can update to
include it as:
```java
public boolean isSameAs(@Nullable SqlCollation that) {
return that != null
&& equals(that)
&& coercibility == that.coercibility;
}
```
Let me know if you other comments.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
