test(trino): add MoR read tests for delete markers, custom payloads and commit-time ordering#19295
Draft
voonhous wants to merge 6 commits into
Draft
test(trino): add MoR read tests for delete markers, custom payloads and commit-time ordering#19295voonhous wants to merge 6 commits into
voonhous wants to merge 6 commits into
Conversation
…-105)
Squash of the 55-commit review history. The connector implementation moves
into the Hudi repo as the hudi-trino module (published as
org.apache.hudi:hudi-trino), targeting Trino SPI 481 on JDK 25, behind the
opt-in hudi-trino Maven profile. The Trino repo keeps only a thin trino-hudi
shim depending on the published artifact. Removes hudi-trino-bundle and the
trino{base,coordinator,worker} docker modules.
Co-authored-by: vamsikarnika <vamsikarnika@gmail.com>
Co-authored-by: Aditya Goenka <ad1happy2go@gmail.com>
… parquet reader MOR tables written at table version 10 store delta-log blocks as native parquet files (RFC-103). Reading one calls HoodieIOFactory.getFileFormatUtils(PARQUET).readFooter(...) to pull the log-block header out of the parquet footer, but HudiTrinoIOFactory stubbed getFileFormatUtils out, so any realtime read of a v10 MOR table failed with "FileFormatUtils not supported in HudiTrinoIOFactory" (surfaced by TestHudiCustomMerger and TestHudiCustomMergerEndToEnd, which write their tables at the default version). Add HudiTrinoParquetFileFormatUtils, implementing readFooter with Trino's own parquet reader (MetadataReader.readFooter -> FileMetadata.getKeyValueMetaData), mirroring ParquetUtils.readFooter semantics, and return it from HudiTrinoIOFactory.getFileFormatUtils for PARQUET; non-parquet native log formats now throw a clear message. Record decoding already flows through Trino's page-source reader, so the footer read was the only missing piece. No new dependency is added (hudi-hadoop-common stays excluded).
The reader context ignored its path/schema arguments and re-served the pre-built base page source, so RFC-103 native (parquet) log records were never read and v10 MOR reads returned base-only values. Build a Trino parquet page source on demand for native log files, projected on the file-group reader's requiredSchema with predicate pushdown off, via a factory injected from HudiPageSourceProvider. Synthesize VARCHAR handles for Hudi meta columns the file-group reader adds to requiredSchema beyond the connector projection (e.g. _hoodie_commit_time); all meta columns are UTF8 strings on disk and parquet columns are resolved by name, so the placeholder ordinal is never read (refactor tracked by apache#19249). Custom mergers must be projection compatible: a non-projection-compatible merger makes the file-group reader demand the full table schema, whose unprojected data columns cannot be typed at this layer; fail such reads with a clear NOT_SUPPORTED error and declare isProjectionCompatible() on the test mergers, which merge on their declared mandatory fields only. Also adapt the TestHudiSmokeTest footer-read call site to the parameterized createPageSource helper.
voonhous
changed the title
Collaborator
voonhous
marked this pull request as draft
July 15, 2026 06:14
voonhous
force-pushed
the
18898--mor-merge-semantics-read-tests
branch
2 times, most recently
from
July 15, 2026 08:39
ba38225 to
b597403
Compare
3 tasks
voonhous
force-pushed
the
18898--mor-merge-semantics-read-tests
branch
from
July 15, 2026 09:16
b597403 to
4c8ae68
Compare
3 tasks
…K 25 compile Master commit b0a2563 declares Lombok in the root pom's annotationProcessorPaths (a parallel-build deflake). hudi-trino inherits that path but compiles on JDK 25, where Lombok 1.18.36 crashes javac with ExceptionInInitializerError: com.sun.tools.javac.code.TypeTag :: UNKNOWN -- this broke the hudi-trino CI on every PR merge ref once that commit landed. hudi-trino uses no annotation processors, so set proc=none and override the inherited processor path away.
Fixes apache#19249, including the scope update in issue comment 4937143911; relates to the base read path of apache#18837. The file-group reader can require merge columns the query projection does not carry: Hudi meta fields like _hoodie_commit_time, delete markers, record-key data columns, a custom merger's mandatory fields, or the whole table schema when a CUSTOM merger is not projection compatible. buildRequiredColumnHandles previously hand-synthesized a HIVE_STRING/VARCHAR handle behind a HOODIE_META_COLUMNS guard and threw NOT_SUPPORTED for anything else, and the base-file read silently returned projection-only records that made such mergers see nulls. Log side: - Every required-schema field carries its real Avro type from the table schema, so unresolved handles are typed directly from the field (NativeLogicalTypesAvroTypeBlockHandler + HiveTypeTranslator) via the new HudiUtil.toColumnHandle; the synthesis, guard and exception are gone. trino-hive-formats moves from runtime to compile scope. Base side: - HudiPageSourceProvider mirrors the file-group reader's decision: HudiUtil.resolveMergeModeAndStrategyId applies the same version-gated inference as hudi-common (merge mode below table version 9, strategy id below version 8), and when the resolved CUSTOM merger is not projection compatible (interface default; also every payload-based table, since the payload strategy resolves HoodieAvroRecordMerger), the read projection expands to every table-schema column via HudiUtil.appendMissingSchemaColumns. Matches Spark; the extra I/O is inherent to such mergers. - getMergeRequiredColumnHandles predicts every column FileGroupReaderSchemaHandler.getMandatoryFieldsForMerging can demand: ordering columns under the inferred (not raw) merge mode, _hoodie_is_deleted and _hoodie_operation unconditionally (dropped when not table columns), the custom delete key when both hoodie.payload.delete.field and .marker are set, and the record-key data columns when hoodie.populate.meta.fields is false. - The base read reuses the pre-built page source, so its projection MUST cover requiredSchema: HudiTrinoReaderContext now guards that invariant and fails with HUDI_SCHEMA_ERROR naming any missing columns instead of merging with nulls. MOR splits with log files but no base file fail with a clear NOT_SUPPORTED instead of a bare NoSuchElementException. Cleanups in the same area: - Replace SynthesizedColumnHandler with PrefilledColumnValues, a thin per-split adapter over trino-hive's HiveUtil.getPrefilledColumnValue. Fixes $partition rendering multi-key partitions in hash order, handles the hive null marker (backslash-N) in partition values, adds previously unsupported partition-key types, and deletes SynthesizedColumnStrategy plus the hand-rolled appendPartitionKey dispatch. - Remove dead code (HudiTrinoReaderContext.colToPosMap and dataHandles, HudiPageSource.readerContext); make fields private final. - remapColumnIndicesToPhysical throws a diagnosable TrinoException instead of an autoboxing NPE for columns absent from the parquet file schema, and lowercases with Locale.ROOT. Tests: - TestHudiNonProjectionCompatibleMerger + NonProjectionCompatibleRankMerger (no isProjectionCompatible/getMandatoryFieldsForMerging overrides) + initializer: narrow-projection MOR queries that never project merge_rank, laid out so one key's winning rank is on the log record and the other's on the base record; sum(value)=199 discriminates the correct merge from base-only (110) and newest-wins (103). - Unit coverage: toColumnHandle, appendMissingSchemaColumns, usesNonProjectionCompatibleMerger, resolveMergeModeAndStrategyId (table versions 6/8/9), merge-required column prediction, PrefilledColumnValues (incl. the $partition ordering fix), and the typed missing-column remap error (both remap tests). - Existing projection-compatible merger tests stay as fast-path regression coverage; stale MaxRankRecordMerger javadoc refreshed.
…nd commit-time ordering Fixes apache#18898 (follow-up from the RFC-105 review of HudiTrinoReaderContext.getRecordMerger): the merge-mode dispatch had no end-to-end coverage where the merger choice matters. Adds runtime-written v10 MOR tables and snapshot-read tests for: - Deletes under EVENT_TIME_ORDERING (mor_deletes): hard deletes -- which at v10 are native delete log files read back through the connector's own getFileRecordIterator with the synthetic delete-log schema, a previously untested path enabled by the required-column resolution this branch stacks on -- plus _hoodie_is_deleted soft deletes, an OBSOLETE soft delete and an OBSOLETE update (lower ordering value) that must LOSE against the base row. - COMMIT_TIME_ORDERING (mor_commit_time): a lower-ordering update that must WIN (latest write), the exact mirror of the event-time case, discriminating OverwriteWithLatestMerger from event-time merging. - Payload-driven semantics without any hudi.record-merger-impls property: AWSDmsAvroPayload (v10-translated to COMMIT_TIME + delete-key props; Op='D' log records delete rows at merge time), OverwriteNonDefaultsWithLatestAvroPayload (v10-translated to PARTIAL_UPDATE_MODE=IGNORE_DEFAULTS; null update columns keep stored values), and a user-defined SummingTestPayload riding the payload-based CUSTOM strategy whose merged value (old + new) proves the payload's combineAndGetUpdateValue executed at read time. Test rows are written with the pass-through HoodieAvroPayload (not a BaseAvroPayload) so delete-flagged rows land as data records and every merge decision happens at read time from the table config. Bugs found by these tests and fixed here: - HudiUtil.mergeRequiredColumnNames read the custom delete key/marker from raw table props, but v9+ table creation persists them PREFIXED (hoodie.record.merge.property.*). The file-group reader strips the prefix via getTableMergeProperties before DeleteContext reads the plain keys, so its required schema included the delete column while the connector's base-read prediction missed it and the base-read projection guard failed every narrow projection on such tables. Now resolved the same way the reader does: table merge properties first, raw props as fallback. Unit-tested in TestHudiMergeRequiredColumns. - buildColumnHandles matched predicted merge columns against metastore data columns case-sensitively; the prediction carries the table schema's casing (AWSDms hardcodes 'Op') while metastore names are lowercased, so the handle was never built. Matching is now case-insensitive. - buildRequiredColumnHandles emitted projection handles with the lowercased metastore name on the log projection, but hudi-common reads merge fields off log records by the table schema's exact field name (Avro lookups are case-sensitive), so e.g. the DMS delete marker was never seen on log records. Handles are now re-labeled with the required-schema field's exact name; parquet columns resolve case-insensitively either way. - count(*) over a MoR split with log files failed with "Failed to construct Avro schema": the query projects no columns, and HudiPageSource's serializer fed the empty projection to AvroHiveFileUtils.determineSchemaOrThrowException, which rejects an empty column list. HudiUtil.constructSchema now short-circuits an empty projection to an empty record schema; the page source already counts positions fine with zero channels. HudiUtilTest's empty-columns test now pins the empty-record schema instead of the former exception. - Payload-based merging read garbage on the summing table ([, -44] instead of [k1, 109]): records handed to hudi-common carried a schema reconstructed from Hive metastore types (every column a nullable union, fields in projection order), while the file-group reader tracks its required schema for those records. BaseAvroPayload round-trips the newer record through Avro binary with the tracked schema, so the structural mismatch misaligned the binary decode. HudiTrinoReaderContext now builds the log-side AND base-side serializers with requiredSchema itself, via a new HudiAvroSerializer constructor that takes the record schema explicitly and maps page channels to record fields BY NAME (the base projection's order can differ from the required schema's field order; containment holds in both directions through the existing base-read guard and generateRequiredSchema always containing the requested schema). This also aligns record layouts with the positions hudi-common derives from the tracked schema (partial-update merging, _hoodie_operation lookup). - (hudi-common, engine-agnostic) The file-group reader passed its raw input props to initRecordMerger, the MERGE_TYPE lookup and the schema handler instead of the ConfigUtils.getMergeProps view, which layers the table's prefixed merge properties (hoodie.record.merge.property.*) in as plain keys -- a regression from commit 19961e7 replacing the previous in-place putAll with a copy. DeleteContext therefore missed the translated DMS delete key/marker, the delete column was not a mandatory merge field, and Op='D' log records merged as regular data updates instead of deletes on snapshot reads. Fixed in HoodieFileGroupReader and HoodieLsmFileGroupReader alike by passing this.props downstream. Also re-ports a shim-lineage guard (trino repo commit 78209f83b50) that the RFC-105 migration squash predates: HudiAvroSerializer's default constructor now maps channels past hidden (synthesized, split-prefilled) columns instead of relying on serialize() never being called with them; with identity positions a hidden column would shift every later value into the wrong field and overrun the record. Also replaces the TODO(apache#18898) in getRecordMerger with a pointer to the new suites.
voonhous
force-pushed
the
18898--mor-merge-semantics-read-tests
branch
from
July 15, 2026 16:49
4c8ae68 to
b57f97e
Compare
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
2 participants
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.
Stacked on #19288 (which stacks on #18837) -- only the head commit is new here.
Describe the issue this Pull Request addresses
Closes #18898
HudiTrinoReaderContext.getRecordMergerdispatches on the table's merge mode, but every existing MoR test uses a default payload where all mergers give the same answer. Deletes, custom payloads, and commit-time ordering -- the cases where the dispatch actually matters -- had no coverage.Summary and Changelog
Adds runtime-written MOR tables (current table version, native logs) and snapshot-read tests:
mor_deletes(event-time): hard deletes, which at v10 are native delete log files read back through the connector'sgetFileRecordIterator(previously untested, and only readable on top of feat(trino): resolve merge-required columns from the table schema #19288);_hoodie_is_deletedsoft deletes; and an obsolete delete + update with a lower ordering value that must lose to the base row.mor_commit_time: the same lower-ordering update must win here (latest write wins). The pair tells the two mergers apart.hudi.record-merger-implsset: AWSDmsOp='D'marker deletes, OverwriteNonDefaults partial updates, and a test payload that sums the old and new values -- the summed result can only come fromcombineAndGetUpdateValuerunning at read time.Writing these flushed out three bugs, fixed here:
hoodie.record.merge.property.*). Narrow projections on e.g. AWSDms tables tripped the base-read projection guard.buildColumnHandlesmatched predicted columns against metastore names case-sensitively, so a schema-cased delete key likeOpnever got a handle.Also drops the
TODO(apache/hudi#18898)comment ingetRecordMerger.Impact
Read-path fixes only: narrow projections on tables with prefixed delete-marker props no longer fail, and mixed-case merge columns resolve on both base and log reads. No API or config changes.
Risk Level
low -- mostly tests; each of the three fixes is small and pinned by a new test.
Documentation Update
none
Contributor's checklist