Skip to content

fix(workflow-operator, v1.2): File Scan operator using offset with an empty limit emits no rows - #7530

Open
eugenegujing wants to merge 1 commit into
apache:release/v1.2from
eugenegujing:backport/file-scan-offset-overflow-v1.2
Open

fix(workflow-operator, v1.2): File Scan operator using offset with an empty limit emits no rows#7530
eugenegujing wants to merge 1 commit into
apache:release/v1.2from
eugenegujing:backport/file-scan-offset-overflow-v1.2

Conversation

@eugenegujing

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Backport of #7348 to release/v1.2, cherry-picked from main commit 8982803.

FileScanUtils.createTuplesFromFile computed the end of its line slice as offset + limit.getOrElse(Int.MaxValue). With Offset ≥ 1 and Limit left empty, the addition overflows Int to a negative bound, and Iterator.slice clamps a negative bound to 0 and returns an empty iterator. The File Scan operator therefore emitted zero rows, silently, with the workflow reporting success. Both FileScan and FileScanOp delegate to this helper, so both were affected.

The fix replaces the slice arithmetic with drop(offset) plus an optional take(limit) — the shape CSVScanSourceOpExec and ArrowSourceOpExec already use — so "no limit" is expressed by not bounding the iterator rather than by a sentinel value that arithmetic can overflow. FileScanUtils.scala on release/v1.2 is byte-identical to main's pre-fix version, so the fix applies unchanged.

One adaptation was needed: FileScanUtilsSpec.scala was created on main (#6077) after v1.2 branched, so the cherry-pick hit a modify/delete conflict; this PR adds the file with main's full post-fix content. As a side effect, v1.2 also gains the spec's 3 pre-existing main-only tests (zip extraction, __MACOSX filtering, per-line flat-map) in addition to the 8 fix-related ones. They target FileScanUtils behavior that is identical on v1.2 and all pass.

Any related issues, documentation, discussions?

Backport of #7348 (originally closed #7345).

How was this PR tested?

The regression tests from #7348 come along with the cherry-pick. On this branch:

sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.scan.file.FileScanUtilsSpec org.apache.texera.amber.operator.source.scan.file.FileScanSourceOpDescSpec org.apache.texera.amber.operator.source.scan.file.FileScanOpDescSpec"
# 3 suites, 25 tests, all passed
# (main has 29: four getPhysicalOp/propagateSchema coverage tests were added
#  to these specs after v1.2 branched and are unrelated to this fix)

sbt "WorkflowOperator/scalafmtCheck" "WorkflowOperator/Test/scalafmtCheck"
# passed

sbt "WorkflowOperator/scalafixAll --check"
# passed

Was this PR authored or co-authored using generative AI tooling?

Co-authored by: Claude Code (Claude Fable 5)

… empty limit emits no rows

(cherry picked from commit 8982803)
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @aglinxinyuan
    You can notify them by mentioning @aglinxinyuan in a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants