You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/08 13:13:52 UTC
[GitHub] [arrow-rs] alamb commented on a diff in pull request #4384: feat(flight): harmonize server metadata APIs
alamb commented on code in PR #4384:
URL: https://github.com/apache/arrow-rs/pull/4384#discussion_r1223018314
##########
arrow-flight/src/sql/metadata/sql_info.rs:
##########
@@ -15,26 +15,34 @@
// specific language governing permissions and limitations
// under the License.
-//! [`SqlInfoList`] for building responses to [`CommandGetSqlInfo`] queries.
+//! Helpers for building responses to [`CommandGetSqlInfo`] metadata requests.
Review Comment:
👍
##########
arrow-flight/src/sql/metadata/sql_info.rs:
##########
@@ -427,7 +395,88 @@ impl SqlInfoList {
}
}
-// The schema produced by [`SqlInfoList`]
+/// A builder for [`SqlInfoData`] which is used to create [`CommandGetSqlInfo`] responses.
+///
+/// # Example
+/// ```
+/// # use arrow_flight::sql::{metadata::SqlInfoDataBuilder, SqlInfo, SqlSupportedTransaction};
+/// // Create the list of metadata describing the server
+/// let mut builder = SqlInfoDataBuilder::new();
+/// builder.append(SqlInfo::FlightSqlServerName, "server name");
+/// // ... add other SqlInfo here ..
+/// builder.append(
+/// SqlInfo::FlightSqlServerTransaction,
+/// SqlSupportedTransaction::Transaction as i32,
+/// );
+///
+/// // Create the batch to send back to the client
+/// let info_data = builder.build().unwrap();
+/// ```
+///
+/// [protos]: https://github.com/apache/arrow/blob/6d3d2fca2c9693231fa1e52c142ceef563fc23f9/format/FlightSql.proto#L71-L820
+pub struct SqlInfoData {
+ batch: RecordBatch,
+}
+
+impl SqlInfoData {
+ /// Return the raw (not encoded) RecordBatch that will be returned
+ /// from [`CommandGetSqlInfo`]
Review Comment:
```suggestion
/// Return a [`RecordBatch`] containing only the requested `u32`, if any
/// from [`CommandGetSqlInfo`]
```
##########
arrow-flight/src/sql/metadata/sql_info.rs:
##########
@@ -427,7 +395,88 @@ impl SqlInfoList {
}
}
-// The schema produced by [`SqlInfoList`]
+/// A builder for [`SqlInfoData`] which is used to create [`CommandGetSqlInfo`] responses.
+///
+/// # Example
+/// ```
+/// # use arrow_flight::sql::{metadata::SqlInfoDataBuilder, SqlInfo, SqlSupportedTransaction};
+/// // Create the list of metadata describing the server
+/// let mut builder = SqlInfoDataBuilder::new();
+/// builder.append(SqlInfo::FlightSqlServerName, "server name");
+/// // ... add other SqlInfo here ..
+/// builder.append(
+/// SqlInfo::FlightSqlServerTransaction,
+/// SqlSupportedTransaction::Transaction as i32,
+/// );
+///
+/// // Create the batch to send back to the client
+/// let info_data = builder.build().unwrap();
+/// ```
+///
+/// [protos]: https://github.com/apache/arrow/blob/6d3d2fca2c9693231fa1e52c142ceef563fc23f9/format/FlightSql.proto#L71-L820
+pub struct SqlInfoData {
+ batch: RecordBatch,
+}
+
+impl SqlInfoData {
+ /// Return the raw (not encoded) RecordBatch that will be returned
+ /// from [`CommandGetSqlInfo`]
+ pub fn record_batch(
+ &self,
+ info: impl IntoIterator<Item = u32>,
+ ) -> Result<RecordBatch> {
+ let arr: UInt32Array = downcast_array(self.batch.column(0).as_ref());
+ let type_filter = info
+ .into_iter()
+ .map(|tt| eq_scalar(&arr, tt))
+ .collect::<std::result::Result<Vec<_>, _>>()?
+ .into_iter()
+ // We know the arrays are of same length as they are produced fromn the same root array
+ .reduce(|filter, arr| or(&filter, &arr).unwrap());
+ if let Some(filter) = type_filter {
+ Ok(filter_record_batch(&self.batch, &filter)?)
+ } else {
+ Ok(self.batch.clone())
Review Comment:
💯 for optimizing the common case of no filtering and simply returning the (pre-computed) RecordBatch
##########
arrow-flight/src/sql/metadata/sql_info.rs:
##########
@@ -427,7 +395,88 @@ impl SqlInfoList {
}
}
-// The schema produced by [`SqlInfoList`]
+/// A builder for [`SqlInfoData`] which is used to create [`CommandGetSqlInfo`] responses.
+///
+/// # Example
+/// ```
+/// # use arrow_flight::sql::{metadata::SqlInfoDataBuilder, SqlInfo, SqlSupportedTransaction};
+/// // Create the list of metadata describing the server
+/// let mut builder = SqlInfoDataBuilder::new();
+/// builder.append(SqlInfo::FlightSqlServerName, "server name");
+/// // ... add other SqlInfo here ..
+/// builder.append(
+/// SqlInfo::FlightSqlServerTransaction,
+/// SqlSupportedTransaction::Transaction as i32,
+/// );
+///
+/// // Create the batch to send back to the client
+/// let info_data = builder.build().unwrap();
+/// ```
+///
+/// [protos]: https://github.com/apache/arrow/blob/6d3d2fca2c9693231fa1e52c142ceef563fc23f9/format/FlightSql.proto#L71-L820
+pub struct SqlInfoData {
+ batch: RecordBatch,
+}
+
+impl SqlInfoData {
+ /// Return the raw (not encoded) RecordBatch that will be returned
+ /// from [`CommandGetSqlInfo`]
+ pub fn record_batch(
+ &self,
+ info: impl IntoIterator<Item = u32>,
+ ) -> Result<RecordBatch> {
+ let arr: UInt32Array = downcast_array(self.batch.column(0).as_ref());
+ let type_filter = info
+ .into_iter()
+ .map(|tt| eq_scalar(&arr, tt))
+ .collect::<std::result::Result<Vec<_>, _>>()?
+ .into_iter()
+ // We know the arrays are of same length as they are produced fromn the same root array
+ .reduce(|filter, arr| or(&filter, &arr).unwrap());
+ if let Some(filter) = type_filter {
+ Ok(filter_record_batch(&self.batch, &filter)?)
+ } else {
+ Ok(self.batch.clone())
+ }
+ }
+
+ /// Return the schema of the RecordBatch that will be returned
+ /// from [`CommandGetSqlInfo`]
+ pub fn schema(&self) -> SchemaRef {
+ self.batch.schema()
+ }
+}
+
+/// A builder for a [`CommandGetSqlInfo`] response.
+pub struct GetSqlInfoBuilder<'a> {
+ info: Vec<u32>,
Review Comment:
```suggestion
/// requested `SqlInfo`s. If empty means return all infos.
info: Vec<u32>,
```
##########
arrow-flight/src/sql/metadata/sql_info.rs:
##########
@@ -427,7 +395,88 @@ impl SqlInfoList {
}
}
-// The schema produced by [`SqlInfoList`]
+/// A builder for [`SqlInfoData`] which is used to create [`CommandGetSqlInfo`] responses.
+///
+/// # Example
+/// ```
+/// # use arrow_flight::sql::{metadata::SqlInfoDataBuilder, SqlInfo, SqlSupportedTransaction};
+/// // Create the list of metadata describing the server
+/// let mut builder = SqlInfoDataBuilder::new();
+/// builder.append(SqlInfo::FlightSqlServerName, "server name");
+/// // ... add other SqlInfo here ..
+/// builder.append(
+/// SqlInfo::FlightSqlServerTransaction,
+/// SqlSupportedTransaction::Transaction as i32,
+/// );
+///
+/// // Create the batch to send back to the client
+/// let info_data = builder.build().unwrap();
+/// ```
+///
+/// [protos]: https://github.com/apache/arrow/blob/6d3d2fca2c9693231fa1e52c142ceef563fc23f9/format/FlightSql.proto#L71-L820
+pub struct SqlInfoData {
+ batch: RecordBatch,
+}
+
+impl SqlInfoData {
+ /// Return the raw (not encoded) RecordBatch that will be returned
+ /// from [`CommandGetSqlInfo`]
+ pub fn record_batch(
+ &self,
+ info: impl IntoIterator<Item = u32>,
+ ) -> Result<RecordBatch> {
+ let arr: UInt32Array = downcast_array(self.batch.column(0).as_ref());
+ let type_filter = info
Review Comment:
❤️ this is very clever
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org