voonhous commented on code in PR #19236:
URL: https://github.com/apache/hudi/pull/19236#discussion_r3560399130
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLanceDataSource.scala:
##########
@@ -1422,11 +1441,30 @@ class TestLanceDataSource extends
HoodieSparkClientTestBase {
s"DESCRIPTOR default should populate reference on plain read (id=$id)")
}
- // read_blob() under CONTENT mode is what we use to verify the
post-compaction bytes
- // because read_blob() on INLINE rows throws under the DESCRIPTOR default.
The bytes can
- // only come back if HoodieSparkLanceReader's CONTENT pin held during the
compactor's
- // base-file read — otherwise untouched ids 3..5 would have been rewritten
with null
- // `data` and CONTENT-mode read would surface that.
+ // Plain projection under CONTENT first: on a regression this fails with
an explicit
Review Comment:
Right -- with `validateBlobRow` in the writer, a DESCRIPTOR leak now aborts
the rewrite itself, so this projection can never see `{INLINE, null, populated
reference}`. Reworded both comments (compaction + clustering) to say what the
projection actually backstops -- the `{INLINE, null, null}` shape the guard
deliberately allows -- and pointed at `TestSparkReaderContextFactory` for the
CONTENT pin.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLanceDataSource.scala:
##########
@@ -1443,6 +1481,150 @@ class TestLanceDataSource extends
HoodieSparkClientTestBase {
}
}
+ /**
+ * Clustering must preserve INLINE blob bytes under the DESCRIPTOR default.
Clustering rewrites
+ * ALL rows through
MultipleSparkJobExecutionStrategy.readRecordsForGroupAsRow, which reads base
+ * files through the same internal write-side reader context as compaction
+ * (SparkReaderContextFactory -> SparkFileFormatInternalRowReaderContext ->
SparkLanceReaderBase).
+ * Before SparkReaderContextFactory pinned {@code
hoodie.read.blob.inline.mode=CONTENT} for
+ * internal reads, the first clustering of a Lance table with INLINE blobs
read null {@code data}
+ * (the DESCRIPTOR default) and rewrote every row's blob with null bytes,
silently losing all
+ * blob content (#19232). This test bulk-inserts INLINE blobs, triggers
inline clustering,
+ * asserts a replacecommit actually completed, and verifies every row's
bytes survived.
+ */
+ @Test
+ def testBlobInlineClusteringRoundTrip(): Unit = {
Review Comment:
Added `testBlobInlineCowUpsertMergeRoundTrip`: CoW `bulk_insert` of INLINE
blobs into a single file group, then `upsert` of a subset.
- Pins that the merge actually ran: two commits, no deltacommits,
post-upsert base file disjoint from the pre-upsert one.
- Checks touched rows carry the new bytes and untouched rows keep the
originals, with the same DESCRIPTOR/CONTENT checks as the compaction and
clustering tests.
The class's write helper uses `DefaultSparkRecordMerger`, so this exercises
the SPARK branch of `getReaderContextFactoryForWrite`
(`SparkReaderContextFactory`), i.e. the default datasource path.
--
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]
