voonhous commented on code in PR #19460:
URL: https://github.com/apache/hudi/pull/19460#discussion_r3701287195
##########
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:
Good catch -- `bucket: 5` is a prefix of the buggy `bucket: 5.describe`
output, so the positive match alone could not discriminate. Fixed:
- `interceptParse` now returns the `ParseException`
- the case additionally asserts `!e2.getMessage.contains(".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:
Addressed rather than deferring: `blob_e5` now asserts the exception carries
the thrower via the copied stack trace:
```scala
assert(e5.getStackTrace.exists(_.getClassName.contains("IntervalUtils")))
```
Verified against Spark 3.5 and 4.2 (the top frames are
`IntervalUtils$.stringToInterval` on 3.x and
`SparkIntervalUtils.stringToInterval` via `IntervalUtils$` on 4.x, so the
substring match holds on both).
--
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]
