diff --git a/python/python/lancedb/namespace.py b/python/python/lancedb/namespace.py index f8675ebb5..a5d88eb1f 100644 --- a/python/python/lancedb/namespace.py +++ b/python/python/lancedb/namespace.py @@ -11,6 +11,7 @@ through a namespace abstraction. from __future__ import annotations import sys +import warnings from typing import Any, Dict, Iterable, List, Optional, Union from uuid import UUID @@ -82,6 +83,31 @@ from ._lancedb import Session _MAX_QUERY_K = 2**31 - 1 +def _create_table_storage_options( + storage_options: Optional[Dict[str, str]], + data_storage_version: Optional[str], + enable_v2_manifest_paths: Optional[bool], +) -> Optional[Dict[str, str]]: + if data_storage_version is None and enable_v2_manifest_paths is None: + return storage_options + options = dict(storage_options or {}) + for name, value in ( + ("data_storage_version", data_storage_version), + ("enable_v2_manifest_paths", enable_v2_manifest_paths), + ): + if value is not None: + warnings.warn( + f"setting {name} directly on create_table is deprecated. " + "Use storage_options instead.", + DeprecationWarning, + stacklevel=3, + ) + options[f"new_table_{name}"] = ( + str(value).lower() if isinstance(value, bool) else value + ) + return options + + def _query_to_namespace_request( table_id: List[str], query: "Query", @@ -587,6 +613,9 @@ class LanceNamespaceDBConnection(DBConnection): if mode.lower() not in ["create", "overwrite"]: raise ValueError("mode must be either 'create' or 'overwrite'") validate_table_name(name) + storage_options = _create_table_storage_options( + storage_options, data_storage_version, enable_v2_manifest_paths + ) async_table = LOOP.run( self._inner.create_table( name, @@ -1230,6 +1259,9 @@ class AsyncLanceNamespaceDBConnection: if mode.lower() not in ["create", "overwrite"]: raise ValueError("mode must be either 'create' or 'overwrite'") validate_table_name(name) + storage_options = _create_table_storage_options( + storage_options, data_storage_version, enable_v2_manifest_paths + ) table = await self._inner.create_table( name, data, diff --git a/python/python/tests/test_namespace.py b/python/python/tests/test_namespace.py index 520a6992d..b55da4d57 100644 --- a/python/python/tests/test_namespace.py +++ b/python/python/tests/test_namespace.py @@ -6,6 +6,7 @@ import tempfile import shutil import importlib +import warnings import http.server import json import threading @@ -25,6 +26,125 @@ PUSHDOWN_DATA = pa.table( ) +class TestNamespaceCreateTableOptions: + @pytest.mark.parametrize("version", ["legacy", "stable"]) + @pytest.mark.parametrize("empty", [False, True]) + def test_deprecated_storage_version(self, tmp_path, version, empty): + db = lancedb.connect_namespace("dir", {"root": str(tmp_path / "namespace")}) + data = None if empty else [{"id": 1}] + schema = pa.schema([("id", pa.int64())]) + other_version = "stable" if version == "legacy" else "legacy" + options = {"new_table_data_storage_version": other_version} + control = lancedb.connect(tmp_path / "control").create_table( + "test", + data=data, + schema=schema, + storage_options={"new_table_data_storage_version": version}, + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", DeprecationWarning) + table = db.create_table( + "test", + data=data, + schema=schema, + storage_options=options, + data_storage_version=version, + ) + + assert ( + table.to_lance().data_storage_version + == control.to_lance().data_storage_version + ) + assert table.to_arrow().to_pylist() == ([] if empty else data) + assert options == {"new_table_data_storage_version": other_version} + assert any("data_storage_version" in str(w.message) for w in caught) + + @pytest.mark.asyncio + @pytest.mark.parametrize("version", ["legacy", "stable"]) + @pytest.mark.parametrize("empty", [False, True]) + async def test_deprecated_storage_version_async(self, tmp_path, version, empty): + root = str(tmp_path / "namespace") + db = lancedb.connect_namespace_async("dir", {"root": root}) + data = None if empty else [{"id": 1}] + schema = pa.schema([("id", pa.int64())]) + other_version = "stable" if version == "legacy" else "legacy" + options = {"new_table_data_storage_version": other_version} + control = lancedb.connect(tmp_path / "control").create_table( + "test", + data=data, + schema=schema, + storage_options={"new_table_data_storage_version": version}, + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", DeprecationWarning) + table = await db.create_table( + "test", + data=data, + schema=schema, + storage_options=options, + data_storage_version=version, + ) + + reopened = lancedb.connect_namespace("dir", {"root": root}).open_table("test") + assert ( + reopened.to_lance().data_storage_version + == control.to_lance().data_storage_version + ) + assert (await table.to_arrow()).to_pylist() == ([] if empty else data) + assert options == {"new_table_data_storage_version": other_version} + assert any("data_storage_version" in str(w.message) for w in caught) + + @pytest.mark.parametrize("enabled", [False, True]) + @pytest.mark.parametrize("empty", [False, True]) + def test_deprecated_manifest_paths(self, tmp_path, enabled, empty): + db = lancedb.connect_namespace("dir", {"root": str(tmp_path)}) + data = None if empty else [{"id": 1}] + options = {"new_table_enable_v2_manifest_paths": str(not enabled).lower()} + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", DeprecationWarning) + table = db.create_table( + "test", + data=data, + schema=pa.schema([("id", pa.int64())]), + storage_options=options, + enable_v2_manifest_paths=enabled, + ) + + assert table.uses_v2_manifest_paths() == enabled + assert table.to_arrow().to_pylist() == ([] if empty else data) + assert db.open_table("test").uses_v2_manifest_paths() == enabled + assert options == { + "new_table_enable_v2_manifest_paths": str(not enabled).lower() + } + assert any("enable_v2_manifest_paths" in str(w.message) for w in caught) + + @pytest.mark.asyncio + @pytest.mark.parametrize("enabled", [False, True]) + @pytest.mark.parametrize("empty", [False, True]) + async def test_deprecated_manifest_paths_async(self, tmp_path, enabled, empty): + db = lancedb.connect_namespace_async("dir", {"root": str(tmp_path)}) + data = None if empty else [{"id": 1}] + options = {"new_table_enable_v2_manifest_paths": str(not enabled).lower()} + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", DeprecationWarning) + table = await db.create_table( + "test", + data=data, + schema=pa.schema([("id", pa.int64())]), + storage_options=options, + enable_v2_manifest_paths=enabled, + ) + + assert await table.uses_v2_manifest_paths() == enabled + assert (await table.to_arrow()).to_pylist() == ([] if empty else data) + reopened = await db.open_table("test") + assert await reopened.uses_v2_manifest_paths() == enabled + assert options == { + "new_table_enable_v2_manifest_paths": str(not enabled).lower() + } + assert any("enable_v2_manifest_paths" in str(w.message) for w in caught) + + def _ipc_file(table: pa.Table = PUSHDOWN_DATA) -> bytes: sink = pa.BufferOutputStream() with pa.ipc.new_file(sink, table.schema) as writer: