rangareddy commented on issue #17356:
URL: https://github.com/apache/hudi/issues/17356#issuecomment-5351238315
This issue was reviewed as part of the JIRA-migrated backlog triage
(HUDI-8923).
**Findings: partially delivered. All six logical types are supported on
column-stats index version V2, but not on V1 - which is every table at table
version 8 or below.**
**What has landed.**
`hudi-common/src/main/java/org/apache/hudi/metadata/stats/ValueType.java`
declares `TIME_MILLIS`, `TIME_MICROS`, `TIMESTAMP_MILLIS`, `TIMESTAMP_MICROS`,
`LOCAL_TIMESTAMP_MILLIS` and `LOCAL_TIMESTAMP_MICROS` (plus nanos variants),
each with its own cast/to/from converters - for example `castToTimeMillis` at
`:471`, `toTimeMillis` at `:582`, `fromTimeMillis` at `:586`. That arrived with
PR #13711, `feat(metadata): Improve Logical Type Handling on Col Stats`
(`be6f08d56643`, merged 2025-09-23).
**The gate.** `HoodieIndexVersion.getCurrentVersion(...)`:
```java
case COLUMN_STATS:
case PARTITION_STATS:
case EXPRESSION_INDEX:
// column stats, partition stats, expression index must be updated together
if (tableVersion.lesserThan(HoodieTableVersion.NINE)) {
return V1;
}
return V2;
```
and on V1 both `ValueType.wrapValue()` (`:162`) and `unwrapValue()` (`:181`)
short-circuit into the legacy path:
```java
if (meta.getValueType() == V1) {
return primitiveWrapperType.wrap(val);
}
```
#13711's description confirms this is deliberate: *"For backwards
compatibility we have a V1 type that will call the legacy methods. So when v1
col stats index is used, every value will have a type of V1."*
**The V1 path still has exactly the gap described here.**
`hudi-common/src/main/avro/HoodieMetadata.avsc` declares only two
time/timestamp wrappers - `TimeMicrosWrapper` and `TimestampMicrosWrapper`.
There is no `TimeMillisWrapper`, `TimestampMillisWrapper`,
`LocalTimestampMillisWrapper` or `LocalTimestampMicrosWrapper`. Correspondingly
`HoodieAvroWrapperUtils` unwraps only `DateWrapper`, `LocalDateWrapper`,
`TimestampMicrosWrapper` and `DecimalWrapper` (`:207-221`), and its
`PrimitiveWrapperType` enum (`:248-254`) covers only `BOOLEAN`, `INT`, `LONG`,
`FLOAT`, `DOUBLE`, `STRING`, `BYTES`.
Against the list in the description:
| logical type | V2 (table version 9+) | V1 (table version <= 8) |
| --- | --- | --- |
| `TimeMillis` | supported | **not supported** |
| `TimeMicros` | supported | wrapper exists |
| `TimestampMillis` | supported | **not supported** |
| `TimestampMicros` | supported | supported |
| `LocalTimestampMillis` | supported | **not supported** |
| `LocalTimestampMicros` | supported | **not supported** |
Two further signals that this area is not settled: `fd79a1682e7e`,
`fix(ingest): Repair affected logical timestamp milli tables (#14161)`, is a
**data repair** follow-up, and #13711's own commit log includes *"Timestamp
millis seem to be converted to micros in the reader"*.
**Remaining work**, part of which is a product decision rather than a patch:
either support these logical types on the V1 column-stats and partition-stats
path, or declare "upgrade to table version 9" the official answer and record
that here along with the migration path for existing tables. At the moment
neither is written down, so users on table version 8 have no documented answer.
Keeping this open.
--
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]
