Skip to content

fix: delegate merge_columns_df fast path to PyLance - #60

Merged
universalmind303 merged 6 commits into
daft-engine:mainfrom
FANNG1:fix/native-reader-fast-path
Sep 10, 2026
Merged

universalmind303 merged 6 commits into
daft-engine:mainfrom
FANNG1:fix/native-reader-fast-path

Conversation

@FANNG1

@FANNG1 FANNG1 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the manual full-table merge_columns_df writer with PyLance's fragment.merge_columns(RecordBatchReader) API
  • add focused regression coverage for deletion vectors, fast-path eligibility, and storage versions

Closes #59
Closes #20

Why these issues are systemic

#59 and #20 look like separate bugs, but they have the same root cause: daft-lance was maintaining a second implementation of Lance's merge-column write path.

The old fast path directly handled Lance file writing, storage-version selection, field-ID allocation, fragment metadata construction, file URI composition, local file-size lookup, and visible-to-physical row mapping. Those are Lance implementation details rather than daft-lance's integration contract. This caused the deletion-vector path to commit metadata that could not be read (#59), and made object-store merges call os.path.getsize() on an s3:// URI (#20).

This maintenance burden will grow as Lance's physical formats and fragment semantics evolve. Delegating file creation and metadata construction to PyLance keeps daft-lance responsible only for its real boundary: supplying correctly aligned columns and committing the returned fragment metadata.

Correctness

The fragment.merge_columns(reader) overload is positional. The fast path groups rows by fragment_id and sorts each group by _rowaddr before passing the new columns to PyLance. As before, this path assumes a full-table input carrying the correct fragment_id and _rowaddr metadata from daft.read_lance.

PyLance maps the visible reader rows onto the fragment's physical rows, preserving deletion-vector semantics without daft-lance duplicating that mapping logic.

Partial-fragment inputs continue to use the existing slow path.

The object-store failure is removed structurally: daft-lance no longer constructs data-file paths or calls local filesystem APIs for the positional fast path. PyLance owns file creation and returned fragment metadata.

Validation

  • focused fast-path and namespace tests: 96 passed
  • full test assertions: 354 passed, 7 skipped, 2 xfailed, 2 xpassed
  • focused Mypy and git diff --check: passed
  • the full-suite process encountered the repository's existing exit 139 during teardown, after all assertions completed

@FANNG1 FANNG1 changed the title fix: use native reader for merge_columns_df fast path fix: use pylance native reader for merge_columns_df fast path Aug 5, 2026
@FANNG1 FANNG1 changed the title fix: use pylance native reader for merge_columns_df fast path fix: delegate merge_columns_df fast path to PyLance Aug 5, 2026
@FANNG1

FANNG1 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@universalmind303 could you help review this PR? thanks

@universalmind303
universalmind303 merged commit a756f26 into daft-engine:main Sep 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants