Skip to content

fix: respect disable_restatement remove intervals across env#3838

Merged
eakmanrq merged 1 commit intomainfrom
eakmanrq/erspect_disable_restate_clear_intervals
Feb 14, 2025
Merged

fix: respect disable_restatement remove intervals across env#3838
eakmanrq merged 1 commit intomainfrom
eakmanrq/erspect_disable_restate_clear_intervals

Conversation

@eakmanrq
Copy link
Contributor

@eakmanrq eakmanrq commented Feb 14, 2025

Prior to this PR, when we removed intervals across environments, introduced in #3511, we would also remove intervals for downstream snapshots even if they had disable_restatement enabled. One tricky part is that the model, in most cases, would not be backfilled in that plan and this would not be communicated in console output. A case where it would be shown is if that downstream model was also an upstream model of one of the restated models or it's downstream models. I get that was confusing - lets give this example:

A -> B -> C

If B had disable_restatement, and A was being restated, then B would both have it's intervals removed and it would be backfilled in the plan since it is upstream of C. But if C had disable_restatement then it would have it's intervals removed but would not be backfilled in the plan since it was not upstream of anything else.

Going through the effort of explaining this in case someone needs to try to reproduce this in the future because I kept trying patterns where just C was disabled and couldn't repro.

This PR fixes the issue by doing the following:

  • EvaluatablePlan now includes disabled_restatement_models
    • This is needed since EvaluatablePlan doesn't include any Snapshots, just SnapshotTableInfo, and SnapshotTableInfo just includes the model kind type but not addition info for the given kind.
  • When finding downstream snapshots across environments, we now filter out the snapshots that have disabled_restatement_models

We are basing what is disabled or not on what is in prod but it seems the worst case scenario for this is that a dev snapshot could be restated when it wasn't intended but that seems fine since it is just in development.

