rusackas opened a new pull request, #45507:
URL: https://github.com/apache/superset/pull/45507
### SUMMARY
`oceanbase_py`'s SQLAlchemy dialect
(`oceanbase_py.sqlalchemy.dialect.OceanBaseDialect`) has two bugs that break it
under SQLAlchemy 2.0, which this repo requires:
1. `has_table()` and `get_columns()` both pass a raw SQL string straight to
`Connection.execute()` (e.g. `connection.execute(f"DESCRIBE {full_name}")`),
which SQLAlchemy 2.0 rejects outright with `ObjectNotExecutableError`. Every
*other* raw-SQL method in the same dialect module (`get_schema_names`,
`get_table_names`, `get_view_names`) already uses
`connection.exec_driver_sql(...)` instead, so this looks like an isolated
oversight in just these two methods rather than a deliberate choice.
2. `has_table()` also never catches the error `DESCRIBE` raises for a table
that doesn't exist (MySQL/OceanBase error 1146), so even with (1) fixed it can
only ever return `True` — raising instead of returning `False` for exactly the
case `MetaData.create_all()`'s default `checkfirst=True` exists to handle.
`oceanbase_py` has no public source repository to open an issue or PR
against (checked all three released PyPI versions' metadata, the listed
author's GitHub repos, and the official `oceanbase` org — nothing), so this
patches `OceanBaseEngineSpec`'s own dialect import at module-load time instead,
the same pattern `superset/db_engine_specs/databricks.py`'s
`monkeypatch_dialect()` already uses for a similar bug in PyHive's
`HiveDialect`. Filing the fix upstream by email as well, since there's no repo
to PR against.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only fix, no UI change.
### TESTING INSTRUCTIONS
- `tests/unit_tests/db_engine_specs/test_oceanbase.py` adds unit coverage
for the patch itself, stubbing a minimal fake `oceanbase_py` package
(reproducing the exact original bug) so it runs in the base dev environment
without the real driver installed — `oceanbase_py` pins `sqlalchemy<2`,
incompatible with this repo's own pin, so it's never part of the base install
(see `requirements/development.in`). Covers: the patched methods call
`exec_driver_sql` and never `execute`, `has_table()` returns `False` (not an
exception) for a missing table, `get_columns()` raises `NoSuchTableError` for a
missing table and returns correctly parsed columns otherwise, and
`monkeypatch_dialect()` is a no-op (doesn't raise) when the driver isn't
installed at all.
- `tests/testcontainers/db_engine_specs/test_oceanbase.py` (real OceanBase
instance via testcontainers, nightly-only CI job) previously carried its own
local copy of this exact patch to make its own tests pass; that's now removed
since importing `OceanBaseEngineSpec` applies the fix automatically, and those
tests now exercise the production code path instead of a test-only workaround.
- Ran locally: `pytest tests/unit_tests/db_engine_specs/test_oceanbase.py
-v` (19 passed), `ruff check`/`ruff format --check`, and `pre-commit run` on
all three changed files (mypy, ruff, pylint, database engine spec metadata
validation) — all pass.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
