yonipeleg33 commented on code in PR #10297:
URL: https://github.com/apache/arrow-rs/pull/10297#discussion_r3538328005


##########
arrow-array/src/array/map_array.rs:
##########
@@ -347,9 +347,9 @@ impl MapArray {
         let entry_offsets_buffer = Buffer::from(entry_offsets.to_byte_slice());
         let keys_data = StringArray::from_iter_values(keys);
 
-        let keys_field = Arc::new(Field::new("keys", DataType::Utf8, false));
+        let keys_field = Arc::new(Field::new("key", DataType::Utf8, false));
         let values_field = Arc::new(Field::new(
-            "values",
+            "value",

Review Comment:
   Use the new consts here



##########
arrow-array/src/array/map_array.rs:
##########
@@ -846,8 +870,8 @@ mod tests {
 
         let key_array = Arc::new(StringArray::from(vec!["a", "b", "c"])) as 
ArrayRef;
         let value_array = Arc::new(UInt32Array::from(vec![0u32, 10, 20])) as 
ArrayRef;
-        let keys_field = Arc::new(Field::new("keys", DataType::Utf8, false));
-        let values_field = Arc::new(Field::new("values", DataType::UInt32, 
false));
+        let keys_field = Arc::new(Field::new("key", DataType::Utf8, false));
+        let values_field = Arc::new(Field::new("value", DataType::UInt32, 
false));

Review Comment:
   etc. etc. go over all places in the code and replace hard-coded key/value 
with consts



##########
arrow-array/src/array/map_array.rs:
##########
@@ -814,8 +838,8 @@ mod tests {
         // A DictionaryArray has similar buffer layout to a MapArray
         // but the meaning of the values differs
         let struct_t = DataType::Struct(Fields::from(vec![
-            Field::new("keys", DataType::Int32, true),
-            Field::new("values", DataType::UInt32, true),
+            Field::new("key", DataType::Int32, true),
+            Field::new("value", DataType::UInt32, true),

Review Comment:
   Also here



##########
arrow-array/src/builder/map_builder.rs:
##########
@@ -439,7 +447,7 @@ mod tests {
         let mut key_metadata = HashMap::new();
         key_metadata.insert("foo".to_string(), "bar".to_string());
         let key_field = Arc::new(
-            Field::new("keys", DataType::Int32, 
false).with_metadata(key_metadata.clone()),
+            Field::new("other", DataType::Int32, 
false).with_metadata(key_metadata.clone()),

Review Comment:
   Small nit, but use a name that indicates this is a key field for easier read 
of assertion messages
   ```suggestion
               Field::new("other_key", DataType::Int32, 
false).with_metadata(key_metadata.clone()),
   ```



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