Skip to content

fix(spec): encode TIME columns in the batch BinaryRow path - #823

Open
jackylee-ch wants to merge 1 commit into
apache:mainfrom
jackylee-ch:fix/encode-time-batch-binary-row
Open

fix(spec): encode TIME columns in the batch BinaryRow path#823
jackylee-ch wants to merge 1 commit into
apache:mainfrom
jackylee-ch:fix/encode-time-batch-binary-row

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Purpose

downcast_columns has an arm for 16 of Paimon's types but not TIME, so writing
to a table whose partition key, primary key or bucket-key has a TIME column
fails before routing:

Unsupported data type Time(TimeType { nullable: true, precision: 3 })
for batch column downcast at column 0

Measured on all three shapes, which Java accepts: TIME is absent from
PRIMARY_KEY_UNSUPPORTED_LOGICAL_TYPES, gating primary and partition keys alike,
and from validateBucket. They reach paimon-rust from Flink or Spark, since the
DataFusion frontend cannot declare a TIME column.

Two write-path places already treat TIME as a legal key and are unreachable for
it: partition-value rendering in partition_utils (a green test covers the
t=12%3A34%3A56.123/ form) and the hive bucket function.

Brief change log

One arm, mirroring Date. Java writes TIME with the same writeInt as INTEGER
and DATE, so in the binary format a TIME is an int millis-of-day; write_int
reproduces those four bytes and the zeroed upper half of the slot that hash_code
covers. That is also why no precision arm is needed, unlike Timestamp.

Tests

test_batch_vs_per_row_equivalence gains a TIME column with a null, comparing
bytes and hash codes against BinaryRow::from_arrow. Two production-path tests
cover routing (bucket asserted against hash_code, not a constant) and a TIME
partition key including its rendered path. All three fail with the error above
without the arm.

API and Format

No API, wire or storage change. Bytes written for TIME now match Java's.

Follow-up: Rust's key-type validation is thinner than Java's, so Array / Map /
Row / Multiset still reach this catch-all as a write error rather than being
refused at create time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant