HyukjinKwon commented on code in PR #46570:
URL: https://github.com/apache/spark/pull/46570#discussion_r1602427305
##########
connector/connect/common/src/main/protobuf/spark/connect/base.proto:
##########
@@ -199,6 +200,17 @@ message AnalyzePlanRequest {
// (Required) The logical plan to get the storage level.
Relation relation = 1;
}
+
+ message Checkpoint {
Review Comment:
Actually taking a look at `ExecutePlanRequest`/`ExecutePlanResponse`, I
think it's better to keep it in `AnalyzePlanRequest`. `ExecutePlanResponse`
assumes that the output is actual output values such as Arrow batch, map, etc.
vs `AnalyzePlanResponse` can assume it's a DataFrame. This is also consistent
with `persist`/`cache`.
I think it's better to take `RemoveCachedRemoteRelationCommand` as a
separate command that is not dedicated to `checkpoint`. It can be used in other
places if we happen to reuse `CachedRemoteRelationCommand` somewhere.
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
