[GitHub] [arrow-rs] alamb commented on a diff in pull request #3594: Support sending schemas for empty streams

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

alamb commented on code in PR #3594:
URL: https://github.com/apache/arrow-rs/pull/3594#discussion_r1085759716


##########
arrow-flight/src/encode.rs:
##########
@@ -122,6 +125,15 @@ impl FlightDataEncoderBuilder {
         self
     }
 
+    /// Specify a schema for the RecordBatches being sent. If a schema
+    /// is not specified, an encoded Schema message will be sent when
+    /// the first [`RecordBatch`], if any, is encoded. Some clients
+    /// expect a Schema message even if there is no data sent.
+    pub fn with_schema(mut self, schema: SchemaRef) -> Self {

Review Comment:
   I was under the impression that the Flight protocol did not require sending a schema , however the format docs don't seem to specify one way or the other; https://arrow.apache.org/docs/format/Flight.html
   
   Requiring a schema might make the API easier to use correctly with other Flight clients (I am pretty sure the issue we hit upstream in IOx was interoperability with the golang client). 
   
   Downstream in DataFusion we almost always have a https://docs.rs/datafusion/16.1.0/datafusion/physical_plan/type.SendableRecordBatchStream.html which has a schema attached. 
   
   So in summary, I could go either way. Does anyone else have any thoughts?
   
   



##########
arrow-flight/src/encode.rs:
##########
@@ -189,26 +215,30 @@ impl FlightDataEncoder {
         }
     }
 
+    /// Encodes schema as a [`FlightData`] in self.queue.
+    /// Updates `self.schema` and returns the new schema
+    fn encode_schema(&mut self, schema: SchemaRef) -> SchemaRef {

Review Comment:
   changed to     `fn encode_schema(&mut self, schema: &SchemaRef) {`
   



##########
arrow-flight/src/decode.rs:
##########
@@ -101,13 +97,12 @@ impl FlightRecordBatchStream {
     {
         Self {
             inner: FlightDataDecoder::new(inner),
-            got_schema: false,
         }
     }
 
-    /// Has a message defining the schema been received yet?
-    pub fn got_schema(&self) -> bool {

Review Comment:
   I will do that -- and mark it deprecated 👍 



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