fix(python): improve type stubs, data= cleanup safety, and exception specificity

- Update type stubs to document typed exceptions (ParseError, ReaderError,
  etc.) instead of generic ValueError
- Guard data= cleanup against destroying pre-existing tables by checking
  table existence before registration
- Pass replace as keyword arg in bridge register_data_on_reader for
  compatibility with custom readers that omit the parameter
- Tighten test assertions to expect specific exception types
- Remove unused try_native_readers! macro

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: cpsievert

ggsql-python/python/ggsql/_ggsql.pyi

@@ -31,7 +31,7 @@ class DuckDBReader:
 
     Raises
     ------
-    ValueError
+    ReaderError
         If the connection string is invalid or the database cannot be opened.
     """
 
@@ -51,7 +51,7 @@ class DuckDBReader:
 
         Raises
         ------
-        ValueError
+        ReaderError
             If the SQL is invalid or execution fails.
         """
         ...
@@ -74,7 +74,7 @@ class DuckDBReader:
 
         Raises
         ------
-        ValueError
+        ReaderError
             If registration fails or the table name is invalid.
         """
         ...
@@ -89,7 +89,7 @@ class DuckDBReader:
 
         Raises
         ------
-        ValueError
+        ReaderError
             If the table was not registered or unregistration fails.
         """
         ...
@@ -122,9 +122,12 @@ class DuckDBReader:
 
         Raises
         ------
-        ValueError
-            If the query syntax is invalid, has no VISUALISE clause, or
-            SQL execution fails.
+        ParseError
+            If the query syntax is invalid.
+        ValidationError
+            If the query has no VISUALISE clause or fails semantic checks.
+        ReaderError
+            If SQL execution fails.
         """
         ...
 
@@ -154,7 +157,7 @@ class VegaLiteWriter:
 
         Raises
         ------
-        ValueError
+        WriterError
             If rendering fails.
         """
         ...
@@ -388,7 +391,7 @@ def validate(query: str) -> Validated:
 
     Raises
     ------
-    ValueError
+    ParseError
         If validation fails unexpectedly (syntax errors are captured in
         the returned ``Validated`` object, not raised).
     """
@@ -425,7 +428,11 @@ def execute(
 
     Raises
     ------
-    ValueError
-        If parsing, validation, or SQL execution fails.
+    ParseError
+        If the query syntax is invalid.
+    ValidationError
+        If semantic validation fails.
+    ReaderError
+        If SQL execution fails.
     """
     ...

ggsql-python/src/lib.rs

@@ -187,24 +187,6 @@ impl Reader for PyReaderBridge {
     }
 }
 
-// ============================================================================
-// Native Reader Detection Macro
-// ============================================================================
-
-/// Macro to try native readers and fall back to bridge.
-/// Adding new native readers = add to the macro invocation list.
-macro_rules! try_native_readers {
-    ($query:expr, $reader:expr, $($native_type:ty),*) => {{
-        $(
-            if let Ok(native) = $reader.downcast::<$native_type>() {
-                return native.borrow().inner.execute($query)
-                    .map(|s| PySpec { inner: s })
-                    .map_err(ggsql_err_to_py);
-            }
-        )*
-    }};
-}
-
 // ============================================================================
 // PyDuckDBReader
 // ============================================================================
