szehon-ho commented on code in PR #57434:
URL: https://github.com/apache/spark/pull/57434#discussion_r3634883574
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -1430,13 +1433,40 @@ class AstBuilder extends DataTypeAstBuilder
visitIdentifierSeq(c.exceptCols).map(UnresolvedAttribute.quoted)
}
+ // STORED AS SCD TYPE <n>. Absent clause defaults to SCD Type 1. Only 1
and 2 are
+ // supported; reject anything else with a clear error rather than a
generic parse failure.
+ val storedAsScdType = Option(params.autoCdcStoredAsClause()) match {
+ case Some(c) =>
+ val scdType = c.`type`.getText.toInt
Review Comment:
Minor robustness nit: `INTEGER_VALUE` is `DIGIT+`, so an overflowing literal
such as `STORED AS SCD TYPE 99999999999999999999` makes `.toInt` throw an
uncaught `NumberFormatException` -- which surfaces as an internal error/stack
trace rather than the intended `Unsupported SCD type` message this block is
trying to produce.
AstBuilder already guards this pattern elsewhere (the nearest-by `num`,
~L2627):
```scala
val value = try n.getText.toLong catch {
case _: NumberFormatException => throw ...outOfRange...
}
```
Since only `1` and `2` are valid, the simplest fix is to branch on the
literal text instead of parsing to Int -- e.g. match `getText` on `"1"` / `"2"`
and route everything else (including oversized numbers) through the existing
`operationNotAllowed(...)`. That closes the overflow hole and keeps the clear
error message.
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -790,6 +792,16 @@ autoCdcColumnsClause
ASTERISK EXCEPT LEFT_PAREN exceptCols=identifierSeq RIGHT_PAREN)
;
+autoCdcStoredAsClause
+ : STORED AS SCD TYPE type=INTEGER_VALUE
Review Comment:
Minor: the label name `type` collides with the Scala keyword, forcing a
backtick escape when accessing it in the AstBuilder. Renaming the label to
`scdType=INTEGER_VALUE` reads better and avoids the escape.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
