voonhous commented on code in PR #19408:
URL: https://github.com/apache/hudi/pull/19408#discussion_r3691268213
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/ddl/TestCreateTable.scala:
##########
@@ -2340,6 +2341,140 @@ 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]
+
+ test("test create VECTOR table with partition transforms parses (parser
coverage)") {
Review Comment:
Done. Dropped both tests; the section comment now explains transform/literal
coverage lives only in `TestBlobDataType` because the clause visitors are
type-agnostic. The CLUSTERED BY and clause tests keep the VECTOR routing check.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/ddl/TestCreateTable.scala:
##########
@@ -2340,6 +2341,140 @@ 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]
+
+ test("test create VECTOR table with partition transforms parses (parser
coverage)") {
+ val plan = parseCreateTable(
+ s"""
+ |CREATE TABLE vec_parse_tbl (
+ | id BIGINT,
+ | ts DATE,
+ | region STRING,
+ | name STRING,
+ | embedding VECTOR(8)
+ |) USING hudi
+ |PARTITIONED BY (region, bucket(4, id), years(ts), truncate(10, name))
+ """.stripMargin)
+
+ assertEquals(ArrayType(FloatType, containsNull = false),
plan.tableSchema("embedding").dataType)
+
+ assertEquals(Seq(Seq("region")), transformFieldRefs(transformByName(plan,
"identity")))
+ val bucket = transformByName(plan, "bucket")
+ assertEquals(IntegerType, firstLiteralArg(bucket).dataType)
+ assertEquals("4", firstLiteralArg(bucket).value.toString)
+ assertEquals(Seq(Seq("id")), transformFieldRefs(bucket))
+ assertEquals(Seq(Seq("ts")), transformFieldRefs(transformByName(plan,
"years")))
+ val truncate = transformByName(plan, "truncate")
+ assertEquals(IntegerType, firstLiteralArg(truncate).dataType)
+ assertEquals("10", firstLiteralArg(truncate).value.toString)
+ assertEquals(Seq(Seq("name")), transformFieldRefs(truncate))
+ }
+
+ test("test create VECTOR table with typed transform-argument literals parses
(parser coverage)") {
+ // Each transform carries a differently-typed constant argument to
exercise the literal
+ // visitors: string, integer, long, exponent/double, the typed date
constructor, and both
+ // interval forms (multi-units and unit-to-unit). A typed timestamp
constructor is not used
+ // because the Spark 4.x extended parser rejects a bare TIMESTAMP token. A
bare true/false/null
+ // in this position is parsed as a column reference (qualifiedName takes
precedence over a
+ // constant in the grammar under the default non-ANSI config), so the
boolean and null literal
+ // visitors are not reachable from a CREATE TABLE statement.
+ val plan = parseCreateTable(
+ s"""
+ |CREATE TABLE vec_lit_tbl (
+ | id BIGINT,
+ | embedding VECTOR(4)
+ |) USING hudi
+ |PARTITIONED BY (
+ | str_t('x', id),
+ | int_t(4, id),
+ | long_t(9000000000L, id),
+ | exp_t(1E3, id),
+ | date_t(DATE '2020-01-01', id),
+ | mu_ivl_t(INTERVAL '1' DAY, id),
+ | uu_ivl_t(INTERVAL '1-2' YEAR TO MONTH, id)
+ |)
+ """.stripMargin)
+
+ assertEquals(StringType, firstLiteralArg(transformByName(plan,
"str_t")).dataType)
+ assertEquals("x", firstLiteralArg(transformByName(plan,
"str_t")).value.toString)
+ assertEquals(IntegerType, firstLiteralArg(transformByName(plan,
"int_t")).dataType)
+ assertEquals(LongType, firstLiteralArg(transformByName(plan,
"long_t")).dataType)
+ assertEquals(DoubleType, firstLiteralArg(transformByName(plan,
"exp_t")).dataType)
+ assertEquals(DateType, firstLiteralArg(transformByName(plan,
"date_t")).dataType)
+ assertEquals(
+ DayTimeIntervalType(DayTimeIntervalType.DAY, DayTimeIntervalType.DAY),
+ firstLiteralArg(transformByName(plan, "mu_ivl_t")).dataType)
+ assertEquals(
+ YearMonthIntervalType(YearMonthIntervalType.YEAR,
YearMonthIntervalType.MONTH),
+ firstLiteralArg(transformByName(plan, "uu_ivl_t")).dataType)
+ }
+
+ test("test create VECTOR table with CLUSTERED BY bucket spec parses (parser
coverage)") {
+ val plan = parseCreateTable(
+ "CREATE TABLE vec_bucket_tbl (id BIGINT, embedding VECTOR(4)) USING hudi
" +
+ "CLUSTERED BY (id) INTO 4 BUCKETS")
+ val bucket = transformByName(plan, "bucket")
+ assertEquals("4", firstLiteralArg(bucket).value.toString)
+ assertEquals(Seq(Seq("id")), transformFieldRefs(bucket))
+
+ // SORTED BY ... ASC is accepted by the bucket-spec visitor.
+ val sortedPlan = parseCreateTable(
+ "CREATE TABLE vec_sbucket_tbl (id BIGINT, ts BIGINT, embedding
VECTOR(4)) USING hudi " +
+ "CLUSTERED BY (id, ts) SORTED BY (id ASC) INTO 8 BUCKETS")
+
assertTrue(sortedPlan.partitioning.exists(_.name.toLowerCase.contains("bucket")))
Review Comment:
Done. Now `assertEquals("sorted_bucket", sortedPlan.partitioning.head.name)`.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/ddl/TestCreateTable.scala:
##########
@@ -2340,6 +2341,140 @@ 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]
+
+ test("test create VECTOR table with partition transforms parses (parser
coverage)") {
+ val plan = parseCreateTable(
+ s"""
+ |CREATE TABLE vec_parse_tbl (
+ | id BIGINT,
+ | ts DATE,
+ | region STRING,
+ | name STRING,
+ | embedding VECTOR(8)
+ |) USING hudi
+ |PARTITIONED BY (region, bucket(4, id), years(ts), truncate(10, name))
+ """.stripMargin)
+
+ assertEquals(ArrayType(FloatType, containsNull = false),
plan.tableSchema("embedding").dataType)
+
+ assertEquals(Seq(Seq("region")), transformFieldRefs(transformByName(plan,
"identity")))
+ val bucket = transformByName(plan, "bucket")
+ assertEquals(IntegerType, firstLiteralArg(bucket).dataType)
+ assertEquals("4", firstLiteralArg(bucket).value.toString)
+ assertEquals(Seq(Seq("id")), transformFieldRefs(bucket))
+ assertEquals(Seq(Seq("ts")), transformFieldRefs(transformByName(plan,
"years")))
+ val truncate = transformByName(plan, "truncate")
+ assertEquals(IntegerType, firstLiteralArg(truncate).dataType)
+ assertEquals("10", firstLiteralArg(truncate).value.toString)
+ assertEquals(Seq(Seq("name")), transformFieldRefs(truncate))
+ }
+
+ test("test create VECTOR table with typed transform-argument literals parses
(parser coverage)") {
+ // Each transform carries a differently-typed constant argument to
exercise the literal
+ // visitors: string, integer, long, exponent/double, the typed date
constructor, and both
+ // interval forms (multi-units and unit-to-unit). A typed timestamp
constructor is not used
+ // because the Spark 4.x extended parser rejects a bare TIMESTAMP token. A
bare true/false/null
+ // in this position is parsed as a column reference (qualifiedName takes
precedence over a
+ // constant in the grammar under the default non-ANSI config), so the
boolean and null literal
+ // visitors are not reachable from a CREATE TABLE statement.
+ val plan = parseCreateTable(
+ s"""
+ |CREATE TABLE vec_lit_tbl (
+ | id BIGINT,
+ | embedding VECTOR(4)
+ |) USING hudi
+ |PARTITIONED BY (
+ | str_t('x', id),
+ | int_t(4, id),
+ | long_t(9000000000L, id),
+ | exp_t(1E3, id),
+ | date_t(DATE '2020-01-01', id),
+ | mu_ivl_t(INTERVAL '1' DAY, id),
+ | uu_ivl_t(INTERVAL '1-2' YEAR TO MONTH, id)
+ |)
+ """.stripMargin)
+
+ assertEquals(StringType, firstLiteralArg(transformByName(plan,
"str_t")).dataType)
+ assertEquals("x", firstLiteralArg(transformByName(plan,
"str_t")).value.toString)
+ assertEquals(IntegerType, firstLiteralArg(transformByName(plan,
"int_t")).dataType)
+ assertEquals(LongType, firstLiteralArg(transformByName(plan,
"long_t")).dataType)
+ assertEquals(DoubleType, firstLiteralArg(transformByName(plan,
"exp_t")).dataType)
+ assertEquals(DateType, firstLiteralArg(transformByName(plan,
"date_t")).dataType)
+ assertEquals(
+ DayTimeIntervalType(DayTimeIntervalType.DAY, DayTimeIntervalType.DAY),
+ firstLiteralArg(transformByName(plan, "mu_ivl_t")).dataType)
+ assertEquals(
+ YearMonthIntervalType(YearMonthIntervalType.YEAR,
YearMonthIntervalType.MONTH),
+ firstLiteralArg(transformByName(plan, "uu_ivl_t")).dataType)
+ }
+
+ test("test create VECTOR table with CLUSTERED BY bucket spec parses (parser
coverage)") {
+ val plan = parseCreateTable(
+ "CREATE TABLE vec_bucket_tbl (id BIGINT, embedding VECTOR(4)) USING hudi
" +
+ "CLUSTERED BY (id) INTO 4 BUCKETS")
+ val bucket = transformByName(plan, "bucket")
+ assertEquals("4", firstLiteralArg(bucket).value.toString)
+ assertEquals(Seq(Seq("id")), transformFieldRefs(bucket))
+
+ // SORTED BY ... ASC is accepted by the bucket-spec visitor.
+ val sortedPlan = parseCreateTable(
+ "CREATE TABLE vec_sbucket_tbl (id BIGINT, ts BIGINT, embedding
VECTOR(4)) USING hudi " +
+ "CLUSTERED BY (id, ts) SORTED BY (id ASC) INTO 8 BUCKETS")
+
assertTrue(sortedPlan.partitioning.exists(_.name.toLowerCase.contains("bucket")))
+
+ // SORTED BY ... DESC is rejected by the bucket-spec visitor.
+ checkExceptionContain(
+ "CREATE TABLE vec_bad_bucket_tbl (id BIGINT, embedding VECTOR(4)) USING
hudi " +
+ "CLUSTERED BY (id) SORTED BY (id DESC) INTO 4 BUCKETS")(
+ "Column ordering must be ASC")
+ }
+
+ test("test create VECTOR table with LOCATION/COMMENT/OPTIONS/TBLPROPERTIES
parses (parser coverage)") {
Review Comment:
Done. Now asserting `tableSpec.comment`, `.location` and the three `prop_*`
entries, and `Some("/tmp/vec_path_tbl")` for the path-folding case. Left
`.options` alone since it is not on the 3.5+ `TableSpecBase`.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/feature/index/TestIndexSyntax.scala:
##########
@@ -95,6 +95,26 @@ class TestIndexSyntax extends HoodieSparkSqlTestBase {
resolvedLogicalPlan = analyzer.execute(logicalPlan)
assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[RefreshIndexCommand].table,
databaseName, tableName)
assertResult("idx_name")(resolvedLogicalPlan.asInstanceOf[RefreshIndexCommand].indexName)
+
+ // CREATE INDEX without a USING clause: the index type defaults to
empty
+ logicalPlan = sqlParser.parsePlan(s"create index idx_default on
$tableName (name)")
+ resolvedLogicalPlan = analyzer.execute(logicalPlan)
+
assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].table,
databaseName, tableName)
+
assertResult("idx_default")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexName)
+
assertResult("")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexType)
+
assertResult(false)(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].ignoreIfExists)
Review Comment:
Done. Dropped the duplicated assert; the PR description now frames these as
plan-field pinning rather than previously untested branches.
##########
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:
Done. `parseCreateTable` now lives in `ExtendedParserTestHelpers` (abstract
`def spark`, satisfied by `HoodieSparkSqlTestBase`'s lazy val); both suites use
it.
--
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]
