mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-29 06:56:15 +00:00
fix: correct remote materialized view refresh results and options (#4672)
Remote materialized-view refreshes drained the SQL result and then fabricated `mode=rebuild`, `rows_written=<total view rows>`, and `source_version=0`. This made incremental refreshes report incorrect statistics. Make these three result fields optional across Rust, Python, and TypeScript, and leave them unset for remote SQL refreshes. Remove the post-refresh row count request. Local refreshes continue to populate all metrics, and `version` remains the view version observed after completion. Remote per-operation statistics remain unavailable until the SQL job adapter can retrieve them. Expose the forced-rebuild option through the Rust builder, synchronous/asynchronous Python refresh and job methods, and TypeScript refresh. Remote `full=true` sends `WITH (full = true)`; the default lets the server choose its refresh mode. Local source-version pinning and remote job cancellation are preserved. Local refresh regression tests respect the `experimental-functions` build gate, while remote refresh coverage also runs in default builds. Validation: - Rust: 20 materialized-view tests and 21 remote SQL tests passed with `remote,experimental-functions`; `cargo fmt --all` passed. - Python: rebuilt the extension in the uv-managed environment. The default build passed 23 tests with 27 feature-dependent skips. The opt-in build passed 61 tests with 3 expected default-build-only skips, covering the materialized-view suite and 14 async query-serialization cases from current main. Python Ruff lint and formatting checks passed. - TypeScript: default and opt-in native builds and TypeScript compilation passed. The default build passed 5 tests with 10 expected feature-dependent skips; the opt-in build passed all 14 materialized-view tests with the default-build-only test skipped. Lint/format checks and documentation generation passed. - Repository-wide `ruff check .` reports 48 pre-existing violations in unchanged `docs/web-tests` files; `ruff check python` passes. Fixes #4670 <!-- lance-gatekeeper-fix:v1 agent=ece43ec4b88a48d1789f60005fa3ecfc generation=1 --> Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
co-authored by
Xuanwo
parent
c167b99fa0
commit
2cd635d41c
@@ -6,6 +6,7 @@ import * as tmp from "tmp";
|
||||
import { Connection, connect } from "../lancedb";
|
||||
import {
|
||||
DEFINITION_META_KEY,
|
||||
MaterializedView,
|
||||
definitionFromMetadata,
|
||||
} from "../lancedb/materialized_view";
|
||||
import { experimentalFunctionsEnabled } from "../lancedb/native";
|
||||
@@ -94,20 +95,33 @@ describe("materialized views", () => {
|
||||
);
|
||||
});
|
||||
|
||||
experimentalTest("fully refreshes after an append", async () => {
|
||||
const view = await db.createMaterializedView("copy", "people", {
|
||||
withNoData: true,
|
||||
});
|
||||
await view.refresh();
|
||||
experimentalTest.each([false, true])(
|
||||
"fully refreshes after an append (full=%s)",
|
||||
async (full) => {
|
||||
const view = await db.createMaterializedView("copy", "people", {
|
||||
withNoData: true,
|
||||
});
|
||||
await view.refresh();
|
||||
|
||||
const people = await db.openTable("people");
|
||||
await people.add([{ name: "alan", age: 41 }]);
|
||||
const result = await view.refresh();
|
||||
expect(result.mode).toBe("rebuild");
|
||||
expect(Number(result.rowsWritten)).toBe(4);
|
||||
expect(await view.table().countRows()).toBe(4);
|
||||
const people = await db.openTable("people");
|
||||
await people.add([{ name: "alan", age: 41 }]);
|
||||
const result = await view.refresh({ full });
|
||||
expect(result.mode).toBe("rebuild");
|
||||
expect(Number(result.rowsWritten)).toBe(4);
|
||||
expect(await view.table().countRows()).toBe(4);
|
||||
|
||||
expect((await view.refresh()).mode).toBe("rebuild");
|
||||
expect((await view.refresh()).mode).toBe("rebuild");
|
||||
},
|
||||
);
|
||||
|
||||
it("forwards a forced refresh and preserves absent remote metrics", async () => {
|
||||
const view = new MaterializedView(await db.openTable("people"));
|
||||
const remoteResult = { version: 42 };
|
||||
const refresh = jest
|
||||
.spyOn(view.table(), "refreshMaterializedView")
|
||||
.mockResolvedValue(remoteResult);
|
||||
expect(await view.refresh({ full: true })).toEqual(remoteResult);
|
||||
expect(refresh).toHaveBeenCalledWith(undefined, true);
|
||||
});
|
||||
|
||||
experimentalTest("lists views and rejects non-views", async () => {
|
||||
|
||||
@@ -188,14 +188,20 @@ export class MaterializedView {
|
||||
* Recompute the view from its source.
|
||||
* Local refresh requires a source build with `experimental-functions`.
|
||||
*
|
||||
* Every refresh rebuilds the complete view. For local views,
|
||||
* `sourceVersion` refreshes to that source version instead of the latest;
|
||||
* remote SQL refreshes do not support source-version pinning.
|
||||
* `full: true` forces a remote rebuild; otherwise the server chooses the
|
||||
* refresh mode. Local views always rebuild, and `sourceVersion` selects a
|
||||
* source version instead of the latest. Remote SQL refreshes do not support
|
||||
* source-version pinning. Their results leave `mode`, `rowsWritten`, and
|
||||
* `sourceVersion` absent because the SQL job adapter does not expose these metrics.
|
||||
*/
|
||||
async refresh(options?: {
|
||||
full?: boolean;
|
||||
sourceVersion?: number;
|
||||
}): Promise<RefreshMaterializedViewResult> {
|
||||
validateNonNegativeInteger(options?.sourceVersion, "sourceVersion");
|
||||
return await this.inner.refreshMaterializedView(options?.sourceVersion);
|
||||
return await this.inner.refreshMaterializedView(
|
||||
options?.sourceVersion,
|
||||
options?.full,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -699,6 +699,7 @@ export abstract class Table {
|
||||
*/
|
||||
abstract refreshMaterializedView(
|
||||
sourceVersion?: number,
|
||||
full?: boolean,
|
||||
): Promise<RefreshMaterializedViewResult>;
|
||||
|
||||
/** @ignore */
|
||||
@@ -1442,8 +1443,9 @@ export class LocalTable extends Table {
|
||||
|
||||
async refreshMaterializedView(
|
||||
sourceVersion?: number,
|
||||
full?: boolean,
|
||||
): Promise<RefreshMaterializedViewResult> {
|
||||
return await this.inner.refreshMaterializedView(sourceVersion);
|
||||
return await this.inner.refreshMaterializedView(sourceVersion, full);
|
||||
}
|
||||
|
||||
async materializedViewDefinition(): Promise<string> {
|
||||
|
||||
+13
-10
@@ -470,11 +470,12 @@ impl Table {
|
||||
pub async fn refresh_materialized_view(
|
||||
&self,
|
||||
source_version: Option<i64>,
|
||||
full: Option<bool>,
|
||||
) -> napi::Result<RefreshMaterializedViewResult> {
|
||||
let view = lancedb::MaterializedView::from_table(self.inner_ref()?.clone())
|
||||
.await
|
||||
.default_error()?;
|
||||
let mut builder = view.refresh();
|
||||
let mut builder = view.refresh().full(full.unwrap_or(false));
|
||||
if let Some(version) = source_version {
|
||||
let version = u64::try_from(version).map_err(|_| {
|
||||
napi::Error::from_reason("sourceVersion must be a non-negative integer")
|
||||
@@ -1615,22 +1616,24 @@ impl From<lancedb::function::FunctionErrors> for FunctionErrors {
|
||||
|
||||
#[napi(object)]
|
||||
pub struct RefreshMaterializedViewResult {
|
||||
/// How the view was brought up to date: "rebuild".
|
||||
pub mode: String,
|
||||
pub rows_written: i64,
|
||||
pub source_version: i64,
|
||||
/// How the view was brought up to date: "rebuild", or absent for remote SQL refreshes.
|
||||
pub mode: Option<String>,
|
||||
/// Rows written by this refresh, or absent for remote SQL refreshes.
|
||||
pub rows_written: Option<i64>,
|
||||
/// Source version refreshed to, or absent for remote SQL refreshes.
|
||||
pub source_version: Option<i64>,
|
||||
pub version: i64,
|
||||
}
|
||||
|
||||
impl From<lancedb::RefreshMaterializedViewResult> for RefreshMaterializedViewResult {
|
||||
fn from(value: lancedb::RefreshMaterializedViewResult) -> Self {
|
||||
let mode = match value.mode {
|
||||
let mode = value.mode.map(|mode| match mode {
|
||||
lancedb::RefreshMode::Rebuild => "rebuild",
|
||||
};
|
||||
});
|
||||
Self {
|
||||
mode: mode.to_string(),
|
||||
rows_written: value.rows_written as i64,
|
||||
source_version: value.source_version as i64,
|
||||
mode: mode.map(str::to_string),
|
||||
rows_written: value.rows_written.map(|rows| rows as i64),
|
||||
source_version: value.source_version.map(|version| version as i64),
|
||||
version: value.version as i64,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user