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]

Reply via email to