(datafusion) branch main updated: fix: Substrait serialization mishandles sliced map literals (#25543)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new dd1d709929 fix: Substrait serialization mishandles sliced map literals (#25543)
dd1d709929 is described below

commit dd1d709929c349fc1dfd47dc1069a8d7a5e6f0ea
Author: Neil Conway <ne...@gmail.com>
AuthorDate: Wed Sep 30 10:14:27 2026 +0000

    fix: Substrait serialization mishandles sliced map literals (#25543)
    
    ## Which issue does this PR close?
    
    - N/A
    
    ## Rationale for this change
    
    `ScalarValue::Map` has an unfortunate property: it exposes the `keys()`
    and `values()` methods from the underlying Arrow `MapArray`, but those
    methods return all of the keys/values in the backing array, _not_ just
    keys/values in the visible span. As a result, calling `keys()` on a
    `ScalarValue::Map` can return values outside the logical content of the
    scalar map.
    
    Fix an instance of this bug in the substrait serialization code.
    
    ## What changes are included in this PR?
    
    * Fix substrait map literal serialization bug
    * Add unit test
    
    ## What is the testing strategy for this PR?
    
    Existing tests pass, new test added.
    
    ## Are there any user-facing changes?
    
    No, aside from fixing the bug.
---
 .../src/logical_plan/producer/expr/literal.rs      | 35 +++++++++++++++++++---
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/datafusion/substrait/src/logical_plan/producer/expr/literal.rs b/datafusion/substrait/src/logical_plan/producer/expr/literal.rs
index f26984ad4a..f81333e630 100644
--- a/datafusion/substrait/src/logical_plan/producer/expr/literal.rs
+++ b/datafusion/substrait/src/logical_plan/producer/expr/literal.rs
@@ -286,19 +286,20 @@ pub(crate) fn to_substrait_literal(
                 }?;
                 LiteralType::EmptyMap(mt)
             } else {
-                let keys = (0..m.keys().len())
+                let entries = m.value(0);
+                let keys = (0..entries.len())
                     .map(|i| {
                         to_substrait_literal(
                             producer,
-                            &ScalarValue::try_from_array(&m.keys(), i)?,
+                            &ScalarValue::try_from_array(entries.column(0), i)?,
                         )
                     })
                     .collect::<datafusion::common::Result<Vec<_>>>()?;
-                let values = (0..m.values().len())
+                let values = (0..entries.len())
                     .map(|i| {
                         to_substrait_literal(
                             producer,
-                            &ScalarValue::try_from_array(&m.values(), i)?,
+                            &ScalarValue::try_from_array(entries.column(1), i)?,
                         )
                     })
                     .collect::<datafusion::common::Result<Vec<_>>>()?;
@@ -548,6 +549,32 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn round_trip_sliced_map_literals() -> Result<()> {
+        let mut builder =
+            MapBuilder::new(None, StringBuilder::new(), Int64Builder::new());
+        builder.keys().append_value("prefix");
+        builder.values().append_value(10);
+        builder.append(true)?;
+        builder.keys().append_value("selected");
+        builder.values().append_value(20);
+        builder.keys().append_value("null_value");
+        builder.values().append_null();
+        builder.append(true)?;
+        builder.append(true)?;
+        builder.append(false)?;
+        builder.keys().append_value("suffix");
+        builder.values().append_value(30);
+        builder.append(true)?;
+        let maps = builder.finish();
+
+        // Scalar extraction retains the other rows' entries in the backing arrays.
+        for row in 0..maps.len() {
+            round_trip_literal(ScalarValue::try_from_array(&maps, row)?)?;
+        }
+        Ok(())
+    }
+
     fn round_trip_literal(scalar: ScalarValue) -> Result<()> {
         println!("Checking round trip of {scalar:?}");
         let state = SessionContext::default().state();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org