Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Automated Reviewer SuggestionsBased on the
|
📊 Arrow Flight E2E bench(no arrow-flight-e2e.csv in artifact) |
carloea2
left a comment
There was a problem hiding this comment.
One text-content mismatch remains in the single-string read. Checked Python's text reader locally against the native byte-decoding path.
… export it Read as a single string, the export opened the file in text mode, which turns a CRLF into a LF. The engine decodes the file's bytes as they are, so the two disagreed on any file with Windows line endings. Open it with newline="" for both File Scan operators; reading by line keeps the default, which already ends a line where the engine does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8705 +/- ##
============================================
- Coverage 92.73% 92.70% -0.03%
- Complexity 4926 4945 +19
============================================
Files 1241 1243 +2
Lines 52476 52657 +181
Branches 6475 6504 +29
============================================
+ Hits 48662 48818 +156
- Misses 2206 2215 +9
- Partials 1608 1624 +16
*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:
|
The engine splits with linesIterator, which ends a line only at LF, CR and CRLF. The export used splitlines, which also ends one at eight other characters such as U+2028 and NEL, so a row holding one came out as two. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
carloea2
left a comment
There was a problem hiding this comment.
Retested the current head. The Unicode line separator regression is fixed. All 62 focused tests and scalafmtCheckAll pass with the #8341 source API and the existing Windows test cleanup from #8796 supplied locally. Without #8796, only that UTF-16 cleanup test fails on Windows.
Command: sbt -batch "WorkflowOperator/testOnly *FileScanOpDescSpec *FileScanSourceOpDescSpec *TextInputSourceOpDescSpec *URLFetcherOpDescSpec -- -oDF" "scalafmtCheckAll".
No new finding in the line source changes. The current checks only cover metadata, so this is not a green build confirmation. #8341 and #8597 remain prerequisites.
int() and float() read "1_0", "nan", "inf" and an INTEGER past 32 bits, which end the engine's run, and float() refused "2d" and "0x1p3", which the engine reads. Each line is now trimmed as Java trims and checked against the grammar parseInt, parseLong or parseDouble accepts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What changes were proposed in this PR?
File Scan, the file-scan source, Text Input and URL Fetcher implement
StandaloneCodeGenerator. Each slices the raw lines before parsing, as the engine does, so a line outside the window is never converted. A boolean line is read asparseFieldreads one, where 1 is true and other text ends the run. A timestamp line is kept as text and parsed as a column with the helper #8597 adds, as the CSV readers parse theirs.Two sources are reported as unverifiable rather than exported blind. File Scan takes its filenames from an input port at run time, which a source harness has nothing to feed. URL Fetcher reads a live URL, so two runs are not required to agree and a comparison would only measure the network.
Any related issues, documentation, discussions?
Part of #8325, step 12c; that issue lists the set in order. It needs #8341 for the shared source code and #8597 for the timestamp helpers, so both merge first.
Closes #8703, the task this change is the whole of.
One engine defect was found while matching these readers, and it is fixed here rather than worked around: the export alone could not close it. #8598, where a file scan extracting an archive could not also include the filename, because the line-by-line branch emitted a one-field row against the two-column schema its own flag declares. A second, #8596, where the Encoding field never reached the executor, was fixed upstream by #8605, and this branch takes that fix.
Closes #8598.
How was this PR tested?
Each operator asserts the block it emits in its own spec. Where a difference is one of behaviour rather than of text, the spec runs the engine's own executor and the generated script over the same input and compares what each produced: the boolean parsing rules and the entries read out of an archive. Once the verification lands, every operator is also run both ways on every configuration its schema offers; this branch is cut from main and does not carry that machinery, so those runs are not on this diff's CI.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)