@eakmanrq eakmanrq force-pushed the eakmanrq/erspect_disable_restate_clear_intervals branch from 8d2cfef to f782b15 Compare February 14, 2025 00:32
@eakmanrq eakmanrq enabled auto-merge (squash) February 14, 2025 00:33
@eakmanrq eakmanrq force-pushed the eakmanrq/erspect_disable_restate_clear_intervals branch 2 times, most recently from 1f09db7 to c03ff88 Compare February 14, 2025 00:55
@eakmanrq eakmanrq force-pushed the eakmanrq/erspect_disable_restate_clear_intervals branch from c03ff88 to 62ebf2d Compare February 14, 2025 01:06
@eakmanrq eakmanrq merged commit a91ddd5 into main Feb 14, 2025
21 checks passed
@eakmanrq eakmanrq deleted the eakmanrq/erspect_disable_restate_clear_intervals branch February 14, 2025 01:16
justinjoseph89 added a commit to trygforsikring/sqlmesh that referenced this pull request Feb 20, 2025
* Chore: replace mysql-connector-python with pymysql (TobikoData#3788)

* feat: don't force db connect if using serverless (TobikoData#3786)

* Feat: Add 'auditing' and 'promoting' runtime stages (TobikoData#3791)

* fix: databricks set default catalog for both connections (TobikoData#3793)

* Docs: Prod Env Observability updates (TobikoData#3790)

Co-authored-by: Trey Spiller <1831878+treysp@users.noreply.github.com>
Co-authored-by: Trey Spiller <treyspiller@gmail.com>

* Docs: update jinja gateway variable syntax (TobikoData#3795)

* Fix: Only promote all snapshots if the target environment expired (TobikoData#3797)

* Chore: remove [missing dates] from CLI model backfills title (TobikoData#3796)

* Fix: Make sure that physical tables exist for promoted snapshots (TobikoData#3798)

* fix: db properly support `with_log_level` (TobikoData#3799)

* fix: run integration tests in session (TobikoData#3801)

* Fix: specify init duckdb database so quickstart works (TobikoData#3800)

* Docs: clarify in faq that run ignores local definitions  (TobikoData#3794)

* feat: add airflow operator and hook for ClickHouse (TobikoData#3699)

* Feat: Allow macros in python model properties (TobikoData#3740)

* Docs: update CLI quickstart's CLI output (TobikoData#3802)

* Fix: Ensure diff sample displays when table names have been uppercased (TobikoData#3806)

* Fix: update github links for pdoc api docs (TobikoData#3807)

* Feat: Run audits as non-blocking on dev previews (TobikoData#3809)

* Chore!: bump sqlglot to v26.6.0 (TobikoData#3810)

* Feat!: Adjust physical_properties evaluation and add macro to resolve physical table names (TobikoData#3772)

* Chore: Use 'dev' suffix instead of 'temp' for non-deployable physical tables (TobikoData#3803)

* Fix!: Propagate the grain attribute when converting dbt models (TobikoData#3804)

* Docs(dagster): Fix installation instructions (TobikoData#3812)

* fix: add kwargs to build_table_properties_exp (TobikoData#3817)

* Feat: include alter statements in destructive change error message (TobikoData#3805)

* Dagster demo and tutorial video (TobikoData#3822)

* fix: databricks with_log_level (TobikoData#3823)

* Feat: improve audit error message formatting (TobikoData#3818)

* Fix(postgres): Quote role names if required when running SET ROLE on cursor init (TobikoData#3825)

* Fix: handle quoted projects properly in bigquery adapter (TobikoData#3820)

* Chore: Fix flaky test (TobikoData#3828)

* docs: adding self hosted executor docs (TobikoData#3816)

* Revert "Fix(postgres): Quote role names if required when running SET ROLE on cursor init" (TobikoData#3834)

* Fix: Don't fail because of an unrestorable change if the target model is forward-only (TobikoData#3835)

* Chore: improve metadata update console printing (TobikoData#3824)

* fix: respect disable_restatement remove intervals across env (TobikoData#3838)

* fix: respect disable restate dev unpaused snapshots (TobikoData#3840)

* Feat: add [WARNING] to console warning messages (TobikoData#3826)

* Feat: Extend support of project wide model properties (TobikoData#3832)

* Fix: Streamline execution of pre- / post- statements when creating a physical table (TobikoData#3837)

* fix: signals that return an empty list are considered ready (TobikoData#3841)

* Chore: Add the ingress section to the self-hosted executor docs

* Fix: Snapshots promoted in prod shouldn't be restated in dev (TobikoData#3843)

* Fix: Inference of python model names from the file system (TobikoData#3844)

* feat: add support for datetime/date in macros (TobikoData#3846)

* fix: only expand restatement range if incremental (TobikoData#3847)

* feat: improve gcp postgres connection config options (TobikoData#3842)

* Fix: Pin PyGithub to 2.5.0 so tests can run (TobikoData#3851)

* Fix: Warn when SQLMesh automatically adjusts a restatement range to cover the whole model (TobikoData#3850)

* fix!: normalize catalog override name (TobikoData#3849)

* Feat: allow different warning messages for logger and console (TobikoData#3836)

* Chore: fix audit doc typos (TobikoData#3856)

* Add airflow tutorial video (TobikoData#3860)

* Feat!: add model blueprinting (TobikoData#3848)

* Fix: Make sure that pending restatement intervals are always recorded last during compaction (TobikoData#3862)

* Chore: Break up the plan_builder method in Context (TobikoData#3867)

* fix: give better error message when object not serializable (TobikoData#3861)

* Feat: make date_spine macro less strict to allow dynamic behavior (TobikoData#3865)

* Add Tcloud SSO docs (TobikoData#3827)

* Fix: Unexpected backfill of a parent of a changed forward-only child when the child runs before the parent (TobikoData#3871)

* Feat: Allow CustomKind subclasses for custom materializations (TobikoData#3863)

* Chore: consolidate `make install-*` (TobikoData#3874)

---------

Co-authored-by: Trey Spiller <1831878+treysp@users.noreply.github.com>
Co-authored-by: Ryan Eakman <6326532+eakmanrq@users.noreply.github.com>
Co-authored-by: Iaroslav Zeigerman <zeigerman.ia@gmail.com>
Co-authored-by: Marisa Smith <66020208+mesmith027@users.noreply.github.com>
Co-authored-by: Trey Spiller <treyspiller@gmail.com>
Co-authored-by: Anton Parfenyuk <spar9a@gmail.com>
Co-authored-by: Themis Valtinos <73662635+themisvaltinos@users.noreply.github.com>
Co-authored-by: Gerasimos Kounadis <gerasimoskounadis@gmail.com>
Co-authored-by: Erin Drummond <erin.dru@gmail.com>
Co-authored-by: Jo <46752250+georgesittas@users.noreply.github.com>
Co-authored-by: Sung Won Chung <sungwonchung3@gmail.com>
Co-authored-by: Ben <9087625+benfdking@users.noreply.github.com>
Co-authored-by: Philippe Laflamme <484152+plaflamme@users.noreply.github.com>
Co-authored-by: Toby Mao <toby.mao@gmail.com>
Co-authored-by: Afzal Jasani <amj355@nyu.edu>
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.

2 participants