eschutho opened a new pull request, #43725:
URL: https://github.com/apache/superset/pull/43725
### SUMMARY
`DatasetRestApi.get_or_create_dataset()` (`POST
/api/v1/dataset/get_or_create/`) has two lookup branches. When the caller omits
`schema`, `DatasetDAO.get_table_by_name()`'s `.one_or_none()` call is wrapped
in `try/except MultipleResultsFound`, returning a clean 400 if more than one
dataset matches (added in commit cb1694575c6, fixing #30377). When the caller
*does* supply `schema`, the parallel call to
`DatasetDAO.get_table_by_catalog_schema_and_name()` — added in that same commit
— has no such guard, so a raw `sqlalchemy.exc.MultipleResultsFound` propagates
uncaught as an opaque 500 instead of a 400.
This is reachable in practice, not just theoretical: `SqlaTable` has
`UniqueConstraint("database_id", "catalog", "schema", "table_name")`, but
standard SQL uniqueness semantics (Postgres, MySQL) treat NULL as distinct from
NULL, so two rows sharing `(database_id, schema, table_name)` with
`catalog=NULL` on both do not violate that constraint. Since multi-catalog
support is recent, plenty of existing datasets have `catalog=None` — see the
method's own comment acknowledging this. If such a duplicate pair exists and a
caller specifies `schema` explicitly, the endpoint 500s instead of returning an
actionable error.
This PR applies the exact same guard the sibling branch already uses,
mirroring it into the schema-scoped path.
This follows the same cleanup pattern as prior PRs in this series, e.g.
https://github.com/apache/superset/pull/42401 and
https://github.com/apache/superset/pull/43433 — a raw exception from a library
call is replaced with the Superset-idiomatic caught-and-classified error,
matching a sibling call site's existing pattern in the same file.
### TESTING INSTRUCTIONS
Added `test_get_or_create_dataset_with_schema_returns_400_when_ambiguous` in
`tests/integration_tests/datasets/api_tests.py`, next to the existing sibling
tests for this endpoint (`test_get_or_create_dataset_disambiguates_by_schema`,
`test_get_or_create_dataset_no_schema_returns_400_when_ambiguous`). It seeds
two datasets sharing `database_id`/`schema`/`table_name` (both `catalog=None`),
then POSTs to `get_or_create/` with that `schema` explicitly set, and asserts a
400 with an actionable message instead of an unhandled 500. Skipped on SQLite
for the same pre-existing reason the sibling tests are (legacy single-column
`table_name` unique constraint in the SQLite test schema). Verified locally
against a fresh SQLite DB using the composite constraint only: confirmed
`MultipleResultsFound` propagates unhandled pre-fix, and the endpoint returns a
clean 400 post-fix.
### 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
--
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]
