Jefffrey commented on code in PR #11160:
URL: https://github.com/apache/arrow-rs/pull/11160#discussion_r4067547509


##########
arrow-schema/src/datatype_parse.rs:
##########
@@ -634,13 +642,19 @@ impl<'a> Parser<'a> {
         ))
     }
 
-    /// Parses `"name": [non-null] Type` used in the verbose REE form.
+    /// Parses `"name": non-null Type` for the run_ends field in verbose REE 
form.
     fn parse_ree_verbose_field(&mut self) -> ArrowResult<Field> {

Review Comment:
   probably rename this to be specific to run_ends field, or inline this since 
its only used once



##########
arrow-schema/src/datatype_parse.rs:
##########
@@ -580,6 +580,14 @@ impl<'a> Parser<'a> {
     fn parse_map(&mut self) -> ArrowResult<DataType> {
         self.expect_token(Token::LParen)?;
         let field = self.parse_field()?;
+        if let DataType::Struct(fields) = field.data_type()

Review Comment:
   we probably need to be strict with this:
   
   - enforce its a struct child type (non-nullable) with 2 children
   - take key as the first field, instead of searching by name
   
   for reference, how we validate in `MapArray`:
   
   
https://github.com/apache/arrow-rs/blob/b825e98ff68a20e875ecbf1771e5fab0878271d9/arrow-array/src/array/map_array.rs#L91-L119
   
   and see the format doc for this:
   
   
https://github.com/apache/arrow/blob/5c2ff723410f0841b0ea1158d455bbfa23ff1ca9/format/Schema.fbs#L118-L146



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