Skip to content

#857 Add source-level record limit - #860

Merged
yruslan merged 1 commit into
masterfrom
codex/issue-857-record-limit
Jul 27, 2026
Merged

#857 Add source-level record limit#860
yruslan merged 1 commit into
masterfrom
codex/issue-857-record-limit

Conversation

@lokm01

@lokm01 lokm01 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Closes #857.

What changed

  • Added the record_limit read option and parsed it as a non-negative 32-bit integer.
  • Propagated the option through CobolParameters and DefaultSource into CobolRelation.
  • Applied the cap centrally after format-specific scan construction, so it covers fixed-length, text, variable-length, indexed, compressed, and hierarchical readers.
  • Added writer-side validation because record_limit is read-only.
  • Documented semantics, usage, and operational trade-offs in the README.

Behavior

  • Missing record_limit: existing unlimited behavior is unchanged.
  • record_limit = 0: returns an empty RDD without evaluating parent scan partitions.
  • Positive value: returns at most that many decoded rows globally across all input paths/files.
  • Negative, malformed, or overflowing values: fail with a clear IllegalArgumentException.

The implementation uses a narrow single-partition coalesce and lazily consumes rows until the requested count is reached. This provides a source-level cap and can avoid opening later partitions/files once the cap is satisfied. While enabled, the scan runs through one task. Indexed readers may still build indexes before row consumption.

This does not make df.limit(N) push down automatically. Cobrix currently implements Spark DataSource V1 TableScan, whose buildScan() API does not receive an optimizer limit. Automatic logical-plan limit pushdown would require a separate DataSource V2 implementation using SupportsPushDownLimit and a compatibility design spanning Spark 2.4 through 3.5.

Validation

Focused suites passed across every supported CI matrix combination (96 test executions total):

  • CobolParametersParserSuite
  • CobolRelationSpec
  • CobolParametersValidatorSuite
  • Test43RecordLimitSpec

Matrix:

  • Scala 2.11.12 / Spark 2.4.8 / JDK 8
  • Scala 2.12.21 / Spark 3.4.4 / JDK 8
  • Scala 2.12.21 / Spark 3.5.7
  • Scala 2.13.18 / Spark 3.5.7 / JDK 8

git diff --check also passes.

Summary by CodeRabbit

  • New Features

    • Added the record_limit Spark read option to cap decoded rows across all input files and paths.
    • Supports zero, positive limits, or no limit when omitted.
    • Added support across fixed-length, variable-length, and D-format records.
    • Documented usage with a Spark read example.
  • Bug Fixes

    • Writing configurations now reject the read-only record_limit option.
    • Invalid or negative limit values produce clear validation errors.

@lokm01
lokm01 requested a review from yruslan July 27, 2026 07:39
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1cd38fe7-f731-47cb-a5b9-4d59b83fa39e

📥 Commits

Reviewing files that changed from the base of the PR and between 5937f2d and cc92f67.

📒 Files selected for processing (10)
  • README.md
  • cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/parameters/CobolParameters.scala
  • cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/parameters/CobolParametersParser.scala
  • cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/reader/parameters/CobolParametersParserSuite.scala
  • spark-cobol/src/main/scala/za/co/absa/cobrix/spark/cobol/source/CobolRelation.scala
  • spark-cobol/src/main/scala/za/co/absa/cobrix/spark/cobol/source/DefaultSource.scala
  • spark-cobol/src/main/scala/za/co/absa/cobrix/spark/cobol/source/parameters/CobolParametersValidator.scala
  • spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/source/CobolRelationSpec.scala
  • spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/source/integration/Test43RecordLimitSpec.scala
  • spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/source/parameters/CobolParametersValidatorSuite.scala

Walkthrough

The PR adds a record_limit Spark read option, parses it into CobolParameters, applies it to decoded Spark rows, rejects it for writes, documents usage, and adds unit and integration coverage.

Changes

Record limit support

Layer / File(s) Summary
Option contract and parsing
cobol-parser/.../parameters/CobolParameters.scala, cobol-parser/.../parameters/CobolParametersParser.scala, cobol-parser/src/test/..., README.md
Adds the recordLimit configuration field, parses non-negative integer values from record_limit, validates malformed inputs, and documents the option and usage example.
Scan propagation and row limiting
spark-cobol/.../CobolRelation.scala, spark-cobol/.../DefaultSource.scala, spark-cobol/src/test/.../CobolRelationSpec.scala
Passes the parsed limit into CobolRelation and limits decoded RDD rows for absent, zero, and positive values across partitions.
Write validation and integration coverage
spark-cobol/.../CobolParametersValidator.scala, spark-cobol/src/test/.../parameters/CobolParametersValidatorSuite.scala, spark-cobol/src/test/.../integration/Test43RecordLimitSpec.scala
Rejects record_limit in writer options and tests limiting across supported input formats, paths, and pedantic mode.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SparkRead
  participant DefaultSource
  participant CobolRelation
  participant DecodedRDD
  SparkRead->>DefaultSource: supply record_limit
  DefaultSource->>CobolRelation: pass recordLimit
  CobolRelation->>DecodedRDD: build decoded rows
  CobolRelation->>DecodedRDD: applyRecordLimit
  DecodedRDD-->>SparkRead: return capped rows
Loading

Possibly related PRs

  • AbsaOSS/cobrix#846: Both changes modify CobolParameters and CobolParametersParser option handling.

Suggested reviewers: yruslan

Poem

A rabbit found rows in a burrow so wide,
And capped their parade with a limit supplied.
“Zero!” it whispered—no rows hopped through,
“A thousand!”—just enough for the view.
The parser, scan, and tests danced in delight,
While writers were told, “Reads only tonight!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a source-level record limit.
Linked Issues check ✅ Passed The PR implements the requested record_limit source option, propagates it through reading, and enforces the documented validation behavior.
Out of Scope Changes check ✅ Passed The changes stay focused on the source-level record limit and related docs, tests, and writer validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-857-record-limit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

JaCoCo code coverage report - 'cobol-parser'

Overall Project 90.17% 🍏

There is no coverage information present for the Files changed

@github-actions

Copy link
Copy Markdown

JaCoCo code coverage report - 'spark-cobol'

Overall Project 83.54% -0.07% 🍏
Files changed 94.44% 🍏

File Coverage
CobolParametersValidator.scala 86.01% 🍏
CobolRelation.scala 85.5% -2.11% 🍏
DefaultSource.scala 83.37% 🍏

@yruslan
yruslan marked this pull request as ready for review July 27, 2026 07:50
@yruslan
yruslan merged commit 997d999 into master Jul 27, 2026
7 checks passed
@yruslan
yruslan deleted the codex/issue-857-record-limit branch July 27, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a source-level row limit (record_limit / limit pushdown) to bound decoding work when sampling

2 participants