feat(python): give a port that carried no rows its declared columns - #8488
feat(python): give a port that carried no rows its declared columns#8488kz930 wants to merge 2 commits into
Conversation
A port can finish having carried no rows: an upstream filter that matches nothing still ends its channel, and DataProcessor calls on_finish either way. TableOperator then built its table out of no tuples, and since the column names are read off the tuples, the operator was handed a frame of no columns at all. Every table operator that names one of its own columns raised KeyError on it. The runtime now records what each input port was declared to carry, and the table falls back to it where the tuples cannot say. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8488 +/- ##
=========================================
Coverage 93.69% 93.69%
Complexity 4826 4826
=========================================
Files 1209 1209
Lines 49871 49890 +19
Branches 6099 6101 +2
=========================================
+ Hits 46727 46746 +19
Misses 1652 1652
Partials 1492 1492
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 359 | 0.219 | 26,476/41,648/41,648 us | 🔴 +24.1% / 🔴 +166.1% |
| 🔴 | bs=100 sw=10 sl=64 | 786 | 0.48 | 125,375/162,143/162,143 us | 🔴 +16.7% / 🔴 +55.1% |
| 🔴 | bs=1000 sw=10 sl=64 | 904 | 0.552 | 1,099,612/1,246,237/1,246,237 us | 🔴 +6.3% / 🔴 +24.8% |
Baseline details
Latest main 1fbd346 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 359 tuples/sec | 427 tuples/sec | 794.98 tuples/sec | -15.9% | -54.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.219 MB/s | 0.26 MB/s | 0.485 MB/s | -15.8% | -54.9% |
| bs=10 sw=10 sl=64 | p50 | 26,476 us | 23,091 us | 12,443 us | +14.7% | +112.8% |
| bs=10 sw=10 sl=64 | p95 | 41,648 us | 33,560 us | 15,648 us | +24.1% | +166.1% |
| bs=10 sw=10 sl=64 | p99 | 41,648 us | 33,560 us | 19,794 us | +24.1% | +110.4% |
| bs=100 sw=10 sl=64 | throughput | 786 tuples/sec | 840 tuples/sec | 1,034 tuples/sec | -6.4% | -24.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.48 MB/s | 0.513 MB/s | 0.631 MB/s | -6.4% | -24.0% |
| bs=100 sw=10 sl=64 | p50 | 125,375 us | 115,226 us | 97,860 us | +8.8% | +28.1% |
| bs=100 sw=10 sl=64 | p95 | 162,143 us | 138,946 us | 104,570 us | +16.7% | +55.1% |
| bs=100 sw=10 sl=64 | p99 | 162,143 us | 138,946 us | 117,811 us | +16.7% | +37.6% |
| bs=1000 sw=10 sl=64 | throughput | 904 tuples/sec | 912 tuples/sec | 1,066 tuples/sec | -0.9% | -15.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.552 MB/s | 0.557 MB/s | 0.651 MB/s | -0.9% | -15.2% |
| bs=1000 sw=10 sl=64 | p50 | 1,099,612 us | 1,092,413 us | 958,266 us | +0.7% | +14.8% |
| bs=1000 sw=10 sl=64 | p95 | 1,246,237 us | 1,172,813 us | 998,734 us | +6.3% | +24.8% |
| bs=1000 sw=10 sl=64 | p99 | 1,246,237 us | 1,172,813 us | 1,032,953 us | +6.3% | +20.6% |
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,557.27,200,128000,359,0.219,26475.61,41647.51,41647.51
1,100,10,64,20,2545.40,2000,1280000,786,0.480,125374.82,162142.79,162142.79
2,1000,10,64,20,22126.73,20000,12800000,904,0.552,1099611.57,1246236.74,1246236.74|
@carloea2 this one is a platform bug rather than an export change: a port that finishes having carried no rows hands a table operator a frame with no columns at all, so every operator naming one of its own columns raises KeyError. Would you take a look when you have a moment? |
The frame Arrow builds is a plain DataFrame, so an operator reading its input with as_tuples() raised AttributeError on the branch that gives a port with no rows its declared columns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changes were proposed in this PR?
The runtime now tells an executor what each input port was declared to carry, and
TableOperatorfalls back to that when a port finishes with no rows.Operatorgains aninput_schemasmapping, keyed by port index.DataProcessorwrites the finishing port's schema into it just before callingon_finish, reading it from the input manager's own port; a source has no input port to ask, so it is left alone.TableOperator.on_finishuses it only when there are no tuples to read column names off, which is the only case where the tuples do not already say the same thing.Table.empty_ofbuilds that frame through Arrow, so each column carries the dtype it would have had with rows in it rather than object.Any related issues, documentation, discussions?
Found while building #8325, but not part of it: the bug is in the engine, not in
the export. The export's verification is what surfaced it, by running each
operator on a table with no rows, but it reaches Sort, the visualization
operators and any user-written
UDFTableOperatorin an ordinary run.Closes #8487, the bug this change is the whole of.
How was this PR tested?
A new case in
TestTableOperator,test_on_finish_with_no_rows_keeps_the_declared_columns, gives a port an INTEGER and a STRING column, finishes it with no rows, and asserts the operator receives both columns, an empty frame, andint32for the integer one. Removing the fallback turns it red on the missing columns.The rest of
amber/src/test/python/corepasses unchanged, 1107 tests. The one failure istest_iceberg_rest_catalog_integration, which needs a running catalog and fails the same way without this change.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
🤖 Generated with Claude Code