fix(spec): encode TIME columns in the batch BinaryRow path - #823
Open
jackylee-ch wants to merge 1 commit into
Open
fix(spec): encode TIME columns in the batch BinaryRow path#823jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
downcast_columnshas an arm for 16 of Paimon's types but notTIME, so writingto a table whose partition key, primary key or
bucket-keyhas a TIME columnfails before routing:
Measured on all three shapes, which Java accepts:
TIMEis absent fromPRIMARY_KEY_UNSUPPORTED_LOGICAL_TYPES, gating primary and partition keys alike,and from
validateBucket. They reach paimon-rust from Flink or Spark, since theDataFusion 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 thet=12%3A34%3A56.123/form) and thehivebucket function.Brief change log
One arm, mirroring
Date. Java writes TIME with the samewriteIntas INTEGERand DATE, so in the binary format a TIME is an int millis-of-day;
write_intreproduces those four bytes and the zeroed upper half of the slot that
hash_codecovers. That is also why no precision arm is needed, unlike
Timestamp.Tests
test_batch_vs_per_row_equivalencegains a TIME column with a null, comparingbytes and hash codes against
BinaryRow::from_arrow. Two production-path testscover routing (bucket asserted against
hash_code, not a constant) and a TIMEpartition 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.