Skip to content

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

Open
Izeren wants to merge 1 commit into
apache:masterfrom
Izeren:fix/FLINK-25802-timestamp-range-over-codegen
Open

[FLINK-25802][FLINK-30499][table-planner] Fix TIMESTAMP codegen for RANGE OVER window bounds#28858
Izeren wants to merge 1 commit into
apache:masterfrom
Izeren:fix/FLINK-25802-timestamp-range-over-codegen

Conversation

@Izeren

@Izeren Izeren commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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 Jul 31, 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

…ANGE OVER 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 millis via .getMillisecond(), and promote child member code
to the parent context via addReusableMember().

Generated-by: Claude Code
@Izeren
Izeren force-pushed the fix/FLINK-25802-timestamp-range-over-codegen branch from d9de951 to 4933b95 Compare August 3, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants