JingsongLi commented on code in PR #8985:
URL: https://github.com/apache/paimon/pull/8985#discussion_r3712299780
##########
paimon-python/pypaimon/table/row/generic_row.py:
##########
@@ -447,6 +459,26 @@ def to_bytes(cls, row: Union[GenericRow, BinaryRow]) ->
bytes:
absolute_offset = fixed_part_size + offset_in_variable_part
offset_and_nano = (absolute_offset << 32) | nano_of_millisecond
struct.pack_into('<q', fixed_part, field_fixed_offset,
offset_and_nano)
+ elif is_variant_type:
Review Comment:
[P1] Complete the VARIANT read path before accepting it here. This branch
now lets a VARIANT bucket/primary key serialize and commit, but
`GenericRowDeserializer` has no VARIANT case and falls through to
`_parse_string()` when manifest `_MIN_KEY`/`_MAX_KEY` values are loaded. I
reproduced this with a valid `GenericVariant.from_python(-1)`: the fixed-bucket
commit succeeds, but reopening the table and calling `new_scan().plan()` raises
`UnicodeDecodeError` on the payload's `0xff` byte. Please add the matching
VARIANT decoder (offset/size, followed by the 4-byte value length, value, and
metadata) and an end-to-end commit/reopen/scan test using a real variant value
with non-UTF-8 payload bytes.
##########
paimon-python/pypaimon/table/row/generic_row.py:
##########
@@ -420,13 +426,21 @@ def to_bytes(cls, row: Union[GenericRow, BinaryRow]) ->
bytes:
'CHAR', 'VARCHAR', 'STRING', 'BINARY', 'VARBINARY', 'BYTES',
'BLOB'])
is_decimal_type = type_name.startswith('DECIMAL') or
type_name.startswith('NUMERIC')
is_timestamp_type = type_name.startswith('TIMESTAMP')
+ is_ltz_type = type_name.startswith('TIMESTAMP_LTZ') or 'WITH LOCAL
TIME ZONE' in type_name
+ is_variant_type = type_name == 'VARIANT'
if is_decimal_type or is_timestamp_type:
precision, scale = _parse_type_precision_scale(field.type)
else:
precision, scale = 0, 0
is_high_precision_decimal = is_decimal_type and precision > 18
is_non_compact_timestamp = is_timestamp_type and precision > 3
+ if is_timestamp_type:
+ if is_ltz_type:
+ value = _normalize_ltz(value)
Review Comment:
[P1] Preserve LTZ awareness when deserializing manifest statistics.
Normalizing an aware LTZ value to naive UTC makes the BinaryRow hash match
Java, but `_parse_timestamp()` still returns a naive `datetime` for both
TIMESTAMP and TIMESTAMP_LTZ. I reproduced a valid LTZ primary-key fixed-bucket
commit succeeding, followed by an equality-filtered scan failing while
comparing the naive manifest min/max with the aware Arrow predicate literal:
`TypeError: can't compare offset-naive and offset-aware datetimes`. Please
return UTC-aware values when the declared type is TIMESTAMP_LTZ (including the
WITH LOCAL TIME ZONE spelling) and add a commit/reopen/filter regression test,
ideally covering both compact and non-compact precisions.
--
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]
