[
https://issues.apache.org/jira/browse/CALCITE-3786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17141516#comment-17141516
]
Haisheng Yuan commented on CALCITE-3786:
----------------------------------------
{quote}
Just implement the #explainTerms correctly with the items you want to involve
in.
{quote}
Really? Could you please kindly show me the code snippet? Won't it introduce
tons of "meaningless test plan changes"?
{quote}
Use different SqlExplainLevel to distinguish just like before.
{quote}
How? By setting explain level to NO_ATTRIBUTES? Won't all the plan nodes be
affected?
{quote}
especially when we are talking about "semantic equivalent" not strict #equals.
{quote}
There is no "semantic equivalent" or "strict #equals", which are terms coined
by you. If one rel instance can be used to represent the other instance, then
they are equal. The digestEquals was introduced (which I have changed to
private, because you said it is a mess), because we can't override #equals and
#hashCode, which are final. If we change them directly, then it will be a large
breaking change, when downstream projects use RelNode as HashMap key, in which
case they have to change the HashMap to IdentityHashMap.
{quote}
Just like Julian said, Cons is a basic data structure, it can be used in many
codes, but digest is a planner internal thing
{quote}
Well, are you aware of that, in non-federation and non-MV project that uses
Calcite, it may generate thousands or tens of thousands RelNodes during query
planning, but how many ConsList will be generated? Zero. The use case is not
uncommon, it is even dominant. Do you think we should care more about a basic
data structure that may not be used at all, than a "planner internal thing"
that is ubiquitous in Calcite, and being used no matter what case?
{quote}
it is not necessary from my side if we can make the code more concise.
{quote}
Do you think a 256 lines Digest.java is conciser than a 52 lines
RelDigest.java, which is essentially a 4 line interface:
{code:java}
public interface RelDigest {
void clear();
RelNode getRel();
}
{code}
just like you think 9 lines of code are simpler that one line of code?
Could you please kindly point out which part of
[PR#2039|https://github.com/apache/calcite/pull/2039] is not concise so that I
can make it conciser?
> 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
> Fix For: 1.24.0
>
> Time Spent: 8h 40m
> 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)
