Implement state:modified for saved queries (#10295)

.changes/unreleased/Fixes-20240612-152139.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: Implement state:modified for saved queries
+time: 2024-06-12T15:21:39.851426-04:00
+custom:
+  Author: gshank
+  Issue: "10294"

core/dbt/contracts/graph/manifest.py

@@ -1095,6 +1095,7 @@ def from_writable_manifest(cls, writable_manifest: WritableManifest) -> "Manifes
             metrics=cls._map_resources_to_map_nodes(writable_manifest.metrics),
             groups=cls._map_resources_to_map_nodes(writable_manifest.groups),
             semantic_models=cls._map_resources_to_map_nodes(writable_manifest.semantic_models),
+            saved_queries=cls._map_resources_to_map_nodes(writable_manifest.saved_queries),
             selectors={
                 selector_id: selector
                 for selector_id, selector in writable_manifest.selectors.items()

core/dbt/contracts/graph/nodes.py

@@ -1562,7 +1562,6 @@ def same_group(self, old: "SavedQuery") -> bool:
         return self.group == old.group
 
     def same_exports(self, old: "SavedQuery") -> bool:
-        # TODO: This isn't currently used in `same_contents` (nor called anywhere else)
         if len(self.exports) != len(old.exports):
             return False
 
@@ -1592,6 +1591,7 @@ def same_contents(self, old: Optional["SavedQuery"]) -> bool:
             and self.same_label(old)
             and self.same_config(old)
             and self.same_group(old)
+            and self.same_exports(old)
             and True
         )
 

core/dbt/graph/selector_methods.py

@@ -109,7 +109,7 @@ def is_selected_node(fqn: List[str], node_selector: str, is_versioned: bool) ->
 
 
 SelectorTarget = Union[
-    SourceDefinition, ManifestNode, Exposure, Metric, SemanticModel, UnitTestDefinition
+    SourceDefinition, ManifestNode, Exposure, Metric, SemanticModel, UnitTestDefinition, SavedQuery
 ]
 
 
@@ -202,6 +202,7 @@ def all_nodes(
             self.metric_nodes(included_nodes),
             self.unit_tests(included_nodes),
             self.semantic_model_nodes(included_nodes),
+            self.saved_query_nodes(included_nodes),
         )
 
     def configurable_nodes(
@@ -680,7 +681,8 @@ def check_modified_content(
         self, old: Optional[SelectorTarget], new: SelectorTarget, adapter_type: str
     ) -> bool:
         if isinstance(
-            new, (SourceDefinition, Exposure, Metric, SemanticModel, UnitTestDefinition)
+            new,
+            (SourceDefinition, Exposure, Metric, SemanticModel, UnitTestDefinition, SavedQuery),
         ):
             # these all overwrite `same_contents`
             different_contents = not new.same_contents(old)  # type: ignore
@@ -775,6 +777,8 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu
                 previous_node = SemanticModel.from_resource(manifest.semantic_models[unique_id])
             elif unique_id in manifest.unit_tests:
                 previous_node = UnitTestDefinition.from_resource(manifest.unit_tests[unique_id])
+            elif unique_id in manifest.saved_queries:
+                previous_node = SavedQuery.from_resource(manifest.saved_queries[unique_id])
 
             keyword_args = {}
             if checker.__name__ in [

tests/functional/saved_queries/fixtures.py

@@ -3,8 +3,6 @@
 """
 
 saved_queries_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query
     description: "{{ doc('saved_query_description') }}"
@@ -27,8 +25,6 @@
 """
 
 saved_queries_with_defaults_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query
     description: "{{ doc('saved_query_description') }}"
@@ -50,8 +46,6 @@
 """
 
 saved_queries_with_diff_filters_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query_where_list
     description: "{{ doc('saved_query_description') }}"
@@ -83,8 +77,6 @@
 """
 
 saved_query_with_extra_config_attributes_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query
     description: "{{ doc('saved_query_description') }}"
@@ -105,8 +97,6 @@
 """
 
 saved_query_with_export_configs_defined_at_saved_query_level_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query
     description: "{{ doc('saved_query_description') }}"
@@ -131,8 +121,6 @@
 """
 
 saved_query_without_export_configs_defined_yml = """
-version: 2
-
 saved_queries:
   - name: test_saved_query
     description: "{{ doc('saved_query_description') }}"

tests/functional/saved_queries/test_saved_query_parsing.py

@@ -1,3 +1,5 @@
+import os
+import shutil
 from typing import List
 
 import pytest
@@ -6,13 +8,15 @@
 )
 
 from dbt.contracts.graph.manifest import Manifest
-from dbt.tests.util import write_file
+from dbt.tests.util import run_dbt, write_file
 from dbt_common.events.base_types import BaseEvent
 from tests.functional.assertions.test_runner import dbtTestRunner
 from tests.functional.saved_queries.fixtures import (
+    saved_queries_with_defaults_yml,
     saved_queries_with_diff_filters_yml,
     saved_queries_yml,
     saved_query_description,
+    saved_query_with_cache_configs_defined_yml,
 )
 from tests.functional.semantic_models.fixtures import (
     fct_revenue_sql,
@@ -32,6 +36,11 @@ def models(self):
             "docs.md": saved_query_description,
         }
 
+    def copy_state(self):
+        if not os.path.exists("state"):
+            os.makedirs("state")
+        shutil.copyfile("target/manifest.json", "state/manifest.json")
+
     def test_semantic_model_parsing(self, project):
         runner = dbtTestRunner()
         result = runner.invoke(["parse", "--no-partial-parse"])
@@ -52,6 +61,31 @@ def test_semantic_model_parsing(self, project):
         assert saved_query.exports[0].config.export_as == ExportDestinationType.TABLE
         assert saved_query.exports[0].config.schema_name == "my_export_schema_name"
 
+        # Save state
+        self.copy_state()
+        # Nothing has changed, so no state:modified results
+        results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"])
+        assert len(results) == 0
+
+        # Change saved_query
+        write_file(
+            saved_query_with_cache_configs_defined_yml,
+            project.project_root,
+            "models",
+            "saved_queries.yml",
+        )
+        # State modified finds changed saved_query
+        results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"])
+        assert len(results) == 1
+
+        # change exports
+        write_file(
+            saved_queries_with_defaults_yml, project.project_root, "models", "saved_queries.yml"
+        )
+        # State modified finds changed saved_query
+        results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"])
+        assert len(results) == 1
+
     def test_saved_query_error(self, project):
         error_schema_yml = saved_queries_yml.replace("simple_metric", "metric_not_found")
         write_file(error_schema_yml, project.project_root, "models", "saved_queries.yml")

tests/unit/graph/test_selector_methods.py

@@ -588,6 +588,27 @@ def test_select_saved_query_by_tag(manifest: Manifest) -> None:
     search_manifest_using_method(manifest, method, "any_tag")
 
 
+def test_modified_saved_query(manifest: Manifest) -> None:
+    metric = make_metric("test", "my_metric")
+    saved_query = make_saved_query(
+        "pkg",
+        "test_saved_query",
+        "my_metric",
+    )
+    manifest.metrics[metric.unique_id] = metric
+    manifest.saved_queries[saved_query.unique_id] = saved_query
+    # Create PreviousState with a saved query, this deepcopies manifest
+    previous_state = create_previous_state(manifest)
+    method = statemethod(manifest, previous_state)
+
+    # create another metric and add to saved query
+    alt_metric = make_metric("test", "alt_metric")
+    manifest.metrics[alt_metric.unique_id] = alt_metric
+    saved_query.query_params.metrics.append("alt_metric")
+
+    assert search_manifest_using_method(manifest, method, "modified") == {"test_saved_query"}
+
+
 def test_select_unit_test(manifest: Manifest) -> None:
     test_model = make_model("test", "my_model", "select 1 as id")
     unit_test = make_unit_test("test", "my_unit_test", test_model)
@@ -606,8 +627,7 @@ def test_select_unit_test(manifest: Manifest) -> None:
     }
 
 
-@pytest.fixture
-def previous_state(manifest):
+def create_previous_state(manifest):
     writable = copy.deepcopy(manifest).writable_manifest()
     state = PreviousState(
         state_path=Path("/path/does/not/exist"),
@@ -618,6 +638,11 @@ def previous_state(manifest):
     return state
 
 
+@pytest.fixture
+def previous_state(manifest):
+    return create_previous_state(manifest)
+
+
 def add_node(manifest, node):
     manifest.nodes[node.unique_id] = node