SteNicholas opened a new issue, #477: URL: https://github.com/apache/paimon-cpp/issues/477
### Search before asking - [x] I searched in the [issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar. ### Motivation Paimon Java accepts `TIME` columns in Lance data files: `LanceFileFormat.LanceRowTypeVisitor.visit(TimeType)` accepts the type, `ArrowFieldTypeConversion` writes it as Arrow `TIMEMILLI` (`time32[ms]`), and the Lance type mapping in Paimon's `docs/docs/concepts/spec/fileformat.md` lists `TIME → TIME32 / TIME64`. Paimon C++ has represented `TIME` as `time32[ms]` since #378, and Parquet and Vortex already support it. Lance does not: - `SchemaValidation::ValidateLanceDataField()` has no `TIME32` case. A TIME column falls through to `default` and is rejected with `Lance file format does not support type time32[ms]` (`src/paimon/core/schema/schema_validation.cpp:1026`). `SchemaValidationTest.TestLanceDataTypes` asserts this rejection (`src/paimon/core/schema/schema_validation_test.cpp:480`). - `LanceStatsExtractor::CreateEmptyStats()` has no `TIME32` case either (`src/paimon/format/lance/lance_stats_extractor.cpp:108`). Lifting only the validation would therefore still fail when an append data file with a TIME column is finalized, with `cannot create empty Lance statistics for type time32[ms]`. This is the same failure `VortexTimeColumnWriteInteTest.TestAppendTimeColumnFinalize` guards against for Vortex. Both sides use the same physical type, so the gap is only the C++ gating. ### Solution 1. Accept `TIME32` in `ValidateLanceDataField()`, including when nested in ARRAY and ROW. Non-millisecond units are already rejected earlier by `TableSchema::Create()` (`Only millisecond TIME is supported`). 2. Add a `TIME32` case to `LanceStatsExtractor::CreateEmptyStats()`, following `VortexStatsExtractor` (`src/paimon/format/vortex/vortex_stats_extractor.cpp:109`), which returns empty `CreateIntColumnStats`. 3. No reader change is expected. The declared precision comes from the table schema (`paimon.time.precision` field metadata), and `LanceFileBatchReader::AlignBatch()` aligns each batch to the read type. ### Anything else? Suggested validation: - Update the `TestLanceDataTypes` expectations so millisecond TIME, top level and nested, is accepted. - Add a format-level round trip in `src/paimon/format/lance/lance_file_format_test.cpp` covering nulls and TIME nested in ARRAY and ROW. - Add a table-level append write and finalize test, like `VortexTimeColumnWriteInteTest.TestAppendTimeColumnFinalize` (`test/inte/write_and_read_inte_test.cpp:5984`). - Add a Java compatibility fixture with a TIME column, read from C++. The README of `test/test_data/lance/append_java_compat.db/append_java_compat` says "TIME32 is omitted because the C++ table path does not support it", which no longer holds. Prefer adding a new fixture over rewriting the existing one. This needs no storage format or protocol change. `TIMESTAMP_LTZ` support for Lance is tracked separately. ### Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- 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]
