[GitHub] [arrow-rs] avantgardnerio commented on a diff in pull request #4250: feat: update flight-sql to latest specs

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

avantgardnerio commented on code in PR #4250:
URL: https://github.com/apache/arrow-rs/pull/4250#discussion_r1200634008


##########
arrow-flight/examples/flight_sql_server.rs:
##########
@@ -486,10 +512,58 @@ impl FlightSqlService for FlightSqlServiceImpl {
         &self,
         _query: ActionClosePreparedStatementRequest,
         _request: Request<Action>,
-    ) {
+    ) -> Result<(), Status> {
         unimplemented!("Implement do_action_close_prepared_statement")
     }
 
+    async fn do_action_create_prepared_substrait_plan(
+        &self,
+        _query: ActionCreatePreparedSubstraitPlanRequest,
+        _request: Request<Action>,
+    ) -> Result<ActionCreatePreparedStatementResult, Status> {
+        unimplemented!("Implement do_action_create_prepared_substrait_plan")

Review Comment:
   The nice thing about `unimplemented!("xxx")` is that it contains information both readable by code (unimplemented) and humans ("Implement do_action_create_prepared_substrait_plan"). This information only exists to get serialized via FlightSQL and sent back to the client. So when someone runs a query in say DataGrip (or whatever their favorite tool is) they get an error message telling them specifically which one of these many methods failed, which makes it much easier to debug.
   
   The alternative is that they get a basic error "unimplemented" and they have to drill into the library files in their IDE, and add breakpoints to each one of these methods in order to track down what isn't implemented.
   
   So my argument for retaining this is that more information is better than losing information.



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