lidavidm commented on code in PR #13492:
URL: https://github.com/apache/arrow/pull/13492#discussion_r963706905
##########
format/FlightSql.proto:
##########
@@ -90,6 +90,55 @@ enum SqlInfo {
*/
FLIGHT_SQL_SERVER_READ_ONLY = 3;
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports executing
+ * Substrait plans.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT = 4;
+
+ /*
+ * Retrieves a string value indicating the minimum supported Substrait
version, or null
+ * if Substrait is not supported.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT_MIN_VERSION = 5;
+
+ /*
+ * Retrieves a string value indicating the maximum supported Substrait
version, or null
+ * if Substrait is not supported.
Review Comment:
As I understand, the backwards compatibility story is not yet worked out, so
in the future, this may be less useful, but for now, it's the only way to
reliably determine whether a plan can really be executed.
##########
format/FlightSql.proto:
##########
@@ -1416,14 +1477,47 @@ message ActionCreatePreparedStatementRequest {
// The valid SQL string to create a prepared statement for.
string query = 1;
+ // Create/execute the prepared statement as part of this transaction (if
+ // unset, executions of the prepared statement will be auto-committed).
+ optional bytes transaction_id = 2;
+}
+
+/*
+ * An embedded message describing a Substrait plan to execute.
+ */
+message SubstraitPlan {
+ // The serialized substrait.Plan to create a prepared statement for.
+ // XXX(ARROW-16902): this is bytes instead of an embedded message
+ // because Protobuf does not really support one DLL using Protobuf
+ // definitions from another DLL.
+ bytes plan = 1;
+ // The Substrait release, e.g. "0.12.0". This information is not
+ // tracked in the plan itself, so this is the only way for consumers
+ // to potentially know if they can handle the plan.
+ string version = 2;
+}
+
+/*
+ * Request message for the "CreatePreparedSubstraitPlan" action on a Flight
SQL enabled backend.
+ */
+message ActionCreatePreparedSubstraitPlanRequest {
+ option (experimental) = true;
Review Comment:
No, I just missed it :) I'll fix that.
##########
format/FlightSql.proto:
##########
@@ -1416,14 +1477,47 @@ message ActionCreatePreparedStatementRequest {
Review Comment:
I think not in this RFC, but we should do it in the near future (perhaps
after the JDBC driver has seen some use)
##########
format/FlightSql.proto:
##########
@@ -90,6 +90,55 @@ enum SqlInfo {
*/
FLIGHT_SQL_SERVER_READ_ONLY = 3;
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports executing
+ * Substrait plans.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT = 4;
+
+ /*
+ * Retrieves a string value indicating the minimum supported Substrait
version, or null
+ * if Substrait is not supported.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT_MIN_VERSION = 5;
+
+ /*
+ * Retrieves a string value indicating the maximum supported Substrait
version, or null
+ * if Substrait is not supported.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT_MAX_VERSION = 6;
+
+ /*
+ * Retrieves an int32 indicating whether the Flight SQL Server supports the
+ * BeginTransaction/EndTransaction/BeginSavepoint/EndSavepoint actions.
+ *
+ * Even if this is not supported, the database may still support explicit
"BEGIN
+ * TRANSACTION"/"COMMIT" SQL statements (see SQL_TRANSACTIONS_SUPPORTED);
this property
+ * is only about whether the server implements the Flight SQL API endpoints.
+ *
+ * The possible values are listed in `SqlSupportedTransaction`.
+ */
+ FLIGHT_SQL_SERVER_TRANSACTION = 7;
+
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports explicit
+ * query cancellation (the CancelQuery action).
+ */
+ FLIGHT_SQL_SERVER_CANCEL = 8;
+
+ /*
+ * Retrieves an int32 indicating the timeout (in milliseconds) for prepared
statement handles.
Review Comment:
I'll update this.
##########
format/FlightSql.proto:
##########
@@ -90,6 +90,55 @@ enum SqlInfo {
*/
FLIGHT_SQL_SERVER_READ_ONLY = 3;
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports executing
+ * Substrait plans.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT = 4;
+
+ /*
+ * Retrieves a string value indicating the minimum supported Substrait
version, or null
+ * if Substrait is not supported.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT_MIN_VERSION = 5;
+
+ /*
+ * Retrieves a string value indicating the maximum supported Substrait
version, or null
+ * if Substrait is not supported.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT_MAX_VERSION = 6;
+
+ /*
+ * Retrieves an int32 indicating whether the Flight SQL Server supports the
+ * BeginTransaction/EndTransaction/BeginSavepoint/EndSavepoint actions.
+ *
+ * Even if this is not supported, the database may still support explicit
"BEGIN
+ * TRANSACTION"/"COMMIT" SQL statements (see SQL_TRANSACTIONS_SUPPORTED);
this property
+ * is only about whether the server implements the Flight SQL API endpoints.
+ *
+ * The possible values are listed in `SqlSupportedTransaction`.
+ */
+ FLIGHT_SQL_SERVER_TRANSACTION = 7;
+
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports explicit
+ * query cancellation (the CancelQuery action).
+ */
+ FLIGHT_SQL_SERVER_CANCEL = 8;
+
+ /*
+ * Retrieves an int32 indicating the timeout (in milliseconds) for prepared
statement handles.
+ *
+ * If 0, there is no timeout. Servers should reset the timeout when the
handle is used in a command.
+ */
+ FLIGHT_SQL_SERVER_STATEMENT_TIMEOUT = 100;
+
+ /*
+ * Retrieves an int32 indicating the timeout (in milliseconds) for
transactions, since transactions are not tied to a connection.
Review Comment:
Maybe @jduo can chime in; this timeout existed implicitly before, but
something here is necessary because unlike JDBC/ODBC which can tie these to the
lifetime of an actual connection, Flight SQL makes fewer assumptions about
state being tied to the gRPC connection (which are more disposable).
##########
format/FlightSql.proto:
##########
@@ -90,6 +90,55 @@ enum SqlInfo {
*/
FLIGHT_SQL_SERVER_READ_ONLY = 3;
+ /*
+ * Retrieves a boolean value indicating whether the Flight SQL Server
supports executing
+ * Substrait plans.
+ */
+ FLIGHT_SQL_SERVER_SUBSTRAIT = 4;
Review Comment:
Hmm, I think that makes sense, though current clients are going to assume
SQL support.
--
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]
