fixup! Add validation rule for SCD query with no time

metricflow-semantics/metricflow_semantics/query/issues/parsing/scd_requires_metric_time.py

@@ -11,23 +11,24 @@
     MetricFlowQueryResolutionIssue,
 )
 from metricflow_semantics.query.resolver_inputs.base_resolver_inputs import MetricFlowQueryResolverInput
+from metricflow_semantics.specs.instance_spec import InstanceSpec
 
 
 @dataclass(frozen=True)
 class ScdRequiresMetricTimeIssue(MetricFlowQueryResolutionIssue):
     """Describes an issue with a query that includes a SCD group by but does not include metric_time."""
 
-    scd_qualified_names: Sequence[str]
+    scds_in_query: Sequence[InstanceSpec]
 
     @override
     def ui_description(self, associated_input: MetricFlowQueryResolverInput) -> str:
-        dim_str = ", ".join(self.scd_qualified_names)
+        dim_str = ", ".join(scd.qualified_name for scd in self.scds_in_query)
         return (
-            f"Your query contains the Slowly Changing Dimensions (SCDs): [{dim_str}]. "
-            "A query containing SCDs must also contain the metric_time dimension in order "
-            "to join the SCD table to the valid time range. Please add metric_time "
-            "to the query and try again. If you're using agg_time_dimension, use "
-            "metric_time instead."
+            "Your query contains the group bys which are SCDs or contain SCDs in the "
+            f"join path: [{dim_str}]. \n\nA query containing SCDs must also contain the "
+            "metric_time dimension in order to join the SCD table to the valid time "
+            "range. Please add metric_time to the query and try again. If you're "
+            "using agg_time_dimension, use metric_time instead."
         )
 
     @override
@@ -36,16 +37,16 @@ def with_path_prefix(self, path_prefix: MetricFlowQueryResolutionPath) -> ScdReq
             issue_type=self.issue_type,
             parent_issues=self.parent_issues,
             query_resolution_path=self.query_resolution_path.with_path_prefix(path_prefix),
-            scd_qualified_names=self.scd_qualified_names,
+            scds_in_query=self.scds_in_query,
         )
 
     @staticmethod
     def from_parameters(  # noqa: D102
-        scd_qualified_names: Sequence[str], query_resolution_path: MetricFlowQueryResolutionPath
+        scds_in_query: Sequence[InstanceSpec], query_resolution_path: MetricFlowQueryResolutionPath
     ) -> ScdRequiresMetricTimeIssue:
         return ScdRequiresMetricTimeIssue(
             issue_type=MetricFlowQueryIssueType.ERROR,
             parent_issues=(),
             query_resolution_path=query_resolution_path,
-            scd_qualified_names=scd_qualified_names,
+            scds_in_query=scds_in_query,
         )

metricflow-semantics/metricflow_semantics/query/validation_rules/metric_time_requirements.py

@@ -29,14 +29,15 @@
     ResolverInputForQuery,
 )
 from metricflow_semantics.query.validation_rules.base_validation_rule import PostResolutionQueryValidationRule
+from metricflow_semantics.specs.instance_spec import InstanceSpec
 from metricflow_semantics.specs.time_dimension_spec import TimeDimensionSpec
 
 
 @dataclass(frozen=True)
 class QueryItemsAnalysis:
     """Contains data about which items a query contains."""
 
-    scds: Sequence[str]
+    scds: Sequence[InstanceSpec]
     has_metric_time: bool
     has_agg_time_dimension: bool
 
@@ -67,7 +68,7 @@ def _get_query_items_analysis(
     ) -> QueryItemsAnalysis:
         has_agg_time_dimension = False
         has_metric_time = False
-        scds: List[str] = []
+        scds: List[InstanceSpec] = []
 
         valid_agg_time_dimension_specs = self._manifest_lookup.metric_lookup.get_valid_agg_time_dimensions_for_metric(
             metric_reference
@@ -83,7 +84,7 @@ def _get_query_items_analysis(
                 has_agg_time_dimension = True
 
             scd_matches = group_by_item_input.spec_pattern.match(scd_specs)
-            scds.extend(match.qualified_name for match in scd_matches)
+            scds.extend(scd_matches)
 
         return QueryItemsAnalysis(
             scds=scds,
@@ -109,7 +110,7 @@ def validate_metric_in_resolution_dag(
         if len(query_items_analysis.scds) > 0 and not query_items_analysis.has_metric_time:
             issues = issues.add_issue(
                 ScdRequiresMetricTimeIssue.from_parameters(
-                    scd_qualified_names=query_items_analysis.scds,
+                    scds_in_query=query_items_analysis.scds,
                     query_resolution_path=resolution_path,
                 )
             )