zhang-arvin commented on PR #10200: URL: https://github.com/apache/paimon/pull/10200#issuecomment-5886548513
## Narrowing the type is the wrong entry point — the loss is a design-wide millisecond contract, not a declaration bug Moving the fail-fast from `TimeType` down to the write paths does not work either. Please consider before we spend CI cycles on it: **1. The current tests assert truncation-to-millis is correct.** `ArrowFormatWriterTest#testArrowBundleRecordsWithTimeAndFixedBinaryVectors` (paimon-arrow, lines 752-796) declares `TIME(6)`/`TIME(9)` and asserts micros `12345678` and nanos `12345678901` both read back as `12345` ms. It comments that external Arrow producers legitimately use `TimeMicroVector`/`TimeNanoVector`. **2. `TIME(4..9)` declaration is deliberately allowed.** `SchemaValidation.validateIcebergTimePrecisions` (line 643) gates only under Iceberg metadata; `SchemaManagerTest:289` builds a historical table with `TIME(6)`; `DataTypesTest:119` asserts `new TimeType(9)`; `SchemaMergingUtilsTest:848/896/910/918` merge `TimeType(6)/(9)`. **3. CDC produces `TIME(6)` on purpose** (`PostgresRecordParser:171`, `DebeziumSchemaUtils:453`), and `TypeConverterTest:72`/`PaimonMetadataApplierTest:418` assert it must be creatable. **Where the value is actually *changed* (not just representation-limited):** `DateTimeUtils.parseFraction` (line 340, 399-412) rounds on the 4th fractional digit, so `12:34:56.1234567` becomes `...124`. That, plus `BinaryStringUtils.toTime` (line 294, no precision arg) and the CDC `toMillisOfDay()` cast (`FlinkCDCToPaimonDataConverter:125`), are the only paths that alter input values. **Minimal viable落点**: reject/warn in the string→TIME cast (`CAST(x AS TIME(p))`, `parseFraction`) where a real value change occurs, or emit a warning in `FlinkCDCToPaimonTypeConverter` when a `TIME(p>3)` route is registered. Both keep declaration semantics and existing tests intact. Avro already does the format-level rejection correctly (`AvroSchemaConverter:154`) — that is the pattern, but it only applies to formats that truly cannot represent the value; Arrow/Parquet/ORC define milliseconds as the contract. Risk: extending TIME to micros/nanos is a breaking change touching `RowCompactedSerializer`, Parquet `TIME_MICROS`, Arrow vector mapping and Iceberg conversions — needs its own issue. *This analysis was generated by an AI agent (zhang-arvin's assistant).* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
