[carloea2]

What changes were proposed in this PR?

Read Arrow fields by their global row index. Reusing the first column's chunk coordinates for other columns can raise IndexError or return the wrong value when their chunk boundaries differ.

Refreshed onto upstream main at 1023593. The diff contains only the provider and its regressions.

Any related issues, documentation, discussions?

Closes #8209

Replaces #8210, which was closed by the stale bot.

How was this PR tested?

Fresh detached worktree, Python 3.12 on Windows. Both chunk layouts fail before the source change: a shorter second-column chunk raises IndexError; a longer one returns 30 instead of 20 in row two. Both pass afterward.

After bash bin/python-proto-gen.sh, from amber:

python -m pytest src/test/python/core/models --deselect src/test/python/core/models/test_tuple.py::TestTuple::test_hash -q --tb=short -p no:cacheprovider
python -m ruff check src/main/python src/test/python
python -m ruff format --check src/main/python src/test/python
git diff --check

328 passed and one expected failure. Two parameterized cases of the existing Windows epoch test were deselected; that separate change is #8957. Both Arrow regressions ran. Ruff lint and formatting passed for all 217 Python files.

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

Generated-by: Codex

[carloea2]

This replaces stale-closed #8210. The problem still reproduces on current main. The patch passes its regression, 350 broader tests and Ruff checks.

/request-review @eugenegujing

[carloea2]

/request-review @eugenegujing

[github-actions]

Automated Reviewer Suggestions

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

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.17%. Comparing base (1023593) to head (2894330).

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8956   +/-   ##
=========================================
  Coverage     92.17%   92.17%           
  Complexity     4986     4986           
=========================================
  Files          1284     1284           
  Lines         55605    55598    -7     
  Branches       7120     7118    -2     
=========================================
- Hits          51254    51248    -6     
  Misses         2592     2592           
+ Partials       1759     1758    -1     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.44% <ø> (ø)		Carriedforward from 1023593
agent-service	99.54% <ø> (ø)		Carriedforward from 1023593
amber	86.83% <ø> (ø)		Carriedforward from 1023593
computing-unit-managing-service	64.02% <ø> (ø)		Carriedforward from 1023593
config-service	87.61% <ø> (ø)		Carriedforward from 1023593
file-service	82.29% <ø> (ø)		Carriedforward from 1023593
frontend	96.48% <ø> (ø)		Carriedforward from 1023593
notebook-migration-service	83.77% <ø> (ø)		Carriedforward from 1023593
pyamber	98.56% <100.00%> (+0.01%)	⬆️
workflow-compiling-service	77.95% <ø> (ø)		Carriedforward from 1023593

*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

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

Compared against main 1023593 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	429	0.262	20,858/37,034/37,034 us	🔴 -15.8% / 🔴 +161.9%
🟢	bs=100 sw=10 sl=64	972	0.593	100,124/132,044/132,044 us	🟢 -13.7% / 🔴 +38.0%
⚪	bs=1000 sw=10 sl=64	1,148	0.701	858,737/971,778/971,778 us	⚪ within ±5% / ⚪ within ±5%

Baseline details

Latest main 1023593 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	429 tuples/sec	509 tuples/sec	881.37 tuples/sec	-15.7%	-51.3%
bs=10 sw=10 sl=64	MB/s	0.262 MB/s	0.311 MB/s	0.538 MB/s	-15.8%	-51.3%
bs=10 sw=10 sl=64	p50	20,858 us	18,371 us	11,399 us	+13.5%	+83.0%
bs=10 sw=10 sl=64	p95	37,034 us	33,053 us	14,143 us	+12.0%	+161.9%
bs=10 sw=10 sl=64	p99	37,034 us	33,053 us	17,513 us	+12.0%	+111.5%
bs=100 sw=10 sl=64	throughput	972 tuples/sec	988 tuples/sec	1,148 tuples/sec	-1.6%	-15.3%
bs=100 sw=10 sl=64	MB/s	0.593 MB/s	0.603 MB/s	0.701 MB/s	-1.7%	-15.4%
bs=100 sw=10 sl=64	p50	100,124 us	95,620 us	89,815 us	+4.7%	+11.5%
bs=100 sw=10 sl=64	p95	132,044 us	153,054 us	95,717 us	-13.7%	+38.0%
bs=100 sw=10 sl=64	p99	132,044 us	153,054 us	107,566 us	-13.7%	+22.8%
bs=1000 sw=10 sl=64	throughput	1,148 tuples/sec	1,121 tuples/sec	1,175 tuples/sec	+2.4%	-2.3%
bs=1000 sw=10 sl=64	MB/s	0.701 MB/s	0.684 MB/s	0.717 MB/s	+2.5%	-2.2%
bs=1000 sw=10 sl=64	p50	858,737 us	887,651 us	885,446 us	-3.3%	-3.0%
bs=1000 sw=10 sl=64	p95	971,778 us	950,060 us	933,809 us	+2.3%	+4.1%
bs=1000 sw=10 sl=64	p99	971,778 us	950,060 us	979,021 us	+2.3%	-0.7%

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,466.00,200,128000,429,0.262,20858.48,37033.73,37033.73
1,100,10,64,20,2058.28,2000,1280000,972,0.593,100124.08,132044.18,132044.18
2,1000,10,64,20,17418.66,20000,12800000,1148,0.701,858737.16,971777.76,971777.76

[carloea2]

/request-review @aglinxinyuan

Could you review this Arrow chunk-boundary change? The regression and focused tests pass locally, and CI is green.

[carloea2]

/request-review @sshiv012 @parshimers

[eugenegujing]

LGTM. The test only has the layout where b's first chunk is shorter than a's, which crashes with IndexError in the old code. The other case from the issue, where b's first chunk is longer, doesn't crash but reads the wrong row, e.g. a = [[1], [2, 3]], b = [[10, 20], [30]] gives b=30 for row 2. Maybe parametrize over that one as well?

[aglinxinyuan]

@Yicong-Huang please review the PR. I don't have enough context.

[carloea2]

@eugenegujing Added the opposite chunk layout and assertions per row, so the test catches the silent b=30 instead of b=20 error as well as IndexError. The focused suites give 141 passes; the existing Windows epoch-hashing test remains separate in #8957. Ruff passes, and CI has restarted.

[carloea2]

/request-review @aglinxinyuan

Both Arrow chunk layouts are covered and 328 model tests pass locally. Could you review the refreshed head once CI is green?