fix: last value accumulator merge indexing

[peterxcli]

Which issue does this PR close?

• Related to FIRST/LAST returns wrong result with PartialMerge datafusion-comet#4131.

Rationale for this change

TrivialLastValueAccumulator::merge_batch filtered out unset states but then read row 0 from the remaining value array. As a result, merging multiple LAST_VALUE partial states returned the first valid state instead of the last one.

What changes are included in this PR?

• Select the final valid state row when merging unordered LAST_VALUE states.
• Extend the existing merge test to verify the evaluated result.

Are these changes tested?

Yes:

cargo test -p datafusion-functions-aggregate test_first_last_state_after_merge

[codecov-commenter]

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.66%. Comparing base (9facaaa) to head (3a98268).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/functions-aggregate/src/first_last.rs	54.54%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #23905      +/-   ##
==========================================
- Coverage   80.67%   80.66%   -0.01%     
==========================================
  Files        1095     1095              
  Lines      372416   372426      +10     
  Branches   372416   372426      +10     
==========================================
- Hits       300434   300431       -3     
- Misses      54055    54060       +5     
- Partials    17927    17935       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[peterxcli]

cc @comphead, @andygrove, tagging you because you involved in the discussion of apache/datafusion-comet#4131, please take a look if you have time, Thanks!
This is a small change.

[comphead]

Checking this

[comphead]

Thanks @peterxcli the PR makes sense to me, and likely other accumulators shouldn't be affected.

For the test it would be nice having another one

  #[test]
  fn test_trivial_last_value_merge_all_flags_false() -> Result<()> {
      let mut acc = TrivialLastValueAccumulator::try_new(&DataType::Int64, false)?;
      let states: Vec<ArrayRef> = vec![
          Arc::new(Int64Array::from(vec![None, None])) as ArrayRef,
          Arc::new(BooleanArray::from(vec![false, false])) as ArrayRef,
      ];
      acc.merge_batch(&states)?;
      assert_eq!(acc.evaluate()?, ScalarValue::Int64(None));
      Ok(())
  }

to test is_set flags

[comphead]

Thanks @peterxcli
Since this is a core change I'd like to keep it open to let other committers have a chance to look at it