FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Add a non-JVM read path for Lance data sources by haoxu0 · Pull Request #6943 · feast-dev/feast · GitHub

Repository navigation

feat: Add a non-JVM read path for Lance data sources - #6943

Merged
ntkathole merged 2 commits into
feast-dev:masterfrom
haoxu0:feat/lance-read
Oct 7, 2026
Merged

ntkathole merged 2 commits into
feast-dev:masterfrom
haoxu0:feat/lance-read

Conversation

haoxu0 commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Completes the read half of #6899. LanceFormat landed in #6925 as a format descriptor; nothing read Lance until now.

Lance already worked through SparkSource, which drives its reader generically from table_format.format_type.value and table_format.properties. What was missing is a path that needs no JVM — which is also the real test of whether the DataSource abstraction is engine-agnostic rather than Spark-agnostic in name only.

Placement, and why not the two obvious alternatives

A new LanceSource read by the existing DuckDB store — not a Lance offline store, and not an extension of FileSource.

FileSource is the wrong host. Its format axis is already occupied by file_format (ParquetFormat/DeltaFormat), so adding table_format gives one source two overlapping format axes. It is also read by two stores with incompatible contracts: duckdb._read_data_source dispatches on type, while dask._read_datasource has no dispatch seam at all — it reads file_options.uri unconditionally as Parquet and hard-asserts isinstance(..., FileSource) in three places, so a third format would break silently under Dask. Decisively: Lance's catalog addressing (catalog/namespace/table) has no path to put in FileSource.path, so the catalog-based layer would not fit the class even if the path-based one did.

A Lance offline store would be the wrong 120 lines. duckdb.py is not an engine — it is a binding that injects _read_data_source/_write_data_source callbacks into the real engine in ibis.py. A LanceOfflineStore would be a near-verbatim copy of it plus a repo_config.py entry, and would force users to choose Lance instead of Parquet rather than mixing them in one feature service.

Reading a source as ibis.memtable(arrow_table) in the DuckDB store already has two precedents — IcebergSource and MlflowDatasetSource. Following them means ibis.py is untouched, so the point-in-time join, TTL handling, field mapping and ODFVs all work unchanged, and neither repo_config.py nor data_source.py needs an edit because CUSTOM_SOURCE + data_source_class_type is self-describing in the registry proto.

Net: +47 lines to duckdb.py, one new source module.

Both addressing modes work

# path-based
LanceSource(uri="s3://bucket/poi_embeddings.lance", timestamp_field="generated_at")

# catalog-based, through namespace_client + table_id
LanceSource(
    table="poi_embeddings",
    namespace_impl="rest",
    table_format=LanceFormat(catalog="my_catalog", namespace="features"),
    timestamp_field="generated_at",
)

Pin semantics: a pin selects data, never shape

Following what was argued on #5782 and #6925. Three things enforce it:

  • get_table_column_names_and_types reads the pinned schema, so feast apply infers the schema reads will actually see.
  • A pre-flight check in get_historical_features fails with a message naming the pin when a pinned version cannot satisfy the declared schema. It reads schemas only — one metadata round trip, no scan.
  • Vector widths go through _validate_vector_field_lengths from fix: Make vector length validation reachable, schema-driven and vectorized #6909 rather than a second validator. This is exact for Lance because Lance stores vectors as fixed_size_list, where that check is a property of the Arrow type.

Demonstrated rather than asserted — the dataset is overwritten with a corrected value for the same (entity, timestamp), and the pinned read still returns the original:

latest  (        latest version): raw=[0.77, 1.17]  get_historical_features@12:30 -> 0.77
tag pin (    tag 'eval_2026_q1'): raw=[0.5,  0.9 ]  get_historical_features@12:30 -> 0.50
ver pin (             version 1): raw=[0.5,  0.9 ]  get_historical_features@12:30 -> 0.50

Testing

43 tests, all using the dir namespace implementation — it exercises the identical namespace_client + table_id code path as a remote catalog, so the suite needs no server. Point-in-time correctness is covered for both the path-based and catalog-based variants, with a 12:30 query selecting the 11:00 row and never the 13:00 one.

Regression evidence, rebased onto current master. Failure names compared rather than counts, since the suite has flaky members:

with this change : 40 failures
master           : 40 failures
only with change : (none)
only on master   : (none)

ruff check and ruff format --check clean on all four files. mypy --ignore-missing-imports clean. (Bare mypy reports pre-existing missing-stub noise for pyarrow/pandas/ibis/pyiceberg/deltalake; the repo has no mypy config section.)

