From 2ef4c5fdf7f75ad83b674803019750bf0dfe3a44 Mon Sep 17 00:00:00 2001 From: Jayashanker Padishala Date: Thu, 23 Jul 2026 03:33:42 -0700 Subject: [PATCH 1/2] fix(sql): don't classify objects with permissive __getattr__ as connections Instantiating any pytorch-ignite Metric hung the executing cell forever: ignite metrics implement __getattr__ to return a lazy MetricsLambda for any attribute name (with a traceback.format_stack call on each access), so they pass every getattr-based duck-typing check. The post-execution data-source hook then classified the metric as an ADBC connection and adbc_get_objects()/read_all() walked into the lazy object graph and never returned real data. Probe one attribute that no real connection defines; if the object fabricates a callable for it, reject it in AdbcDBAPIEngine.is_compatible and DBAPIEngine.is_compatible. Genuine connection proxies that forward __getattr__ to an underlying connection are unaffected, since the probe does not exist there either. Fixes #10213 --- marimo/_sql/engines/adbc.py | 6 ++++++ marimo/_sql/engines/dbapi.py | 7 ++++++- marimo/_sql/engines/types.py | 22 ++++++++++++++++++++++ tests/_sql/test_adbc.py | 14 ++++++++++++++ tests/_sql/test_dbapi.py | 14 ++++++++++++++ tests/_sql/test_get_engines.py | 16 ++++++++++++++++ 6 files changed, 78 insertions(+), 1 deletion(-) diff --git a/marimo/_sql/engines/adbc.py b/marimo/_sql/engines/adbc.py index 7133f6e3443..4f790c892f9 100644 --- a/marimo/_sql/engines/adbc.py +++ b/marimo/_sql/engines/adbc.py @@ -17,6 +17,7 @@ InferenceConfig, SQLConnection, default_inference_config, + fabricates_attributes, ) from marimo._sql.utils import convert_to_output, is_cheap_dialect from marimo._types.ids import VariableName @@ -461,6 +462,11 @@ def is_compatible(var: Any) -> bool: if var_type_name == "ibis.common.deferred.Deferred": return False + # Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics) + # pass any getattr-based check and hang catalog introspection. #10213 + if fabricates_attributes(var): + return False + try: # First, validate the connection-level surface area to avoid # accidentally classifying regular DB-API connections as ADBC. diff --git a/marimo/_sql/engines/dbapi.py b/marimo/_sql/engines/dbapi.py index 3ed9d8ac1a6..e1d6277f25a 100644 --- a/marimo/_sql/engines/dbapi.py +++ b/marimo/_sql/engines/dbapi.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING, Any, Protocol from marimo import _loggers -from marimo._sql.engines.types import QueryEngine +from marimo._sql.engines.types import QueryEngine, fabricates_attributes from marimo._sql.utils import convert_to_output LOGGER = _loggers.marimo_logger() @@ -112,6 +112,11 @@ def is_compatible(var: Any) -> bool: if var_type_name == "ibis.common.deferred.Deferred": return False + # Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics) + # pass any getattr-based check and hang catalog introspection. #10213 + if fabricates_attributes(var): + return False + try: required_methods = ["cursor", "commit", "rollback", "close"] has_required_methods = all( diff --git a/marimo/_sql/engines/types.py b/marimo/_sql/engines/types.py index a2b493f034f..c80f3172e0a 100644 --- a/marimo/_sql/engines/types.py +++ b/marimo/_sql/engines/types.py @@ -23,6 +23,28 @@ NO_SCHEMA_NAME = "" +# Attribute that no real database connection defines; used to detect objects +# that fabricate attributes via a permissive __getattr__. +_MISSING_ATTRIBUTE_PROBE = "_marimo_does_not_exist_" + + +def fabricates_attributes(var: Any) -> bool: + """Whether ``var`` fabricates attributes via a permissive ``__getattr__``. + + Some objects return a new object for *any* attribute name (for example + pytorch-ignite metrics, which build lazy ``MetricsLambda`` graphs). Such + objects pass every ``getattr``-based duck-typing check, and catalog + introspection on them can hang. Genuine connection proxies that forward + ``__getattr__`` to an underlying connection are unaffected, since the + probe attribute does not exist there either. + """ + try: + return getattr(var, _MISSING_ATTRIBUTE_PROBE, None) is not None + except Exception: + # A __getattr__ that raises something other than AttributeError is + # not a usable connection either. + return True + @dataclass class InferenceConfig(ABC): diff --git a/tests/_sql/test_adbc.py b/tests/_sql/test_adbc.py index 7e6c2812393..5f526091826 100644 --- a/tests/_sql/test_adbc.py +++ b/tests/_sql/test_adbc.py @@ -425,6 +425,20 @@ def test_adbc_execute_native_returns_arrow_table(monkeypatch) -> None: assert cursor.did_close is True +def test_adbc_is_compatible_rejects_fabricated_attributes() -> None: + # Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics) + # resolve any attribute name, so they pass getattr-based duck typing + # and catalog introspection on them hangs. See #10213. + class FabricatingAttributes: + def __getattr__(self, name: str) -> object: + def _lazy(*_args: object, **_kwargs: object) -> object: + return FabricatingAttributes() + + return _lazy + + assert AdbcDBAPIEngine.is_compatible(FabricatingAttributes()) is False + + def test_adbc_is_compatible_does_not_create_cursor() -> None: conn = FakeAdbcDbApiConnection( cursor=FakeAdbcDbApiCursor(description=None), diff --git a/tests/_sql/test_dbapi.py b/tests/_sql/test_dbapi.py index 1922b50e15a..4aac1aab695 100644 --- a/tests/_sql/test_dbapi.py +++ b/tests/_sql/test_dbapi.py @@ -58,6 +58,20 @@ def test_is_compatible() -> None: assert not isinstance(engine, EngineCatalog) +def test_is_compatible_rejects_fabricated_attributes() -> None: + # Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics) + # resolve any attribute name, so they pass getattr-based duck typing + # and catalog introspection on them hangs. See #10213. + class FabricatingAttributes: + def __getattr__(self, name: str) -> object: + def _lazy(*_args: object, **_kwargs: object) -> object: + return FabricatingAttributes() + + return _lazy + + assert not DBAPIEngine.is_compatible(FabricatingAttributes()) + + def test_execute_native(dbapi_engine: DBAPIEngine) -> None: with patch.object( dbapi_engine, "sql_output_format", return_value="native" diff --git a/tests/_sql/test_get_engines.py b/tests/_sql/test_get_engines.py index 80f9515fe18..342c218ffc8 100644 --- a/tests/_sql/test_get_engines.py +++ b/tests/_sql/test_get_engines.py @@ -482,3 +482,19 @@ def test_variables_without_datasource_engine() -> None: variables = [("deferred_for_test", deferred_for_test)] engines = get_engines_from_variables(variables) assert not engines + + +def test_fabricated_attributes_not_treated_as_engine() -> None: + # Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics) + # resolve any attribute name, so they pass getattr-based duck typing + # and catalog introspection on them hangs. See #10213. + class FabricatingAttributes: + def __getattr__(self, name: str) -> object: + def _lazy(*_args: object, **_kwargs: object) -> object: + return FabricatingAttributes() + + return _lazy + + variables = [(VariableName("metric"), FabricatingAttributes())] + engines = get_engines_from_variables(variables) + assert not engines From 5d65a9298f3ebef50d450547465a1a2e467252fb Mon Sep 17 00:00:00 2001 From: Jayashanker Padishala Date: Thu, 23 Jul 2026 07:22:29 -0700 Subject: [PATCH 2/2] fix: use single backticks in docstring per style guide --- marimo/_sql/engines/types.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/marimo/_sql/engines/types.py b/marimo/_sql/engines/types.py index c80f3172e0a..a4604cdfa7d 100644 --- a/marimo/_sql/engines/types.py +++ b/marimo/_sql/engines/types.py @@ -29,13 +29,13 @@ def fabricates_attributes(var: Any) -> bool: - """Whether ``var`` fabricates attributes via a permissive ``__getattr__``. + """Whether `var` fabricates attributes via a permissive `__getattr__`. Some objects return a new object for *any* attribute name (for example - pytorch-ignite metrics, which build lazy ``MetricsLambda`` graphs). Such - objects pass every ``getattr``-based duck-typing check, and catalog + pytorch-ignite metrics, which build lazy `MetricsLambda` graphs). Such + objects pass every `getattr`-based duck-typing check, and catalog introspection on them can hang. Genuine connection proxies that forward - ``__getattr__`` to an underlying connection are unaffected, since the + `__getattr__` to an underlying connection are unaffected, since the probe attribute does not exist there either. """ try: