From 43a869007147f292cd1938c556cc3f7712e0cd3a Mon Sep 17 00:00:00 2001 From: Evert Lammerts Date: Fri, 12 Dec 2025 17:49:53 +0100 Subject: [PATCH] Quote view names in unregister --- src/duckdb_py/pyconnection.cpp | 3 ++- tests/fast/api/test_duckdb_connection.py | 32 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/duckdb_py/pyconnection.cpp b/src/duckdb_py/pyconnection.cpp index b88b88ed..11a7ea9d 100644 --- a/src/duckdb_py/pyconnection.cpp +++ b/src/duckdb_py/pyconnection.cpp @@ -1776,7 +1776,8 @@ shared_ptr DuckDBPyConnection::UnregisterPythonObject(const D_ASSERT(py::gil_check()); py::gil_scoped_release release; // FIXME: DROP TEMPORARY VIEW? doesn't exist? - connection.Query("DROP VIEW \"" + name + "\""); + const auto quoted_name = KeywordHelper::WriteOptionallyQuoted(name, '\"'); + connection.Query("DROP VIEW " + quoted_name + ""); registered_objects.erase(name); return shared_from_this(); } diff --git a/tests/fast/api/test_duckdb_connection.py b/tests/fast/api/test_duckdb_connection.py index 4fa32749..246b9d92 100644 --- a/tests/fast/api/test_duckdb_connection.py +++ b/tests/fast/api/test_duckdb_connection.py @@ -313,6 +313,38 @@ def test_unregister_problematic_behavior(self, duckdb_cursor): # This should not have affected the existing view: assert duckdb_cursor.execute("select * from vw").fetchone() == (0,) + def test_unregister_quoted_table_names(self, duckdb_cursor): + """Test that unregister works for quoted tables.""" + rel = duckdb_cursor.sql("select 'test', 'data'") + + table_name = 'test with .s and "s and s' + duckdb_cursor.register(table_name, rel) + duckdb_cursor.unregister(table_name) + + escaped_table_name = table_name.replace('"', '""') + with pytest.raises(duckdb.CatalogException): + duckdb_cursor.sql(f'select * from "{escaped_table_name}"') + + def test_unregister_with_scary_name(self, duckdb_cursor): + """Test that unregister doesn't have side effects.""" + rel = duckdb_cursor.sql("select 'test', 'data'") + + scary_name = 'test";create table foo as select * from range(10);--' + # make sure a view with the name "test" exists + duckdb_cursor.register("test", rel) + duckdb_cursor.register(scary_name, rel) + # try to trick unregister (which uses DROP VIEW) to run another statement + duckdb_cursor.unregister(scary_name) + + # hopefully that didn't happen + with pytest.raises(duckdb.CatalogException): + duckdb_cursor.sql("select * from foo") + + # verify the scary name table was properly unregistered + escaped_scary_name = scary_name.replace('"', '""') + with pytest.raises(duckdb.CatalogException): + duckdb_cursor.sql(f'select * from "{escaped_scary_name}"') + @pytest.mark.parametrize("pandas", [NumpyPandas(), ArrowPandas()]) def test_relation_out_of_scope(self, pandas): def temporary_scope():