Skip to content

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

Open
Izeren wants to merge 1 commit into
apache:release-1.20from
Izeren:backport-28858-1.20
Open

[FLINK-25802][FLINK-30499][table-planner] Fix TIMESTAMP codegen for RANGE OVER window bounds#28971
Izeren wants to merge 1 commit into
apache:release-1.20from
Izeren:backport-28858-1.20

Conversation

@Izeren

@Izeren Izeren commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Cherry-pick of #28858 to release-1.20.

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

  • Add explicit handling of TIMESTAMP_WITHOUT_TIME_ZONE and TIMESTAMP_WITH_LOCAL_TIME_ZONE that scales the Calcite millisecond bound to microseconds and derives epoch-microsecond values via getMillisecond() * 1000L + getNanoOfMillisecond() / 1000, preserving sub-millisecond precision without risking Long overflow
  • 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(6) and TIMESTAMP_LTZ(6) ORDER BY columns with a sub-millisecond boundary row

Verifying this change

This change added tests and can be verified as follows:

  • Added testRangeOverWithTimestamp and testRangeOverWithTimestampLtz to OverAggregateITCase (old-style BatchTestBase IT tests, consistent with release-1.20 conventions). Both tests include a sub-millisecond row to verify microsecond-precision window boundary exclusion.

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 force-pushed the backport-28858-1.20 branch 2 times, most recently from 5c729e0 to c638ac8 Compare August 13, 2026 14:47
@Izeren
Izeren force-pushed the backport-28858-1.20 branch from c638ac8 to 18c43ad Compare August 14, 2026 10:46
@Izeren
Izeren force-pushed the backport-28858-1.20 branch from 18c43ad to 3866794 Compare August 14, 2026 15:52
@Izeren
Izeren marked this pull request as ready for review August 17, 2026 08:05
@Izeren

Izeren commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

@snuyanzin, I have used old style tests for this cherry-pick to avoid resolving conflicts with batch tests. Could you please have a look when you have time?

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.

2 participants