Skip to content

[FLINK-35854][table] Upgrade Calcite version to 1.35.0 #26547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

snuyanzin
Copy link
Contributor

What is the purpose of the change

Upgrade Calcite to 1.35.0

Brief change log

TBD

Verifying this change

existing tests

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes )
  • The public API, i.e., is any changed class annotated with @Public(Evolving): ( no)
  • The serializers: ( no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no)
  • The S3 file system connector: ( no)

Documentation

  • Does this pull request introduce a new feature? ( no)
  • If yes, how is the feature documented? (not applicable)

@flinkbot
Copy link
Collaborator

flinkbot commented May 12, 2025

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

[INFO] | +- org.apache.calcite.avatica:avatica-core:jar:1.23.0:compile
[INFO] | +- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO] | +- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO] | +- org.checkerframework:checker-qual:jar:3.10.0:compile
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not related to this commit, however seems at some point the version was downgraded in Flink.
There is no critical changes, so should be ok


@Override
public DataType getResultType() {
return DataTypes.BOOLEAN();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the main question: should we have it for different types or boolean is enough?
So far in existing tests there is only boolean in use

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@@ -3061,34 +3061,6 @@ SqlNode SqlReset() :
}
}


/** Parses a TRY_CAST invocation. */
Copy link
Contributor Author

@snuyanzin snuyanzin May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since now TRY_CAST comes with Calcite https://issues.apache.org/jira/browse/CALCITE-4771

@@ -168,8 +168,8 @@ public SqlLibrary semantics() {
}

@Override
public boolean allowCoercionStringToArray() {
return SqlConformanceEnum.DEFAULT.allowCoercionStringToArray();
public boolean allowLenientCoercion() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suddenly was just renamed in Calcite apache/calcite@b042094

@snuyanzin snuyanzin force-pushed the flink35854_1 branch 4 times, most recently from 0bab0b8 to 6c34459 Compare May 14, 2025 07:19
@@ -293,6 +292,8 @@
"DATETIME_DIFF"
"DATETIME_INTERVAL_CODE"
"DATETIME_INTERVAL_PRECISION"
"DAYOFWEEK"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious why these 2 re in this list and not in nonReservedKeywordsToAdd.

@@ -224,7 +224,6 @@
"RESUME"
"TABLES"
"TIMESTAMP_LTZ"
"TRY_CAST"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious why TRY_CAST is removed as a keyword. I had assumed it would still be a keyword but Calcite would now provide the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it was added to keywords on Calcite level, no need to add it one more time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants