hudi-agent commented on code in PR #19408:
URL: https://github.com/apache/hudi/pull/19408#discussion_r3685736424


##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestBlobDataType.scala:
##########
@@ -282,4 +285,267 @@ class TestBlobDataType extends HoodieSparkSqlTestBase {
         "Expected at least one .clean instant on the timeline after 
compaction")
     })
   }
+
+  // The following cases are parser-coverage only: a BLOB column routes the 
whole CREATE TABLE
+  // through the extended AST builder, so its clause visitors run. parsePlan 
is purely syntactic
+  // (no catalog, no execution), which lets us exercise clauses Hudi does not 
support at execution
+  // time (transform partitioning, STORED AS / ROW FORMAT, interval columns). 
The BLOB column type
+  // itself proves routing because the stock Spark parser rejects the BLOB 
type name.
+
+  private def parse(sql: String): CreateTable =

Review Comment:
   🤖 nit: `transformByName`, `transformFieldRefs`, and `firstLiteralArg` look 
identical to the helpers just added in `TestCreateTable` — have you considered 
extracting them into a small shared trait or companion so there's one copy to 
maintain?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/ddl/TestCreateTable.scala:
##########
@@ -2340,6 +2342,151 @@ class TestCreateTable extends HoodieSparkSqlTestBase {
     }
   }
 
+  // The following cases are parser-coverage only: a VECTOR column routes the 
whole CREATE TABLE
+  // through the extended AST builder, so its clause visitors run. parsePlan 
is purely syntactic
+  // (no catalog, no execution), matching how TestIndexSyntax exercises the 
index statements, which
+  // lets us cover clauses that are not supported at execution time (transform 
partitioning,
+  // CLUSTERED BY, typed literal arguments). The VECTOR column type proves the 
statement routed
+  // here because the stock Spark parser rejects the VECTOR type name.
+
+  private def parseCreateTable(sql: String): CreateTable =
+    spark.sessionState.sqlParser.parsePlan(sql).asInstanceOf[CreateTable]
+
+  private def transformByName(plan: CreateTable, name: String): Transform =

Review Comment:
   🤖 nit: could you swap `sys.error(...)` for `fail(...)` here (and in 
`firstLiteralArg` on line 2362)? The parallel helpers in `TestBlobDataType` 
already use `fail`, which produces a clean test-failure rather than an 
unchecked `RuntimeException`.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



-- 
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]

Reply via email to