Known limitations, deliberately

  • Read-only. _write_data_source is untouched, so a LanceSource is not yet a persist target and there is no SavedDatasetLanceStorage. materialize into an online store does work, via pull_latest_from_table_or_query. Happy to follow up — _write_data_source already dispatches on IcebergSource, so the seam exists.
  • DuckDB only. Dask cannot read Lance, by the reasoning above. RayOfflineStore would be a natural second host since it already has a _resolve_source_dataset dispatch, but it is not in this PR.
  • Only the dir impl is exercised. rest, glue, unity, polaris and the Hive/Iceberg impls need lance-namespace-impls plus a live catalog. The code path is identical, which is why dir is a meaningful test, but I have not run against a remote catalog.
  • Eager materialisation. The read pulls the dataset into Arrow before ibis sees it, matching what IcebergSource and MlflowDatasetSource already do. Column projection is pushed down via to_arrow(columns=...), but the retrieval path does not pass a projection and no predicate pushdown reaches Lance.
  • Vector-width validation is exact only for fixed_size_list. For a plain list/large_list column the pre-flight runs on a zero-row table and infers nothing. Lance writes fixed_size_list, so this is the right trade in practice. Making it exact in general needs the feature view threaded into the data_source_reader callback in ibis.py, whose signature is (DataSource, str) — a separate change that would benefit every source, not just this one.

Related: #6899, #6925, #6909, #5782, #5652

haoxu0 requested a review from a team as a code owner October 5, 2026 19:29

codecov-commenter commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 1.19760% with 165 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.99%. Comparing base (7087f1f) to head (8b8d17c).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...t/infra/data_sources/contrib/lance/lance_source.py 0.00% 147 Missing ⚠️
sdk/python/feast/infra/offline_stores/duckdb.py 11.11% 16 Missing ⚠️
...feast/infra/data_sources/contrib/lance/__init__.py 0.00% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6943      +/-   ##
==========================================
- Coverage   49.08%   48.99%   -0.09%     
==========================================
  Files         433      435       +2     
  Lines       54332    54505     +173     
  Branches     7917     7948      +31     
==========================================
+ Hits        26667    26703      +36     
- Misses      25788    25926     +138     
+ Partials     1877     1876       -1     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 50.37% <1.19%> (-0.11%) ⬇️
Files with missing lines Coverage Δ
...feast/infra/data_sources/contrib/lance/__init__.py 0.00% <0.00%> (ø)
sdk/python/feast/infra/offline_stores/duckdb.py 33.60% <11.11%> (-1.16%) ⬇️
...t/infra/data_sources/contrib/lance/lance_source.py 0.00% <0.00%> (ø)

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5eaa36...8b8d17c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

haoxu0 requested review from a team, ejscribner, nquinn408 and robhowley and a balanced review from Copilot and removed request for a team October 6, 2026 06:55

Copilot AI left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🟡 Changes recommended

Vector schema inference and validation have correctness gaps, and the required Lance dependencies are not installable through a Feast extra.

Review effort: Balanced
Findings: 1 · 3 · 1

Open (5) What changed in this PR

Adds native, non-JVM Lance reads through the DuckDB offline store, including catalog addressing, version pinning, and schema validation.

Changes:

  • Introduces LanceSource for URI and namespace-based datasets.
  • Adds DuckDB dispatch and historical-retrieval validation.
  • Adds Lance read, pinning, schema, and point-in-time tests.
File Description
lance_source.py Implements Lance source reads, serialization, and validation.
lance/​__init__.py Exports the Lance source API.
duckdb.py Integrates Lance reads with DuckDB retrieval.
test_lance_source.py Tests source behavior and DuckDB integration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

haoxu0 commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

@ntkathole @franciscojavierarceo PTAL when you have time

haoxu0 and others added 2 commits October 7, 2026 11:19
Adds LanceSource and teaches the DuckDB offline store to read it, completing the
read half of feast-dev#6899. LanceFormat landed in feast-dev#6925 as a format descriptor; nothing
read Lance until now.

Lance already worked through SparkSource, which drives its reader generically
from table_format.format_type.value and table_format.properties. What was
missing is a path that needs no JVM, which is also the real test of whether the
DataSource abstraction is engine-agnostic rather than Spark-agnostic in name
only.

