feat(workflow-operator): answer the filter on an empty cell instead of throwing - #7566
Open
kz930 wants to merge 1 commit into
Open
feat(workflow-operator): answer the filter on an empty cell instead of throwing#7566kz930 wants to merge 1 commit into
kz930 wants to merge 1 commit into
Conversation
Contributor
Backport auto-label reportThis
|
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7566 +/- ##
============================================
- Coverage 87.88% 87.88% -0.01%
+ Complexity 4276 4275 -1
============================================
Files 1176 1176
Lines 47018 47024 +6
Branches 5245 5247 +2
============================================
+ Hits 41323 41325 +2
- Misses 3972 3973 +1
- Partials 1723 1726 +3
*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:
|
…f throwing Substring Search and Unnest String both read their column and called toString on it with no null check, so a single blank cell took the workflow down with a NullPointerException. An empty value is ordinary input here. A blank CSV cell arrives as null: univocity returns null for an empty field, and AttributeTypeUtils.parseField passes it through by design, its first line being `if (field == null) return null`. So the null these two did not expect is the null the core layer promises to deliver. Both now skip, which is what the rest of the codebase does with a value that is not there. FilterPredicate answers false for every condition but IS_NULL / IS_NOT_NULL once a field is null. COUNT(column) counts only non-null rows, and CONCAT and MIN pass over them. Twenty-four operators open their generated Python with `dropna(subset=[...]) #remove missing values`. Substring Search filters the row out; Unnest String produces no rows, the same way its existing filter drops the empty pieces a run of delimiters produces. Each spec gains the case; both fail on the previous behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kz930
force-pushed
the
fix/7548-null-cell-filters
branch
from
August 11, 2026 20:51
c012a51 to
a5130fe
Compare
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 370 | 0.226 | 25,551/35,782/35,782 us | 🔴 +27.7% / 🔴 +123.8% |
| 🔴 | bs=100 sw=10 sl=64 | 777 | 0.474 | 126,781/179,644/179,644 us | 🔴 +11.9% / 🔴 +66.2% |
| ⚪ | bs=1000 sw=10 sl=64 | 912 | 0.557 | 1,095,902/1,130,755/1,130,755 us | ⚪ within ±5% / 🔴 -11.5% |
Baseline details
Latest main e878df3 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 370 tuples/sec | 445 tuples/sec | 784.3 tuples/sec | -16.9% | -52.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.226 MB/s | 0.271 MB/s | 0.479 MB/s | -16.6% | -52.8% |
| bs=10 sw=10 sl=64 | p50 | 25,551 us | 21,682 us | 12,590 us | +17.8% | +102.9% |
| bs=10 sw=10 sl=64 | p95 | 35,782 us | 28,020 us | 15,991 us | +27.7% | +123.8% |
| bs=10 sw=10 sl=64 | p99 | 35,782 us | 28,020 us | 18,694 us | +27.7% | +91.4% |
| bs=100 sw=10 sl=64 | throughput | 777 tuples/sec | 809 tuples/sec | 1,002 tuples/sec | -4.0% | -22.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.474 MB/s | 0.494 MB/s | 0.612 MB/s | -4.0% | -22.5% |
| bs=100 sw=10 sl=64 | p50 | 126,781 us | 121,668 us | 101,285 us | +4.2% | +25.2% |
| bs=100 sw=10 sl=64 | p95 | 179,644 us | 160,575 us | 108,068 us | +11.9% | +66.2% |
| bs=100 sw=10 sl=64 | p99 | 179,644 us | 160,575 us | 118,235 us | +11.9% | +51.9% |
| bs=1000 sw=10 sl=64 | throughput | 912 tuples/sec | 913 tuples/sec | 1,030 tuples/sec | -0.1% | -11.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.557 MB/s | 0.558 MB/s | 0.629 MB/s | -0.2% | -11.4% |
| bs=1000 sw=10 sl=64 | p50 | 1,095,902 us | 1,093,504 us | 991,882 us | +0.2% | +10.5% |
| bs=1000 sw=10 sl=64 | p95 | 1,130,755 us | 1,134,818 us | 1,038,496 us | -0.4% | +8.9% |
| bs=1000 sw=10 sl=64 | p99 | 1,130,755 us | 1,134,818 us | 1,068,265 us | -0.4% | +5.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,540.68,200,128000,370,0.226,25551.01,35782.02,35782.02
1,100,10,64,20,2572.68,2000,1280000,777,0.474,126781.43,179643.54,179643.54
2,1000,10,64,20,21929.12,20000,12800000,912,0.557,1095901.76,1130754.68,1130754.68
kz930
changed the title
xuang7
removed their request for review
August 11, 2026 21:05
Contributor
Author
|
@Yicong-Huang, @aglinxinyuan May you take a look at this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Substring Search and Unnest String both read their column and called
toStringon it with no null check, so one blank cell took the workflow down with a NullPointerException.An empty value is ordinary input here. A blank CSV cell arrives as null, since univocity returns null for an empty field and
AttributeTypeUtils.parseFieldpasses it through by design, its first line beingif (field == null) return null.Both now skip, which is what the rest of the codebase does with a value that is not there.
FilterPredicateanswers false for every condition but IS_NULL / IS_NOT_NULL once a field is null;COUNT(column)counts only non-null rows, and CONCAT and MIN pass over them; twenty-four operators open their generated Python withdropna(subset=[...]) #remove missing values. Substring Search filters the row out, and Unnest String produces no rows, the same way its existing filter drops the empty pieces a run of delimiters produces.Any related issues, documentation, discussions?
Closes #7548
How was this PR tested?
Each spec gains the case:
SubstringSearchOpExecSpeccovers an empty cell with an ordinary substring and with the empty substring, andUnnestStringOpExecSpeccovers an empty cell. Both fail on the previous behavior, 15 passed / 3 failed before the change and 18 / 0 after.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)