Conversation
…erts Signed-off-by: mday-io <mdaytn@gmail.com>
Strip the injected virtual catalog from every rendered expression rather than only queries and inserts, so CTAS and DELETE statements no longer leak it. Drop the fallback to the configured virtual_catalog when none was injected, matching the other catalog strip sites, and only copy the expression when a reference actually needs rewriting. Consolidate the tests: the unconfigured-catalog test never enabled the feature, and the fallback test only covered the removed fallback. Signed-off-by: mday-io <mdaytn@gmail.com>
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
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 ClickHouse gateway is used alongside catalog-aware gateways, SQLMesh injects a virtual catalog (
__<gateway>__by default, or the configuredvirtual_catalog) so every model has a 3-part name. ClickHouse doesn't support catalogs, so that prefix has to be stripped before SQL reaches the engine. Some DDL paths already handled this (create_view,alter_table,delete_from's target table,create_schema), but the queries themselves still carried the catalog. As a result,INSERT ... SELECT,CREATE TABLE ... AS SELECTand subqueries insideDELETE ... WHEREall failed.This PR strips the virtual catalog centrally in
ClickhouseEngineAdapter._to_sql, so it covers every rendered statement:_default_cataloghas its catalog removed.INSERT ... VALUESstatements aren't deep-copied for no reason.inject_virtual_catalog()has run. This matches the existing strip sites, so a ClickHouse-only project withvirtual_catalogconfigured behaves exactly as before.Test Plan
Added to
tests/core/engine_adapter/test_clickhouse.py:test_virtual_catalog_stripped_from_execute_queries(parametrized) covers INSERT ... SELECT, a plain SELECT, and a JOIN that mixes the virtual catalog with another catalog plus a string literal containing the catalog name. It also checks that the input expression isn't mutated.test_virtual_catalog_stripped_from_ctas_and_deletechecks the exact SQL emitted byctas()and bydelete_from()with a subquery.Both tests fail without the fix. All 38 tests in
test_clickhouse.pypass, and so doesruff/mypyon the changed files.Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO