Skip to content

[FLINK-25802][FLINK-30499][table-planner] Fix TIMESTAMP codegen for RANGE OVER window bounds - #28967

Merged
snuyanzin merged 1 commit into
apache:release-2.3from
Izeren:backport-28858-2.3
Aug 14, 2026
Merged

[FLINK-25802][FLINK-30499][table-planner] Fix TIMESTAMP codegen for RANGE OVER window bounds#28967
snuyanzin merged 1 commit into
apache:release-2.3from
Izeren:backport-28858-2.3

Conversation

@Izeren

@Izeren Izeren commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Cherry-pick of #28858 to release-2.3.

What is the purpose of the change

RangeBoundComparatorCodeGenerator produces Janino compile errors for RANGE OVER windows with TIMESTAMP ORDER BY columns. Two separate bugs:

  1. (FLINK-25802) TIMESTAMP columns are bound as BigIntType but the raw TimestampData term is passed directly to ExprCodeGenerator.bindInput(), causing an invalid (Long) timestampData cast at runtime.

  2. (FLINK-30499) TIMESTAMP_WITH_LOCAL_TIME_ZONE falls through the case _ default with no special handling, and ctx.reuseMemberCode() is emitted inside the compare() method body instead of at class scope, causing a Janino compile error for any type that triggers member code generation.

Both bugs surface as:

InvalidProgramException: Table program cannot be compiled. This is a bug.
  Caused by: CompileException: Cannot cast "org.apache.flink.table.data.TimestampData" to "java.lang.Long"

Brief change log

  • Replace LogicalTypeRoot match with LogicalTypeFamily.TIMESTAMP / LogicalTypeFamily.TIME if/else to cover all three TIMESTAMP variants uniformly (and remain forward-compatible with future additions)
  • Extract epoch milliseconds via .getMillisecond() before passing the value to ExprCodeGenerator.bindInput(), so the variable type matches the declared BigIntType
  • Promote child CodeGeneratorContext member code to the parent context via addReusableMember(), placing class-level declarations at class scope rather than inside compare()
  • Add regression tests in batch/sql/OverAggregateITCase for TIMESTAMP(3) and TIMESTAMP_LTZ(3) ORDER BY columns

Verifying this change

  • Unit 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 (codegen path only; no per-record overhead)
  • Anything that affects deployment or recovery: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no

Was generative AI tooling used to co-author this PR?
  • Yes (Claude Code)

@flinkbot

flinkbot commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

CI report:

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

@Izeren
Izeren marked this pull request as ready for review August 13, 2026 09:52
@Izeren
Izeren marked this pull request as draft August 13, 2026 10:25
…ER` window bounds

RangeBoundComparatorCodeGenerator bound TIMESTAMP columns as BigIntType
but passed the raw TimestampData term to ExprCodeGenerator, causing an
invalid (Long) cast at Janino compile time (FLINK-25802).
TIMESTAMP_WITH_LOCAL_TIME_ZONE fell through to the default case, and
reuseMemberCode() was emitted inside compare() instead of the class scope
(FLINK-30499).

Fix: use LogicalTypeFamily.TIMESTAMP to cover all three TIMESTAMP variants,
extract epoch time with microseconds precision, and promote child member code
to the parent context via addReusableMember().

Generated-by: Claude Code
@Izeren
Izeren force-pushed the backport-28858-2.3 branch from e899647 to 8c612a2 Compare August 13, 2026 10:36
@Izeren
Izeren marked this pull request as ready for review August 13, 2026 14:52
@Izeren

Izeren commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@snuyanzin, CI is green PTAL when you have time

@snuyanzin
snuyanzin merged commit 9b6a46c into apache:release-2.3 Aug 14, 2026
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