[ 
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17133205#comment-17133205
 ] 

Vladimir Sitnikov commented on CALCITE-3786:
--------------------------------------------

>so that we can finally compare 2 RexCall using member equals, and it works 
>well.

1) There are code parts that still use #toString() for the comparison (e.g. 
RexSimplify)

2) I had exactly that suggestion (normalize RexNodes on creation), however, you 
have cast -1 multiple times because that would alter string representation of 
the plans with little gain.

[https://lists.apache.org/thread.html/d89103671efdd813fac768fbc2336bd125e925f0790e9137a2a16375%40%3Cdev.calcite.apache.org%3E]

My suggestion was

Vladimir: It turned out "b" (sort operands in computeDigest) is easier to 
implement.

Vladimir: I've filed a PR: [https://github.com/apache/calcite/pull/1703
]

and your response was "I’m strongly -1 for this way, because it beaks the plan 
test where almost all of the change are meaningless."

Note: PR1703 was updated since that discussion, and the actual implementation 
normalizes nodes at the planning time only, while the user-visible plans are 
displayed without normalization.

 

> Add Digest interface to enable efficient hashCode(equals) for RexNode and 
> RelNode
> ---------------------------------------------------------------------------------
>
>                 Key: CALCITE-3786
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3786
>             Project: Calcite
>          Issue Type: New Feature
>          Components: core
>    Affects Versions: 1.21.0
>            Reporter: Vladimir Sitnikov
>            Assignee: Danny Chen
>            Priority: Major
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Current digests for RexNode, RelNode, RelType, and similar cases use String 
> concatenation.
> It is easy to implement, however, it has drawbacks:
> 1) String objects cannot be reused. For instance, RexCall has operands, 
> however, the digest is duplicated. It causes extra memory use and extra CPU 
> for string copying
> 2) There's no way to have multiple #toString() methods. RelType might need 
> multiple digests: "including field names", "excluding field names".
> A suggested resolution might be behind the lines of
> {code:java}
> class Digest { // immutable
>   final int hashCode; // speedup hashCode and equals
>   final Object[] contents; // The values are either other Digest objects or 
> Strings
>   String toString(); // e.g. for debugging purposes
>   int compareTo(Digest); // e.g. for debugging purposes.
> }
> {code}
> Note how fields in Kotlin are aligned much better, and it makes it easier to 
> read:
> {code:java}
> class Digest { // immutable
>   val hashCode: Int // speedup hashCode and equals
>   val contents: Array<Any> // The values are either other Digest objects or 
> Strings
>   fun toString(): String // e.g. for debugging purposes
>   fun compareTo(other: Digest): Int // e.g. for debugging purposes.
> }
> {code}
> Then the digest for RexCall could be the bits relevant to RexCall itself + 
> digests of the operands (which can be reused as is)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to