[CALCITE-2632] Add hashCode and equals implementations to RexNode.#943
Closed
kgyrtkirk wants to merge 2 commits intoapache:masterfrom
Closed
[CALCITE-2632] Add hashCode and equals implementations to RexNode.#943kgyrtkirk wants to merge 2 commits intoapache:masterfrom
kgyrtkirk wants to merge 2 commits intoapache:masterfrom
Conversation
Previously there was no default hashCode/equals implementations. To ensure that they are working properly is cruical while working with basic collections like sets/maps.
kgyrtkirk
commented
Nov 27, 2018
| LogicalFilter(condition=[=($1, $0)]) | ||
| LogicalProject(D4=[+($0, 4)], D5=[+($0, 5)], D6=[+($0, 6)]) | ||
| LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) | ||
| LogicalJoin(condition=[true], joinType=[inner]) |
Member
Author
There was a problem hiding this comment.
I think this change is "okay" mostly this plan change is due to that now it makes more use of d4=d.d1 and d5=d.d1 which implies d4=d5
vlsi
reviewed
Nov 27, 2018
| } | ||
|
|
||
| @Override public int hashCode() { | ||
| return Objects.hash(digest, type, id); |
Contributor
There was a problem hiding this comment.
is digest initialized by the time hashCode is executed?
Should toString be used here?
Member
Author
There was a problem hiding this comment.
the parent class initializes it - but I understand your concern...I think the general question would be to use digest or not...
vlsi
reviewed
Nov 27, 2018
| if (!(obj instanceof RexCall)) { | ||
| return false; | ||
| } | ||
| return obj == this || Objects.equals(toString(), obj.toString()); |
Contributor
There was a problem hiding this comment.
Do you mean toString should be a long-term solution?
Do you mean this would be reworked in future? (e.g. to avoid string build for the sole purpose of equals)
Member
Author
There was a problem hiding this comment.
I'm not sure...but we shouldn't use node.toString() everywhere when we are comparing RexNode-s...that's even worse...
I think the rational behind using digest instead of real tree comparison is to avoid the recursive comparision/etc - as long as when someone creates digest; remembers this design decision I think we are okay - but; for the record: I wouldn't expect this from a field like "digest"...
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Nov 30, 2018
Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
may be null; and use x == y when objects are primitive.
When implementing Object.equals, use the pattern
return this == obj
|| obj instanceof Type
&& a1.equals(obj.a1) ...
whenever possible.
Close apache#943
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Nov 30, 2018
Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
may be null; and use x == y when objects are primitive.
When implementing Object.equals, use the pattern
return this == obj
|| obj instanceof Type
&& a1.equals(obj.a1) ...
whenever possible.
In the many cases where RexNode.toString() is used as a key in
collections, use the raw RexNode instead.
Close apache#943
julianhyde
pushed a commit
to julianhyde/calcite
that referenced
this pull request
Dec 1, 2018
…Code and equals methods (Zoltan Haindrich)
Previously there was no default hashCode/equals implementations. To
ensure that they are working properly is crucial while working with
basic collections like sets/maps.
Fix ups (Julian Hyde):
* Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
may be null; and use x == y when objects are primitive.
* When implementing Object.equals, use the pattern
return this == obj
|| obj instanceof Type
&& a.equals(((Type) obj).a) ...
whenever possible.
* In the many cases where RexNode.toString() is used as a key in
collections, use the raw RexNode instead.
Close apache#943
F21
pushed a commit
to F21/calcite
that referenced
this pull request
Jan 3, 2019
…Code and equals methods (Zoltan Haindrich)
Previously there was no default hashCode/equals implementations. To
ensure that they are working properly is crucial while working with
basic collections like sets/maps.
Fix ups (Julian Hyde):
* Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
may be null; and use x == y when objects are primitive.
* When implementing Object.equals, use the pattern
return this == obj
|| obj instanceof Type
&& a.equals(((Type) obj).a) ...
whenever possible.
* In the many cases where RexNode.toString() is used as a key in
collections, use the raw RexNode instead.
Close apache#943
wangxlong
pushed a commit
to wangxlong/calcite
that referenced
this pull request
Feb 13, 2020
…Code and equals methods (Zoltan Haindrich)
Previously there was no default hashCode/equals implementations. To
ensure that they are working properly is crucial while working with
basic collections like sets/maps.
Fix ups (Julian Hyde):
* Use Objects.equals(x, y) in preference to x.equals(y) only when x or y
may be null; and use x == y when objects are primitive.
* When implementing Object.equals, use the pattern
return this == obj
|| obj instanceof Type
&& a.equals(((Type) obj).a) ...
whenever possible.
* In the many cases where RexNode.toString() is used as a key in
collections, use the raw RexNode instead.
Close apache#943
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously there was no default hashCode/equals implementations.
To ensure that they are working properly is cruical while working with basic collections like sets/maps.