wombatu-kun commented on code in PR #19455:
URL: https://github.com/apache/hudi/pull/19455#discussion_r3697400937
##########
hudi-spark-datasource/hudi-spark3.5.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_5ExtendedSqlAstBuilder.scala:
##########
@@ -988,8 +945,7 @@ class HoodieSpark3_5ExtendedSqlAstBuilder(conf: SQLConf,
delegate: ParserInterfa
val literal = Option(ctx.constant)
.map(typedVisit[Literal])
.map(lit => LiteralValue(lit.value, lit.dataType))
- reference.orElse(literal)
- .getOrElse(throw new ParseException("Invalid transform argument", ctx))
+ reference.orElse(literal).get
Review Comment:
FALSE is not `ansiNonReserved` (only TRUE is), so under ANSI keyword mode -
the Spark 4.x default - `PARTITIONED BY (f(false, id))` on a BLOB table parses
the argument as `constant` -> `booleanLiteral`, and with `visitBooleanLiteral`
deleted `typedVisit[Literal]` returns null so `visitTransformArgument` NPEs on
`lit.value`. Keep `visitBooleanLiteral` for the same reason `visitNullLiteral`
is kept - FALSE and NULL are in the same keyword class - and correct the
matching claim in the `TestBlobDataType` comment.
##########
hudi-spark-datasource/hudi-spark3.5.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_5ExtendedSqlAstBuilder.scala:
##########
@@ -1044,8 +1000,6 @@ class HoodieSpark3_5ExtendedSqlAstBuilder(conf: SQLConf,
delegate: ParserInterfa
SerdeInfo(storedAs = Some(c.identifier.getText))
case (null, storageHandler) =>
Review Comment:
The `case _` arm of `validateRowFormatFileFormat` is kept on the argument
that `ROW FORMAT SERDE 'x' STORED BY 'y'` reaches it, yet nothing asserts that
and its `// should never happen` comment now states the opposite. Worth adding
that statement to the extended-parser DDL tests and rewording the comment to
the reachability reason, so the next pruning pass does not delete 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]
