feat!: refactor ConnectionInternal into a Database trait (#2067)

This opens up the door for more custom database implementations than the
two we have today. The biggest change should be inivisble:
`ConnectionInternal` has been renamed to `Database`, made public, and
refactored

However, there are a few breaking changes. `data_storage_version` and
`enable_v2_manifest_paths` have been moved from options on
`create_table` to options for the database which are now set via
`storage_options`.

Before:
```
db = connect(uri)
tbl = db.create_table("my_table", data, data_storage_version="legacy", enable_v2_manifest_paths=True)
```

After:
```
db = connect(uri, storage_options={
  "new_table_enable_v2_manifest_paths": "true",
  "new_table_data_storage_version": "legacy"
})
tbl = db.create_table("my_table", data)
```

BREAKING CHANGE: the data_storage_version, enable_v2_manifest_paths
options have moved from options to create_table to storage_options.
BREAKING CHANGE: the use_legacy_format option has been removed,
data_storage_version has replaced it for some time now

nodejs/__test__/connection.test.ts

@@ -17,14 +17,14 @@ describe("when connecting", () => {
   it("should connect", async () => {
     const db = await connect(tmpDir.name);
     expect(db.display()).toBe(
-      `NativeDatabase(uri=${tmpDir.name}, read_consistency_interval=None)`,
+      `ListingDatabase(uri=${tmpDir.name}, read_consistency_interval=None)`,
     );
   });
 
   it("should allow read consistency interval to be specified", async () => {
     const db = await connect(tmpDir.name, { readConsistencyInterval: 5 });
     expect(db.display()).toBe(
-      `NativeDatabase(uri=${tmpDir.name}, read_consistency_interval=5s)`,
+      `ListingDatabase(uri=${tmpDir.name}, read_consistency_interval=5s)`,
     );
   });
 });
@@ -96,14 +96,15 @@ describe("given a connection", () => {
     const data = [...Array(10000).keys()].map((i) => ({ id: i }));
 
     // Create in v1 mode
-    let table = await db.createTable("test", data, { useLegacyFormat: true });
+    let table = await db.createTable("test", data, {
+      storageOptions: { newTableDataStorageVersion: "legacy" },
+    });
 
     const isV2 = async (table: Table) => {
       const data = await table
         .query()
         .limit(10000)
         .toArrow({ maxBatchLength: 100000 });
-      console.log(data.batches.length);
       return data.batches.length < 5;
     };
 
@@ -122,7 +123,7 @@ describe("given a connection", () => {
     const schema = new Schema([new Field("id", new Float64(), true)]);
 
     table = await db.createEmptyTable("test_v2_empty", schema, {
-      useLegacyFormat: false,
+      storageOptions: { newTableDataStorageVersion: "stable" },
     });
 
     await table.add(data);

nodejs/lancedb/connection.ts

@@ -52,6 +52,8 @@ export interface CreateTableOptions {
    *
    * The default is `stable`.
    * Set to "legacy" to use the old format.
+   *
+   * @deprecated Pass `new_table_data_storage_version` to storageOptions instead.
    */
   dataStorageVersion?: string;
 
@@ -61,17 +63,11 @@ export interface CreateTableOptions {
    * turning this on will make the dataset unreadable for older versions
    * of LanceDB (prior to 0.10.0). To migrate an existing dataset, instead
    * use the {@link LocalTable#migrateManifestPathsV2} method.
+   *
+   * @deprecated Pass `new_table_enable_v2_manifest_paths` to storageOptions instead.
    */
   enableV2ManifestPaths?: boolean;
 
-  /**
-   * If true then data files will be written with the legacy format
-   *
-   * The default is false.
-   *
-   * Deprecated. Use data storage version instead.
-   */
-  useLegacyFormat?: boolean;
   schema?: SchemaLike;
   embeddingFunction?: EmbeddingFunctionConfig;
 }
@@ -256,6 +252,28 @@ export class LocalConnection extends Connection {
     return new LocalTable(innerTable);
   }
 
+  private getStorageOptions(
+    options?: Partial<CreateTableOptions>,
+  ): Record<string, string> | undefined {
+    if (options?.dataStorageVersion !== undefined) {
+      if (options.storageOptions === undefined) {
+        options.storageOptions = {};
+      }
+      options.storageOptions["newTableDataStorageVersion"] =
+        options.dataStorageVersion;
+    }
+
+    if (options?.enableV2ManifestPaths !== undefined) {
+      if (options.storageOptions === undefined) {
+        options.storageOptions = {};
+      }
+      options.storageOptions["newTableEnableV2ManifestPaths"] =
+        options.enableV2ManifestPaths ? "true" : "false";
+    }
+
+    return cleanseStorageOptions(options?.storageOptions);
+  }
+
   async createTable(
     nameOrOptions:
       | string
@@ -272,20 +290,14 @@ export class LocalConnection extends Connection {
       throw new Error("data is required");
     }
     const { buf, mode } = await parseTableData(data, options);
-    let dataStorageVersion = "stable";
-    if (options?.dataStorageVersion !== undefined) {
-      dataStorageVersion = options.dataStorageVersion;
-    } else if (options?.useLegacyFormat !== undefined) {
-      dataStorageVersion = options.useLegacyFormat ? "legacy" : "stable";
-    }
+
+    const storageOptions = this.getStorageOptions(options);
 
     const innerTable = await this.inner.createTable(
       nameOrOptions,
       buf,
       mode,
-      cleanseStorageOptions(options?.storageOptions),
-      dataStorageVersion,
-      options?.enableV2ManifestPaths,
+      storageOptions,
     );
 
     return new LocalTable(innerTable);
@@ -309,22 +321,14 @@ export class LocalConnection extends Connection {
       metadata = registry.getTableMetadata([embeddingFunction]);
     }
 
-    let dataStorageVersion = "stable";
-    if (options?.dataStorageVersion !== undefined) {
-      dataStorageVersion = options.dataStorageVersion;
-    } else if (options?.useLegacyFormat !== undefined) {
-      dataStorageVersion = options.useLegacyFormat ? "legacy" : "stable";
-    }
-
+    const storageOptions = this.getStorageOptions(options);
     const table = makeEmptyTable(schema, metadata);
     const buf = await fromTableToBuffer(table);
     const innerTable = await this.inner.createEmptyTable(
       name,
       buf,
       mode,
-      cleanseStorageOptions(options?.storageOptions),
-      dataStorageVersion,
-      options?.enableV2ManifestPaths,
+      storageOptions,
     );
     return new LocalTable(innerTable);
   }

nodejs/src/connection.rs

@@ -2,17 +2,15 @@
 // SPDX-FileCopyrightText: Copyright The LanceDB Authors
 
 use std::collections::HashMap;
-use std::str::FromStr;
 
+use lancedb::database::CreateTableMode;
 use napi::bindgen_prelude::*;
 use napi_derive::*;
 
-use crate::error::{convert_error, NapiErrorExt};
+use crate::error::NapiErrorExt;
 use crate::table::Table;
 use crate::ConnectionOptions;
-use lancedb::connection::{
-    ConnectBuilder, Connection as LanceDBConnection, CreateTableMode, LanceFileVersion,
-};
+use lancedb::connection::{ConnectBuilder, Connection as LanceDBConnection};
 use lancedb::ipc::{ipc_file_to_batches, ipc_file_to_schema};
 
 #[napi]
@@ -124,8 +122,6 @@ impl Connection {
         buf: Buffer,
         mode: String,
         storage_options: Option<HashMap<String, String>>,
-        data_storage_options: Option<String>,
-        enable_v2_manifest_paths: Option<bool>,
     ) -> napi::Result<Table> {
         let batches = ipc_file_to_batches(buf.to_vec())
             .map_err(|e| napi::Error::from_reason(format!("Failed to read IPC file: {}", e)))?;
@@ -137,14 +133,6 @@ impl Connection {
                 builder = builder.storage_option(key, value);
             }
         }
-        if let Some(data_storage_option) = data_storage_options.as_ref() {
-            builder = builder.data_storage_version(
-                LanceFileVersion::from_str(data_storage_option).map_err(|e| convert_error(&e))?,
-            );
-        }
-        if let Some(enable_v2_manifest_paths) = enable_v2_manifest_paths {
-            builder = builder.enable_v2_manifest_paths(enable_v2_manifest_paths);
-        }
         let tbl = builder.execute().await.default_error()?;
         Ok(Table::new(tbl))
     }
@@ -156,8 +144,6 @@ impl Connection {
         schema_buf: Buffer,
         mode: String,
         storage_options: Option<HashMap<String, String>>,
-        data_storage_options: Option<String>,
-        enable_v2_manifest_paths: Option<bool>,
     ) -> napi::Result<Table> {
         let schema = ipc_file_to_schema(schema_buf.to_vec()).map_err(|e| {
             napi::Error::from_reason(format!("Failed to marshal schema from JS to Rust: {}", e))
@@ -172,14 +158,6 @@ impl Connection {
                 builder = builder.storage_option(key, value);
             }
         }
-        if let Some(data_storage_option) = data_storage_options.as_ref() {
-            builder = builder.data_storage_version(
-                LanceFileVersion::from_str(data_storage_option).map_err(|e| convert_error(&e))?,
-            );
-        }
-        if let Some(enable_v2_manifest_paths) = enable_v2_manifest_paths {
-            builder = builder.enable_v2_manifest_paths(enable_v2_manifest_paths);
-        }
         let tbl = builder.execute().await.default_error()?;
         Ok(Table::new(tbl))
     }