Placement: a new source read by the existing DuckDB store, rather than a Lance
offline store or an extension of FileSource.

FileSource is the wrong host. Its format axis is already taken by file_format,
so adding table_format would give one source two overlapping format axes. It is
also read by two stores with incompatible contracts: duckdb._read_data_source
dispatches on type, while dask._read_datasource has no dispatch seam and reads
file_options.uri unconditionally as Parquet, and asserts isinstance(...,
FileSource) in three places. Decisively, Lance's catalog addressing has no path
to put in FileSource.path, so the catalog-based layer would not fit the class
even if the path-based one did.

A Lance offline store would be the wrong 120 lines. duckdb.py is a binding that
injects reader and writer callbacks into the engine in ibis.py, so a Lance store
would be a near-copy of it plus a repo_config entry, and would force a choice
between Lance and Parquet instead of mixing them in one feature service.

Reading a source as ibis.memtable(arrow_table) in the DuckDB store already has
two precedents, IcebergSource and MlflowDatasetSource. Following them leaves
ibis.py untouched, so the point-in-time join, TTL handling, field mapping and
ODFVs work unchanged, and no edit to repo_config.py or data_source.py is needed
because CUSTOM_SOURCE plus data_source_class_type is self-describing.

Both addressing modes work: a uri, and catalog/namespace/table through
namespace_client and table_id.

Pin semantics follow what was argued on feast-dev#5782 and feast-dev#6925: a pin selects data,
never shape. get_table_column_names_and_types reads the pinned schema so
feast apply infers what reads will actually see; a pre-flight check fails with a
message naming the pin when a pinned version cannot satisfy the declared schema;
and vector widths go through _validate_vector_field_lengths from feast-dev#6909 rather
than a second validator.

Tests use the dir namespace implementation, which exercises the same
namespace_client and table_id code path as a remote catalog with no server
required. 43 tests, including a demonstration that a tag pin returns earlier
data after the dataset has been overwritten for the same entity and timestamp.

Read-only for now: _write_data_source is untouched, so a LanceSource is not yet
a persist target and there is no SavedDatasetLanceStorage.

Signed-off-by: hao-xu5 <hxu44@apple.com>
Signed-off-by: HaoXuAI <sduxuhao@gmail.com>
ntkathole merged commit e2a53bb into feast-dev:master Oct 7, 2026
18 of 23 checks passed
haoxu0 added a commit to haoxu0/feast that referenced this pull request Oct 7, 2026
feast-dev#6943 made a `LanceSource` readable without Spark. This makes one writable,
through the same two addressing modes: a uri, or a `namespace_client` plus
`table_id` resolved through a Lance namespace, so a catalog-addressed
dataset is committed through its catalog rather than behind its back.

`_write_data_source` gains a `LanceSource` branch, following the
`IcebergSource` branch already there, which makes the path reachable from
`DuckDBOfflineStore.offline_write_batch`. The `isinstance` narrowing the
read path open-coded is now a shared `_as_lance_source` helper used by
both, rather than a second copy of the optional-import guard.

Three things are settled before Lance is called.

A pinned source is refused. A Lance commit always produces a new version,
so `write_dataset` has no `version` argument at all; a write through a
source pinned to version 1 or to a tag would succeed and then be invisible
through the very source that performed it. Measured: after appending to a
dataset tagged `prod` at version 1, the unpinned source reads 5 rows and
the pinned source still reads 3.

An absent dataset is created rather than appended to, because Lance has no
create-or-append mode and rejects `create` on an existing dataset.

An existing dataset's vector widths are compared against the incoming
data. Lance rejects an `append` whose schema disagrees, but it accepts an
`overwrite` that replaces a 8-wide embedding column with a 32-wide one,
silently rewriting the declared shape and leaving a version history whose
vectors are not mutually comparable, with every index built on the old
width invalidated. No legitimate schema evolution changes an embedding's
dimension, so this is refused. The guard is narrow on purpose: only vector
widths are policed, and other type changes remain Lance's business.

The declared `vector_length` is checked at the store entry point rather
than in the writer callback, which is handed a `DataSource` and so cannot
see the feature view. It reuses `_validate_vector_field_lengths` from
 feast-dev#6909 rather than adding a second implementation. Without it, creating a
dataset at a width other than the declared one would succeed and fail only
on the next read.

19 tests added, all using the `dir` namespace implementation for the
catalog-based cases so no server is needed.

Signed-off-by: hao-xu5 <hxu44@apple.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL