linliu-code opened a new pull request, #19713:
URL: https://github.com/apache/hudi/pull/19713

   ### Describe the issue this Pull Request addresses
   
   Closes #19712
   
   Stacked on #19709 — review that one first. Until it merges this PR's diff 
shows its commit as well as this one's.
   
   ### Summary and Changelog
   
   Under MOR partial updates the merge in `HoodieIndexUtils` produces a record 
carrying only the columns
   named in `UPDATE SET`. That is the intended contract: `HoodieAppendHandle` 
takes the partial schema as
   its writer schema and `BaseWriteHelper` does the same for dedup. But the 
global-index merge wrapped
   that record against the **full** write schema, and 
`HoodieAvroIndexedRecord#prependMetaFields` infers
   the meta-field count as `targetSchema.size() - record.size()`. For a 
two-field record against a
   ten-field target that is 8, so `JoinedGenericRecord` treated eight slots as 
meta and the data landed
   at indices 8 and 9 rather than 7 and 8.
   
   Wrap the merged record against its own schema instead, taken from the same 
config the append handle
   and dedup path read, resolved once in the caller rather than per record, 
falling back to the write
   schema when partial updates are off. That makes `prependMetaFields`' 
existing inference correct, so
   `HoodieAvroIndexedRecord` and `JoinedGenericRecord` are untouched.
   
   Second change, required with the first: `inferPartitionPath` was deriving a 
partition from that same
   merged record, and `KeyGenUtils#getPartitionPath` substitutes the default 
partition for a field it
   cannot find, so the payload already carried `dt=__HIVE_DEFAULT_PARTITION__`. 
Harmless only because the
   write failed first; fixing the schema alone would turn a loud failure into a 
silently mis-partitioned
   row. A merge cannot change a field the record does not carry, so the 
existing record's partition is
   correct by construction. Gated on partial updates so full-schema paths keep 
resolving through the key
   generator, and the partition-field check strips the mandatory `field:TYPE` 
spec that the custom key
   generators keep verbatim in `getPartitionPathFields`.
   
   Tests: five parameterized cases over `GLOBAL_BLOOM`, `GLOBAL_SIMPLE`, 
`RECORD_INDEX` and
   `GLOBAL_RECORD_LEVEL_INDEX`, plus a boundary case pinning the analysis-time 
rejection when the index
   updates the partition path. They assert row values, 
`_hoodie_partition_path`, and the log block via
   the existing `validateLogBlock`, which checks `IS_PARTIAL` and that the 
block schema is the meta
   fields over the assigned columns only. `validateLogBlock` gained a defaulted 
`partitionPath`
   parameter so it can address a partitioned table.
   
   ### Impact
   
   Fixes `MERGE INTO` with a partial `UPDATE SET` on MOR with a global bloom or 
simple index. No
   behaviour change when partial updates are off, since both new schemas then 
resolve to the write schema
   and the partition guard is gated off.
   
   Two points a reviewer may want to push on, stated rather than left to be 
found:
   
   The partial schema is read from `config.getPartialUpdateSchema()`. 
`RecordContext#getSchemaFromBufferRecord`
   would give the merged record's schema directly and be correct by 
construction; it returned null when
   tried, because each `RecordContext` holds its own schema cache and the 
merger is built from the
   incoming context. The config route mirrors `HoodieAppendHandle` and 
`BaseWriteHelper`, but the other
   shape may be preferable and is easy to swap if so.
   
   Relatedly, `WRITE_PARTIAL_UPDATE_SCHEMA` is the union across update clauses 
while each clause projects
   only its own assignments, so config-equals-record holds for the 
single-clause shape the tests cover.
   A divergent multi-clause statement faults earlier today, inside the 
payload's serializer, so this
   change does not introduce that gap, but it is not an invariant either.
   
   ### Risk Level
   
   low
   
   Two methods in one file, gated so that only the partial-update path changes. 
Verified on this branch:
   the two global bloom and simple cases fail on the parent commit with the 
exception from #19712 and
   pass here; `TestPartialUpdateForMergeInto`, `TestMergeIntoTable` and 
`TestMergeIntoTable2` pass
   together (67 tests); checkstyle and scalastyle clean.
   
   An earlier revision of the partition guard was not gated and checked the 
partition fields by raw name.
   Since the custom key generators return `field:TYPE` verbatim, that would 
have silently disabled
   partition re-keying for every table using them, including full-schema merges 
unrelated to partial
   updates. Both the gate and the suffix stripping exist to close that.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


-- 
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]

Reply via email to