[CALCITE-7393] Support RelDataTypeDigest by zhuwenzhuang · Pull Request #4761 · apache/calcite

zhuwenzhuang · 2026-01-22T10:45:20Z

Use RelDataTypeDigest to reduce composite type's digest memory and digest relative operation's latency.
test case: HepPlannerTest.testLargeTypeDigest
This is the first part of latency/memory optimization of large plan.

xiedeyantu · 2026-01-22T13:33:39Z

Thank you for your contribution! I have a few suggestions:
First, please ensure consistency between the Jira title, commit message, and PR title.
Second, please do not arbitrarily change or delete public/protected methods or variables in existing classes (they can be marked as deprecated).
Third, any errors in the CI need to be checked again.

mihaibudiu · 2026-01-22T18:50:06Z

core/src/main/java/org/apache/calcite/rel/type/RelDataTypeDigest.java

+/**
+ * Digest of a RelDataType.
+ */
+public interface RelDataTypeDigest {


This could be two interfaces, in case Digest is useful somewhere else.

I noticed that the hashCode caching in RelDigest and RexCall already addresses some of the hashCode caching/comparison latency issues. Could you please clarify what you mean by “be two interfaces”?

interface HasDigestString { String getDigestString() } interface RelDataTypeDigest extends HasDigestString { RelDataType getType(); }

Other classes may implement HasDigestString.

mihaibudiu · 2026-01-22T18:50:38Z

core/src/main/java/org/apache/calcite/jdbc/JavaRecordType.java

  }

