wombatu-kun commented on code in PR #19583:
URL: https://github.com/apache/hudi/pull/19583#discussion_r3772045934
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/MergeOnReadSnapshotRelation.scala:
##########
@@ -132,7 +133,15 @@ abstract class BaseMergeOnReadSnapshotRelation(sqlContext:
SQLContext,
getPartitionColumnsAsInternalRow(file.getPathInfo),
file.getPathInfo.getPath, 0, file.getFileSize)
}
- HoodieMergeOnReadFileSplit(partitionedBaseFile, logFiles)
+ // These values are empty unless the partition columns are omitted from
the data files, which is
Review Comment:
These values are also non-empty when
hoodie.datasource.read.extract.partition.values.from.path is set on a table
that does persist its partition columns, so "empty unless the partition columns
are omitted from the data files" is not accurate. Worth naming
shouldExtractPartitionValuesFromPartitionPath's three triggers instead.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/cdc/InternalRowToJsonStringConverter.scala:
##########
@@ -90,10 +91,40 @@ class InternalRowToJsonStringConverter(schema: StructType) {
structMap.toMap
case _ => value // fallback
}
+ case dt if dt.typeName ==
InternalRowToJsonStringConverter.VARIANT_TYPE_NAME =>
+ // VariantVal.toString renders the variant as JSON; embed it as a
real JSON node so
+ // the image carries the variant's structure. Falling through to the
default would
+ // serialize the VariantVal bean, i.e. its raw value/metadata bytes
as base64.
+ // Matched on the type name rather than SparkAdapter.isVariantType:
this guard is
+ // evaluated for every non-string/array/map/struct field, and
resolving the adapter
+ // needs a version module that is not on hudi-spark-common's own
test classpath.
+ val variantJson = value.toString
+ try {
+ mapper.readTree(variantJson)
+ } catch {
+ // A variant can hold a field name, string or nesting depth past
Jackson's default
+ // StreamReadConstraints (50k chars, 20M chars, 1000 levels) while
staying well inside
+ // the variant size limit, and all three arrive here as
StreamConstraintsException. A
+ // CDC image is diagnostic data rather than the table's data, so
keep the rendering as
+ // a plain string instead of failing the query over it.
+ // NOTE: value.toString is deliberately outside this block. It
throws MALFORMED_VARIANT
+ // on corrupt bytes, which is a data-integrity problem an operator
has to see, not a
+ // rendering quirk to paper over -- and there would be no
rendering left to fall back to.
+ case _: JsonProcessingException => variantJson
Review Comment:
A variant rendering nested exactly 1000 levels clears readTree but then
trips Jackson's write-side nesting cap inside convert's writeValueAsString,
which sits outside this fallback, so the query still fails (the window is
999-1000 when the variant sits inside a struct). Embedding the validated
rendering as a Jackson RawValue instead of the parsed JsonNode would bring that
case back behind the existing catch - follow-up, not a blocker.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -448,6 +449,105 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
})
}
+ test("Test CDC captures VARIANT values from shredded and unshredded base
files") {
Review Comment:
The comment in "Test Query Log Only MOR Table With VARIANT column triggers
compaction" still says the no-catalyst-schema legs (CDC, and legacy streaming)
are tracked in #19578, which this PR closes and covers here and in
TestStreamingSource. Worth repointing it at the new tests.
--
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]
