wombatu-kun commented on code in PR #19691:
URL: https://github.com/apache/hudi/pull/19691#discussion_r3826685285
##########
hudi-common/src/test/resources/variant_backward_compat/README.md:
##########
@@ -39,3 +39,15 @@ The test runs on these four arguments:
COW tables generated are the same for both AVRO/SPARK. But for MOR, the log
files metadata are
different. Hence, we only need to generate test files for either 1/2, 3 and 4,
hence, 3 test
resource files.
+
+# variant_schema_on_read_cow.zip
+
+A Spark 4.1-written COW variant table carrying a COMMITTED INTERNAL SCHEMA:
one insert, then a
+schema-on-read DDL (`alter table add columns (note string)`) under
`hoodie.schema.on.read.enable`,
+then a second insert. Used by the Spark 3.x schema-on-read rejection test
(#18021). Generated by
Review Comment:
The other fixtures here are regenerable because the entry points at the test
that writes them, but this one records neither the create-table DDL nor the row
values. Add the exact statements, or a Spark-4-gated generator test next to the
existing one, so the binary can be rebuilt when the format moves.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -977,6 +977,61 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
}
}
+ test("Test Spark 3.x schema-on-read reads of a variant table with a
committed internal schema") {
+ // #18021: hoodie.schema.on.read.enable resolves the schema through the
InternalSchema round
+ // trip, whose sentinel detection restores the VARIANT logical type - the
exact input
+ // HoodieSparkSchemaConverters rejects on Spark 3.x. Verified 2026-08-20:
every leg fails
+ // LOUDLY with the same actionable error as the plain auto-resolve path;
there is no silent
+ // wrong data and no obscure secondary failure. Notably that includes the
documented
+ // struct-DDL compat mode, which works WITHOUT schema-on-read (see the
backward-compat test
+ // below) but breaks once the table carries an internal schema and the
conf is on, because
+ // internal-schema resolution overrides the user's DDL. Real support is
#18285; until then
+ // Spark 3.x compat-mode readers must keep hoodie.schema.on.read.enable
off.
+ assume(HoodieSparkUtils.isSpark3, "This test verifies Spark 3.x behavior
with schema-on-read")
Review Comment:
`org.apache.spark.sql.hudi.dml.schema` is in none of the Azure spark3.5
scala wildcard sets (`dml.others`, `dml.insert`, `feature`, and the DDL &
Others set), and the only GHA lane using the `dml` filter runs spark4.2, where
this `assume` cancels the test. Was the pin meant to be CI-enforced - if so the
package needs adding to one of those Azure jobs, otherwise the description's
"runs in the spark3.5 CI lane" should go.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -977,6 +977,61 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
}
}
+ test("Test Spark 3.x schema-on-read reads of a variant table with a
committed internal schema") {
+ // #18021: hoodie.schema.on.read.enable resolves the schema through the
InternalSchema round
+ // trip, whose sentinel detection restores the VARIANT logical type - the
exact input
+ // HoodieSparkSchemaConverters rejects on Spark 3.x. Verified 2026-08-20:
every leg fails
+ // LOUDLY with the same actionable error as the plain auto-resolve path;
there is no silent
+ // wrong data and no obscure secondary failure. Notably that includes the
documented
+ // struct-DDL compat mode, which works WITHOUT schema-on-read (see the
backward-compat test
+ // below) but breaks once the table carries an internal schema and the
conf is on, because
+ // internal-schema resolution overrides the user's DDL. Real support is
#18285; until then
+ // Spark 3.x compat-mode readers must keep hoodie.schema.on.read.enable
off.
+ assume(HoodieSparkUtils.isSpark3, "This test verifies Spark 3.x behavior
with schema-on-read")
+
+ withTempDir { tmpDir =>
+
HoodieTestUtils.extractZipToDirectory("variant_backward_compat/variant_schema_on_read_cow.zip",
tmpDir.toPath, getClass)
+ val tablePath =
tmpDir.toPath.resolve("variant_schema_on_read_cow").toString
+
+ def assertVariantRejected(leg: String)(f: => Unit): Unit = {
+ val ex = intercept[HoodieSchemaException](f)
+ assert(ex.getCause.getMessage.contains("VARIANT type is only supported
in Spark 4.0+"),
+ s"[$leg] expected the actionable variant rejection, got:
${ex.getCause}")
+ }
+
+ assertVariantRejected("auto-resolve, plain") {
+ spark.read.format("hudi").load(tablePath).collect()
+ }
+ assertVariantRejected("auto-resolve, schema-on-read") {
+ spark.read.format("hudi").option("hoodie.schema.on.read.enable",
"true").load(tablePath).collect()
Review Comment:
This asserts the same exception as the plain leg above, so it still passes
if the internal schema fails to load and `HoodieBaseRelation` falls back to the
commit-metadata schema. Assert first that
`TableSchemaResolver.getTableInternalSchemaFromCommitMetadata` is non-empty for
this fixture, so the leg cannot pass without exercising the InternalSchema path.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -977,6 +977,61 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
}
}
+ test("Test Spark 3.x schema-on-read reads of a variant table with a
committed internal schema") {
+ // #18021: hoodie.schema.on.read.enable resolves the schema through the
InternalSchema round
Review Comment:
`HoodieSparkSchemaConverters` still carries `TODO: Check if internalSchema
will throw any errors here: #18021` on the VARIANT case, which this PR answers
and closes. Worth dropping it in the same change so the marker does not outlive
its issue.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -977,6 +977,61 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
}
}
+ test("Test Spark 3.x schema-on-read reads of a variant table with a
committed internal schema") {
+ // #18021: hoodie.schema.on.read.enable resolves the schema through the
InternalSchema round
+ // trip, whose sentinel detection restores the VARIANT logical type - the
exact input
+ // HoodieSparkSchemaConverters rejects on Spark 3.x. Verified 2026-08-20:
every leg fails
+ // LOUDLY with the same actionable error as the plain auto-resolve path;
there is no silent
+ // wrong data and no obscure secondary failure. Notably that includes the
documented
+ // struct-DDL compat mode, which works WITHOUT schema-on-read (see the
backward-compat test
+ // below) but breaks once the table carries an internal schema and the
conf is on, because
+ // internal-schema resolution overrides the user's DDL. Real support is
#18285; until then
+ // Spark 3.x compat-mode readers must keep hoodie.schema.on.read.enable
off.
+ assume(HoodieSparkUtils.isSpark3, "This test verifies Spark 3.x behavior
with schema-on-read")
+
+ withTempDir { tmpDir =>
+
HoodieTestUtils.extractZipToDirectory("variant_backward_compat/variant_schema_on_read_cow.zip",
tmpDir.toPath, getClass)
+ val tablePath =
tmpDir.toPath.resolve("variant_schema_on_read_cow").toString
+
+ def assertVariantRejected(leg: String)(f: => Unit): Unit = {
+ val ex = intercept[HoodieSchemaException](f)
+ assert(ex.getCause.getMessage.contains("VARIANT type is only supported
in Spark 4.0+"),
+ s"[$leg] expected the actionable variant rejection, got:
${ex.getCause}")
+ }
+
+ assertVariantRejected("auto-resolve, plain") {
+ spark.read.format("hudi").load(tablePath).collect()
+ }
+ assertVariantRejected("auto-resolve, schema-on-read") {
+ spark.read.format("hudi").option("hoodie.schema.on.read.enable",
"true").load(tablePath).collect()
+ }
+ assertVariantRejected("compat struct DDL, schema-on-read") {
Review Comment:
The backward-compat test this points at reads `variant_cow.zip`, which
carries no internal schema, so nothing here shows that
`hoodie.schema.on.read.enable` is what breaks the struct DDL on a table that
has one. Add a fourth leg that reads this same table through the same DDL with
the conf off and asserts it succeeds.
--
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]
