roeap commented on code in PR #4359:
URL: https://github.com/apache/arrow-rs/pull/4359#discussion_r1218479700
##########
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:
> I may have missed it, but this doesn't seem to ever construct a
XdbcTypeInfoListBuilder
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.
> What do you think about making this mirror the other builders, so the code
would look like
That was my initial instinct too, somehow I felt though that xdbc and sql
info are different a bit nature as discussed above. If we want to construct the
record batch at runtime, I think we should do that though...
> Maybe we can write some sort of generic function (either on
FlightDataEncoderBuilder or as a free function) that takes a impl
Into<SchemaAndBatch>
Hmm, I have no too strong feelings about this, in the end, the `stream`
function only "saves" a very few lines of code, so maybe either do it
everywhere (includig catalogs) or never? In that case I would go for never, and
just return the batch, since I expect the `FlightDataEncoderBuilder ` will be
used very frequently throughout the codebase, so users will be quite familiar
with it.
--
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]