Skip to content

[client] Implement BatchScanner with limit-based scan#515

Open
Arnav-panjla wants to merge 3 commits intoapache:mainfrom
Arnav-panjla:feat/batch-scanner-316
Open

[client] Implement BatchScanner with limit-based scan#515
Arnav-panjla wants to merge 3 commits intoapache:mainfrom
Arnav-panjla:feat/batch-scanner-316

Conversation

@Arnav-panjla
Copy link
Copy Markdown
Contributor

Purpose

Fixes #316

Implement one-shot BatchScanner using LimitScanRequest.


Brief change log

  • Added limit to TableScan
  • Added create_batch_scanner
  • Implemented BatchScanner (eager RPC, single poll_batch)
  • Added Arrow IPC (log) and KV → RecordBatch (PK) decoding
  • Added projection support

Tests

  • All tests pass (cargo test)
  • Verified with cargo check, clippy, fmt

API and Format

API Change: Yes

  • New BatchScanner, TableScan::limit, create_batch_scanner

Storage Format: No


Documentation

No

Copy link
Copy Markdown
Contributor

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Arnav-panjla Ty for the PR, left some comments, PTAL

Comment thread Cargo.lock
"rustc_version",
]

[[package]]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's sync Cargo.lock in a separate PR, otherwise it's weird.
Want to pick this up?

@@ -0,0 +1,254 @@
// Licensed to the Apache Software Foundation (ASF) under one
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have any tests, do you mind to add unit and integration tests?

Ok(self
.batch
.take()
.map(|b| ScanBatch::new(self.bucket.clone(), b, 0)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we hardcode 0 here?

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.

Core BatchScanner implementation

2 participants