[GitHub] [arrow-rs] roeap commented on a diff in pull request #4359: feat(flight): add xdbc type info helpers

["roeap (via GitHub)" <gi...@apache.org>]

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. 



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