chore

Author: Weijun-H

datafusion/functions-nested/src/map.rs

@@ -66,15 +66,6 @@ fn make_map_batch(args: &[ColumnarValue]) -> Result<ColumnarValue> {
 
     match keys_arg {
         ColumnarValue::Array(_) => {
-            // For array inputs, keys is a List array where each element represents
-            // the keys of one map. Some list elements may be NULL, which represents
-            // a NULL map (not a null key within a map).
-            //
-            // We should NOT check keys.null_count() here because:
-            // - list_to_arrays() will flatten the list and skip NULL elements (via flatten())
-            // - Those NULL elements represent NULL maps, which are valid
-            // - Null keys WITHIN maps are checked later in check_unique_keys() and
-            //   make_map_array_internal()
             let row_keys = match key_array.data_type() {
                 DataType::List(_) => list_to_arrays::<i32>(&keys),
                 DataType::LargeList(_) => list_to_arrays::<i64>(&keys),
@@ -87,36 +78,12 @@ fn make_map_batch(args: &[ColumnarValue]) -> Result<ColumnarValue> {
                 }
             };
 
-            // Check that keys within each map are unique and non-null
-            // Note: row_keys only contains non-NULL map entries due to flatten()
             row_keys
                 .iter()
-                .try_for_each(|key| check_unique_keys(key.as_ref()))?;
+                .try_for_each(|key| validate_map_keys(key.as_ref()))?;
         }
         ColumnarValue::Scalar(_) => {
-            // For scalar inputs, process based on data type
-            match key_array.data_type() {
-                DataType::List(_)
-                | DataType::LargeList(_)
-                | DataType::FixedSizeList(_, _) => {
-                    // The scalar wraps a List<List<T>> which represents multiple maps
-                    // (e.g., from make_array(['a','b'], NULL, ['c']) in constant evaluation)
-                    // Some list elements may be NULL (representing NULL maps), so we use list_to_arrays
-                    let row_keys = match key_array.data_type() {
-                        DataType::List(_) => list_to_arrays::<i32>(&keys),
-                        DataType::LargeList(_) => list_to_arrays::<i64>(&keys),
-                        DataType::FixedSizeList(_, _) => fixed_size_list_to_arrays(&keys),
-                        _ => unreachable!(),
-                    };
-                    row_keys
-                        .iter()
-                        .try_for_each(|key| check_unique_keys(key.as_ref()))?;
-                }
-                _ => {
-                    // For non-list scalars (e.g., a single array of keys), check directly
-                    check_unique_keys(key_array)?;
-                }
-            }
+            validate_map_keys(key_array)?;
         }
     }
 
@@ -136,15 +103,19 @@ fn make_map_batch(args: &[ColumnarValue]) -> Result<ColumnarValue> {
     make_map_batch_internal(keys, values, can_evaluate_to_const, keys_arg.data_type())
 }
 
-fn check_unique_keys(array: &dyn Array) -> Result<()> {
+/// Validates that map keys are non-null and unique.
+fn validate_map_keys(array: &dyn Array) -> Result<()> {
     let mut seen_keys = HashSet::with_capacity(array.len());
 
     for i in 0..array.len() {
         let key = ScalarValue::try_from_array(array, i)?;
-        // Map keys cannot be null
+
+        // Validation 1: Map keys cannot be null
         if key.is_null() {
             return exec_err!("map key cannot be null");
         }
+
+        // Validation 2: Map keys must be unique
         if seen_keys.contains(&key) {
             return exec_err!("map key must be unique, duplicate key found: {}", key);
         }
@@ -428,9 +399,7 @@ fn make_map_array_internal<O: OffsetSizeTrait>(
     let mut non_null_idx = 0;
 
     for i in 0..original_len {
-        let is_null = nulls_bitmap
-            .as_ref()
-            .map_or(false, |nulls| nulls.is_null(i));
+        let is_null = nulls_bitmap.as_ref().is_some_and(|nulls| nulls.is_null(i));
         if is_null {
             // NULL map: offset doesn't advance (empty range)
             offset_buffer.push(running_offset);

datafusion/sqllogictest/test_files/map.slt

@@ -233,7 +233,7 @@ SELECT map(column5, column6) FROM duplicate_keys_table;
 
 # key is a nested type
 query error DataFusion error: Execution error: map key must be unique, duplicate key found: \[1, 2\]
-SELECT MAP([[1,2], [1,2], [NULL]], [41, 33, null]);
+SELECT MAP([[1,2], [1,2]], [41, 33]);
 
 query error DataFusion error: Execution error: map key must be unique, duplicate key found: \[\{1:1\}\]
 SELECT MAP([Map {1:'1'}, Map {1:'1'}, Map {2:'2'}], [41, 33, null]);