Feature/schema ownership#5806
Draft
gabepesco wants to merge 5 commits into
Draft
Conversation
Signed-off-by: Gabe Pesco <PescoG@medinsight.milliman.com>
Add `physical_owner` field to `OwnershipConfig` so that SQLMesh__* physical tables get ownership applied at creation time, not just virtual-layer views and schemas. * `OwnershipConfig.physical_owner` - optional plain string, no env-pattern matching needed * `EngineAdapterBase.alter_table_owner` - no-op default * `SparkEngineAdapter.alter_table_owner` - ALTER TABLE ... OWNER TO ... with backtick-quoting * `SnapshotEvaluator.create_snapshot` - calls alter_table_owner after _execute_create (skips ViewKind) * `SnapshotEvaluator.create` / `create_physical_schemas` - accept and thread owner param * `BuiltInPlanEvaluator` - resolves physical_owner in both PhysicalLayerSchemaCreation and PhysicalLayerUpdate stages Signed-off-by: Gabe Pesco <gabe.pesco@milliman.com> Signed-off-by: Gabe Pesco <PescoG@medinsight.milliman.com>
Run ruff-format on changed files. Remove DuckDB attach assertions accidentally included in test_config_ownership_defaults_to_empty — those are already covered by test_load_duckdb_attach_config. Signed-off-by: Gabe Pesco <gabe.pesco@milliman.com> Signed-off-by: Gabe Pesco <PescoG@medinsight.milliman.com>
Signed-off-by: Gabe Pesco <PescoG@medinsight.milliman.com>
- OwnershipConfig gains environment_owner_resolver and physical_owner_resolver callable fields for cases where the owner principal must be resolved at plan execution time (e.g. dynamic SPN identity via adapter.current_user()). - Callable resolvers take precedence over the static mapping/string fields when both are set. - resolve_owner() and resolve_physical_owner() now accept the active EngineAdapter so callables can query the connection. - Add current_user() to the base EngineAdapter (SELECT CURRENT_USER()). - Replace the getattr/isinstance guard in BuiltInSchedulerConfig with OwnershipConfig.is_active. - Update all resolve_owner/resolve_physical_owner call sites in BuiltInPlanEvaluator to thread the adapter through. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.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
SQLMesh creates schemas, views, and tables owned by whichever user/SPN identity executes the plan. The
after_allhook is the only existing correction point, but it only fires on successful completion — a failed or partial run leaves objects owned by the executing principal. Two cleanup paths are affected:sushi__dev_alice.*): the janitor drops dev environment schemas on expiry. Unity Catalog rejects the DROP if the janitor doesn't own the schema.sqlmesh__sushi.*): the janitor drops orphaned versioned tables aftersnapshot_ttl(default 1 week). Same ownership problem.This PR adds an
ownershipconfig block that sets owner principals at object-creation time, so even a partially-completed run leaves objects in a manageable state.Config API:
How it works:
OwnershipConfigmaps environment name regex patterns to owner principals for the virtual layer (first match wins, same style asenvironment_catalog_mapping), plus a plainphysical_ownerstring for the physical layer.SnapshotEvaluator.promote()and applied immediately after eachCREATE SCHEMAandCREATE OR REPLACE VIEW.physical_owneris passed intoSnapshotEvaluator.create()andcreate_physical_schemas(), applied after each physical table and schema is created.ViewKindsnapshots are skipped (their ownership is managed by the virtual layer).alter_schema_owner(),alter_view_owner(), andalter_table_owner()are no-ops on the base adapter.SparkEngineAdapter(covering Databricks Unity Catalog) issues the correspondingALTER ... OWNER TO ...DDL with properly backtick-quoted principals, handling names that contain:and@(Unity Catalog group format).Test plan:
OwnershipConfigcoveringresolve_owner()pattern matching, first-match-wins ordering, case sensitivity, dict deserialization,update_withmerge behavior,physical_ownerfield, and defaults.SnapshotEvaluator.promote()verifyingalter_schema_ownerandalter_view_ownerare called correctly when owner is set and skipped when omitted.SnapshotEvaluator.create()andcreate_physical_schemas()verifyingalter_table_ownerandalter_schema_ownerfire for non-view tables and are suppressed forViewKindsnapshots and when no owner is configured.SparkEngineAdapterverifying correct backtick-quotedALTER SCHEMA/VIEW/TABLE ... OWNER TO ...DDL, including principals with:and@. Includes a base adapter no-op test confirmingDuckDBEngineAdapteremits noOWNER-related SQL.@pytest.mark.slow) running a full prod + dev plan/apply cycle with ownership config against DuckDB, verifying no errors and correct schema creation.Note: the
ALTER ... OWNER TOcontract on a live Databricks Unity Catalog connection is not tested here — the SQL generation tests verify the correct DDL is produced.Checklist:
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO