[FLINK-39812][table] Migrate `FlinkTypeFactory` to java by snuyanzin · Pull Request #28284 · apache/flink

snuyanzin · 2026-06-01T09:59:01Z

What is the purpose of the change

This is a preliminary change before bumping Calcite to 1.39.0

The main reason here is that this class is relatively often a problem while Calcite upgrade if something is changed in interfaces (new default methods for instance)
maven just fails as

 [ERROR] error: Error while emitting FlinkTypeFactory.scala
  [ERROR] assertion failed:
  [ERROR]   cannot invokespecial org/apache/calcite/rel/type/RelDataTypeFactory.leastRestrictive, the interface is not a direct parent.
  [ERROR]      while compiling: .../flink-table-planner/src/main/scala/org/apache/flink/table/planner/calcite/FlinkTypeFactory.scala
  [ERROR]         during phase: jvm
  [ERROR]      library version: version 2.12.20
  [ERROR]     compiler version: version 2.12.20
  ...
  [INFO]   last tree to typer: TypeTree(class FlinkHintStrategies)
  [INFO]             tree tpe: org.apache.flink.table.planner.hint.FlinkHintStrategies
  ...
  [WARNING] 42 warnings found
  [ERROR] one error found

Brief change log

FlinkTypeFactory to java

Verifying this change

existing tests

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

Dependencies (does it add or upgrade a dependency): ( no)
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 )

Was generative AI tooling used to co-author this PR?

Yes (please specify the tool below)

flinkbot · 2026-06-01T10:06:27Z

CI report:

002e953 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

liuyongvs

LGTM +1

twalthr · 2026-06-01T11:50:12Z

+            String[] fieldNames, LogicalType[] fieldTypes, StructKind structKind) {
+        FieldInfoBuilder b = builder();
+        b.kind(structKind);
+        // mirror Scala's `fieldNames.zip(fieldTypes)`, which truncates to the shorter sequence


Can be simplified. Remove comment and Math.min

twalthr · 2026-06-01T11:51:29Z

                        .map(PlannerTypeUtils::removeLegacyTypes)
-                        .collect(Collectors.toList());
+                        .collect(Collectors.toList())
+                        .toArray(new LogicalType[0]);


nit: isn't List easier to handle than arrays?

made using lists here

fhueske

Thanks for porting the type factory @snuyanzin.

I've checked the ported code and didn't spot any correctness issues.
Left a few comments with minor improvement suggestions.
Maybe we can use Java lists rather than arrays where possible?

Cheers, Fabian

fhueske · 2026-06-01T12:38:46Z

                        .map(PlannerTypeUtils::removeLegacyTypes)
-                        .collect(Collectors.toList());
+                        .collect(Collectors.toList())
+                        .toArray(new LogicalType[0]);


fhueske · 2026-06-01T13:01:56Z

+                return createSqlType(SqlTypeName.TINYINT);
+            case SMALLINT:
+                return createSqlType(SqlTypeName.SMALLINT);
+


nit

Suggested change

fhueske · 2026-06-01T13:46:12Z

+        final Optional<RelDataType> resolved = resolveAllIdenticalTypes(types);
+        final RelDataType leastRestrictive =
+                resolved.isPresent() ? resolved.get() : super.leastRestrictive(types);


Nit:

Should be possible to simplified to something like

Suggested change

final Optional<RelDataType> resolved = resolveAllIdenticalTypes(types);

final RelDataType leastRestrictive =

resolved.isPresent() ? resolved.get() : super.leastRestrictive(types);

final RelDataType leastRestrictive = resolveAllIdenticalTypes(types)

.getOrElse(super.leastRestrictive(types));

fhueske · 2026-06-01T14:27:41Z

+            case DECIMAL:
+                return new DecimalType(relDataType.getPrecision(), relDataType.getScale());
+
+            // time indicators


nit:
the comment does not really apply anymore because the outer else branch of case TIMESTAMP does not handle a time indicator but a regular TIMESTAMP type.

fhueske · 2026-06-01T14:29:23Z

+                    }
+                }
+                return new LocalZonedTimestampType(relDataType.getPrecision());
+            // temporal types


TIMESTAMP and TIMESTAMP_WITH_LOCAL_TIME_ZONE are missing from the temporal types because they are handled above.

removed, not sure we need it there...

fhueske · 2026-06-01T14:56:26Z

+                                new String[] {"f1", "f2", "f3"},
+                                new LogicalType[] {
+                                    new IntType(), new BigIntType(), new VarCharType()


Suggested change

new String[] {"f1", "f2", "f3"},

new LogicalType[] {

new IntType(), new BigIntType(), new VarCharType()

List.of("f1", "f2", "f3"),

List.of(new IntType(), new BigIntType(), new VarCharType())

should work as well?

fhueske

+1 to merge.

Thank you, Fabian

[FLINK-39812][table] Migrate FlinkTypeFactory to java

3add135

liuyongvs approved these changes Jun 1, 2026

View reviewed changes

twalthr reviewed Jun 1, 2026

View reviewed changes

snuyanzin force-pushed the flink39812 branch 2 times, most recently from 9a1c183 to 2c97164 Compare June 1, 2026 14:23

Address feedback

037ef8f

snuyanzin force-pushed the flink39812 branch from 2c97164 to 037ef8f Compare June 1, 2026 14:24

fhueske reviewed Jun 1, 2026

View reviewed changes

Address feedback

002e953

snuyanzin force-pushed the flink39812 branch from 4118851 to 002e953 Compare June 1, 2026 15:07

fhueske approved these changes Jun 1, 2026

View reviewed changes

github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jun 1, 2026

snuyanzin merged commit 50ff839 into apache:master Jun 1, 2026

Conversation

snuyanzin commented Jun 1, 2026

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

Was generative AI tooling used to co-author this PR?

Uh oh!

flinkbot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

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

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

Choose a reason for hiding this comment

Uh oh!

snuyanzin Jun 1, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhueske left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

flinkbot commented Jun 1, 2026 •

edited

Loading

snuyanzin Jun 1, 2026 •

edited

Loading