kou commented on code in PR #38256:
URL: https://github.com/apache/arrow/pull/38256#discussion_r1361539912


##########
format/FlightSql.proto:
##########
@@ -1778,6 +1789,44 @@ message CommandPreparedStatementUpdate {
   bytes prepared_statement_handle = 1;
 }
 
+  /*
+   * Represents a bulk ingestion request. Used in the command member of 
FlightDescriptor
+   * for the the RPC call DoPut to cause the server load the contents of the 
stream's
+   * FlightData into the target destination.
+   */
+message CommandStatementIngest {
+  option (experimental) = true;
+
+  // Describes the behavior for loading bulk data.
+  enum IngestMode {
+    // Ingestion behavior unspecified.
+    INGEST_MODE_UNSPECIFIED = 0;
+    // Create the target table. Fail if the target table already exists.
+    INGEST_MODE_CREATE = 1;
+    // Append to an existing target table. Fail if the target table does not 
exist.
+    INGEST_MODE_APPEND = 2;
+    // Drop the target table if it exists. Then follow INGEST_MODE_CREATE 
behavior.
+    INGEST_MODE_REPLACE = 3;
+    // Create the target table if it does not exist. Then follow 
INGEST_MODE_APPEND behavior.
+    INGEST_MODE_CREATE_APPEND = 4;
+  }
+
+  // The ingestion behavior.
+  IngestMode mode = 1;
+  // The table to load data into.
+  string target_table = 2;
+  // The db_schema of the target_table to load data into. If unset, a 
backend-specific default may be used.
+  optional string target_schema = 3;
+  // The catalog of the target_table to load data into. If unset, a 
backend-specific default may be used.
+  optional string target_catalog = 4;
+  // Use a temporary table for target_table.

Review Comment:
   `target_` prefix may be redundant.



##########
format/FlightSql.proto:
##########
@@ -1778,6 +1789,44 @@ message CommandPreparedStatementUpdate {
   bytes prepared_statement_handle = 1;
 }
 
+  /*
+   * Represents a bulk ingestion request. Used in the command member of 
FlightDescriptor
+   * for the the RPC call DoPut to cause the server load the contents of the 
stream's
+   * FlightData into the target destination.
+   */
+message CommandStatementIngest {
+  option (experimental) = true;
+
+  // Describes the behavior for loading bulk data.
+  enum IngestMode {
+    // Ingestion behavior unspecified.
+    INGEST_MODE_UNSPECIFIED = 0;
+    // Create the target table. Fail if the target table already exists.
+    INGEST_MODE_CREATE = 1;
+    // Append to an existing target table. Fail if the target table does not 
exist.
+    INGEST_MODE_APPEND = 2;
+    // Drop the target table if it exists. Then follow INGEST_MODE_CREATE 
behavior.
+    INGEST_MODE_REPLACE = 3;
+    // Create the target table if it does not exist. Then follow 
INGEST_MODE_APPEND behavior.
+    INGEST_MODE_CREATE_APPEND = 4;
+  }
+
+  // The ingestion behavior.
+  IngestMode mode = 1;
+  // The table to load data into.
+  string target_table = 2;
+  // The db_schema of the target_table to load data into. If unset, a 
backend-specific default may be used.
+  optional string target_schema = 3;
+  // The catalog of the target_table to load data into. If unset, a 
backend-specific default may be used.
+  optional string target_catalog = 4;
+  // Use a temporary table for target_table.
+  optional bool temporary = 5;
+  // Perform the ingestion as part of this transaction (if unset, the query is 
auto-committed).
+  optional bool transaction_id = 6;
+  // Backend-specific options.
+  map<string, string> options = 1000;

Review Comment:
   Why do we want to use `1000` instead of `7` here?



##########
format/FlightSql.proto:
##########
@@ -817,6 +817,17 @@ enum SqlInfo {
    * - true: if invoking user-defined or vendor functions using the stored 
procedure escape syntax is supported.
    */
   SQL_STORED_FUNCTIONS_USING_CALL_SYNTAX_SUPPORTED = 576;
+
+  /*
+   * Retrieves a boolean value indicating whether transactions are supported 
for bulk ingestion. If not, invoking
+   * the method commit in the context of a bulk ingestion is a noop, and the 
isolation level is
+   * `arrow.flight.protocol.sql.SqlTransactionIsolationLevel.TRANSACTION_NONE`.
+   *
+   * Returns:
+   * - false: if bulk ingestion transactions are unsupported;
+   * - true: if bulk ingestion transactions are supported.
+   */
+   INGEST_TRANSACTIONS_SUPPORTED = 577;

Review Comment:
   It seems that other enum values have `SQL_` prefix.
   Do we need it here too?



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