lidavidm commented on code in PR #13492:
URL: https://github.com/apache/arrow/pull/13492#discussion_r913952234
##########
format/FlightSql.proto:
##########
@@ -1450,8 +1484,48 @@ message ActionClosePreparedStatementRequest {
bytes prepared_statement_handle = 1;
}
+/*
+ * Request message for the "BeginTransaction" action.
+ * Begins a transaction or creates a savepoint within a transaction.
+ */
+message ActionBeginTransactionRequest {
+ // Create a savepoint within the identified transaction. Only supported if
+ // FLIGHT_SQL_TRANSACTION is FLIGHT_SQL_TRANSACTION_SUPPORT_SAVEPOINT.
+ bytes transaction_id = 1;
+ // Name for the savepoint (if applicable).
+ string name = 2;
+}
+
+/*
+ * The result of a "BeginTransaction" action.
+ *
+ * The transaction/savepoint can be manipulated with the "EndTransaction"
+ * action, or automatically via server timeout.
+ */
+message ActionBeginTransactionResult {
+ // Opaque handle for the transaction on the server.
+ bytes transaction_id = 1;
+}
+
+/*
+ * Request message for the "EndTransaction" action.
+ * Commit or rollback a transaction, or release/rollback a savepoint within a
transaction.
+ */
+message ActionEndTransactionRequest {
+ enum EndTransaction {
+ END_TRANSACTION_UNSPECIFIED = 0;
+ // Commit the transaction or release the savepoint.
+ END_TRANSACTION_COMMIT = 1;
+ // Roll back the transaction or to a savepoint.
+ END_TRANSACTION_ROLLBACK = 2;
+ }
+ // Opaque handle for the transaction on the server.
Review Comment:
Yes, updated docstring.
##########
format/FlightSql.proto:
##########
@@ -1450,8 +1484,48 @@ message ActionClosePreparedStatementRequest {
bytes prepared_statement_handle = 1;
}
+/*
+ * Request message for the "BeginTransaction" action.
+ * Begins a transaction or creates a savepoint within a transaction.
+ */
+message ActionBeginTransactionRequest {
+ // Create a savepoint within the identified transaction. Only supported if
+ // FLIGHT_SQL_TRANSACTION is FLIGHT_SQL_TRANSACTION_SUPPORT_SAVEPOINT.
+ bytes transaction_id = 1;
+ // Name for the savepoint (if applicable).
+ string name = 2;
+}
+
+/*
+ * The result of a "BeginTransaction" action.
+ *
+ * The transaction/savepoint can be manipulated with the "EndTransaction"
+ * action, or automatically via server timeout.
+ */
+message ActionBeginTransactionResult {
+ // Opaque handle for the transaction on the server.
+ bytes transaction_id = 1;
+}
+
+/*
+ * Request message for the "EndTransaction" action.
+ * Commit or rollback a transaction, or release/rollback a savepoint within a
transaction.
+ */
+message ActionEndTransactionRequest {
Review Comment:
I clarified the docstring, though I'm not sure what you're referring to
here.
##########
format/FlightSql.proto:
##########
@@ -1549,6 +1658,17 @@ message DoPutUpdateResult {
int64 record_count = 1;
}
+/*
+ * Explicitly cancel a running query.
+ *
+ * For distributed queries (ones that use GetFlightInfo->DoGet), this lets a
+ * single client explicitly cancel work across all clients, given server
support.
+ */
+message ActionCancelQuery {
+ // The result of the GetFlightInfo RPC that initated the query.
+ FlightInfo info = 1;
Review Comment:
What would it be? Since most of the data inside FlightInfo isn't meant to be
introspected, and we haven't specified what the contents of Ticket should be,
there's no consistent 'query ID' concept right now. So I chose FlightInfo since
presumably that has all information the server needs to identify the query.
##########
format/FlightSql.proto:
##########
@@ -1475,6 +1549,35 @@ message CommandStatementQuery {
// The SQL syntax.
string query = 1;
+ // Include the query as part of this transaction (by default queries are
auto-committed).
Review Comment:
I suppose we could use schema metadata for that, though.
##########
format/FlightSql.proto:
##########
@@ -1450,8 +1484,48 @@ message ActionClosePreparedStatementRequest {
bytes prepared_statement_handle = 1;
}
+/*
+ * Request message for the "BeginTransaction" action.
+ * Begins a transaction or creates a savepoint within a transaction.
+ */
+message ActionBeginTransactionRequest {
+ // Create a savepoint within the identified transaction. Only supported if
+ // FLIGHT_SQL_TRANSACTION is FLIGHT_SQL_TRANSACTION_SUPPORT_SAVEPOINT.
+ bytes transaction_id = 1;
+ // Name for the savepoint (if applicable).
+ string name = 2;
+}
+
+/*
+ * The result of a "BeginTransaction" action.
+ *
+ * The transaction/savepoint can be manipulated with the "EndTransaction"
+ * action, or automatically via server timeout.
Review Comment:
I chose to follow prepared statements in this regard. I'll add a SqlInfo
value to retrieve the timeout.
##########
format/FlightSql.proto:
##########
@@ -1549,6 +1658,17 @@ message DoPutUpdateResult {
int64 record_count = 1;
}
+/*
+ * Explicitly cancel a running query.
+ *
+ * For distributed queries (ones that use GetFlightInfo->DoGet), this lets a
+ * single client explicitly cancel work across all clients, given server
support.
Review Comment:
I'll update it to specify that the transaction is not rolled back/this is
only to terminate reading of the result set (CC @jduo is that in line with what
you were thinking?)
##########
format/FlightSql.proto:
##########
@@ -1475,6 +1549,35 @@ message CommandStatementQuery {
// The SQL syntax.
string query = 1;
+ // Include the query as part of this transaction (by default queries are
auto-committed).
Review Comment:
The intent was to mimic prepared statements: the server assigns the
transaction ID and gives it to the client. So here there's not a great way to
return the transaction ID to the client. It would be good if FlightInfo could
gain an app_metadata field for such things (since Tickets are not meant to be
client-introspected).
##########
format/FlightSql.proto:
##########
@@ -1549,6 +1658,17 @@ message DoPutUpdateResult {
int64 record_count = 1;
}
+/*
+ * Explicitly cancel a running query.
+ *
+ * For distributed queries (ones that use GetFlightInfo->DoGet), this lets a
+ * single client explicitly cancel work across all clients, given server
support.
+ */
+message ActionCancelQuery {
Review Comment:
What would it contain?
--
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]
