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

snuyanzin · 2025-05-12T06:49:22Z

What is the purpose of the change

Upgrade Calcite to 1.35.0

Brief change log

There are several major changes done in Calcite 1.35.0

About commits
3242726 - Update poms and NOTICE files
bec22cc - Remove RelBuilder
2161225 - Remove SqlTimestampAddFunction
f7c5c96 - Remove CompositeSingleOperandTypeChecker
19cb1e3 - Update Calcite classes to be synced with https://github.com/apache/calcite/releases/tag/calcite-1.35.0
1d284f2 - Update Flink classes
9ba9ab6 - Add FlinkCalciteTableMappingRule
929ce63 - Update plans
985e500 - Update SqlNodeToCallOperationTest
0faf592 - Add LiteralAggFunction
acc7307 - Update plans after LiteralAggFunction
4dc67fd - Update JoinTest.testJoinUDTFWithInvalidJoinHint

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 · 2025-05-12T06:54:26Z

CI report:

746b3f5 UNKNOWN
023a902 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

snuyanzin · 2025-05-12T07:09:05Z

flink-table/flink-sql-parser/pom.xml

 				[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


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

snuyanzin · 2025-05-12T07:47:38Z

.../src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java

+
+    @Override
+    public DataType getResultType() {
+        return DataTypes.BOOLEAN();


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 · 2025-05-12T07:59:11Z

@flinkbot run azure

snuyanzin · 2025-05-12T18:17:21Z

flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl

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

-
-/** Parses a TRY_CAST invocation. */


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

snuyanzin · 2025-05-12T18:19:05Z

...flink-sql-parser/src/main/java/org/apache/flink/sql/parser/validate/FlinkSqlConformance.java

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

    @Override
-    public boolean allowCoercionStringToArray() {
-        return SqlConformanceEnum.DEFAULT.allowCoercionStringToArray();
+    public boolean allowLenientCoercion() {


Suddenly was just renamed in Calcite apache/calcite@b042094

davidradl · 2025-05-16T10:29:18Z

flink-table/flink-sql-parser/src/main/codegen/data/Parser.tdd

@@ -293,6 +292,8 @@
    "DATETIME_DIFF"
    "DATETIME_INTERVAL_CODE"
    "DATETIME_INTERVAL_PRECISION"
+    "DAYOFWEEK"


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

keep in sync with Calcite version https://github.com/apache/calcite/blob/75750b78b5ac692caa654f506fc1515d4d3991d6/core/src/main/codegen/templates/Parser.jj#L7963

davidradl · 2025-05-16T10:29:26Z

flink-table/flink-sql-parser/src/main/codegen/data/Parser.tdd

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


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.

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

snuyanzin · 2025-05-27T21:03:57Z

flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/AggConverter.java

+ *   <li>Added in FLINK-34057, FLINK-34058, FLINK-34312: Lines 452 ~ 469
+ * </ol>
+ */
+class AggConverter implements SqlVisitor<Void> {


The result of this commit apache/calcite@568ce12

where SqlToRelConverter was splitted into 2 classes

snuyanzin · 2025-05-28T06:49:26Z

...nk-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java

+
+/** Rules that determine whether a type is castable from another type. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
+    private static final FlinkCalciteTableMappingRule INSTANCE;


The behavior of SqlTypeCoercionRule was changed (https://issues.apache.org/jira/browse/CALCITE-5662) and started leading to a number of failures in CastFunctionITCase.
New mapping rule makes it working in same way as before

twalthr

Thank you for this PR @snuyanzin. This must have been painful work. I added some comments.

twalthr · 2025-05-28T11:11:07Z

flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java

- *   <li>Added in FLINK-32474: Lines 3046 ~ 3080
- *   <li>Added in FLINK-34312: Lines 5827 ~ 5838
- *   <li>Added in FLINK-34057, FLINK-34058, FLINK-34312: Lines 6285 ~ 6303
+ *   <li>Added in FLINK-29081, FLINK-28682, FLINK-33395: Lines 673 ~ 690


Thank you for updating all these lines. Very helpful, but I'm sure also a lot of work!

twalthr · 2025-05-28T11:14:40Z

...le/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java

+                                SqlStdOperatorTable.POSITION.createCall(
+                                        SqlParserPos.ZERO, call.operand(1), call.operand(0))));
+
+        // "INSTR(string, substring, position, occurrence) is equivalent to


do we need to document those function synonyms?

don't think so
existing position, instr are covered in doc
with other args just fail with validation

twalthr · 2025-05-28T11:17:45Z

flink-table/flink-sql-parser/src/main/codegen/templates/Parser.jj

@@ -8001,6 +8003,8 @@ SqlPostfixOperator PostfixRowOperator() :
 |   < DATETIME_INTERVAL_PRECISION: "DATETIME_INTERVAL_PRECISION" >
 |   < DATETIME_TRUNC: "DATETIME_TRUNC" >
 |   < DAY: "DAY" >
+|   < DAYOFWEEK: "DAYOFWEEK" >


update documentation, do we also support those in runtime code? or at least throw a proper error message?

it's already covered in doc

flink/docs/data/sql_functions.yml

Lines 628 to 633 in a4e3084

- sql: DAYOFYEAR(date)

description: Returns the day of a year (an integer between 1 and 366) from SQL date. Equivalent to EXTRACT(DOY FROM date). E.g., DAYOFYEAR(DATE '1994-09-27') returns 270.

- sql: DAYOFMONTH(date)

description: Returns the day of a month (an integer between 1 and 31) from SQL date. Equivalent to EXTRACT(DAY FROM date). E.g., DAYOFMONTH(DATE '1994-09-27') returns 27.

- sql: DAYOFWEEK(date)

description: Returns the day of a week (an integer between 1 and 7) from SQL date. Equivalent to EXTRACT(DOW FROM date). E.g., DAYOFWEEK(DATE '1994-09-27') returns 3.

...nk-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java

twalthr · 2025-05-28T11:35:42Z

.../src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java

+    }
+
+    /** Built-in Long Literal aggregate function. */
+    public static class LongLiteralAggFunction extends LiteralAggFunction {


No INT? Are byte and short ever called?

I added for int as well
however we don't have any test calling anything else except for boolean

twalthr · 2025-05-28T11:41:54Z

.../src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java

+
+    @Override
+    public UnresolvedReferenceExpression[] aggBufferAttributes() {
+        return new UnresolvedReferenceExpression[] {literalAgg};


Do we actually need to use any state? Can't this be empty array?

looks like with current framework it is not possible to do it...
At least this test requires it JoinWithoutKeyITCase#testNonKeySemi

snuyanzin commented May 12, 2025

View reviewed changes

flinkbot added the component=TableSQL/API label May 12, 2025

snuyanzin force-pushed the flink35854_1 branch from 87464c0 to aeb0cac Compare May 12, 2025 18:02

snuyanzin commented May 12, 2025

View reviewed changes

snuyanzin force-pushed the flink35854_1 branch 4 times, most recently from 0bab0b8 to 6c34459 Compare May 14, 2025 07:19

davidradl reviewed May 16, 2025

View reviewed changes

snuyanzin force-pushed the flink35854_1 branch 3 times, most recently from e616ba0 to b8a8b8b Compare May 26, 2025 20:58

[FLINK-35854][table] Update pom and NOTICE files

3242726

snuyanzin force-pushed the flink35854_1 branch 2 times, most recently from af327eb to 44d710b Compare May 27, 2025 19:59

snuyanzin added 5 commits May 27, 2025 22:01

[FLINK-35854][table] Remove RelBuilder

bec22cc

[FLINK-35854][table] Remove SqlTimestampAddFunction

2161225

[FLINK-35854][table] Remove CompositeSingleOperandTypeChecker

f7c5c96

[FLINK-35854][table] Update Calcite classes

19cb1e3

[FLINK-35854][table] Update Flink classes

1d284f2

snuyanzin force-pushed the flink35854_1 branch 2 times, most recently from a8b5103 to fc3c7e7 Compare May 27, 2025 20:39

snuyanzin commented May 27, 2025

View reviewed changes

snuyanzin added 2 commits May 27, 2025 23:09

[FLINK-35854][table] Add FlinkCalciteTableMappingRule

9ba9ab6

[FLINK-35854][table] Update plans

929ce63

snuyanzin added 4 commits May 27, 2025 23:09

[FLINK-35854][table] SqlNodeToCallOperationTest

985e500

[FLINK-35854][table] Add LiteralAggFunction

0faf592

[FLINK-35854][table] plans after literalAgg

acc7307

[FLINK-35854][table] JoinTest.testJoinUDTFWithInvalidJoinHint

4dc67fd

snuyanzin force-pushed the flink35854_1 branch from fc3c7e7 to 4dc67fd Compare May 27, 2025 21:09

snuyanzin marked this pull request as ready for review May 27, 2025 21:20

snuyanzin commented May 28, 2025

View reviewed changes

twalthr reviewed May 28, 2025

View reviewed changes

snuyanzin added 2 commits May 30, 2025 00:34

address feedback

746b3f5

address feedback

023a902

	- sql: DAYOFYEAR(date)
	description: Returns the day of a year (an integer between 1 and 366) from SQL date. Equivalent to EXTRACT(DOY FROM date). E.g., DAYOFYEAR(DATE '1994-09-27') returns 270.
	- sql: DAYOFMONTH(date)
	description: Returns the day of a month (an integer between 1 and 31) from SQL date. Equivalent to EXTRACT(DAY FROM date). E.g., DAYOFMONTH(DATE '1994-09-27') returns 27.
	- sql: DAYOFWEEK(date)
	description: Returns the day of a week (an integer between 1 and 7) from SQL date. Equivalent to EXTRACT(DOW FROM date). E.g., DAYOFWEEK(DATE '1994-09-27') returns 3.

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

Are you sure you want to change the base?

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

Conversation

snuyanzin commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

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

Documentation

Uh oh!

flinkbot commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin commented May 12, 2025

Uh oh!

snuyanzin May 12, 2025 • 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!

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!

snuyanzin May 27, 2025 • 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!

twalthr 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!

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!

Uh oh!

snuyanzin commented May 12, 2025 •

edited

Loading

flinkbot commented May 12, 2025 •

edited

Loading

snuyanzin May 12, 2025 •

edited

Loading

snuyanzin May 27, 2025 •

edited

Loading