You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/02 23:46:12 UTC
[GitHub] [spark] srielau commented on a diff in pull request #38344: [SPARK-40777][SQL][PROTOBUF] Protobuf import support and move error-classes.
srielau commented on code in PR #38344:
URL: https://github.com/apache/spark/pull/38344#discussion_r1012385639
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -742,6 +832,11 @@
],
"sqlState" : "22023"
},
+ "SQL_TYPE_TO_PROTOBUF_ENUM_TYPE_ERROR" : {
Review Comment:
Rename without _ERROR
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -3212,4 +3212,181 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
messageParameters = Map("expression" -> toSQLExpr(expression))
)
}
+
+ def cannotConvertProtobufTypeToSqlTypeError(
+ protobufColumn: String,
+ sqlColumn: Seq[String],
+ protobufType: String,
+ sqlType: DataType): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_FIELD_TYPE_TO_SQL_TYPE_ERROR",
+ messageParameters = Map(
+ "protobufColumn" -> protobufColumn,
+ "sqlColumn" -> toSQLId(sqlColumn),
+ "protobufType" -> protobufType,
+ "sqlType" -> toSQLType(sqlType)))
+ }
+
+ def cannotConvertCatalystTypeToProtobufTypeError(
+ sqlColumn: Seq[String],
+ protobufColumn: String,
+ sqlType: DataType,
+ protobufType: String): Throwable = {
+ new AnalysisException(
+ errorClass = "CANNOT_CONVERT_SQL_TYPE_TO_PROTOBUF_FIELD_TYPE",
+ messageParameters = Map(
+ "sqlColumn" -> toSQLId(sqlColumn),
+ "protobufColumn" -> protobufColumn,
+ "sqlType" -> toSQLType(sqlType),
+ "protobufType" -> protobufType))
+ }
+
+ def cannotConvertCatalystTypeToProtobufEnumTypeError(
+ sqlColumn: Seq[String],
+ protobufColumn: String,
+ data: String,
+ enumString: String): Throwable = {
+ new AnalysisException(
+ errorClass = "SQL_TYPE_TO_PROTOBUF_ENUM_TYPE_ERROR",
+ messageParameters = Map(
+ "sqlColumn" -> toSQLId(sqlColumn),
+ "protobufColumn" -> protobufColumn,
+ "data" -> data,
+ "enumString" -> enumString))
+ }
+
+ def cannotConvertProtobufTypeToCatalystTypeError(
+ protobufType: String,
+ sqlType: DataType,
+ cause: Throwable): Throwable = {
+ new AnalysisException(
+ errorClass = "CANNOT_CONVERT_PROTOBUF_MESSAGE_TYPE_TO_SQL_TYPE",
+ messageParameters = Map(
+ "protobufType" -> protobufType,
+ "toType" -> toSQLType(sqlType)),
+ cause = Option(cause.getCause))
+ }
+
+ def cannotConvertSqlTypeToProtobufError(
+ protobufType: String,
+ sqlType: DataType,
+ cause: Throwable): Throwable = {
+ new AnalysisException(
+ errorClass = "UNABLE_TO_CONVERT_TO_PROTOBUF_MESSAGE_TYPE",
+ messageParameters = Map(
+ "protobufType" -> protobufType,
+ "toType" -> toSQLType(sqlType)),
+ cause = Option(cause.getCause))
+ }
+
+ def protobufTypeUnsupportedYetError(protobufType: String): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_TYPE_NOT_SUPPORT",
+ messageParameters = Map("protobufType" -> protobufType))
+ }
+
+ def unknownProtobufMessageTypeError(
+ descriptorName: String,
+ containingType: String): Throwable = {
+ new AnalysisException(
+ errorClass = "UNKNOWN_PROTOBUF_MESSAGE_TYPE",
+ messageParameters = Map(
+ "descriptorName" -> descriptorName,
+ "containingType" -> containingType))
+ }
+
+ def cannotFindCatalystTypeInProtobufSchemaError(catalystFieldPath: String): Throwable = {
+ new AnalysisException(
+ errorClass = "NO_SQL_TYPE_IN_PROTOBUF_SCHEMA",
+ messageParameters = Map("catalystFieldPath" -> catalystFieldPath))
+ }
+
+ def cannotFindProtobufFieldInCatalystError(field: String): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_FIELD_MISSING_IN_SQL_SCHEMA",
+ messageParameters = Map("field" -> field))
+ }
+
+ def protobufFieldMatchError(field: String,
+ protobufSchema: String,
+ matchSize: String,
+ matches: String): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_FIELD_MISSING",
+ messageParameters = Map(
+ "field" -> field,
+ "protobufSchema" -> protobufSchema,
+ "matchSize" -> matchSize,
+ "matches" -> matches))
+ }
+
+ def unableToLocateProtobufMessageError(messageName: String): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_MESSAGE_NOT_FOUND",
+ messageParameters = Map("messageName" -> messageName))
+ }
+
+ def descrioptorParseError(descFilePath: String, cause: Throwable): Throwable = {
+ new AnalysisException(
+ errorClass = "CANNOT_PARSE_PROTOBUF_DESCRIPTOR",
+ messageParameters = Map.empty("descFilePath" -> descFilePath),
+ cause = Option(cause.getCause))
+ }
+
+ def cannotFindDescriptorFileError(filePath: String, cause: Throwable): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_DESCRIPTOR_FILE_NOT_FOUND",
+ messageParameters = Map("filePath" -> filePath),
+ cause = Option(cause.getCause))
+ }
+
+ def failedParsingDescriptorError(descFilePath: String, cause: Throwable): Throwable = {
+ new AnalysisException(
+ errorClass = "CANNOT_CONSTRUCT_PROTOBUF_DESCRIPTOR",
+ messageParameters = Map.empty("descFilePath" -> descFilePath),
+ cause = Option(cause.getCause))
+ }
+
+ def foundRecursionInProtobufSchema(fieldDescriptor: String): Throwable = {
+ new AnalysisException(
+ errorClass = "RECURSIVE_PROTOBUF_SCHEMA",
+ messageParameters = Map("fieldDescriptor" -> fieldDescriptor))
+ }
+
+ def protobufFieldTypeMismatchError(field: String): Throwable = {
+ new AnalysisException(
+ errorClass = "PROTOBUF_FIELD_TYPE_MISMATCH",
+ messageParameters = Map("field" -> field))
+ }
+
+ def protobufClassLoadError(
+ protobufClassName: String,
+ hasDots: Boolean,
+ cause: Throwable): Throwable = {
+ val message = if (hasDots) "" else ". Ensure the class name includes package prefix."
+ new AnalysisException(
+ errorClass = "CANNOT_LOAD_PROTOBUF_CLASS",
+ messageParameters = Map("protobufClassName" -> protobufClassName, "message" -> message),
Review Comment:
We don't want text as a parameter. Aside to make this text depends on whether there is any dot seems to be quite subtle. Maybe it simply doesn't have enough dots?
I propose to move this text into the error message (unconditionally).
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -29,12 +44,22 @@
],
"sqlState" : "22007"
},
+ "CANNOT_LOAD_PROTOBUF_CLASS" : {
+ "message" : [
+ "Could not load Protobuf class with name <protobufClassName><message>"
Review Comment:
```suggestion
"Could not load Protobuf class with name <protobufClassName>. Ensure the class name includes package prefix."
```
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -688,6 +733,51 @@
],
"sqlState" : "42000"
},
+ "PROTOBUF_DEPENDENCY_NOT_FOUND" : {
+ "message" : [
+ "Could not find dependency: <dependencyName>"
+ ]
+ },
+ "PROTOBUF_DESCRIPTOR_FILE_NOT_FOUND" : {
+ "message" : [
+ "Error reading Protobuf descriptor file at path: <filePath>"
+ ]
+ },
+ "PROTOBUF_FIELD_MISSING" : {
+ "message" : [
+ "Searching for <field> in Protobuf schema at <protobufSchema> gave <matchSize> matches. Candidates: <matches>"
+ ]
+ },
+ "PROTOBUF_FIELD_MISSING_IN_SQL_SCHEMA" : {
+ "message" : [
+ "Found <field> in Protobuf schema but there is no match in the SQL schema"
+ ]
+ },
+ "PROTOBUF_FIELD_TYPE_MISMATCH" : {
+ "message" : [
+ "Type mismatch encountered for field: <field>"
+ ]
+ },
+ "PROTOBUF_FIELD_TYPE_TO_SQL_TYPE_ERROR" : {
Review Comment:
How about:
```suggestion
"CANNOT_CONVERT_PROTOBUF_FIELD_TYPE_TO_SQL_TYPE" : {
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org