wombatu-kun commented on code in PR #19460:
URL: https://github.com/apache/hudi/pull/19460#discussion_r3701094505
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestBlobDataType.scala:
##########
@@ -502,23 +502,31 @@ class TestBlobDataType extends HoodieSparkSqlTestBase
with ExtendedParserTestHel
}
test("Test parse CREATE TABLE with BLOB column and invalid partition
transforms") {
- // Non-numeric number of buckets. The builders' raw `new
ParseException(message, ctx)` sites
- // surface on Spark 3.4+ as SparkException [INTERNAL_ERROR] wrapping the
message text
- // (#19450), so this case asserts the message via a plain intercept.
- // TODO(#19450): tighten to intercept[ParseException] once the builders
throw it cleanly.
- val e = intercept[Exception] {
- spark.sql("CREATE TABLE blob_e1 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket('x', id))")
- }
- assert(e.getMessage.contains("Invalid number of buckets"))
- // A non-column-reference where a column is required.
- checkExceptionContain(
- "CREATE TABLE blob_e2 (id BIGINT, data BLOB) USING hudi PARTITIONED BY
(bucket(4, 5))")(
- "Expected a column reference")
+ // Each case pins a distinct visitor arm of the extended AST builders; all
must surface as a
+ // clean ParseException on every Spark profile (#19450). Assertions stay
substring-based
+ // because the Spark 4.x builders add an "Operation not allowed: " prefix.
+ // Non-numeric number of buckets.
+ interceptParse("CREATE TABLE blob_e1 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket('x', id))")(
+ "Invalid number of buckets")
+ // A non-column-reference where a column is required; the full text pins
${nonRef.describe}.
+ interceptParse("CREATE TABLE blob_e2 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket(4, 5))")(
+ "Expected a column reference for transform bucket: 5")
Review Comment:
`Expected a column reference for transform bucket: 5` is also a substring of
the pre-fix text `... bucket: 5.describe`, so this case passes with or without
the interpolation fix and the comment's "pins `${nonRef.describe}`" claim does
not hold. Add an assertion that the message does not contain `.describe`.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestBlobDataType.scala:
##########
@@ -502,23 +502,31 @@ class TestBlobDataType extends HoodieSparkSqlTestBase
with ExtendedParserTestHel
}
test("Test parse CREATE TABLE with BLOB column and invalid partition
transforms") {
- // Non-numeric number of buckets. The builders' raw `new
ParseException(message, ctx)` sites
- // surface on Spark 3.4+ as SparkException [INTERNAL_ERROR] wrapping the
message text
- // (#19450), so this case asserts the message via a plain intercept.
- // TODO(#19450): tighten to intercept[ParseException] once the builders
throw it cleanly.
- val e = intercept[Exception] {
- spark.sql("CREATE TABLE blob_e1 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket('x', id))")
- }
- assert(e.getMessage.contains("Invalid number of buckets"))
- // A non-column-reference where a column is required.
- checkExceptionContain(
- "CREATE TABLE blob_e2 (id BIGINT, data BLOB) USING hudi PARTITIONED BY
(bucket(4, 5))")(
- "Expected a column reference")
+ // Each case pins a distinct visitor arm of the extended AST builders; all
must surface as a
+ // clean ParseException on every Spark profile (#19450). Assertions stay
substring-based
+ // because the Spark 4.x builders add an "Operation not allowed: " prefix.
+ // Non-numeric number of buckets.
+ interceptParse("CREATE TABLE blob_e1 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket('x', id))")(
+ "Invalid number of buckets")
+ // A non-column-reference where a column is required; the full text pins
${nonRef.describe}.
+ interceptParse("CREATE TABLE blob_e2 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (bucket(4, 5))")(
+ "Expected a column reference for transform bucket: 5")
// A single-field transform given more than one argument.
- checkExceptionContain(
- "CREATE TABLE blob_e3 (id BIGINT, ts DATE, data BLOB) USING hudi " +
- "PARTITIONED BY (years(id, ts))")(
+ interceptParse("CREATE TABLE blob_e3 (id BIGINT, ts DATE, data BLOB) USING
hudi PARTITIONED BY (years(id, ts))")(
"Too many arguments")
+ // Typed literal that fails to parse (visitTypeConstructor arm).
+ interceptParse("CREATE TABLE blob_e4 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (myfunc(DATE 'nope', id))")(
+ "Cannot parse the DATE value: nope")
+ // Invalid INTERVAL literal (the construct-then-setStackTrace arm).
+ interceptParse("CREATE TABLE blob_e5 (id BIGINT, data BLOB) USING hudi
PARTITIONED BY (myfunc(INTERVAL 'x', id))")(
Review Comment:
The restored `setStackTrace` on the three construct-then-throw sites is
untested - `blob_e5` reaches the INTERVAL arm but only asserts the message.
Assert the thrown exception's stack trace carries an `IntervalUtils` frame;
follow-up, not a blocker.
--
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]
