You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "dimas-b (via GitHub)" <gi...@apache.org> on 2026/03/25 16:17:20 UTC
[PR] Unify equals/hashCode for Index.Element [polaris]
dimas-b opened a new pull request, #4056:
URL: https://github.com/apache/polaris/pull/4056
* Convert the old `IndexElem` class to the root of the `Index.Element` class hierarchy
* Consolidate equals/hashCode logic for all sub-classes in `IndexElem`
* Note: Internal index elements having `null` values are not comparable in runtime. Attempts to invoke `.equals()` or `.hashCode()` on them will result in `NullPointerException`.
* Note: Internal index elements having `null` values are never actually compared in runtime.
Fixes #4049
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
## Checklist
- [ ] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unify equals/hashCode for Index.Element [polaris]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4056:
URL: https://github.com/apache/polaris/pull/4056#discussion_r2995451771
##########
persistence/nosql/persistence/api/src/main/java/org/apache/polaris/persistence/nosql/api/index/IndexElem.java:
##########
@@ -58,8 +73,9 @@ public boolean equals(Object o) {
if (o == this) {
return true;
}
- if (o instanceof Index.Element<?> other) {
- return key.equals(other.key()) && value.equals(other.value());
+ if (o instanceof IndexElem<?> other) {
Review Comment:
It's all good, thanks !
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unify equals/hashCode for Index.Element [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4056:
URL: https://github.com/apache/polaris/pull/4056#discussion_r2995435018
##########
persistence/nosql/persistence/api/src/main/java/org/apache/polaris/persistence/nosql/api/index/IndexElem.java:
##########
@@ -58,8 +73,9 @@ public boolean equals(Object o) {
if (o == this) {
return true;
}
- if (o instanceof Index.Element<?> other) {
- return key.equals(other.key()) && value.equals(other.value());
+ if (o instanceof IndexElem<?> other) {
Review Comment:
comments added
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unify equals/hashCode for Index.Element [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4056:
URL: https://github.com/apache/polaris/pull/4056#discussion_r2995238834
##########
persistence/nosql/persistence/api/src/main/java/org/apache/polaris/persistence/nosql/api/index/IndexElem.java:
##########
@@ -58,8 +73,9 @@ public boolean equals(Object o) {
if (o == this) {
return true;
}
- if (o instanceof Index.Element<?> other) {
- return key.equals(other.key()) && value.equals(other.value());
+ if (o instanceof IndexElem<?> other) {
Review Comment:
Good point about the comment - will add.
Re: "breaking changes", the code affected by this PR has not been released yet. Do we want to be so strict about changes between individual commits?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unify equals/hashCode for Index.Element [polaris]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4056:
URL: https://github.com/apache/polaris/pull/4056#discussion_r2992473954
##########
persistence/nosql/persistence/api/src/main/java/org/apache/polaris/persistence/nosql/api/index/IndexElem.java:
##########
@@ -58,8 +73,9 @@ public boolean equals(Object o) {
if (o == this) {
return true;
}
- if (o instanceof Index.Element<?> other) {
- return key.equals(other.key()) && value.equals(other.value());
+ if (o instanceof IndexElem<?> other) {
Review Comment:
For context (if other reviewers are asking), this is a change in the semantic here.
The "old" `IndexElem` (record) compared against any `Index.Element<?>`, but the new implementation only matches `instanceof IndexElem<?>`. This means a third-party `Index.Element` implementation that isn't an `IndexElem` subclass will no longer be considered equal, even with the same key/value.
The "old" `AbstractIndexElement` also had a fallback to `Index.Element<?>`.
This is an intentional "breaking change". Maybe worth to add a comment here to avoid "surprising" the reviewers ๐
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unify equals/hashCode for Index.Element [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #4056:
URL: https://github.com/apache/polaris/pull/4056
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org