fix: prevent default catalog leak into catalog-unsupported gateways#5752
Merged
themisvaltinos merged 1 commit intoSQLMesh:mainfrom Mar 30, 2026
Conversation
mday-io
force-pushed
the
mday-io/fix/prevent-default-catalog-leak-to-unsupported-gateways
branch
from
6870d2e to
90a3566
Compare
Collaborator
themisvaltinos
left a comment
themisvaltinos
left a comment
There was a problem hiding this comment.
thank you one minor adjustment needed in the code otherwise lgtm. and if possible add also an extra test for the blueprint case I explained
tests/core/test_catalog_leak.py
Outdated
| from sqlmesh.core.model.definition import load_sql_based_models | ||
|
|
||
|
|
||
| def test_default_catalog_not_leaked_to_unsupported_gateway(): |
Collaborator
There was a problem hiding this comment.
nit: if you could move all these tests under core/test_model.py instead of having them in a separate file
Contributor
Author
There was a problem hiding this comment.
Comment on lines
2062
to
2097
| module_path: Path = Path(), | ||
| dialect: DialectType = None, | ||
| default_catalog_per_gateway: t.Optional[t.Dict[str, str]] = None, | ||
| **loader_kwargs: t.Any, | ||
| ) -> t.List[Model]: | ||
| model_blueprints: t.List[Model] = [] | ||
| for blueprint in _extract_blueprints(blueprints, path): | ||
| blueprint_variables = _extract_blueprint_variables(blueprint, path) | ||
|
|
||
| if gateway: | ||
| rendered_gateway = render_expression( | ||
| expression=exp.maybe_parse(gateway, dialect=dialect), | ||
| module_path=module_path, | ||
| macros=loader_kwargs.get("macros"), | ||
| jinja_macros=loader_kwargs.get("jinja_macros"), | ||
| path=path, | ||
| dialect=dialect, | ||
| default_catalog=loader_kwargs.get("default_catalog"), | ||
| blueprint_variables=blueprint_variables, | ||
| ) | ||
| gateway_name = rendered_gateway[0].name if rendered_gateway else None | ||
| else: | ||
| gateway_name = None | ||
|
|
||
| if ( | ||
| default_catalog_per_gateway | ||
| and gateway_name | ||
| and (catalog := default_catalog_per_gateway.get(gateway_name)) is not None | ||
| ): | ||
| loader_kwargs["default_catalog"] = catalog | ||
| if default_catalog_per_gateway and gateway_name: | ||
| catalog = default_catalog_per_gateway.get(gateway_name) | ||
| if catalog is not None: | ||
| loader_kwargs["default_catalog"] = catalog | ||
| else: | ||
| # Gateway exists but has no entry in the dict (e.g., catalog-unsupported | ||
| # engines like ClickHouse). Clear the default catalog so the global | ||
| # default from the primary gateway doesn't leak into this model's name. | ||
| loader_kwargs["default_catalog"] = None | ||
|
|
||
| model_blueprints.append( | ||
| loader( |
Collaborator
There was a problem hiding this comment.
we also need to adapt in line 2068 to keep the original catalog between blueprint iterations since otherwise it might get overwritten and permanently lost:
original_default_catalog = loader_kwargs.get("default_catalog")
for blueprint in _extract_blueprints(blueprints, path):
loader_kwargs["default_catalog"] = original_default_catalogso keeping the catalog and restore it at the start of each iteration to prevent cross contamination between blueprints in the case that a model uses the gateway controlled by blueprint:
MODEL (
name @{blueprint}.my_model,
kind FULL,
gateway @{gw},
blueprints (
(blueprint := my_schema, gw := clickhouse_gw),
(blueprint := other_schema, gw := default_gw)
),
);
SELECT 1 AS id
Contributor
Author
There was a problem hiding this comment.
good catch. applied the update:
https://github.com/SQLMesh/sqlmesh/compare/90a3566963ac634a0d15507996d245efec6da525..f6188e7ab4238e3fa9c050f7554cfa5b9aaac717
Fixes SQLMesh#5748 When the default gateway has a default catalog set (e.g., Trino with catalog: example_catalog), that catalog was silently prepended to model names targeting secondary gateways that do not support catalogs (e.g., ClickHouse), causing UnsupportedCatalogOperationError at evaluation time. The per-gateway catalog dict omits catalog-unsupported gateways entirely, so the model loader could not distinguish "no catalog" from "not checked" and fell through to the global default. This change explicitly sets default_catalog to None when a gateway is known but absent from the dict. Additionally preserves the original default_catalog across blueprint iterations to prevent cross-contamination when blueprints target different gateways. Signed-off-by: Michael Day <michael@mday.io>
mday-io
force-pushed
the
mday-io/fix/prevent-default-catalog-leak-to-unsupported-gateways
branch
from
90a3566 to
f6188e7
Compare
themisvaltinos
approved these changes
Mar 30, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
When a model's gateway is not in
default_catalog_per_gateway, explicitly setsdefault_catalogtoNoneinstead of letting the global default leak through. Implements the fix indefinition.pyas suggested by @themisvaltinos in #5750 (comment).Test Plan:
tests/core/test_catalog_leak.pytest_gateway_macro,test_gateway_macro_jinja,test_gateway_python_model,test_gateway_specific_render,test_model_cache_gatewaytest_multi_virtual_layer,test_multiple_gatewaysruff checkandruff formatpassChecklist:
make stylemake fast-test(and it has pre-existing failures).