feat(csv-scan): keep a numeric column's type when one of its cells is blank - #7568
Conversation
Backport auto-label reportThis
|
… blank
Schema inference and execution disagreed about what a blank cell is, and the
schema side was the one that lost information. Inference set nullValue("") on its
parser, so a blank read as an empty string: tryParseDouble("") fails,
tryParseBoolean("") fails, and inferField lands on tryParseString(). One empty
cell was enough to type a whole numeric column as STRING. Execution builds its
parser without nullValue, so the same blank read as null there, which is what
AttributeTypeUtils.parseField is written to pass through.
The effect reaches well past the scan. Every downstream operator that does
arithmetic on such a column then receives strings and fails on rows whose values
are perfectly good numbers, not on the blank one. Hugging Face Iris Logistic
Regression on a three-row file fails at the first row, where numpy is handed
array([['2.6', '0.75']], dtype='<U32').
Dropping the setting leaves both sides reading a blank as null, and
tryParseDouble(null) already answers DOUBLE, so the column keeps the type its
values give it. One corner changes with it: a column that is blank in every
sampled row now infers as INTEGER rather than STRING. Its values are null either
way, so this renames the empty rather than reinterpreting anything.
CSVScanSourceOpDescSpec gains the case; it fails on the previous behavior. The
module's 2187 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2b08597 to
9f49d90
Compare
Automated Reviewer SuggestionsBased on the
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🟢 | bs=10 sw=10 sl=64 | 531 | 0.324 | 18,012/27,103/27,103 us | 🟢 -6.8% / 🔴 +65.8% |
| 🔴 | bs=100 sw=10 sl=64 | 1,098 | 0.67 | 89,673/102,715/102,715 us | 🟢 -20.8% / 🟢 -11.8% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,270 | 0.775 | 779,973/875,982/875,982 us | 🔴 +5.6% / 🟢 +23.3% |
Baseline details
Latest main 324278e from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 531 tuples/sec | 557 tuples/sec | 774.89 tuples/sec | -4.7% | -31.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.324 MB/s | 0.34 MB/s | 0.473 MB/s | -4.7% | -31.5% |
| bs=10 sw=10 sl=64 | p50 | 18,012 us | 18,017 us | 12,738 us | -0.0% | +41.4% |
| bs=10 sw=10 sl=64 | p95 | 27,103 us | 29,069 us | 16,348 us | -6.8% | +65.8% |
| bs=10 sw=10 sl=64 | p99 | 27,103 us | 29,069 us | 18,848 us | -6.8% | +43.8% |
| bs=100 sw=10 sl=64 | throughput | 1,098 tuples/sec | 1,114 tuples/sec | 1,005 tuples/sec | -1.4% | +9.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.67 MB/s | 0.68 MB/s | 0.613 MB/s | -1.5% | +9.3% |
| bs=100 sw=10 sl=64 | p50 | 89,673 us | 85,057 us | 100,970 us | +5.4% | -11.2% |
| bs=100 sw=10 sl=64 | p95 | 102,715 us | 129,610 us | 107,605 us | -20.8% | -4.5% |
| bs=100 sw=10 sl=64 | p99 | 102,715 us | 129,610 us | 116,429 us | -20.8% | -11.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,270 tuples/sec | 1,300 tuples/sec | 1,030 tuples/sec | -2.3% | +23.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.775 MB/s | 0.794 MB/s | 0.629 MB/s | -2.4% | +23.2% |
| bs=1000 sw=10 sl=64 | p50 | 779,973 us | 768,157 us | 991,433 us | +1.5% | -21.3% |
| bs=1000 sw=10 sl=64 | p95 | 875,982 us | 829,181 us | 1,036,668 us | +5.6% | -15.5% |
| bs=1000 sw=10 sl=64 | p99 | 875,982 us | 829,181 us | 1,070,470 us | +5.6% | -18.2% |
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,376.40,200,128000,531,0.324,18012.06,27102.55,27102.55
1,100,10,64,20,1821.38,2000,1280000,1098,0.670,89672.63,102714.92,102714.92
2,1000,10,64,20,15742.36,20000,12800000,1270,0.775,779972.68,875982.32,875982.32
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7568 +/- ##
============================================
- Coverage 90.14% 90.13% -0.02%
+ Complexity 4417 4413 -4
============================================
Files 1174 1174
Lines 46964 46963 -1
Branches 5262 5262
============================================
- Hits 42334 42328 -6
- Misses 2876 2877 +1
- Partials 1754 1758 +4
*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:
|
|
@aglinxinyuan May you take a look at this? |
What changes were proposed in this PR?
Schema inference and execution disagreed about what a blank cell is, and the schema side was the one that lost information. Inference set
nullValue("")on its parser, so a blank read as an empty string:tryParseDouble("")fails,tryParseBoolean("")fails, andinferFieldlands ontryParseString(). One empty cell was enough to type a whole numeric column as STRING. Execution builds its parser withoutnullValue, so the same blank read as null there, which is whatAttributeTypeUtils.parseFieldis written to pass through.The effect reaches well past the scan. Every downstream operator that does arithmetic on such a column then receives strings and fails on rows whose values are perfectly good numbers, not on the blank one. Hugging Face Iris Logistic Regression on a three-row file fails at the first row, where numpy is handed
array([['2.6', '0.75']], dtype='<U32').Dropping the setting leaves both sides reading a blank as null, and
tryParseDouble(null)already answers DOUBLE, so the column keeps the type its values give it.One corner changes with it: a column that is blank in every sampled row now infers as INTEGER rather than STRING. Its values are null either way, so this renames the empty rather than reinterpreting anything.
Any related issues, documentation, discussions?
Closes #7550
How was this PR tested?
CSVScanSourceOpDescSpecgains a case that writes a two-column file whose numeric column is blank on one row and asserts the inferred type is DOUBLE. It fails on the previous behavior, 16 passed / 1 failed before the change and 17 / 0 after. Since this touches inference every scan goes through, the whole module was run as well: 2187 passed, 0 failed.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)