pitrou commented on code in PR #13492:
URL: https://github.com/apache/arrow/pull/13492#discussion_r924271806


##########
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.

Review Comment:
   So "Flight SQL" is generic enough that it might support things other than 
actual SQL?



##########
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 for prepared statement handles.

Review Comment:
   Er... which unit is that? Seconds? Can we make it a real/float instead, if 
protobuf allows that?



##########
format/FlightSql.proto:
##########
@@ -1450,8 +1500,55 @@ 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 {
+  // The transaction to which a savepoint belongs, if applicable.
+  //
+  // To begin a transaction, leave this field empty.

Review Comment:
   Hmm... should there be two separate commands for starting a transaction and 
a savepoint, so that they can take different parameters?



##########
format/FlightSql.proto:
##########
@@ -760,6 +786,20 @@ enum SqlInfo {
   SQL_STORED_FUNCTIONS_USING_CALL_SYNTAX_SUPPORTED = 576;
 }
 
+// The level of support for Flight SQL transaction RPCs.
+enum FlightSqlTransactionSupport {
+  // Unknown/not indicated
+  FLIGHT_SQL_TRANSACTION_SUPPORT_UNKNOWN = 0;
+  // No support
+  FLIGHT_SQL_TRANSACTION_SUPPORT_NONE = 1;
+  // Transactions, but not savepoints.
+  // A savepoint is a mark within a transaction that can be individually
+  // rolled back to. Not all databases support savepoints.

Review Comment:
   Just for my understanding, savepoints are for two-phase commits, right?



##########
format/FlightSql.proto:
##########
@@ -1475,6 +1572,35 @@ message CommandStatementQuery {
 
   // The SQL syntax.
   string query = 1;
+  // Include the query as part of this transaction (by default queries are 
auto-committed).
+  bytes transaction_id = 2;
+}
+
+/*
+ * Represents a Substrait plan. Used in the command member of FlightDescriptor
+ * for the following RPC calls:
+ *  - GetSchema: return the Arrow schema of the query.
+ *    Fields on this schema may contain the following metadata:
+ *    - ARROW:FLIGHT:SQL:CATALOG_NAME      - Table's catalog name
+ *    - ARROW:FLIGHT:SQL:DB_SCHEMA_NAME    - Database schema name
+ *    - ARROW:FLIGHT:SQL:TABLE_NAME        - Table name
+ *    - ARROW:FLIGHT:SQL:TYPE_NAME         - The data source-specific name for 
the data type of the column.
+ *    - ARROW:FLIGHT:SQL:PRECISION         - Column precision/size
+ *    - ARROW:FLIGHT:SQL:SCALE             - Column scale/decimal digits if 
applicable
+ *    - ARROW:FLIGHT:SQL:IS_AUTO_INCREMENT - "1" indicates if the column is 
auto incremented, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is 
case sensitive, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_READ_ONLY      - "1" indicates if the column is 
read only, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_SEARCHABLE     - "1" indicates if the column is 
searchable via WHERE clause, "0" otherwise.
+ *  - GetFlightInfo: execute the query.
+ *  - DoPut: execute the query.
+ */
+message CommandStatementSubstraitPlan {
+  option (experimental) = true;
+
+  // A serialized substrait.Plan
+  bytes plan = 1;
+  // Include the query as part of this transaction (by default queries are 
auto-committed).

Review Comment:
   Does the "default" happen if `transaction_id` is left unset?



##########
format/FlightSql.proto:
##########
@@ -1475,6 +1572,35 @@ message CommandStatementQuery {
 
   // The SQL syntax.
   string query = 1;
+  // Include the query as part of this transaction (by default queries are 
auto-committed).
+  bytes transaction_id = 2;
+}
+
+/*
+ * Represents a Substrait plan. Used in the command member of FlightDescriptor
+ * for the following RPC calls:
+ *  - GetSchema: return the Arrow schema of the query.
+ *    Fields on this schema may contain the following metadata:
+ *    - ARROW:FLIGHT:SQL:CATALOG_NAME      - Table's catalog name
+ *    - ARROW:FLIGHT:SQL:DB_SCHEMA_NAME    - Database schema name
+ *    - ARROW:FLIGHT:SQL:TABLE_NAME        - Table name
+ *    - ARROW:FLIGHT:SQL:TYPE_NAME         - The data source-specific name for 
the data type of the column.
+ *    - ARROW:FLIGHT:SQL:PRECISION         - Column precision/size
+ *    - ARROW:FLIGHT:SQL:SCALE             - Column scale/decimal digits if 
applicable
+ *    - ARROW:FLIGHT:SQL:IS_AUTO_INCREMENT - "1" indicates if the column is 
auto incremented, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_CASE_SENSITIVE - "1" indicates if the column is 
case sensitive, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_READ_ONLY      - "1" indicates if the column is 
read only, "0" otherwise.
+ *    - ARROW:FLIGHT:SQL:IS_SEARCHABLE     - "1" indicates if the column is 
searchable via WHERE clause, "0" otherwise.
+ *  - GetFlightInfo: execute the query.
+ *  - DoPut: execute the query.
+ */
+message CommandStatementSubstraitPlan {
+  option (experimental) = true;
+
+  // A serialized substrait.Plan
+  bytes plan = 1;
+  // Include the query as part of this transaction (by default queries are 
auto-committed).

Review Comment:
   If so, perhaps replace "by default" with "if unset, " for clarity?



-- 
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]

Reply via email to