zeroshade commented on code in PR #13492:
URL: https://github.com/apache/arrow/pull/13492#discussion_r952993156
##########
format/FlightSql.proto:
##########
@@ -1550,6 +1755,48 @@ message DoPutUpdateResult {
int64 record_count = 1;
}
+/*
+ * Request message for the "CancelQuery" action.
+ *
+ * Explicitly cancel a running query.
+ *
+ * This lets a single client explicitly cancel work, no matter how many clients
+ * are involved/whether the query is distributed or not, given server support.
+ * The transaction/statement is not rolled back; it is the application's job to
+ * commit or rollback as appropriate. This only indicates the client no longer
+ * wishes to read the remainder of the query results or continue submitting
+ * data.
+ *
+ * This command is idempotent.
+ */
+message ActionCancelQueryRequest {
+ option (experimental) = true;
+
+ // The result of the GetFlightInfo RPC that initated the query.
+ // XXX(ARROW-16902): this must be a serialized FlightInfo, but is
+ // rendered as bytes because Protobuf does not really support one
+ // DLL using Protobuf definitions from another DLL.
+ bytes info = 1;
Review Comment:
Should this be a serialized flightinfo? or should it actually be a Ticket?
##########
format/FlightSql.proto:
##########
@@ -761,6 +798,20 @@ enum SqlInfo {
SQL_STORED_FUNCTIONS_USING_CALL_SYNTAX_SUPPORTED = 576;
}
+// The level of support for Flight SQL transaction RPCs.
+enum SqlSupportedTransaction {
+ // Unknown/not indicated
+ SQL_SUPPORTED_TRANSACTION_UNKNOWN = 0;
+ // No support
+ SQL_SUPPORTED_TRANSACTION_NONE = 1;
Review Comment:
Is there a *functional* difference here? Or are we just including an
"Unknown" so that it will be the default? In most cases developers are likely
to treat "unknown" the same as "none" when it comes to transaction support:
(ie. don't try calling them)
##########
format/FlightSql.proto:
##########
@@ -1550,6 +1755,48 @@ message DoPutUpdateResult {
int64 record_count = 1;
}
+/*
+ * Request message for the "CancelQuery" action.
+ *
+ * Explicitly cancel a running query.
+ *
+ * This lets a single client explicitly cancel work, no matter how many clients
+ * are involved/whether the query is distributed or not, given server support.
+ * The transaction/statement is not rolled back; it is the application's job to
+ * commit or rollback as appropriate. This only indicates the client no longer
+ * wishes to read the remainder of the query results or continue submitting
+ * data.
+ *
+ * This command is idempotent.
+ */
+message ActionCancelQueryRequest {
+ option (experimental) = true;
+
+ // The result of the GetFlightInfo RPC that initated the query.
+ // XXX(ARROW-16902): this must be a serialized FlightInfo, but is
+ // rendered as bytes because Protobuf does not really support one
+ // DLL using Protobuf definitions from another DLL.
+ bytes info = 1;
+}
+
+/*
+ * The result of cancelling a query.
+ *
+ * The result should be wrapped in a google.protobuf.Any message.
+ */
+message ActionCancelQueryResult {
+ option (experimental) = true;
+
+ enum CancelResult {
+ CANCEL_RESULT_UNSPECIFIED = 0;
+ CANCEL_RESULT_CANCELLED = 1;
+ CANCEL_RESULT_CANCELLING = 2;
+ CANCEL_RESULT_NOT_CANCELLABLE = 3;
Review Comment:
If the server returns `CANCEL_RESULT_CANCELLING` is a client supposed to
poll with subsequent cancel requests until it receives `CANCELLED`? Or is there
a different way to determine when the cancellation is completed?
##########
format/FlightSql.proto:
##########
@@ -89,6 +90,31 @@ enum SqlInfo {
*/
FLIGHT_SQL_SERVER_READ_ONLY = 3;
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports executing Substrait plans.
+ */
+ FLIGHT_SQL_SUBSTRAIT = 4;
+
+ /*
+ * Retrieves an int32 indicating whether the Flight SQL Server supports
explicit transaction RPCs.
+ *
+ * The possible values are listed in `FlightSqlTransactionSupport`.
+ */
+ FLIGHT_SQL_TRANSACTION = 5;
+
+ /*
+ * Retrieves an int32 indicating the timeout (in milliseconds) for prepared
statement handles.
+ *
+ * If 0, there is no timeout.
+ */
Review Comment:
I think so, we should probably provide any semantics necessary for when a
timeout is refreshed.
##########
format/FlightSql.proto:
##########
@@ -1418,7 +1442,17 @@ message ActionCreatePreparedStatementRequest {
}
/*
- * Wrap the result of a "GetPreparedStatement" action.
+ * Request message for the "CreatePreparedSubstraitPlan" action on a Flight
SQL enabled backend.
+ */
+message ActionCreatePreparedSubstraitPlan {
+ option (experimental) = true;
+
+ // The serialized substrait.Plan to create a prepared statement for.
+ bytes plan = 1;
Review Comment:
If we're expecting the Protobuf serialized plan here, would it make more
sense to just import the substrait proto definition and reference the object
directly rather than having to serialize the plan and then stick the bytes
inside another serialized protobuf?
--
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]
