felipecrv commented on code in PR #3714:
URL: https://github.com/apache/arrow-adbc/pull/3714#discussion_r2528466530


##########
rust/core/src/non_blocking.rs:
##########
@@ -0,0 +1,470 @@
+use arrow_array::{RecordBatch, RecordBatchReader};
+use arrow_schema::Schema;
+
+use std::collections::HashSet;
+use std::future::Future;
+
+use crate::error::Result;
+use crate::options::{self, *};
+use crate::PartitionedResult;
+
+/// Ability to configure an object by setting/getting options.
+pub trait AsyncOptionable {
+    type Option: AsRef<str> + Send;
+
+    /// Set a post-init option.
+    fn set_option(
+        &mut self,
+        key: Self::Option,
+        value: OptionValue,
+    ) -> impl Future<Output = Result<()>> + Send;
+
+    /// Get a string option value by key.
+    fn get_option_string(&self, key: Self::Option) -> impl Future<Output = 
Result<String>>;
+
+    /// Get a bytes option value by key.
+    fn get_option_bytes(&self, key: Self::Option) -> impl Future<Output = 
Result<Vec<u8>>>;
+
+    /// Get an integer option value by key.
+    fn get_option_int(&self, key: Self::Option) -> impl Future<Output = 
Result<i64>>;
+
+    /// Get a float option value by key.
+    fn get_option_double(&self, key: Self::Option) -> impl Future<Output = 
Result<f64>>;
+}
+
+/// A handle to an async ADBC driver.
+pub trait AsyncDriver {
+    type DatabaseType: AsyncDatabase + Send;
+
+    /// Allocate and initialize a new database without pre-init options.
+    fn new_database(&mut self) -> impl Future<Output = 
Result<Self::DatabaseType>> + Send;
+
+    /// Allocate and initialize a new database with pre-init options.
+    fn new_database_with_opts(
+        &mut self,
+        opts: Vec<(options::OptionDatabase, OptionValue)>,
+    ) -> impl Future<Output = Result<Self::DatabaseType>> + Send;
+}
+
+/// A handle to an async ADBC database.
+///
+/// Databases hold state shared by multiple connections. This typically means
+/// configuration and caches. For in-memory databases, it provides a place to
+/// hold ownership of the in-memory database.
+///
+/// Databases must be kept alive as long as any connections exist.
+pub trait AsyncDatabase: AsyncOptionable<Option = OptionDatabase> {
+    type ConnectionType: AsyncConnection + Send;
+
+    /// Allocate and initialize a new connection without pre-init options.
+    fn new_connection(&self) -> impl Future<Output = 
Result<Self::ConnectionType>> + Send;
+
+    /// Allocate and initialize a new connection with pre-init options.
+    fn new_connection_with_opts(
+        &self,
+        opts: Vec<(options::OptionConnection, OptionValue)>,
+    ) -> impl Future<Output = Result<Self::ConnectionType>> + Send;
+}
+
+/// A handle to an async ADBC connection.
+///
+/// Connections provide methods for query execution, managing prepared
+/// statements, using transactions, and so on.
+///
+/// # Autocommit
+///
+/// Connections should start in autocommit mode. They can be moved out by
+/// setting [options::OptionConnection::AutoCommit] to "false". Turning off
+/// autocommit allows customizing the isolation level.
+pub trait AsyncConnection: AsyncOptionable<Option = OptionConnection> {
+    type StatementType: AsyncStatement + Send;
+
+    /// Allocate and initialize a new statement.
+    fn new_statement(&mut self) -> impl Future<Output = 
Result<Self::StatementType>> + Send;
+
+    /// Cancel the in-progress operation on a connection.
+    fn cancel(&mut self) -> impl Future<Output = Result<()>> + Send;
+
+    /// Get metadata about the database/driver.
+    ///
+    /// # Arguments
+    ///
+    /// - `codes` - Requested metadata. If `None`, retrieve all available 
metadata.
+    ///
+    /// # Result
+    ///
+    /// The result is an Arrow dataset with the following schema:
+    ///
+    /// Field Name                  | Field Type
+    /// ----------------------------|------------------------
+    /// info_name                   | uint32 not null
+    /// info_value                  | INFO_SCHEMA
+    ///
+    /// INFO_SCHEMA is a dense union with members:
+    ///
+    /// Field Name (Type Code)      | Field Type
+    /// ----------------------------|------------------------
+    /// string_value (0)            | utf8
+    /// bool_value (1)              | bool
+    /// int64_value (2)             | int64
+    /// int32_bitmask (3)           | int32
+    /// string_list (4)             | list\<utf8\>
+    /// int32_to_int32_list_map (5) | map\<int32, list\<int32\>\>
+    fn get_info(
+        &self,
+        codes: Option<HashSet<options::InfoCode>>,
+    ) -> impl Future<Output = Result<impl RecordBatchReader + Send>>;

Review Comment:
   Please `Box<dyn ..>` the `RecordBatchReader` here.
   
   https://github.com/apache/arrow-adbc/issues/2694



##########
rust/core/src/non_blocking.rs:
##########
@@ -19,48 +19,45 @@ use arrow_array::{RecordBatch, RecordBatchReader};
 use arrow_schema::Schema;
 
 use std::collections::HashSet;
-use std::future::Future;
 
 use crate::error::Result;
 use crate::options::{self, *};
 use crate::PartitionedResult;
 
 /// Ability to configure an object by setting/getting options.
-pub trait AsyncOptionable {
-    type Option: AsRef<str> + Send;
+#[trait_variant::make(AsyncOptionable: Send)]

Review Comment:
   Can we stay with the `impl Future<..>` approach or whatever is done 
automatically by this macro to avoid taking in the dependency in adbc_core?



##########
rust/core/src/non_blocking.rs:
##########
@@ -0,0 +1,470 @@
+use arrow_array::{RecordBatch, RecordBatchReader};
+use arrow_schema::Schema;
+
+use std::collections::HashSet;
+use std::future::Future;
+
+use crate::error::Result;
+use crate::options::{self, *};
+use crate::PartitionedResult;
+
+/// Ability to configure an object by setting/getting options.
+pub trait AsyncOptionable {
+    type Option: AsRef<str> + Send;
+
+    /// Set a post-init option.
+    fn set_option(
+        &mut self,
+        key: Self::Option,
+        value: OptionValue,
+    ) -> impl Future<Output = Result<()>> + Send;
+
+    /// Get a string option value by key.
+    fn get_option_string(&self, key: Self::Option) -> impl Future<Output = 
Result<String>>;
+
+    /// Get a bytes option value by key.
+    fn get_option_bytes(&self, key: Self::Option) -> impl Future<Output = 
Result<Vec<u8>>>;
+
+    /// Get an integer option value by key.
+    fn get_option_int(&self, key: Self::Option) -> impl Future<Output = 
Result<i64>>;
+
+    /// Get a float option value by key.
+    fn get_option_double(&self, key: Self::Option) -> impl Future<Output = 
Result<f64>>;
+}
+
+/// A handle to an async ADBC driver.
+pub trait AsyncDriver {
+    type DatabaseType: AsyncDatabase + Send;
+
+    /// Allocate and initialize a new database without pre-init options.
+    fn new_database(&mut self) -> impl Future<Output = 
Result<Self::DatabaseType>> + Send;
+
+    /// Allocate and initialize a new database with pre-init options.
+    fn new_database_with_opts(
+        &mut self,
+        opts: Vec<(options::OptionDatabase, OptionValue)>,
+    ) -> impl Future<Output = Result<Self::DatabaseType>> + Send;
+}
+
+/// A handle to an async ADBC database.
+///
+/// Databases hold state shared by multiple connections. This typically means
+/// configuration and caches. For in-memory databases, it provides a place to
+/// hold ownership of the in-memory database.
+///
+/// Databases must be kept alive as long as any connections exist.
+pub trait AsyncDatabase: AsyncOptionable<Option = OptionDatabase> {
+    type ConnectionType: AsyncConnection + Send;
+
+    /// Allocate and initialize a new connection without pre-init options.
+    fn new_connection(&self) -> impl Future<Output = 
Result<Self::ConnectionType>> + Send;
+
+    /// Allocate and initialize a new connection with pre-init options.
+    fn new_connection_with_opts(
+        &self,
+        opts: Vec<(options::OptionConnection, OptionValue)>,
+    ) -> impl Future<Output = Result<Self::ConnectionType>> + Send;
+}
+
+/// A handle to an async ADBC connection.
+///
+/// Connections provide methods for query execution, managing prepared
+/// statements, using transactions, and so on.
+///
+/// # Autocommit
+///
+/// Connections should start in autocommit mode. They can be moved out by
+/// setting [options::OptionConnection::AutoCommit] to "false". Turning off
+/// autocommit allows customizing the isolation level.
+pub trait AsyncConnection: AsyncOptionable<Option = OptionConnection> {
+    type StatementType: AsyncStatement + Send;
+
+    /// Allocate and initialize a new statement.
+    fn new_statement(&mut self) -> impl Future<Output = 
Result<Self::StatementType>> + Send;
+
+    /// Cancel the in-progress operation on a connection.
+    fn cancel(&mut self) -> impl Future<Output = Result<()>> + Send;
+
+    /// Get metadata about the database/driver.
+    ///
+    /// # Arguments
+    ///
+    /// - `codes` - Requested metadata. If `None`, retrieve all available 
metadata.
+    ///
+    /// # Result
+    ///
+    /// The result is an Arrow dataset with the following schema:
+    ///
+    /// Field Name                  | Field Type
+    /// ----------------------------|------------------------
+    /// info_name                   | uint32 not null
+    /// info_value                  | INFO_SCHEMA
+    ///
+    /// INFO_SCHEMA is a dense union with members:
+    ///
+    /// Field Name (Type Code)      | Field Type
+    /// ----------------------------|------------------------
+    /// string_value (0)            | utf8
+    /// bool_value (1)              | bool
+    /// int64_value (2)             | int64
+    /// int32_bitmask (3)           | int32
+    /// string_list (4)             | list\<utf8\>
+    /// int32_to_int32_list_map (5) | map\<int32, list\<int32\>\>
+    fn get_info(
+        &self,
+        codes: Option<HashSet<options::InfoCode>>,
+    ) -> impl Future<Output = Result<impl RecordBatchReader + Send>>;
+
+    /// Get a hierarchical view of all catalogs, database schemas, tables, and
+    /// columns.
+    ///
+    /// # Arguments
+    ///
+    /// - `depth` - The level of nesting to query.
+    /// - `catalog` - Only show tables in the given catalog. If `None`,
+    ///   do not filter by catalog. If an empty string, only show tables
+    ///   without a catalog.  May be a search pattern.
+    /// - `db_schema` - Only show tables in the given database schema. If
+    ///   `None`, do not filter by database schema. If an empty string, only 
show
+    ///   tables without a database schema. May be a search pattern.
+    /// - `table_name` - Only show tables with the given name. If `None`, do 
not
+    ///   filter by name. May be a search pattern.
+    /// - `table_type` - Only show tables matching one of the given table
+    ///   types. If `None`, show tables of any type. Valid table types can be 
fetched
+    ///   from [Connection::get_table_types].
+    /// - `column_name` - Only show columns with the given name. If
+    ///   `None`, do not filter by name.  May be a search pattern..
+    ///
+    /// # Result
+    ///
+    /// The result is an Arrow dataset with the following schema:
+    ///
+    /// | Field Name               | Field Type               |
+    /// |--------------------------|--------------------------|
+    /// | catalog_name             | utf8                     |
+    /// | catalog_db_schemas       | list\<DB_SCHEMA_SCHEMA\> |
+    ///
+    /// DB_SCHEMA_SCHEMA is a Struct with fields:
+    ///
+    /// | Field Name               | Field Type              |
+    /// |--------------------------|-------------------------|
+    /// | db_schema_name           | utf8                    |
+    /// | db_schema_tables         | list\<TABLE_SCHEMA\>    |
+    ///
+    /// TABLE_SCHEMA is a Struct with fields:
+    ///
+    /// | Field Name               | Field Type                |
+    /// |--------------------------|---------------------------|
+    /// | table_name               | utf8 not null             |
+    /// | table_type               | utf8 not null             |
+    /// | table_columns            | list\<COLUMN_SCHEMA\>     |
+    /// | table_constraints        | list\<CONSTRAINT_SCHEMA\> |
+    ///
+    /// COLUMN_SCHEMA is a Struct with fields:
+    ///
+    /// | Field Name               | Field Type              | Comments |
+    /// |--------------------------|-------------------------|----------|
+    /// | column_name              | utf8 not null           |          |
+    /// | ordinal_position         | int32                   | (1)      |
+    /// | remarks                  | utf8                    | (2)      |
+    /// | xdbc_data_type           | int16                   | (3)      |
+    /// | xdbc_type_name           | utf8                    | (3)      |
+    /// | xdbc_column_size         | int32                   | (3)      |
+    /// | xdbc_decimal_digits      | int16                   | (3)      |
+    /// | xdbc_num_prec_radix      | int16                   | (3)      |
+    /// | xdbc_nullable            | int16                   | (3)      |
+    /// | xdbc_column_def          | utf8                    | (3)      |
+    /// | xdbc_sql_data_type       | int16                   | (3)      |
+    /// | xdbc_datetime_sub        | int16                   | (3)      |
+    /// | xdbc_char_octet_length   | int32                   | (3)      |
+    /// | xdbc_is_nullable         | utf8                    | (3)      |
+    /// | xdbc_scope_catalog       | utf8                    | (3)      |
+    /// | xdbc_scope_schema        | utf8                    | (3)      |
+    /// | xdbc_scope_table         | utf8                    | (3)      |
+    /// | xdbc_is_autoincrement    | bool                    | (3)      |
+    /// | xdbc_is_generatedcolumn  | bool                    | (3)      |
+    ///
+    /// 1. The column's ordinal position in the table (starting from 1).
+    /// 2. Database-specific description of the column.
+    /// 3. Optional value.  Should be null if not supported by the driver.
+    ///    `xdbc_` values are meant to provide JDBC/ODBC-compatible metadata
+    ///    in an agnostic manner.
+    ///
+    /// CONSTRAINT_SCHEMA is a Struct with fields:
+    ///
+    /// | Field Name               | Field Type              | Comments |
+    /// |--------------------------|-------------------------|----------|
+    /// | constraint_name          | utf8                    |          |
+    /// | constraint_type          | utf8 not null           | (1)      |
+    /// | constraint_column_names  | list\<utf8\> not null     | (2)      |
+    /// | constraint_column_usage  | list\<USAGE_SCHEMA\>      | (3)      |
+    ///
+    /// 1. One of `CHECK`, `FOREIGN KEY`, `PRIMARY KEY`, or `UNIQUE`.
+    /// 2. The columns on the current table that are constrained, in
+    ///    order.
+    /// 3. For `FOREIGN KEY` only, the referenced table and columns.
+    ///
+    /// USAGE_SCHEMA is a Struct with fields:
+    ///
+    /// | Field Name               | Field Type              |
+    /// |--------------------------|-------------------------|
+    /// | fk_catalog               | utf8                    |
+    /// | fk_db_schema             | utf8                    |
+    /// | fk_table                 | utf8 not null           |
+    /// | fk_column_name           | utf8 not null           |
+    ///
+    fn get_objects(
+        &self,
+        depth: options::ObjectDepth,
+        catalog: Option<&str>,
+        db_schema: Option<&str>,
+        table_name: Option<&str>,
+        table_type: Option<Vec<&str>>,
+        column_name: Option<&str>,
+    ) -> impl Future<Output = Result<impl RecordBatchReader + Send>> + Send;

Review Comment:
   Same for all `RecordBatchReader`s. They should be `Box<dyn RecordBatchReader 
...>` otherwise the API is extremely inflexible.



##########
rust/core/src/blocking.rs:
##########
@@ -0,0 +1,793 @@
+// Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   I think we can skip creating the `blocking` namespace altogether and keep 
the symbols where they were. "Blocking" has bad connotations and I don't think 
a blocking database API is as bad as the name "blocking" suggests.



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