heyihong commented on code in PR #54016:
URL: https://github.com/apache/spark/pull/54016#discussion_r2735571435


##########
sql/connect/common/src/main/scala/org/apache/spark/sql/connect/client/SparkConnectClient.scala:
##########
@@ -1103,7 +1104,13 @@ object SparkConnectClient {
             responseListener: ClientCall.Listener[RespT],
             headers: Metadata): Unit = {
           metadata.foreach { case (key, value) =>
-            headers.put(Metadata.Key.of(key, 
Metadata.ASCII_STRING_MARSHALLER), value)
+            if (key != null && key.endsWith("-bin")) {

Review Comment:
   Is it possible for the key value to be null? As a general practice, Scala 
uses Option rather than null for nullable values. It also seems inconsistent 
that we check for null here but not in the other case.
   
   Besides, maybe we should use BINARY_HEADER_SUFFIX instead of "-bin", similar 
to 
https://github.com/grpc/grpc-java/blob/02cb1deb8e5c7ef4339cd124a643c6d5f205e9b4/api/src/main/java/io/grpc/Metadata.java#L866.
   
   So`key.endsWith(BINARY_HEADER_SUFFIX))` should be sufficient?



-- 
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]

Reply via email to