[seed f1a543] Evaluation change; do not merge - #27
Conversation
📝 WalkthroughWalkthroughThe text input source now uses ChangesText input line window
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/text/TextInputSourceOpExec.scala`:
- Around line 39-40: Update the slice endpoint calculation in
TextInputSourceOpExec to avoid Int overflow when fileScanOffset is combined with
an absent or maximal fileScanLimit. Compute the endpoint using Long arithmetic
and cap it at Int.MaxValue, or handle the unbounded limit separately, while
preserving the existing offset and bounded-limit behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a44c8082-8b66-44ae-9eba-99ae6e7f4532
📒 Files selected for processing (1)
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/text/TextInputSourceOpExec.scala
| desc.fileScanOffset.getOrElse(0), | ||
| desc.fileScanOffset.getOrElse(0) + desc.fileScanLimit.getOrElse(Int.MaxValue) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n \
--glob 'build.sbt' \
--glob '*.sbt' \
--glob 'pom.xml' \
--glob 'gradle.properties' \
'scalaVersion|scala-library|scala.binary.version' . || true
rg -n -C 6 \
'fileScanOffset|fileScanLimit|linesIterator\.slice|\.drop\(|\.take\(' \
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/text \
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/text || trueRepository: sshiv012/texera
Length of output: 17393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- descriptor ---'
sed -n '1,110p' common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/text/TextSourceOpDesc.scala
printf '%s\n' '--- relevant tests ---'
sed -n '230,290p' common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/text/TextInputSourceOpDescSpec.scala
printf '%s\n' '--- validation and usages ---'
rg -n -C 4 \
'fileScanOffset|fileScanLimit|TextInputSourceOpDesc' \
common/workflow-operator/src/main common/workflow-operator/src/test \
| head -n 500
printf '%s\n' '--- deterministic overflow probe ---'
python3 - <<'PY'
INT_MAX = 2**31 - 1
def int32(value):
value &= 0xffffffff
return value - 2**32 if value >= 2**31 else value
offset = 1
limit = INT_MAX
endpoint = int32(offset + limit)
print({
"offset": offset,
"limit": limit,
"mathematical_endpoint": offset + limit,
"int32_endpoint": endpoint,
"wrapped_negative": endpoint < 0,
"clamped_long_endpoint": min(offset + limit, INT_MAX),
})
PY
printf '%s\n' '--- Scala 2.13 Iterator.slice implementation ---'
curl -fsSL \
https://raw.githubusercontent.com/scala/scala/v2.13.18/src/library/scala/collection/Iterator.scala \
| rg -n -A 18 -B 4 'def slice'Repository: sshiv012/texera
Length of output: 50371
🌐 Web query:
Scala 2.13.18 Iterator.slice negative until implementation
💡 Result:
In Scala 2.13.18, the Iterator.slice(from: Int, until: Int) method is implemented by delegating to a protected method named sliceIterator [1][2]. The public slice method normalizes the until parameter by calling until max 0 [1][2]. Consequently, if a negative value is passed as the until parameter to the public slice method, it is treated as 0, which results in an empty iterator (unless from is also negative or such that the slice is invalid/empty, as the slice logic handles standard index bounds) [1][3][2]. Specifically, the implementation is as follows: def slice(from: Int, until: Int): Iterator[A] = sliceIterator(from, until max 0) [1][2] The protected sliceIterator method, however, contains logic that explicitly treats negative values as unbounded [1][2]: protected def sliceIterator(from: Int, until: Int): Iterator[A] = { val lo = from max 0 val rest = if (until < 0) -1 // unbounded else if (until <= lo) 0 // empty else until - lo // finite if (rest == 0) Iterator.empty else new Iterator.SliceIterator(this, lo, rest) } [1][2] Because the public slice method always forces until to be at least 0 before passing it to sliceIterator, the "unbounded" path (where until < 0) inside sliceIterator is effectively unreachable through the public API [1][2]. The normalization (until max 0) ensures that any negative input for until is converted to 0, which then triggers the "empty" condition (until <= lo) or results in an empty iterator [1][3][2].
Citations:
- 1: https://github.com/scala/scala/blob/2.13.x/src/library/scala/collection/Iterator.scala
- 2: https://github.com/scala/scala/blob/v2.13.16/src/library/scala/collection/Iterator.scala
- 3: https://github.com/scala/scala/blob/v2.13.8/src/library/scala/collection/Iterator.scala
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- text source test cases ---'
sed -n '40,235p' common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/text/TextInputSourceOpDescSpec.scala
printf '%s\n' '--- file scan implementation ---'
sed -n '1,180p' common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/file/FileScanUtils.scala
printf '%s\n' '--- focused Scala slice source ---'
curl -fsSL \
https://raw.githubusercontent.com/scala/scala/v2.13.18/src/library/scala/collection/Iterator.scala \
| sed -n '/def slice(from: Int, until: Int)/,+8p'
printf '%s\n' '--- behavioral model ---'
python3 - <<'PY'
INT_MAX = 2**31 - 1
def int32(value):
value &= 0xffffffff
return value - 2**32 if value >= 2**31 else value
offset = 1
limit = INT_MAX
wrapped_until = int32(offset + limit)
print(f"Scala Int endpoint: {wrapped_until}")
print(f"Iterator.slice until after normalization: {max(wrapped_until, 0)}")
print(f"Existing slice window is empty: {max(wrapped_until, 0) <= max(offset, 0)}")
long_until = min(offset + limit, INT_MAX)
print(f"Long-and-clamped endpoint: {long_until}")
print(f"Correct offset-only window is non-empty for input length > {offset}: {long_until > offset}")
PYRepository: sshiv012/texera
Length of output: 14534
Prevent Int overflow in the slice endpoint.
When fileScanOffset is positive and fileScanLimit is absent or Int.MaxValue, line 40 can wrap the endpoint to a negative value. Iterator.slice then returns no lines. Use Long arithmetic with an Int.MaxValue cap, or handle the unbounded case separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/text/TextInputSourceOpExec.scala`
around lines 39 - 40, Update the slice endpoint calculation in
TextInputSourceOpExec to avoid Int overflow when fileScanOffset is combined with
an absent or maximal fileScanLimit. Compute the endpoint using Long arithmetic
and cap it at Int.MaxValue, or handle the unbounded limit separately, while
preserving the existing offset and bounded-limit behavior.
1 participant
Evaluation PR. Do not merge.
TextInputSourceOpExec.produceTupleto select the configured line window withlinesIterator.slice(offset, offset + limit).