fix: enforce view stable row ids at creation, not by rollback

The request-level new_table_enable_stable_row_ids option outranks a
connection-level override, so a view can no longer be created unstable and
then destructively dropped by name in cleanup. What made this possible: the
new-table keys used to ride in the store options and fork a fresh store
connection for the request; prepare_write_params now strips them, since
they are creation configuration, not store credentials. The manifest is
still verified as defense against Database impls that ignore the option,
but a failure only reports -- nothing is rolled back.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

rust/lancedb/src/database/listing.rs

@@ -819,6 +819,33 @@ impl ListingDatabase {
             .clone()
             .unwrap_or_default();
 
+        // The new-table keys are creation configuration, not store options:
+        // strip them so a request carrying only them keeps the connection's
+        // store instead of forking a new one.
+        if let Some(store_params) = write_params.store_params.as_mut() {
+            let mut options = store_params.storage_options().cloned().unwrap_or_default();
+            for key in [
+                OPT_NEW_TABLE_STORAGE_VERSION,
+                OPT_NEW_TABLE_V2_MANIFEST_PATHS,
+                OPT_NEW_TABLE_ENABLE_STABLE_ROW_IDS,
+            ] {
+                options.remove(key);
+            }
+            let provider = store_params
+                .storage_options_accessor
+                .as_ref()
+                .and_then(|accessor| accessor.provider().cloned());
+            store_params.storage_options_accessor = match (options.is_empty(), provider) {
+                (true, None) => None,
+                (_, Some(provider)) => Some(Arc::new(
+                    StorageOptionsAccessor::with_initial_and_provider(options, provider),
+                )),
+                (false, None) => Some(Arc::new(StorageOptionsAccessor::with_static_options(
+                    options,
+                ))),
+            };
+        }
+
         // Only modify the storage options if we actually have something to
         // inherit. There is a difference between storage_options=None and
         // storage_options=Some({}). Using storage_options=None will cause the

rust/lancedb/src/materialized_view.rs

@@ -28,6 +28,7 @@ use lance_datafusion::planner::Planner;
 use serde::{Deserialize, Serialize};
 
 use crate::connection::Connection;
+use crate::database::listing::OPT_NEW_TABLE_ENABLE_STABLE_ROW_IDS;
 use crate::table::refresh::quote_identifier;
 use crate::table::{Table, WriteOptions};
 use crate::{Error, Result};
@@ -463,16 +464,14 @@ impl CreateMaterializedViewBuilder {
             definition_to_metadata(&definition)?,
         )]);
         let schema = Arc::new(ArrowSchema::new_with_metadata(fields, metadata));
-        // Stable row ids so the view can itself source another view.
+        // Stable row ids so the view can itself source another view. The
+        // request-level option outranks any connection-level override; the
+        // manifest is still verified because another Database impl may
+        // ignore the option, but nothing is rolled back on failure.
         let table = self
             .connection
             .create_empty_table(&self.name, schema)
-            .write_options(WriteOptions {
-                lance_write_params: Some(lance::dataset::WriteParams {
-                    enable_stable_row_ids: true,
-                    ..Default::default()
-                }),
-            })
+            .storage_option(OPT_NEW_TABLE_ENABLE_STABLE_ROW_IDS, "true")
             .execute()
             .await?;
         let stable = match table.as_native() {
@@ -480,13 +479,11 @@ impl CreateMaterializedViewBuilder {
             None => false,
         };
         if !stable {
-            let _ = self.connection.drop_table(&self.name, &[]).await;
-            return Err(Error::InvalidInput {
+            return Err(Error::Runtime {
                 message: format!(
-                    "view '{}' would be created without stable row ids: the \
-                     connection's new_table_enable_stable_row_ids setting \
-                     overrides view creation; remove it or use a connection \
-                     without it",
+                    "view '{}' was created without stable row ids: the database \
+                     ignored the creation option; the table remains and is not \
+                     usable as a materialized view",
                     self.name
                 ),
             });
@@ -882,42 +879,41 @@ mod tests {
         assert_eq!(definition.filter.as_deref(), Some("age >= 18"));
     }
 
-    /// A connection override that would create the view without stable row
-    /// ids fails loudly instead of committing an unchainable view.
+    /// The creation option outranks a connection configured to create
+    /// unstable tables: the view still gets stable row ids, on the same
+    /// store (no fork -- the table must be reachable through the
+    /// connection afterwards).
     #[tokio::test]
-    async fn test_conflicting_connection_override_is_refused() {
-        let dir = tempfile::tempdir().unwrap();
-        let uri = dir.path().to_str().unwrap();
-        let stable_conn = connect(uri).execute().await.unwrap();
-        let batch = record_batch!(("x", Int32, [1])).unwrap();
-        stable_conn
-            .create_table("src", batch)
-            .write_options(stable_row_ids())
-            .execute()
-            .await
-            .unwrap();
-
-        let unstable_conn = connect(uri)
+    async fn test_view_is_stable_despite_connection_override() {
+        let conn = connect("memory://")
             .storage_options([("new_table_enable_stable_row_ids", "false")])
             .execute()
             .await
             .unwrap();
-        let err = unstable_conn
+        let batch = record_batch!(("x", Int32, [1])).unwrap();
+        conn.create_table("src", batch)
+            .storage_option("new_table_enable_stable_row_ids", "true")
+            .execute()
+            .await
+            .unwrap();
+
+        let view = conn
             .create_materialized_view("v", "src")
             .execute()
             .await
-            .unwrap_err();
-        assert!(
-            matches!(err, Error::InvalidInput { message } if message.contains("stable row ids"))
-        );
-        assert!(
-            !unstable_conn
-                .table_names()
-                .execute()
-                .await
-                .unwrap()
-                .contains(&"v".to_string())
-        );
+            .unwrap();
+        let stable = view
+            .table()
+            .as_native()
+            .unwrap()
+            .dataset
+            .get()
+            .await
+            .unwrap()
+            .manifest
+            .uses_stable_row_ids();
+        assert!(stable);
+        conn.open_materialized_view("v").await.unwrap();
     }
 
     /// A committed filter has to be usable as a predicate.