-  @Override public boolean equals(@Nullable Object obj) {
+  @Override public boolean deepEquals(Object obj) {


I think you'll need to leave the old implementations around too, they may break users who expect them to exist.

If we allow user to implement equals/hashCode for nested type, cache for hashCode and pointer compare for deepEquals will not work for them.
This situation is similiar to https://issues.apache.org/jira/browse/CALCITE-3786

mihaibudiu · 2026-01-22T18:52:14Z

core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java


  protected final @Nullable List<RelDataTypeField> fieldList;
-  protected @Nullable String digest;
+  protected RelDataTypeDigest digest;


Changing protected fields is a breaking change, which has to be reflected in the semantic versioning. This would require a 2.0 release.

On the other hand, I don't know what else you can do to make the digest lazy. Maybe other people can weigh on this dilemma. This is why you should probably file a JIRA issue just for this topic and discuss the design there.

Maybe we can allow user to set String digest to define their own digest. otherwise, we use the RelDataTypeDigest

My suggestion is that some protected/public variables and methods must still be guaranteed to work properly. For example, this
"String digest" might eventually call
"toString" from
"RelDataTypeDigest" to get the value? Because these variables are exposed to users, and someone might currently be using them, we need to provide a compatibility fix. If you think reassigning the original variable would go against your design intention (e.g., it might increase memory usage instead of reducing it), then you need to declare that this is a breaking change.

mihaibudiu · 2026-01-27T17:47:56Z

I don't know how to review this; the change is reasonable in itself, but I don't know if this is an acceptable breaking change. I would appreciate comments from people who have dealt with similar breaking changes in the past.

xiedeyantu

I think this change has a fairly wide impact. I'm wondering if it's possible to control the new/old logic by introducing configuration options. To be honest, I can't judge at this point how the current changes will affect future compatibility. Although your changes may be good, the community probably values compatibility more.

zhuwenzhuang · 2026-02-04T10:40:00Z

@xiedeyantu
Thanks, I think use CalciteSystemProperties to control compatibility is a good idea.
The patch is updated base on 'calcite.disable.generate.type.digest.string', and a benchmark TypeDigestBenchmark added.

Use structured innerDigest instead of string digest for composite/UDT types to reduce memory and improve hashCode/equals latency. Controlled by `calcite.disable.generate.type.digest.string` (default: false). Legacy string digest is still used in hashCode/equals if explicitly set, ensuring backward compatibility. TestCase: TypeDigestBenchmark

mihaibudiu · 2026-02-06T19:12:36Z

In general you should avoid force-pushing until a review is finished, to make the reviewer's job easier by making it clear what is new.

mihaibudiu

This looks pretty good. Let's see if anyone else has comments.

mihaibudiu · 2026-02-06T19:13:46Z

core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java


  protected final @Nullable List<RelDataTypeField> fieldList;
-  protected @Nullable String digest;
+  protected @Deprecated @Nullable String digest;


Please add also a @deprecated JavaDoc comment explaining what users are supposed to do instead.

Should we suggest using RelDataTypeDigest.getDigest().getDigestString() instead of digest? I also have a question: if a user extends and uses this field, will the current modification block the upgrade? Is this considered a blocking point?

1.Yes. It has better performace and ondemand construction behavior.
2.Not a blocking point, if user set the String digest not null, all hashCode/equals/getXXXString logic will fallback to behaviors based on String digest.

My understanding is no. If you use this field and mark it with @Deprecated, the compiler will definitely throw an error. You are returning a string using RelDataTypeDigest.getDigest().getDigestString(), instead of directly using digest.

My first meaning is to add a description like "Use RelDataTypeDigest.getDigest().getDigestString() instead of digest" to the Javadoc.

OK, @depredate mark may cause user's compiler thorw an error. And I think use RelDataTypeDigest to handle digest is better than String digest. I don't know should we add the @depredate mark to indicate this suggestion.

For the java doc.
"Use RelDataTypeDigest.getDigest().getDigestString() instead of digest" is one usage of digest. If users managed the String digest by not standard way(maybe UDT)

digest = "MyType";

, they need implement this in

void generateTypeString(StringBuilder sb,boolean withDetail);

I'm not sure whether we need to add this to the Javadoc, either.

I believe a more detailed description would be better. While I think this breaks compatibility, I think it might be acceptable if no one objects in the next few days.

mihaibudiu · 2026-02-06T21:56:31Z

core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java

+
+  /** Whether to disable generate type digest string.
+   *
+   * <p> Disable generate type digest string for every type can


Please be more explicit here: this is about the RelDataType.digest field.
"Type" can mean many things.

and add a deprecated comment

sonarqubecloud · 2026-02-13T09:26:47Z

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
28.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

xiedeyantu

I don't know if any declarations are needed for the blocking point, but I think this optimization is very good.

zhuwenzhuang force-pushed the CALCITE-6907 branch from 604cf22 to 04538b2 Compare January 22, 2026 11:34

zhuwenzhuang changed the title ~~[CALCITE-6907] support RelDataTypeDigest~~ [CALCITE-6907] Support RelDataTypeDigest Jan 22, 2026

mihaibudiu reviewed Jan 22, 2026

View reviewed changes

zhuwenzhuang force-pushed the CALCITE-6907 branch from 04538b2 to a03c48a Compare January 23, 2026 11:36

zhuwenzhuang changed the title ~~[CALCITE-6907] Support RelDataTypeDigest~~ [CALCITE-7393] Support RelDataTypeDigest Jan 23, 2026

zhuwenzhuang force-pushed the CALCITE-6907 branch 4 times, most recently from 1f17a42 to 955f537 Compare January 26, 2026 01:49

zhuwenzhuang marked this pull request as draft January 26, 2026 01:55

zhuwenzhuang closed this Jan 26, 2026

zhuwenzhuang force-pushed the CALCITE-6907 branch from 955f537 to f33c6a2 Compare January 26, 2026 01:57

zhuwenzhuang reopened this Jan 26, 2026

zhuwenzhuang force-pushed the CALCITE-6907 branch 2 times, most recently from ac7bf74 to 078f2e4 Compare January 26, 2026 03:09

zhuwenzhuang marked this pull request as ready for review January 26, 2026 03:48

zhuwenzhuang requested review from mihaibudiu and xiedeyantu January 26, 2026 03:49

xiedeyantu reviewed Feb 4, 2026

View reviewed changes

zhuwenzhuang force-pushed the CALCITE-6907 branch from 078f2e4 to 7416868 Compare February 4, 2026 10:30

zhuwenzhuang force-pushed the CALCITE-6907 branch from 7416868 to 38fa020 Compare February 4, 2026 11:28

mihaibudiu approved these changes Feb 6, 2026

View reviewed changes

zhuwenzhuang force-pushed the CALCITE-6907 branch 3 times, most recently from 9ca8243 to a38a2d2 Compare February 10, 2026 09:38

zhuwenzhuang force-pushed the CALCITE-6907 branch from a38a2d2 to 892c828 Compare February 10, 2026 12:00

zhuwenzhuang requested a review from xiedeyantu February 11, 2026 03:42

[CALCITE-7393] Change flag name

6ce4690

and add a deprecated comment

zhuwenzhuang force-pushed the CALCITE-6907 branch from 892c828 to 6ce4690 Compare February 13, 2026 08:52

xiedeyantu approved these changes Feb 14, 2026

View reviewed changes

Conversation

zhuwenzhuang commented Jan 22, 2026

Uh oh!

xiedeyantu commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented Jan 27, 2026

Uh oh!

xiedeyantu left a comment

Choose a reason for hiding this comment

Uh oh!

zhuwenzhuang commented Feb 4, 2026

Uh oh!

mihaibudiu commented Feb 6, 2026

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuwenzhuang Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 13, 2026

Quality Gate passed

Uh oh!

xiedeyantu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhuwenzhuang Feb 14, 2026 •

edited

Loading