Repository navigation
feat: keep a table's declared columns when it has no rows - #8766
Conversation
A port that finished with no rows reached a Python table operator as a table with no columns, because the column names were read off the tuples. Any operator naming one of its columns then raised KeyError, Sort and the visualization operators among them. The runtime now records each input port's declared schema on the executor, and a table operator handed no rows gets those columns with no rows under them. Closes apache#8735 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8766 +/- ##
============================================
- Coverage 92.34% 92.34% -0.01%
+ Complexity 5043 5041 -2
============================================
Files 1261 1261
Lines 54105 54131 +26
Branches 6767 6771 +4
============================================
+ Hits 49961 49985 +24
- Misses 2485 2487 +2
Partials 1659 1659
*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 | 377 | 0.23 | 24,089/40,873/40,873 us | 🔴 +20.8% / 🔴 +189.5% |
| 🔴 | bs=100 sw=10 sl=64 | 822 | 0.502 | 119,988/169,293/169,293 us | 🔴 +16.4% / 🔴 +83.1% |
| 🔴 | bs=1000 sw=10 sl=64 | 955 | 0.583 | 1,043,046/1,163,804/1,163,804 us | 🔴 +8.4% / 🔴 +37.3% |
Baseline details
Latest main f674649 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 377 tuples/sec | 419 tuples/sec | 968.33 tuples/sec | -10.0% | -61.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.23 MB/s | 0.256 MB/s | 0.591 MB/s | -10.2% | -61.1% |
| bs=10 sw=10 sl=64 | p50 | 24,089 us | 20,935 us | 10,646 us | +15.1% | +126.3% |
| bs=10 sw=10 sl=64 | p95 | 40,873 us | 33,837 us | 14,120 us | +20.8% | +189.5% |
| bs=10 sw=10 sl=64 | p99 | 40,873 us | 33,837 us | 16,498 us | +20.8% | +147.7% |
| bs=100 sw=10 sl=64 | throughput | 822 tuples/sec | 847 tuples/sec | 1,252 tuples/sec | -3.0% | -34.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.502 MB/s | 0.517 MB/s | 0.764 MB/s | -2.9% | -34.3% |
| bs=100 sw=10 sl=64 | p50 | 119,988 us | 114,098 us | 83,736 us | +5.2% | +43.3% |
| bs=100 sw=10 sl=64 | p95 | 169,293 us | 145,479 us | 92,464 us | +16.4% | +83.1% |
| bs=100 sw=10 sl=64 | p99 | 169,293 us | 145,479 us | 108,561 us | +16.4% | +55.9% |
| bs=1000 sw=10 sl=64 | throughput | 955 tuples/sec | 974 tuples/sec | 1,321 tuples/sec | -2.0% | -27.7% |
| bs=1000 sw=10 sl=64 | MB/s | 0.583 MB/s | 0.595 MB/s | 0.806 MB/s | -2.0% | -27.7% |
| bs=1000 sw=10 sl=64 | p50 | 1,043,046 us | 1,022,486 us | 811,260 us | +2.0% | +28.6% |
| bs=1000 sw=10 sl=64 | p95 | 1,163,804 us | 1,074,070 us | 847,636 us | +8.4% | +37.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,163,804 us | 1,074,070 us | 879,277 us | +8.4% | +32.4% |
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,531.08,200,128000,377,0.230,24089.20,40872.87,40872.87
1,100,10,64,20,2433.97,2000,1280000,822,0.502,119988.13,169293.00,169293.00
2,1000,10,64,20,20950.55,20000,12800000,955,0.583,1043045.51,1163804.36,1163804.36A result with no rows reached the result panel as "Empty result set", because both ends read the column names off the first row. The result pagination now falls back to the stored schema when a page has no rows, and the result table builds its header from that schema when the whole result is empty. An empty page past the end of a non-empty result still leaves the table already shown in place. Part of apache#8735 Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@carloea2 May you take a look at this? |
carloea2
left a comment
There was a problem hiding this comment.
Reproduced a stuck spinner in the new empty-result column search. The 53 existing component tests and 125 Python tests pass; the added search interaction test fails.
…result with no columns A 0-row result now always shows its schema's columns, even none, so a search that matches nothing clears the old headers. Every page that arrives ends the loading state, including an empty page past the end of a non-empty result. Co-Authored-By: Claude Opus 5.5 <[email protected]>
carloea2
left a comment
There was a problem hiding this comment.
The search finding is fixed. All 54 result-table tests and the changed-file formatting and lint checks pass locally. I also ran the FiltersComponent suite after its macOS CI timeout; all 51 pass locally. The backend and Python files are unchanged from the tested head. Code looks good; the two macOS CI failures still need a green run.
|
Please add before and after screenshots of the empty result table, as required for visible frontend changes. |
|
@carloea2 Screenshots are added to the PR description, could you take another look? |
… of a port that carried no rows Since main's apache#8766 a table operator gets its port's declared columns when no rows arrived, but the Loop Start runs its Initialization and Output when it produces the state, on its own collected table and before the port's schema is declared, so D.path on such a port raises.
Resolve the conflicts with apache#8766: a page with no rows takes its columns from the stored schema, and the reply still carries the port's total count and is dropped when it belongs to another operator or port. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…pache#8506) ### What changes were proposed in this PR? Six operators implement `StandaloneCodeGenerator`: the three joins and the three set operations. What they have in common is that they combine or compare tables rather than read what is inside a row. apache#8340 keeps the operators that work on text. Aggregate and the sort family follow in apache#8709, and the samplers and control flow in apache#8710. Review then found two places where the block answered a question the engine answers differently. The hash join asked pandas for a suffix, which appends once and then refuses the duplicate it just made, and let an outer join's holes cost an integer column its type. Running the engine on those cases found the platform out of step with itself. HashJoin declared a right column's new name by checking only the left names, while the executor also avoids the other right names. So a right `x` beside a right `x#@1` was declared as a second `x#@1`, and one value was lost. An unmatched right row in an outer join kept its raw names and landed in the left column. Both now go through one renaming rule in `JoinUtils`. The shared table then found one more. In a right or full outer join on keys both sides name alike, the engine writes an unmatched right row's key into the left key column, and the export left it empty. The engine had its own half: an unmatched right key named like a left payload column landed in that payload. The export now fills the key, and the engine leaves it out of the payload. The set operations matched rows the way pandas does. The engine's HashSet compares rows with Scala's `==`, under which NaN equals nothing, and hashes them by each double's bits, which tell 0.0 from -0.0. The export now selects rows by a key that follows both rules. A probe worker that received no build rows could not name an unmatched right row's columns, because it read the left names off the first build row. This happens when the left input is empty or no left key hashes to that worker. The runtime now records each input port's declared schema on the executor before the port finishes (`OperatorExecutor.inputSchemas`, filled in `EndChannelHandler`), and the probe reads the left names from it. This is the Scala counterpart of what apache#8766 does for Python operators. ### Any related issues, documentation, discussions? Part of apache#8325, step 11a; apache#8709 and apache#8710 follow. That issue lists the set in order. The set operations and joins keyed on a null or a NaN need the nullable dtypes apache#8704 reads an Arrow file into. Without it a null and a NaN share a slot and both behave as they did before, so this PR is no worse on its own and only complete alongside that one. Closes apache#8505, the task this change is the whole of. ### How was this PR tested? Each operator asserts the block it emits in its own spec. The interval join reads its keys off the merged frame rather than writing over an input column that shares a temporary's name. The join cases the review named run in the verify tables of apache#8363, which put the same input through the engine and through the script and compare the two. What those tables cannot reach stays here as executed tests: a null kept apart from a NaN in the set operations and the join key. `HashJoinProbeOpExecSpec` pins the platform fixes. Each set operation's spec runs its native executor and the export over NaN, signed-zero, finite and null rows. `HashJoinProbeOpExecSpec` covers an empty build side, and `EndChannelHandlerSpec` checks that the schema is in place before the operator finishes the port. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
What changes were proposed in this PR?
A port or result with no rows lost its columns in two places, both because the column names were read off the first row.
KeyError: Sort, the visualization operators, and any user-writtenUDFTableOperator.Operatorgains aninput_schemasmapping thatDataProcessorfills with the finishing port's declared schema, andTableOperator.on_finishhands the operator those columns with no rows under them.Table.empty_ofbuilds that frame through Arrow, so each column keeps the dtype it would have had with rows in it.ExecutionResultServicenow reports the stored schema when a page has no rows, and the result table builds its header from that schema when the whole result is empty. An empty page past the end of a non-empty result still leaves the table already shown in place.Any related issues, documentation, discussions?
Closes #8735, which settled on this design (option 3) and asked for the frontend to show a table with no records. The engine half was first proposed in #8488.
How was this PR tested?
TestTableOperator.test_on_finish_with_no_rows_keeps_the_declared_columnsandTestMainLoop.test_second_input_port_with_no_rows_finishes_as_itself. Both fail when the operator is handed the old columnless table. The rest ofamber/src/test/pythonpasses, except the tests undercore/storage/iceberg, which need a running Postgres and Iceberg REST catalog.ExecutionResultServiceSpecgains a case for a stored result with no rows, and the past-the-end case now expects the stored schema. All 41 pass.ResultTableFrameComponentgains two cases: the header of an empty result, and an empty page of a non-empty result keeping its table. All 53 pass.lineheader with no rows.Before and after. The first pair is the result panel of a Filter that matches nothing. The second adds Sort after it, which used to fail with
KeyError: 'line'.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
🤖 Generated with Claude Code