Skip to content

Davis/ordering fields as precombine#570

Draft
Davis-Zhang-Onehouse wants to merge 26 commits into
apache:mainfrom
vinishjail97:davis/ordering-fields-as-precombine
Draft

Davis/ordering fields as precombine#570
Davis-Zhang-Onehouse wants to merge 26 commits into
apache:mainfrom
vinishjail97:davis/ordering-fields-as-precombine

Conversation

@Davis-Zhang-Onehouse

Copy link
Copy Markdown

Description

How are the changes test-covered

  • N/A
  • Automated tests (unit and/or integration tests)
  • Manual tests
    • Details are described below

Comment thread cpp/src/lib.rs Outdated
/// Initialize env_logger exactly once for the lifetime of the loaded shared library.
///
/// Reads RUST_LOG from the environment at the time the first FileGroupReader is created.
/// Example: RUST_LOG=hudi_core=debug enables all hudi-core debug logs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we split the changes into separate PR pls? the title suggests it's about support "ordering fields" as alias, so let's separate out logger changes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, all these PRs are still WIP, please ignore all PRs under my name.

let get_result = if matches!(self, Self::PrecombineField) {
get_result.or_else(|_| {
configs
.get("hoodie.table.ordering.fields")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to add a new enum item called OrderingFields as a table config, and mark PrecombineField as deprecated. Then all applicable callers resolve ordering fields first then fall back to precombine

@xushiyan xushiyan marked this pull request as draft April 2, 2026 17:59
Davis Zhang and others added 24 commits April 8, 2026 01:21
Root Cause

  The HoodieDeleteRecordList.avsc and HoodieDeleteRecord.avsc schemas in hudi-rs were outdated. Java's current
  Hudi codebase uses wrapper record types (BooleanWrapper, IntWrapper, LongWrapper, etc.) for the orderingVal
  union field, while hudi-rs still had raw Avro types (int, long, float, etc.).

  This caused a union index mismatch:
  - Java writes LongWrapper at union index 3 (null=0, BooleanWrapper=1, IntWrapper=2, LongWrapper=3, ...)
  - hudi-rs expected raw long at union index 2 (null=0, int=1, long=2, float=3, ...)

  When hudi-rs read index 3, it interpreted it as float (4 fixed bytes) instead of LongWrapper (a record with
  zig-zag encoded long). This consumed wrong number of bytes, shifting all subsequent reads off alignment,
  producing "Union index 24 out of bounds: 2" on the next record.

  Changes

  1. schemas/HoodieDeleteRecordList.avsc — Updated orderingVal union from raw types to wrapper record types
  matching Java
  2. schemas/HoodieDeleteRecord.avsc — Same update
  3. src/schema/delete.rs — Updated unit tests to use wrapper record values
  4. tests/file_group_reader_tests.rs — Added reproduction test + fixed empty partition path handling
  5. crates/test/src/lib.rs — Added V9MorNonpart3Commits test table variant + id_name_price helper
  6. crates/test/data/.../v9_mor_nonpart_3commits.zip — New test data from failing table
Add 4 new HoodieFileGroupReader e2e tests validating against gold data
from Hudi Spark's TestMORFileSliceLayouts:
- table_log_compaction: log-only with compacted log block
- table_log_only: log-only with data + delete blocks
- table_column_projection: base + log with full schema (23 columns)
- table_all_data_types: base + 3 log files with all data types

Fixes to support these tests:
- Add Decimal128 support in Avro-to-Arrow array reader
- Add Map type support (Avro Map -> Arrow Map, not Dictionary)
- Fix Array/List element field naming to match Parquet convention
- Add direct struct field extraction for nested types in Map/List values
- Fix log-only merge schema resolution (use reader_schema when no base file)
- Add schema reconciliation in records_to_batch for field name differences
  between Avro-derived (log) and Parquet-derived (base) schemas

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
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.

3 participants