rahil-c commented on code in PR #18488:
URL: https://github.com/apache/hudi/pull/18488#discussion_r3067659093
##########
hudi-spark-datasource/hudi-spark3.5.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_5ExtendedSqlAstBuilder.scala:
##########
@@ -2591,7 +2591,7 @@ class HoodieSpark3_5ExtendedSqlAstBuilder(conf: SQLConf,
delegate: ParserInterfa
* Resolve/create a primitive type.
*/
override def visitPrimitiveDataType(ctx: PrimitiveDataTypeContext): DataType
= withOrigin(ctx) {
- val dataType = ctx.identifier.getText.toLowerCase(Locale.ROOT)
+ val dataType = ctx.typeName.getText.toLowerCase(Locale.ROOT)
Review Comment:
Is there any risk with the grammar change made for other types? Maybe some
small test you can run locally to see if this feedback was legit would be great.
Claude:
```
if you write DECIMAL(10, FLOAT):
Grammar now accepts it (since FLOAT is an identifier token)
ctx.INTEGER_VALUE() only sees [10] — the FLOAT identifier token is invisible
to the list
The match fires as ("decimal", List(10)) → silently produces DecimalType(10,
0)
User writes: DECIMAL(10, FLOAT)
Grammar parses as: typeName="DECIMAL", intParams=[10],
identParams=["FLOAT"]
Builder sees: ("decimal", List(10)) ← FLOAT is gone
Builder returns: DecimalType(10, 0) ← no error, wrong semantics
Before this PR: DECIMAL(10, FLOAT) was a grammar error — FLOAT wasn't a
valid token in that position, so the parser would reject it outright. After
this PR it silently succeeds with a wrong type.
The same applies to VARCHAR(255, JUNK) → VarcharType(255), CHAR(10,
WHATEVER) → CharType(10), etc.
```
--
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]