@@ -378,8 +360,16 @@ impl PyDuckDBReader {
 }
 
 impl PyDuckDBReader {
-    /// Register DataFrames from a Python dict. Returns list of registered names for cleanup.
-    /// This is a private Rust helper, not exposed to Python.
+    /// Check whether a table already exists in the reader.
+    fn table_exists(&self, name: &str) -> bool {
+        // A lightweight probe: try to select zero rows from the table.
+        self.inner
+            .execute_sql(&format!("SELECT 1 FROM {name} LIMIT 0"))
+            .is_ok()
+    }
+
+    /// Register DataFrames from a Python dict. Returns list of *newly created*
+    /// table names for cleanup (pre-existing tables are not tracked).
     fn register_data_dict(
         &self,
         py: Python<'_>,
@@ -388,11 +378,14 @@ impl PyDuckDBReader {
         let mut names = Vec::new();
         for (key, value) in data.iter() {
             let name: String = key.extract()?;
+            let existed = self.table_exists(&name);
             let df = py_to_polars(py, &value)?;
             self.inner
                 .register(&name, df, true)
                 .map_err(ggsql_err_to_py)?;
-            names.push(name);
+            if !existed {
+                names.push(name);
+            }
         }
         Ok(names)
     }
@@ -846,6 +839,15 @@ fn execute(py: Python<'_>, query: &str, reader: &Bound<'_, PyAny>, data: Option<
 
 /// Register DataFrames from a Python dict onto a Python reader object.
 /// Returns list of registered names for cleanup.
+/// Check whether a table exists via a Python reader's execute_sql method.
+fn reader_table_exists(reader: &Bound<'_, PyAny>, name: &str) -> bool {
+    reader
+        .call_method1("execute_sql", (format!("SELECT 1 FROM {name} LIMIT 0"),))
+        .is_ok()
+}
+
+/// Register DataFrames from a Python dict onto a Python reader object.
+/// Returns list of *newly created* table names for cleanup.
 fn register_data_on_reader(
     py: Python<'_>,
     reader: &Bound<'_, PyAny>,
@@ -854,10 +856,15 @@ fn register_data_on_reader(
     let mut names = Vec::new();
     for (key, value) in data.iter() {
         let name: String = key.extract()?;
+        let existed = reader_table_exists(reader, &name);
         let df = py_to_polars(py, &value)?;
         let py_df = polars_to_py(py, &df)?;
-        reader.call_method("register", (&name, py_df, true), None)?;
-        names.push(name);
+        let kwargs = PyDict::new(py);
+        kwargs.set_item("replace", true)?;
+        reader.call_method("register", (&name, py_df), Some(&kwargs))?;
+        if !existed {
+            names.push(name);
+        }
     }
     Ok(names)
 }

ggsql-python/tests/test_ggsql.py

@@ -699,20 +699,20 @@ def test_execute_with_data_cleans_up(self):
             data={"temp": df},
         )
         # Table should be cleaned up — querying it should fail
-        with pytest.raises((ggsql.ReaderError, ValueError)):
+        with pytest.raises(ggsql.ReaderError):
             reader.execute_sql("SELECT * FROM temp")
 
     def test_execute_with_data_cleans_up_on_error(self):
         """DataFrames are unregistered even if execution fails."""
         reader = ggsql.DuckDBReader("duckdb://memory")
         df = pl.DataFrame({"x": [1, 2, 3], "y": [10, 20, 30]})
-        with pytest.raises((ggsql.ParseError, ggsql.ValidationError, ValueError)):
+        with pytest.raises(ggsql.ParseError):
             reader.execute(
                 "SELECT * FROM temp VISUALISE DRAW not_a_geom",
                 data={"temp": df},
             )
         # Table should still be cleaned up
-        with pytest.raises((ggsql.ReaderError, ValueError)):
+        with pytest.raises(ggsql.ReaderError):
             reader.execute_sql("SELECT * FROM temp")
 
     def test_execute_without_data_still_works(self):
@@ -730,6 +730,24 @@ def test_execute_with_empty_data(self):
         )
         assert spec.metadata()["rows"] == 1
 
+    def test_execute_with_data_preserves_preexisting_table(self):
+        """data= does not unregister a table that existed before the call."""
+        reader = ggsql.DuckDBReader("duckdb://memory")
+        existing = pl.DataFrame({"x": [1, 2], "y": [10, 20]})
+        reader.register("mytable", existing)
+
+        # Pass same name via data= — should replace temporarily, then NOT unregister
+        override = pl.DataFrame({"x": [3, 4, 5], "y": [30, 40, 50]})
+        spec = reader.execute(
+            "SELECT * FROM mytable VISUALISE x, y DRAW point",
+            data={"mytable": override},
+        )
+        assert spec.metadata()["rows"] == 3
+
+        # The pre-existing table should still be queryable (not unregistered)
+        result = reader.execute_sql("SELECT * FROM mytable")
+        assert result.shape[0] > 0
+
     def test_module_execute_with_data(self):
         """Module-level execute() also supports data= parameter."""
         reader = ggsql.DuckDBReader("duckdb://memory")