Skip to content

fix: Correct FilledAreaPlot x-value validation across groups#6894

Open
VuMartin wants to merge 2 commits into
apache:mainfrom
VuMartin:fix/6728-filled-area-x-values
Open

fix: Correct FilledAreaPlot x-value validation across groups#6894
VuMartin wants to merge 2 commits into
apache:mainfrom
VuMartin:fix/6728-filled-area-x-values

Conversation

@VuMartin

@VuMartin VuMartin commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Fixed the FilledAreaPlot validation logic for grouped data with shared X-values.

- X_values = x_values.union(set(group[$x].unique()))
+ x_values = x_values.union(set(group[$x].unique()))

Previously, the validation incorrectly handled X-value comparisons between groups, causing valid charts to be rejected. The logic was updated so that X-values are tracked across groups correctly when checking whether groups share compatible X attributes.

Before:
Screenshot 2026-07-23 at 2 42 16 AM

After:
Screenshot 2026-07-25 at 2 17 10 AM

Any related issues, documentation, discussions?

Fixes #6728

How was this PR tested?

Tested by running Texera locally and executing a FilledAreaPlot workflow with grouped data.
CSV File:
filled_area_test.csv

Verified:

  • Valid grouped charts are accepted and rendered.
  • Invalid inputs missing X/Y attributes still return the appropriate error message.
  • Python UDF execution works after generating Python proto bindings and installing required Python dependencies.

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

Generated-by: ChatGPT (GPT-5.5-mini)

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

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

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 7e4a9b4 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 393 0.24 25,644/30,735/30,735 us 🔴 +19.7% / 🔴 +100.1%
🔴 bs=100 sw=10 sl=64 789 0.482 125,189/159,743/159,743 us 🔴 +6.6% / 🔴 +46.4%
bs=1000 sw=10 sl=64 902 0.551 1,112,988/1,152,143/1,152,143 us ⚪ within ±5% / 🔴 +11.0%
Baseline details

Latest main 7e4a9b4 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 393 tuples/sec 437 tuples/sec 754.55 tuples/sec -10.1% -47.9%
bs=10 sw=10 sl=64 MB/s 0.24 MB/s 0.267 MB/s 0.461 MB/s -10.1% -47.9%
bs=10 sw=10 sl=64 p50 25,644 us 21,428 us 12,816 us +19.7% +100.1%
bs=10 sw=10 sl=64 p95 30,735 us 34,536 us 16,594 us -11.0% +85.2%
bs=10 sw=10 sl=64 p99 30,735 us 34,536 us 19,806 us -11.0% +55.2%
bs=100 sw=10 sl=64 throughput 789 tuples/sec 806 tuples/sec 969.38 tuples/sec -2.1% -18.6%
bs=100 sw=10 sl=64 MB/s 0.482 MB/s 0.492 MB/s 0.592 MB/s -2.0% -18.5%
bs=100 sw=10 sl=64 p50 125,189 us 121,121 us 103,584 us +3.4% +20.9%
bs=100 sw=10 sl=64 p95 159,743 us 149,888 us 109,097 us +6.6% +46.4%
bs=100 sw=10 sl=64 p99 159,743 us 149,888 us 117,304 us +6.6% +36.2%
bs=1000 sw=10 sl=64 throughput 902 tuples/sec 908 tuples/sec 1,004 tuples/sec -0.7% -10.1%
bs=1000 sw=10 sl=64 MB/s 0.551 MB/s 0.554 MB/s 0.613 MB/s -0.5% -10.1%
bs=1000 sw=10 sl=64 p50 1,112,988 us 1,107,485 us 1,002,357 us +0.5% +11.0%
bs=1000 sw=10 sl=64 p95 1,152,143 us 1,146,636 us 1,046,463 us +0.5% +10.1%
bs=1000 sw=10 sl=64 p99 1,152,143 us 1,146,636 us 1,073,661 us +0.5% +7.3%
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,509.34,200,128000,393,0.240,25643.96,30734.66,30734.66
1,100,10,64,20,2533.79,2000,1280000,789,0.482,125188.64,159742.68,159742.68
2,1000,10,64,20,22160.85,20000,12800000,902,0.551,1112988.13,1152142.76,1152142.76

@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.96%. Comparing base (7e4a9b4) to head (f9f7816).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6894      +/-   ##
============================================
- Coverage     77.97%   76.96%   -1.02%     
- Complexity     3661     3706      +45     
============================================
  Files          1161     1174      +13     
  Lines         46059    47004     +945     
  Branches       5105     5214     +109     
============================================
+ Hits          35915    36176     +261     
- Misses         8521     9187     +666     
- Partials       1623     1641      +18     
Flag Coverage Δ *Carryforward flag
access-control-service 70.66% <ø> (+0.66%) ⬆️
agent-service 76.76% <ø> (ø) Carriedforward from 7e4a9b4
amber 69.15% <100.00%> (-1.53%) ⬇️
computing-unit-managing-service 16.12% <ø> (-4.37%) ⬇️
config-service 63.57% <ø> (-3.10%) ⬇️
file-service 62.37% <ø> (-4.84%) ⬇️
frontend 82.64% <ø> (ø) Carriedforward from 7e4a9b4
notebook-migration-service 78.94% <ø> (ø)
pyamber 92.40% <ø> (ø) Carriedforward from 7e4a9b4
workflow-compiling-service 55.00% <ø> (-0.15%) ⬇️

*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.

@VuMartin VuMartin changed the title Fix FilledAreaPlot x-value validation across groups fix: Correct FilledAreaPlot x-value validation across groups Jul 25, 2026
@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Jul 25, 2026
@Yicong-Huang
Yicong-Huang requested a review from xuang7 July 25, 2026 11:52
@Yicong-Huang

Copy link
Copy Markdown
Contributor

LGTM. cc @xuang7 for backport review

@Yicong-Huang Yicong-Huang removed the release/v1.2 back porting to release/v1.2 label Jul 25, 2026
@Yicong-Huang

Copy link
Copy Markdown
Contributor

let's do the backport separately

@Yicong-Huang
Yicong-Huang removed the request for review from xuang7 July 25, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FilledAreaPlot rejects valid charts due to a likely typo (X_values) in performTableCheck()

3 participants