Andrew Lamb

> This is top of my list to review tomorrow morning I am sorry -- I am just finding other PRs like https://github.com/apache/datafusion/pull/12978 and https://github.com/apache/datafusion/pull/13133 very subtle and take a...

I am giving this a final review now

Performance results: ``` -------------------- Benchmark clickbench_partitioned.json -------------------- ┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓ ┃ Query ┃ main_base ┃ vectorize-append-value ┃ Change ┃ ┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩ │ QQuery 0 │ 2.30ms │ 2.33ms │ no change │ │...

As my admittedly sparse help for this PR I have filed some additional tickets for follow on work after this PR is merged: - https://github.com/apache/datafusion/issues/13262 - https://github.com/apache/datafusion/issues/13263

I don't think we need to wait on this PR anymore, let's merge it in and keep moving forward. Thank you everyone again!

Update here is that this is looking like it results in some sweet clickbench improvements: - https://github.com/apache/datafusion/issues/13983#issuecomment-2632053433

> @alamb Could you help reivew this PR? Thanks @xiedeyantu ! We normally need to add tests as part of any code PR -- could you look into adding some...

> > > @alamb Could you help reivew this PR? > > > > > > Thanks @xiedeyantu ! We normally need to add tests as part of any code...

@blaginin also did an end to end test with S3 in the CI tests. The instructions are here: - https://github.com/apache/datafusion/blob/main/datafusion-cli/CONTRIBUTING.md#L30-L29

I think ScalarValue::Map should follow the same pattern as `ScalarValue::List` `ScalarValue::Struct` https://github.com/apache/datafusion/blob/8216e32e87b2238d8814fe16215c8770d6c327c8/datafusion/common/src/scalar/mod.rs#L244 So that would mean a single element `MapArray`: https://docs.rs/arrow/latest/arrow/array/struct.MapArray.html ```rust enum ScalarValue { ... Map(Arc) ... } ```...