Skip to content

fix(workflow-operator, v1.2): Text Input operator using offset with an empty limit emits no rows - #7529

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

fix(workflow-operator, v1.2): Text Input operator using offset with an empty limit emits no rows#7529
eugenegujing wants to merge 1 commit into
apache:release/v1.2from
eugenegujing:backport/text-input-offset-overflow-v1.2

Conversation

@eugenegujing

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Backport of #7347 to release/v1.2, cherry-picked from main commit 91235fb (clean, no conflicts).

TextInputSourceOpExec computed its line window as slice(offset, offset + limit.getOrElse(Int.MaxValue)). With an Offset set and the Limit left empty, the addition overflows Int to a negative bound, which Scala 2.13's Iterator.slice clamps to 0 and then returns an empty iterator — so the operator silently emitted zero rows while the workflow reported success. Any Offset ≥ 1 with an empty Limit is affected, and an explicit large Limit (e.g. Int.MaxValue) overflows the same way. This contradicts the Limit property's own description, "Leave empty to read all lines."

The fix replaces the slice with drop(offset) + take(limit), the same idiom the CSV, Arrow, and JSONL scan sources already use. There is no addition, so nothing can overflow; every configuration that previously worked is unchanged.

Any related issues, documentation, discussions?

Backport of #7347 (originally closed #7346).

How was this PR tested?

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

sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.scan.text.TextInputSourceOpDescSpec"
# 15 tests, all passed (8 pre-existing on release/v1.2 + 7 new)
# (main has 17: two getPhysicalOp/propagateSchema coverage tests were added
#  to this spec 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)

…n empty limit emits no rows

(cherry picked from commit 91235fb)
@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: @Yicong-Huang
    You can notify them by mentioning @Yicong-Huang 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