alamb commented on code in PR #4359:
URL: https://github.com/apache/arrow-rs/pull/4359#discussion_r1218502420
##########
arrow-flight/examples/flight_sql_server.rs:
##########
@@ -544,12 +580,11 @@ impl FlightSqlService for FlightSqlServiceImpl {
async fn do_get_xdbc_type_info(
&self,
- _query: CommandGetXdbcTypeInfo,
+ query: CommandGetXdbcTypeInfo,
_request: Request<Ticket>,
) -> Result<Response<<Self as FlightService>::DoGetStream>, Status> {
- Err(Status::unimplemented(
- "do_get_xdbc_type_info not implemented",
- ))
+ let stream = INSTANCE_XDBC_INFO.encode(query).map_err(Status::from);
Review Comment:
> The builder is used to statically create the instance of XdbcTypeInfoList,
which then directly contains the sorted etc record batch. Since the data never
changes at runtime, I thought encoding it should also be done only once? Then
again, I do not really have a good feeling for how expensive then encoding is
and if it is worthwhile to save that effort at query time.
I thought about this a bunch too. Since this is a metadata query that gets
run over and over again I do think it is worth optimizing the performance for
that common case. However, given the Builders for the other metadata endpoints
are builders for handling the parameters (e.g. filters) having an XDBC builder
do something different may be confusing.
Maybe we can use the pattern in this PR but change the names a bit? For
example
```rust
// create a builder with pre-defined Xdbc data:
let mut builder = query.into_builder(&INSTANCE_XBDC_DATA);
let schema = builder.schema();
let batch = builder.build();
let stream = FlightDataEncoderBuilder::new()
.with_schema(schema)
.build(futures::stream::once(async { batch }))
.map_err(Status::from);
Ok(Response::new(Box::pin(stream)))
```
And then pre-define `INSTANCE_XDBC_Data` with another builder? Something
like
```rust
static INSTANCE_XDBC_INFO: Lazy<XdbcTypeInfoData> = Lazy::new(|| {
let mut builder = XdbcTypeInfoDataBuilder::new();
builder.append(XdbcTypeInfo {
type_name: "INTEGER".into(),
data_type: XdbcDataType::XdbcInteger,
column_size: Some(32),
literal_prefix: None,
literal_suffix: None,
create_params: None,
nullable: Nullable::NullabilityNullable,
case_sensitive: false,
searchable: Searchable::Full,
unsigned_attribute: Some(false),
fixed_prec_scale: false,
auto_increment: Some(false),
local_type_name: Some("INTEGER".into()),
minimum_scale: None,
maximum_scale: None,
sql_data_type: XdbcDataType::XdbcInteger,
datetime_subcode: None,
num_prec_radix: Some(2),
interval_precision: None,
});
builder.build().unwrap()
});
```
🤔
--
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]
