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/10/21 02:37:57 UTC

[GitHub] [spark] HyukjinKwon commented on a diff in pull request #38300: [SPARK-40854] [CONNECT] Use proper JSON encoding until we have Arrow collection.

HyukjinKwon commented on code in PR #38300:
URL: https://github.com/apache/spark/pull/38300#discussion_r1001301224


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectStreamHandler.scala:
##########
@@ -57,21 +60,67 @@ class SparkConnectStreamHandler(responseObserver: StreamObserver[Response]) exte
     processRows(request.getClientId, rows)
   }
 
-  private def processRows(clientId: String, rows: DataFrame) = {
+  private[connect] def processRows(clientId: String, rows: DataFrame): Unit = {
     val timeZoneId = SQLConf.get.sessionLocalTimeZone
-    val schema =
-      ByteString.copyFrom(ArrowUtils.toArrowSchema(rows.schema, timeZoneId).toByteArray)
-
-    val textSchema = rows.schema.fields.map(f => f.name).mkString("|")
-    val data = rows.collect().map(x => x.toSeq.mkString("|")).mkString("\n")
-    val bbb = proto.Response.CSVBatch.newBuilder
-      .setRowCount(-1)
-      .setData(textSchema ++ "\n" ++ data)
-      .build()
-    val response = proto.Response.newBuilder().setClientId(clientId).setCsvBatch(bbb).build()
 
-    // Send all the data
-    responseObserver.onNext(response)
+    // Only process up to 10MB of data.
+    val sb = new StringBuilder
+    var rowCount = 0
+    rows.toJSON
+      .toLocalIterator()

Review Comment:
   Actually it's better to avoid using `toLocalIterator` ...  this can trigger multiple Spark jobs if it includes a shuffle IIRC. We would have to implement a logic like `collectAsArrowToPython` or `collectAsArrowToR` manually.



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