rangadi commented on code in PR #38384:
URL: https://github.com/apache/spark/pull/38384#discussion_r1024692244
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/utils/ProtobufUtils.scala:
##########
@@ -155,21 +155,52 @@ private[sql] object ProtobufUtils extends Logging {
* Loads the given protobuf class and returns Protobuf descriptor for it.
*/
def buildDescriptorFromJavaClass(protobufClassName: String): Descriptor = {
+
+ // Default 'Message' class here is shaded while using the package (as in
production).
+ // The incoming classes might not be shaded. Check both.
+ val shadedMessageClass = classOf[Message] // Shaded in prod, not in unit
tests.
+ val missingShadingErrorMessage = "The jar with Protobuf classes needs to
be shaded " +
+ s"(com.google.protobuf.* --> ${shadedMessageClass.getPackage.getName}.*)"
+
val protobufClass = try {
Utils.classForName(protobufClassName)
} catch {
case e: ClassNotFoundException =>
- throw QueryCompilationErrors.protobufClassLoadError(protobufClassName,
e)
+ val explanation =
+ if (protobufClassName.contains(".")) "Ensure the class include in
the jar"
+ else "Ensure the class name includes package prefix"
+ throw QueryCompilationErrors.protobufClassLoadError(protobufClassName,
explanation, e)
+
+ case e: NoClassDefFoundError if
e.getMessage.matches("com/google/proto.*Generated.*") =>
+ // This indicates the the the Java classes are not shaded.
+ throw QueryCompilationErrors.protobufClassLoadError(
+ protobufClassName, missingShadingErrorMessage, e)
}
- if (!classOf[Message].isAssignableFrom(protobufClass)) {
- throw QueryCompilationErrors.protobufMessageTypeError(protobufClassName)
- // TODO: Need to support V2. This might work with V2 classes too.
+ if (!shadedMessageClass.isAssignableFrom(protobufClass)) {
+ // Check if this extends 2.x Message class included in spark, that does
not work.
+ val unshadedMessageClass = Utils.classForName(
+
"com.escape-shading.google.protobuf.Message".replace("escape-shading.", "")
Review Comment:
Updated this in #38680. It adds an explanation and uses join rather than
replace().
--
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]
