fix(workflow-operator): MAX aggregation checks the wrong empty-group sentinel

[eugenegujing]

What changes were proposed in this PR?

One-line fix in AggregationOperation.scala: the max aggregation starts its running maximum from the type's minimum value, so its final "was this group empty?" check must compare against that same minimum value, but it compared against the type's maximum value instead (the line was copied from min, where that comparison is correct because min starts from the maximum).

Because the check looked at the wrong sentinel, two results were silently wrong while the workflow completed with no error:

• A group with only null values returned the sentinel itself instead of null, e.g. -2147483648 for INTEGER or 1970-01-01 00:00:00 for TIMESTAMP.
• A true maximum equal to the type's maximum value was mistaken for "no value seen" and discarded, so max over {1, 5, 2147483647} reported 5 (the largest value from the other local aggregation worker) instead of 2147483647.

Before-and-after

Test data: group g1 has only nulls, g2 contains {1, 5, 2147483647}, g3 contains {10, 42}.

Before the fix, max(v) grouped by k returns -2147483648 for g1 (expected null) and 5 for g2 (expected 2147483647):

min(v) on the same data is correct (null for g1, 1 for g2, 10 for g3), confirming only max's empty-group check is broken:

After the fix, max(v) returns null for g1, 2147483647 for g2, and 42 for g3:

Any related issues, documentation, discussions?

Closes #7531

The regression was introduced by #1840, which generalised the hard-coded Double sentinels to per-type minValue/maxValue helpers and updated maxAgg's initialiser but not its finaliser.

How was this PR tested?

Added 5 regression tests, all of which fail without the one-line fix and pass with it (verified in both directions):

• AggregateOpSpec: max over empty input and over all-null input returns null; max keeps a true maximum equal to the type's maximum value, covering INTEGER, LONG, DOUBLE, and TIMESTAMP.
• AggregationOperationSpec: a worker-to-final pipeline via getFinal keeps Int.MaxValue when partial results are re-aggregated, reproducing the two-worker scenario shown above; max's merge stays neutral when one side saw no values.

Full aggregate suite: sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.aggregate.*" — 68 tests, all passing.

Also verified end to end in the UI with the workflow shown above (screenshots are from before and after rebuilding the backend with this fix).

Was this PR authored or co-authored using generative AI tooling?

Co-authored by: Claude Code (Claude Fable 5)

[github-actions]

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch	Analysis
✅ release/v1.2	Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @tanishqgandhi1908, @aglinxinyuan
  You can notify them by mentioning @tanishqgandhi1908, @aglinxinyuan in a comment.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.23%. Comparing base (5e41efa) to head (c879f75).
⚠️ Report is 58 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7532      +/-   ##
============================================
+ Coverage     84.70%   85.23%   +0.53%     
- Complexity     4153     4224      +71     
============================================
  Files          1169     1173       +4     
  Lines         46740    46808      +68     
  Branches       5202     5212      +10     
============================================
+ Hits          39592    39899     +307     
+ Misses         5433     5162     -271     
- Partials       1715     1747      +32     

Flag	Coverage Δ		*Carryforward flag
access-control-service	70.00% <ø> (ø)
agent-service	85.50% <ø> (ø)		Carriedforward from 5e41efa
amber	82.10% <100.00%> (+1.23%)	⬆️
computing-unit-managing-service	60.38% <ø> (+9.66%)	⬆️
config-service	65.97% <ø> (ø)
file-service	69.05% <ø> (ø)
frontend	86.45% <ø> (ø)		Carriedforward from 5e41efa
notebook-migration-service	78.89% <ø> (ø)
pyamber	97.55% <ø> (ø)		Carriedforward from 5e41efa
workflow-compiling-service	26.31% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 3 worse · ⚪ 12 noise (<±5%) · 0 without baseline

Compared against main 133da7b benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
⚪	bs=10 sw=10 sl=64	503	0.307	18,397/29,294/29,294 us	⚪ within ±5% / 🔴 +76.3%
🔴	bs=100 sw=10 sl=64	1,040	0.635	95,296/119,124/119,124 us	🔴 +9.9% / 🟢 +10.7%
⚪	bs=1000 sw=10 sl=64	1,250	0.763	807,214/861,629/861,629 us	⚪ within ±5% / 🟢 +29.4%

Baseline details

Latest main 133da7b from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	503 tuples/sec	525 tuples/sec	740.3 tuples/sec	-4.2%	-32.1%
bs=10 sw=10 sl=64	MB/s	0.307 MB/s	0.321 MB/s	0.452 MB/s	-4.4%	-32.1%
bs=10 sw=10 sl=64	p50	18,397 us	18,108 us	13,173 us	+1.6%	+39.7%
bs=10 sw=10 sl=64	p95	29,294 us	27,921 us	16,615 us	+4.9%	+76.3%
bs=10 sw=10 sl=64	p99	29,294 us	27,921 us	19,272 us	+4.9%	+52.0%
bs=100 sw=10 sl=64	throughput	1,040 tuples/sec	1,093 tuples/sec	939.86 tuples/sec	-4.8%	+10.7%
bs=100 sw=10 sl=64	MB/s	0.635 MB/s	0.667 MB/s	0.574 MB/s	-4.8%	+10.7%
bs=100 sw=10 sl=64	p50	95,296 us	90,323 us	106,062 us	+5.5%	-10.2%
bs=100 sw=10 sl=64	p95	119,124 us	108,354 us	113,212 us	+9.9%	+5.2%
bs=100 sw=10 sl=64	p99	119,124 us	108,354 us	123,853 us	+9.9%	-3.8%
bs=1000 sw=10 sl=64	throughput	1,250 tuples/sec	1,253 tuples/sec	966.11 tuples/sec	-0.2%	+29.4%
bs=1000 sw=10 sl=64	MB/s	0.763 MB/s	0.765 MB/s	0.59 MB/s	-0.3%	+29.4%
bs=1000 sw=10 sl=64	p50	807,214 us	794,502 us	1,038,733 us	+1.6%	-22.3%
bs=1000 sw=10 sl=64	p95	861,629 us	885,223 us	1,085,289 us	-2.7%	-20.6%
bs=1000 sw=10 sl=64	p99	861,629 us	885,223 us	1,115,555 us	-2.7%	-22.8%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,397.51,200,128000,503,0.307,18396.94,29294.22,29294.22
1,100,10,64,20,1922.23,2000,1280000,1040,0.635,95296.05,119124.20,119124.20
2,1000,10,64,20,16000.08,20000,12800000,1250,0.763,807214.20,861629.15,861629.15