dongjoon-hyun commented on a change in pull request #31296:
URL: https://github.com/apache/spark/pull/31296#discussion_r562868528
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
##########
@@ -2889,6 +2889,24 @@ class Dataset[T] private[sql](
flatMap(func)(encoder)
}
+ /**
+ * Return a new Dataset of string created by piping elements to a forked
external process.
+ * The resulting Dataset is computed by executing the given process once per
partition.
+ * All elements of each input partition are written to a process's stdin as
lines of input
+ * separated by a newline. The resulting partition consists of the process's
stdout output, with
+ * each line of stdout resulting in one element of the output partition. A
process is invoked
+ * even for empty partitions.
Review comment:
This sounds like risky to me, `A process is invoked even for empty
partitions`.
This may cause a hang situation if the command is expecting input.
For example, this PR's test case is using `cat`. And, `cat | wc -l` hangs.
Could you add a test case of empty partition to make it sure that we handle
those cases?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
##########
@@ -2889,6 +2889,24 @@ class Dataset[T] private[sql](
flatMap(func)(encoder)
}
+ /**
+ * Return a new Dataset of string created by piping elements to a forked
external process.
+ * The resulting Dataset is computed by executing the given process once per
partition.
+ * All elements of each input partition are written to a process's stdin as
lines of input
+ * separated by a newline. The resulting partition consists of the process's
stdout output, with
+ * each line of stdout resulting in one element of the output partition. A
process is invoked
+ * even for empty partitions.
Review comment:
This sounds a little risky to me, `A process is invoked even for empty
partitions`.
This may cause a hang situation if the command is expecting input.
For example, this PR's test case is using `cat`. And, `cat | wc -l` hangs.
Could you add a test case of empty partition to make it sure that we handle
those cases?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
