Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat!: forward-only changes schema modification check #2576

Merged
merged 46 commits into from May 21, 2024

Conversation

treysp
Copy link
Contributor

@treysp treysp commented May 7, 2024

Some forward-only changes require schema modifications that drop existing data (e.g., changing a column type from str to int). If a user inadvertently makes such a change, they will lose existing data. This feature identifies destructive changes and optionally warns/errors before they occur to avoid accidental data loss.

The feature introduces a new model kind property on_destructive_change with valid values allow, warn, and error (default is error). The on_destructive_change property only applies to incremental models and can be specified as a model default.

Users may want to temporarily allow a destructive change to a forward-only model. The feature introduces to the plan command a new --allow-destructive-model selector to temporarily allow destructive changes.

Of note

  • Defaults

    • on_destructive_change is a property of a model kind
      • Only incremental models are checked for destructive changes, and their default is ERROR
      • Non-incremental models default is ALLOW
    • Users may specify a default in the model_defaults configuration key - it will only apply to incremental models, as non-incremental models are always allowed
  • Plan time check

    • Uses SchemaDiffer and engine-specific rules to determine whether a change is destructive.
    • We pass the current engine's SchemaDiffer into the plan builder to perform the comparison.
    • Evaluating a change requires that columns_to_types is known.
    • Evaluates both direct and indirect changes to a model. A complete evaluation requires knowing columns_to_types for the model and all directly modified ancestors.
  • Run time check

    • Uses the set of Alters passed from SchemaDiffer to alter_table() to determine whether a change is destructive.
  • dbt

    • We map dbt's incremental model on_schema_change setting to sqlmesh on_destructive_change
    • on_schema_change values are mapped to on_destructive_change as follows:
      • ignore -> WARN
      • append_new_columns -> WARN
      • fail -> ERROR
      • sync_all_columns -> ALLOW

@treysp treysp force-pushed the trey/forward-only-additive-only branch from 0a0eb2e to d60e09e Compare May 7, 2024 17:12
sqlmesh/core/model/kind.py Outdated Show resolved Hide resolved
sqlmesh/core/model/kind.py Outdated Show resolved Hide resolved
@treysp treysp changed the title Feat: forward-only changes schema modification check Feat!: forward-only changes schema modification check May 8, 2024
@treysp treysp force-pushed the trey/forward-only-additive-only branch 5 times, most recently from 50d801e to 9215b86 Compare May 15, 2024 16:09
@treysp treysp force-pushed the trey/forward-only-additive-only branch from 1815d71 to 7d10bec Compare May 15, 2024 16:52
@treysp treysp force-pushed the trey/forward-only-additive-only branch from b602daa to 0995c7e Compare May 21, 2024 22:52
@treysp treysp merged commit 397bc56 into main May 21, 2024
15 checks passed
@treysp treysp deleted the trey/forward-only-additive-only branch May 21, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants