Skip to content

Refactor: Unify execute functions in zarr_exec.rs (Phase 2)#15

Merged
jayendra13 merged 1 commit into
mainfrom
refactor/phase2-unify-execute
Feb 4, 2026
Merged

Refactor: Unify execute functions in zarr_exec.rs (Phase 2)#15
jayendra13 merged 1 commit into
mainfrom
refactor/phase2-unify-execute

Conversation

@jayendra13

Copy link
Copy Markdown
Owner

Summary

Phase 2 of refactoring to eliminate duplicated code. This builds on Phase 1 (#13).

Note: This PR includes Phase 1 commits. After #13 is merged, this PR will auto-update to show only Phase 2 changes.

Phase 2 Changes

Extract common patterns from the three execute functions (execute_remote, execute_virtualizarr, execute_virtualizarr_with_adapter) into shared helpers:

  • build_exec_projected_schema() - Common schema projection logic
  • AsyncReadParams - Encapsulates common async read parameters
  • execute_async_read() - Generic async execution with store setup closure

This consolidates the repeated stream creation pattern:

  1. Build projected schema
  2. Create async stream with store setup
  3. Call read_zarr_async
  4. Collect batches and wrap in RecordBatchStreamAdapter

Each execute function now focuses only on its unique store setup logic.

Impact

  • Phase 2 net change: -17 lines (132 additions, 149 deletions)
  • Clearer separation of concerns
  • No functional changes - all tests pass

Test plan

  • cargo test - All tests pass
  • cargo clippy - No warnings
  • cargo fmt - Properly formatted

Depends on: #13 (Phase 1)

Part of #5

Extract common patterns from execute_remote, execute_virtualizarr, and
execute_virtualizarr_with_adapter into shared helpers:

- build_exec_projected_schema(): Common schema projection logic
- AsyncReadParams: Encapsulates common async read parameters
- execute_async_read(): Generic async execution with store setup closure

This consolidates the repeated stream creation pattern:
1. Build projected schema
2. Create async stream with store setup
3. Call read_zarr_async
4. Collect batches and wrap in RecordBatchStreamAdapter

Impact: -17 lines (132 additions, 149 deletions) with clearer separation
of concerns. Each execute function now focuses only on its unique store
setup logic.

Part of: #5
@jayendra13 jayendra13 merged commit 3b548f9 into main Feb 4, 2026
1 of 3 checks passed
@jayendra13 jayendra13 deleted the refactor/phase2-unify-execute branch February 4, 2026 01:58
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.

1 participant