You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2025/07/09 00:37:34 UTC
(spark) branch master updated: [SPARK-52706][SQL] Fix inconsistencies and refactor primitive types in parser
This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new a46296e74408 [SPARK-52706][SQL] Fix inconsistencies and refactor primitive types in parser
a46296e74408 is described below
commit a46296e74408626d1e462ff0c68c27b3fa0b4d47
Author: Mihailo Milosevic <mi...@databricks.com>
AuthorDate: Wed Jul 9 08:37:22 2025 +0800
[SPARK-52706][SQL] Fix inconsistencies and refactor primitive types in parser
### What changes were proposed in this pull request?
This PR proposes a change in how our parser treats datatypes. We introduce types with/without parameters and group accordingly.
### Why are the changes needed?
Changes are needed for many reasons:
1. Context of primitiveDataType is constantly getting bigger. This is not a good practice, as we have many null fields which only take up memory.
2. We have inconsistencies in where we use each type. We get TIMESTAMP_NTZ in a separate rule, but we also mention it in primitive types.
3. Primitive types should stay related to primitive types, adding ARRAY, STRUCT, MAP in the rule just because it is convenient is not good practice.
4. Current structure does not give option of extending types with different features. For example, we introduced STRING collations, but what if we were to introduce CHAR/VARCHAR with collations. Current structure gives us 0 possibility of making a type CHAR(5) COLLATE UTF8_BINARY (We can only do CHAR COLLATE UTF8_BINARY (5)).
### Does this PR introduce _any_ user-facing change?
No. This is internal refactoring.
### How was this patch tested?
All existing tests should pass, this is just code refactoring.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #51335 from mihailom-db/restructure-primitive.
Authored-by: Mihailo Milosevic <mi...@databricks.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../spark/sql/catalyst/parser/SqlBaseParser.g4 | 46 ++++---
.../sql/catalyst/parser/DataTypeAstBuilder.scala | 144 ++++++++++++---------
.../spark/sql/errors/QueryParsingErrors.scala | 6 +-
3 files changed, 110 insertions(+), 86 deletions(-)
diff --git a/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4 b/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
index fc3d86ca858f..698afa486002 100644
--- a/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
+++ b/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
@@ -1340,7 +1340,20 @@ collateClause
: COLLATE collationName=multipartIdentifier
;
-type
+nonTrivialPrimitiveType
+ : STRING collateClause?
+ | (CHARACTER | CHAR) (LEFT_PAREN length=INTEGER_VALUE RIGHT_PAREN)?
+ | VARCHAR (LEFT_PAREN length=INTEGER_VALUE RIGHT_PAREN)?
+ | (DECIMAL | DEC | NUMERIC)
+ (LEFT_PAREN precision=INTEGER_VALUE (COMMA scale=INTEGER_VALUE)? RIGHT_PAREN)?
+ | INTERVAL
+ (fromYearMonth=(YEAR | MONTH) (TO to=MONTH)? |
+ fromDayTime=(DAY | HOUR | MINUTE | SECOND) (TO to=(HOUR | MINUTE | SECOND))?)?
+ | TIMESTAMP (WITHOUT TIME ZONE)?
+ | TIME (LEFT_PAREN precision=INTEGER_VALUE RIGHT_PAREN)? (WITHOUT TIME ZONE)?
+ ;
+
+trivialPrimitiveType
: BOOLEAN
| TINYINT | BYTE
| SMALLINT | SHORT
@@ -1349,32 +1362,23 @@ type
| FLOAT | REAL
| DOUBLE
| DATE
- | TIME
- | TIMESTAMP | TIMESTAMP_NTZ | TIMESTAMP_LTZ
- | STRING collateClause?
- | CHARACTER | CHAR
- | VARCHAR
+ | TIMESTAMP_LTZ | TIMESTAMP_NTZ
| BINARY
- | DECIMAL | DEC | NUMERIC
| VOID
- | INTERVAL
| VARIANT
- | ARRAY | STRUCT | MAP
- | unsupportedType=identifier
+ ;
+
+primitiveType
+ : nonTrivialPrimitiveType
+ | trivialPrimitiveType
+ | unsupportedType=identifier (LEFT_PAREN INTEGER_VALUE(COMMA INTEGER_VALUE)* RIGHT_PAREN)?
;
dataType
- : complex=ARRAY LT dataType GT #complexDataType
- | complex=MAP LT dataType COMMA dataType GT #complexDataType
- | complex=STRUCT (LT complexColTypeList? GT | NEQ) #complexDataType
- | INTERVAL from=(YEAR | MONTH) (TO to=MONTH)? #yearMonthIntervalDataType
- | INTERVAL from=(DAY | HOUR | MINUTE | SECOND)
- (TO to=(HOUR | MINUTE | SECOND))? #dayTimeIntervalDataType
- | TIME (LEFT_PAREN precision=INTEGER_VALUE RIGHT_PAREN)?
- (WITHOUT TIME ZONE)? #timeDataType
- | (TIMESTAMP_NTZ | TIMESTAMP WITHOUT TIME ZONE) #timestampNtzDataType
- | type (LEFT_PAREN INTEGER_VALUE
- (COMMA INTEGER_VALUE)* RIGHT_PAREN)? #primitiveDataType
+ : complex=ARRAY (LT dataType GT)? #complexDataType
+ | complex=MAP (LT dataType COMMA dataType GT)? #complexDataType
+ | complex=STRUCT ((LT complexColTypeList? GT) | NEQ)? #complexDataType
+ | primitiveType #primitiveDataType
;
qualifiedColTypeWithPositionList
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/DataTypeAstBuilder.scala b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/DataTypeAstBuilder.scala
index e83a987263db..beb7061a841a 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/DataTypeAstBuilder.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/DataTypeAstBuilder.scala
@@ -65,74 +65,89 @@ class DataTypeAstBuilder extends SqlBaseParserBaseVisitor[AnyRef] {
ctx.parts.asScala.map(_.getText).toSeq
}
- /**
- * Resolve/create the TIME primitive type.
- */
- override def visitTimeDataType(ctx: TimeDataTypeContext): DataType = withOrigin(ctx) {
- val precision = if (ctx.precision == null) {
- TimeType.DEFAULT_PRECISION
- } else {
- ctx.precision.getText.toInt
- }
- TimeType(precision)
- }
-
- /**
- * Create the TIMESTAMP_NTZ primitive type.
- */
- override def visitTimestampNtzDataType(ctx: TimestampNtzDataTypeContext): DataType = {
- withOrigin(ctx)(TimestampNTZType)
- }
-
/**
* Resolve/create a primitive type.
*/
override def visitPrimitiveDataType(ctx: PrimitiveDataTypeContext): DataType = withOrigin(ctx) {
- val typeCtx = ctx.`type`
- (typeCtx.start.getType, ctx.INTEGER_VALUE().asScala.toList) match {
- case (BOOLEAN, Nil) => BooleanType
- case (TINYINT | BYTE, Nil) => ByteType
- case (SMALLINT | SHORT, Nil) => ShortType
- case (INT | INTEGER, Nil) => IntegerType
- case (BIGINT | LONG, Nil) => LongType
- case (FLOAT | REAL, Nil) => FloatType
- case (DOUBLE, Nil) => DoubleType
- case (DATE, Nil) => DateType
- case (TIMESTAMP, Nil) => SqlApiConf.get.timestampType
- case (TIMESTAMP_LTZ, Nil) => TimestampType
- case (STRING, Nil) =>
- typeCtx.children.asScala.toSeq match {
- case Seq(_) => StringType
- case Seq(_, ctx: CollateClauseContext) =>
- val collationNameParts = visitCollateClause(ctx).toArray
- val collationId = CollationFactory.collationNameToId(
- CollationFactory.resolveFullyQualifiedName(collationNameParts))
- StringType(collationId)
- }
- case (CHARACTER | CHAR, length :: Nil) => CharType(length.getText.toInt)
- case (VARCHAR, length :: Nil) => VarcharType(length.getText.toInt)
- case (BINARY, Nil) => BinaryType
- case (DECIMAL | DEC | NUMERIC, Nil) => DecimalType.USER_DEFAULT
- case (DECIMAL | DEC | NUMERIC, precision :: Nil) =>
- DecimalType(precision.getText.toInt, 0)
- case (DECIMAL | DEC | NUMERIC, precision :: scale :: Nil) =>
- DecimalType(precision.getText.toInt, scale.getText.toInt)
- case (VOID, Nil) => NullType
- case (INTERVAL, Nil) => CalendarIntervalType
- case (VARIANT, Nil) => VariantType
- case (CHARACTER | CHAR | VARCHAR, Nil) =>
- throw QueryParsingErrors.charTypeMissingLengthError(ctx.`type`.getText, ctx)
- case (ARRAY | STRUCT | MAP, Nil) =>
- throw QueryParsingErrors.nestedTypeMissingElementTypeError(ctx.`type`.getText, ctx)
- case (_, params) =>
- val badType = ctx.`type`.getText
- val dtStr = if (params.nonEmpty) s"$badType(${params.mkString(",")})" else badType
- throw QueryParsingErrors.dataTypeUnsupportedError(dtStr, ctx)
+ val typeCtx = ctx.primitiveType
+ if (typeCtx.nonTrivialPrimitiveType != null) {
+ // This is a primitive type with parameters, e.g. VARCHAR(10), DECIMAL(10, 2), etc.
+ val currentCtx = typeCtx.nonTrivialPrimitiveType
+ currentCtx.start.getType match {
+ case STRING =>
+ currentCtx.children.asScala.toSeq match {
+ case Seq(_) => StringType
+ case Seq(_, ctx: CollateClauseContext) =>
+ val collationNameParts = visitCollateClause(ctx).toArray
+ val collationId = CollationFactory.collationNameToId(
+ CollationFactory.resolveFullyQualifiedName(collationNameParts))
+ StringType(collationId)
+ }
+ case CHARACTER | CHAR =>
+ if (currentCtx.length == null) {
+ throw QueryParsingErrors.charVarcharTypeMissingLengthError(typeCtx.getText, ctx)
+ } else CharType(currentCtx.length.getText.toInt)
+ case VARCHAR =>
+ if (currentCtx.length == null) {
+ throw QueryParsingErrors.charVarcharTypeMissingLengthError(typeCtx.getText, ctx)
+ } else VarcharType(currentCtx.length.getText.toInt)
+ case DECIMAL | DEC | NUMERIC =>
+ if (currentCtx.precision == null) {
+ DecimalType.USER_DEFAULT
+ } else if (currentCtx.scale == null) {
+ DecimalType(currentCtx.precision.getText.toInt, 0)
+ } else {
+ DecimalType(currentCtx.precision.getText.toInt, currentCtx.scale.getText.toInt)
+ }
+ case INTERVAL =>
+ if (currentCtx.fromDayTime != null) {
+ visitDayTimeIntervalDataType(currentCtx)
+ } else if (currentCtx.fromYearMonth != null) {
+ visitYearMonthIntervalDataType(currentCtx)
+ } else {
+ CalendarIntervalType
+ }
+ case TIMESTAMP =>
+ if (currentCtx.WITHOUT() == null) {
+ SqlApiConf.get.timestampType
+ } else TimestampNTZType
+ case TIME =>
+ val precision = if (currentCtx.precision == null) {
+ TimeType.DEFAULT_PRECISION
+ } else {
+ currentCtx.precision.getText.toInt
+ }
+ TimeType(precision)
+ }
+ } else if (typeCtx.trivialPrimitiveType != null) {
+ // This is a primitive type without parameters, e.g. BOOLEAN, TINYINT, etc.
+ typeCtx.trivialPrimitiveType.start.getType match {
+ case BOOLEAN => BooleanType
+ case TINYINT | BYTE => ByteType
+ case SMALLINT | SHORT => ShortType
+ case INT | INTEGER => IntegerType
+ case BIGINT | LONG => LongType
+ case FLOAT | REAL => FloatType
+ case DOUBLE => DoubleType
+ case DATE => DateType
+ case TIMESTAMP_LTZ => TimestampType
+ case TIMESTAMP_NTZ => TimestampNTZType
+ case BINARY => BinaryType
+ case VOID => NullType
+ case VARIANT => VariantType
+ }
+ } else {
+ val badType = typeCtx.unsupportedType.getText
+ val params = typeCtx.INTEGER_VALUE().asScala.toList
+ val dtStr =
+ if (params.nonEmpty) s"$badType(${params.mkString(",")})"
+ else badType
+ throw QueryParsingErrors.dataTypeUnsupportedError(dtStr, ctx)
}
}
- override def visitYearMonthIntervalDataType(ctx: YearMonthIntervalDataTypeContext): DataType = {
- val startStr = ctx.from.getText.toLowerCase(Locale.ROOT)
+ private def visitYearMonthIntervalDataType(ctx: NonTrivialPrimitiveTypeContext): DataType = {
+ val startStr = ctx.fromYearMonth.getText.toLowerCase(Locale.ROOT)
val start = YearMonthIntervalType.stringToField(startStr)
if (ctx.to != null) {
val endStr = ctx.to.getText.toLowerCase(Locale.ROOT)
@@ -146,8 +161,8 @@ class DataTypeAstBuilder extends SqlBaseParserBaseVisitor[AnyRef] {
}
}
- override def visitDayTimeIntervalDataType(ctx: DayTimeIntervalDataTypeContext): DataType = {
- val startStr = ctx.from.getText.toLowerCase(Locale.ROOT)
+ private def visitDayTimeIntervalDataType(ctx: NonTrivialPrimitiveTypeContext): DataType = {
+ val startStr = ctx.fromDayTime.getText.toLowerCase(Locale.ROOT)
val start = DayTimeIntervalType.stringToField(startStr)
if (ctx.to != null) {
val endStr = ctx.to.getText.toLowerCase(Locale.ROOT)
@@ -165,6 +180,9 @@ class DataTypeAstBuilder extends SqlBaseParserBaseVisitor[AnyRef] {
* Create a complex DataType. Arrays, Maps and Structures are supported.
*/
override def visitComplexDataType(ctx: ComplexDataTypeContext): DataType = withOrigin(ctx) {
+ if (ctx.LT() == null && ctx.NEQ() == null) {
+ throw QueryParsingErrors.nestedTypeMissingElementTypeError(ctx.getText, ctx)
+ }
ctx.complex.getType match {
case SqlBaseParser.ARRAY =>
ArrayType(typedVisit(ctx.dataType(0)))
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala b/sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
index 12f986b89fd2..60ccf7a9282c 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
@@ -324,7 +324,9 @@ private[sql] object QueryParsingErrors extends DataTypeErrorsBase {
ctx)
}
- def charTypeMissingLengthError(dataType: String, ctx: PrimitiveDataTypeContext): Throwable = {
+ def charVarcharTypeMissingLengthError(
+ dataType: String,
+ ctx: PrimitiveDataTypeContext): Throwable = {
new ParseException(
errorClass = "DATATYPE_MISSING_SIZE",
messageParameters = Map("type" -> toSQLType(dataType)),
@@ -333,7 +335,7 @@ private[sql] object QueryParsingErrors extends DataTypeErrorsBase {
def nestedTypeMissingElementTypeError(
dataType: String,
- ctx: PrimitiveDataTypeContext): Throwable = {
+ ctx: ComplexDataTypeContext): Throwable = {
dataType.toUpperCase(Locale.ROOT) match {
case "ARRAY" =>
new ParseException(
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org