From 0cdf7f08cff93fb2cf96769ebfe07e95bdd4d0bc Mon Sep 17 00:00:00 2001 From: Superset Dev Date: Fri, 28 Aug 2026 19:16:33 -0700 Subject: [PATCH] fix(testcontainers): monkeypatch oceanbase_py's has_table instead of skipping checkfirst checkfirst=False only sidestepped create_all()'s own call to has_table(); Inspector.get_columns() (used by OceanBaseEngineSpec.get_columns(), which the second test needs to actually exercise) calls the same broken has_table() internally and hit the identical ObjectNotExecutableError (confirmed on real CI). Every other raw-SQL method in the same dialect module correctly uses connection.exec_driver_sql(...) -- this looks like an isolated oversight in just has_table(), not a deliberate design choice, so this monkeypatches it to do the same thing the rest of the dialect already does, fixing the root cause for both call sites instead of routing around one of them. --- .../db_engine_specs/_pagination.py | 10 ++-- .../db_engine_specs/test_oceanbase.py | 49 +++++++++++++------ 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/tests/testcontainers/db_engine_specs/_pagination.py b/tests/testcontainers/db_engine_specs/_pagination.py index 351b64e31cd..582bce888f5 100644 --- a/tests/testcontainers/db_engine_specs/_pagination.py +++ b/tests/testcontainers/db_engine_specs/_pagination.py @@ -26,12 +26,9 @@ real execution can. Each call site keeps its own test function (and dialect-specific docstring) so failures still report against the right module; this only factors out the identical table setup/assert body, via an optional post-insert hook for -dialects (CrateDB) that need one, an optional extra-table-args hook for +dialects (CrateDB) that need one, and an optional extra-table-args hook for dialects (ClickHouse) whose CREATE TABLE requires a schema item a plain -Column/primary key can't express, and an optional checkfirst override for -a dialect (OceanBase) whose has_table() -- create_all()'s default -checkfirst=True calls it before creating each table -- passes a raw string -to Connection.execute(), which SQLAlchemy 2.0 rejects outright. +Column/primary key can't express. """ from collections.abc import Callable @@ -45,7 +42,6 @@ def assert_paginated_query_returns_correct_rows_in_order( engine: Engine, after_insert: Callable[[Connection], None] | None = None, extra_table_args: tuple[Any, ...] = (), - checkfirst: bool = True, ) -> None: metadata = MetaData() t = SATable( @@ -59,7 +55,7 @@ def assert_paginated_query_returns_correct_rows_in_order( Column("id", Integer, primary_key=True, autoincrement=False), *extra_table_args, ) - metadata.create_all(engine, checkfirst=checkfirst) + metadata.create_all(engine) with engine.begin() as conn: conn.execute(insert(t), [{"id": i} for i in range(10)]) if after_insert is not None: diff --git a/tests/testcontainers/db_engine_specs/test_oceanbase.py b/tests/testcontainers/db_engine_specs/test_oceanbase.py index c1add061ff6..9c5070f318c 100644 --- a/tests/testcontainers/db_engine_specs/test_oceanbase.py +++ b/tests/testcontainers/db_engine_specs/test_oceanbase.py @@ -34,6 +34,17 @@ has a pre-existing, unrelated native-library linking issue against this machine's Homebrew-installed libmysqlclient, and this dialect wasn't pulled/run locally at all given its heavier resource footprint -- CI-only verification, matching the nightly_only gating. + +oceanbase_py.sqlalchemy.dialect.OceanBaseDialect.has_table() -- called by +both create_all()'s default checkfirst=True and by Inspector.get_columns() +internally -- passes a raw string straight to Connection.execute() +(`connection.execute(f"DESCRIBE {full_name}")`), which SQLAlchemy 2.0 +rejects outright (ObjectNotExecutableError, confirmed on real CI). Every +*other* raw-SQL method in the same dialect module correctly uses +`connection.exec_driver_sql(...)` instead -- this looks like an isolated +oversight in just this one method, not a deliberate design choice, so this +test monkeypatches has_table() to do the same thing the rest of the +dialect already does, rather than working around it from the test side. """ from collections.abc import Iterator @@ -47,7 +58,7 @@ from sqlalchemy import ( MetaData, Table as SATable, ) -from sqlalchemy.engine import Engine, URL +from sqlalchemy.engine import Connection, Engine, URL from superset.db_engine_specs.oceanbase import OceanBaseEngineSpec from superset.sql.parse import Table @@ -60,6 +71,7 @@ from ._driver import require_driver # noqa: E402 require_driver("testcontainers.core.container") require_driver("oceanbase_py") +from oceanbase_py.sqlalchemy.dialect import OceanBaseDialect # noqa: E402 from testcontainers.core.container import DockerContainer # noqa: E402 from testcontainers.core.wait_strategies import LogMessageWaitStrategy # noqa: E402 @@ -67,6 +79,26 @@ from ._pagination import ( # noqa: E402 assert_paginated_query_returns_correct_rows_in_order, ) + +def _has_table( + self: OceanBaseDialect, + connection: Connection, + table_name: str, + schema: str | None = None, + **kw: object, +) -> bool: + if schema is None: + schema = self.default_schema_name + quote = self.identifier_preparer.quote_identifier + full_name = quote(table_name) + if schema: + full_name = f"{quote(schema)}.{full_name}" + res = connection.exec_driver_sql(f"DESCRIBE {full_name}") + return res.first() is not None + + +OceanBaseDialect.has_table = _has_table + PORT = 2881 PASSWORD = "pilot" # noqa: S105 -- fixed test-fixture password, not a secret @@ -106,15 +138,8 @@ def test_paginated_query_returns_correct_rows_in_order(engine: Engine) -> None: a real instance. Mocked tests cannot catch a dialect compiling this incorrectly (see apache/superset#42899, where Trino emitted OFFSET before LIMIT) -- only real execution can. - - checkfirst=False: create_all()'s default checkfirst=True calls - oceanbase_py's has_table() first, which passes a raw string straight to - Connection.execute() -- SQLAlchemy 2.0 requires an executable construct - (text(...), select(...), etc.) and rejects a bare string outright - (confirmed on real CI: ObjectNotExecutableError). Safe to skip the - existence check here: each test gets a genuinely fresh container. """ - assert_paginated_query_returns_correct_rows_in_order(engine, checkfirst=False) + assert_paginated_query_returns_correct_rows_in_order(engine) def test_get_columns_maps_native_types(engine: Engine) -> None: @@ -122,10 +147,6 @@ def test_get_columns_maps_native_types(engine: Engine) -> None: OceanBaseEngineSpec.get_columns wraps a real SQLAlchemy Inspector; this exercises that against actual server-reported column metadata rather than a mocked Inspector. - - checkfirst=False: see the note on the previous test -- create_all()'s - default checkfirst=True calls oceanbase_py's has_table(), which is - broken under SQLAlchemy 2.0. """ metadata = MetaData() SATable( @@ -134,7 +155,7 @@ def test_get_columns_maps_native_types(engine: Engine) -> None: Column("id", Integer, primary_key=True), Column("amount", Integer), ) - metadata.create_all(engine, checkfirst=False) + metadata.create_all(engine) inspector = inspect(engine) columns = OceanBaseEngineSpec.get_columns(inspector, Table("pilot_types"))