flink icon indicating copy to clipboard operation
flink copied to clipboard

[FLINK-31362][table] Upgrade Calcite version to 1.33.0

Open snuyanzin opened this issue 2 years ago • 5 comments

What is the purpose of the change

The PR is about to bump Calcite to 1.33.0

There are several major changes done in Calcite 1.33.0

About commits https://github.com/apache/flink/pull/24255/commits/a7c3543ca6d9f048b8bc4da484fc16a4ea540292 ( [hotfix] Add missing Flink modification comment ) - there are some places in Calcite classes where comments about Flink modifications are missed, this commit fixes that https://github.com/apache/flink/pull/24255/commits/aca3a51e49e6a9ba94a4685a9b248b07130ac750 - NOTICE and pom files https://github.com/apache/flink/pull/24255/commits/6b470ee416a1283a4aea143fc923d17ce54cfcf9 - Sync of Calcite classes with https://github.com/apache/calcite/releases/tag/calcite-1.33.0 https://github.com/apache/flink/pull/24255/commits/8f0958e555647a93f121bd82857ccef2a01bfa64 - adapt an option to make VALUE became a synonym for VALUES (https://issues.apache.org/jira/browse/CALCITE-5393) and parsing string literal as array literal(https://issues.apache.org/jira/browse/CALCITE-5159) (currently both are turned off as in 1.32.0) https://github.com/apache/flink/pull/24255/commits/18023c34e299ae17a8543959386c821f42f3086b - api change for FlinkLogicalRelFactories.scala https://github.com/apache/flink/pull/24255/commits/73f17e2dc144640dc5d9e1fa14aacd27cb9f0487 removes unnecessary parentheses as a result of https://issues.apache.org/jira/browse/CALCITE-5265 https://github.com/apache/flink/pull/24255/commits/5ffbf40faa384d8eb5bf8a29561f4ec15c743938 - after https://issues.apache.org/jira/browse/CALCITE-5336 filter push down stopped working for a number of cases, so that's the reason to have RexUtil with some tiny change, also after refactoring in Calcite there is a bug which leads to having CompositeSingleOperandTypeChecker here https://github.com/apache/flink/pull/24255/commits/612335d783ff06a9db5c384ea8d0ba155e01dbe7 - there was fixed https://issues.apache.org/jira/browse/CALCITE-4351 (RelMdUtil#numDistinctVals always returns 0 for large inputs) https://github.com/apache/flink/pull/24255/commits/3ed6e49f050adc3532be5483f685289d5df42cdd - updated plans

Verifying this change

This change is already covered by existing tests + some tests are changed because of changes in Calcite

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)

snuyanzin avatar Feb 03 '24 00:02 snuyanzin

CI report:

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

flinkbot avatar Feb 03 '24 01:02 flinkbot

Hey there, just wondering what would need to happen to get this and the 1.34 PR merged https://github.com/apache/flink/pull/24256 Is there anything I can do to help move this along? Happy to take a stab at the merge conflicts.

tjbanghart avatar Jul 17 '24 16:07 tjbanghart

hi @tjbanghart thanks for reaching out rebasing PR should be pretty straightforward (I can do that on the upcoming weekend)

IIRC there was mentioned somewhere that together with Calcite 1.30/1.31 https://issues.apache.org/jira/browse/CALCITE-6266 came to Flink and it would be nice to fix it as well (i think still discussable )

snuyanzin avatar Jul 17 '24 16:07 snuyanzin

Hey @snuyanzin thanks for the response!

Is CALCITE-6266 blocking? Are we ignoring some tests in Flink because of it? I'll take a look and see if we can get some traction there.

Edit: Just saw that it was marked as blocking. I'll see if we can resolve that. Taking a look at your existing PR now https://github.com/apache/calcite/pull/3690/files

tjbanghart avatar Jul 17 '24 19:07 tjbanghart

@flinkbot run azure

snuyanzin avatar Jul 22 '24 22:07 snuyanzin