flink icon indicating copy to clipboard operation
flink copied to clipboard

[FLINK-17224][table] Support precision of TIME type

Open raminqaf opened this issue 3 months ago • 3 comments

What is the purpose of the change

Add precision supports for the TIME function. Currently, Flink only takes precision 0 and ignores any other type of precision.

Brief change log

  • Pass precision to TIME funciton.
  • Fixed the TIME conversion formula in JsonToRowDataConverters.convertToTime() from localTime.toNanoOfDay() * 1000_000 to localTime.toNanoOfDay() / 1000_000
  • Enhanced test coverage in JsonRowDataSerDeSchemaTest.testSerDe() to include comprehensive testing of all supported data types
  • Updated test assertions to use correct field indexing and proper row size

Verifying this change

This change is already covered by existing tests, such as:

  • JsonRowDataSerDeSchemaTest.testSerDe() - Enhanced to comprehensively test TIME type conversion for both JsonParser and non-JsonParser deserializers
  • The test now verifies correct handling of TIME(0) and TIME(3) precision levels
  • Added test coverage for the previously failing scenario that caused the DateTimeException

This change added tests and can be verified as follows:

  • The enhanced testSerDe() method now tests the complete round-trip serialization/deserialization for TIME types
  • Verified that the fix resolves the DateTimeException: Invalid value for HourOfDay error
  • Confirmed that both isJsonParser=true and isJsonParser=false code paths work correctly
  • Manually verified that TIME values like "12:12:43" and "12:12:43.123" are properly converted without precision loss

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: yes - affects JSON format serialization/deserialization of TIME types
  • The runtime per-record code paths (performance sensitive): no - this is format-specific conversion logic
  • 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 - this is a bug fix for existing functionality

raminqaf avatar Sep 01 '25 10:09 raminqaf

The TimeLongConvert converts the time to millisecond precision. I was wondering if we need to change this to TimeLongConverter implements DataStructureConverter<Long, Long> so that we internally support nanosecond precision.

raminqaf avatar Sep 01 '25 10:09 raminqaf

CI report:

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

flinkbot avatar Sep 01 '25 10:09 flinkbot

A similar PR exists: https://github.com/apache/flink/pull/22775

raminqaf avatar Sep 01 '25 11:09 raminqaf