You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ar...@apache.org on 2024/03/25 17:17:18 UTC

(superset) 03/10: Table with Time Comparison:

This is an automated email from the ASF dual-hosted git repository.

arivero pushed a commit to branch table-time-comparison
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 0726994292dd9eff1fbd4b3955b6adf1c1ac1b53
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Fri Mar 1 16:56:08 2024 +0100

    Table with Time Comparison:
    
    - Stop using the new instant_time_comparison_info as a direct propery of queryObject. Put it in the extras
---
 .../superset-ui-core/src/query/types/Query.ts         |  4 +---
 .../plugins/plugin-chart-table/src/buildQuery.ts      |  4 ++--
 superset/charts/schemas.py                            | 19 +++++++++++--------
 superset/common/query_object.py                       |  4 ----
 superset/connectors/sqla/models.py                    | 11 +++++++----
 tests/unit_tests/connectors/test_models.py            | 15 +++++++++------
 tests/unit_tests/queries/query_object_test.py         |  1 -
 7 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts
index db3a090dd6..83b90253eb 100644
--- a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts
+++ b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts
@@ -71,6 +71,7 @@ export type QueryObjectExtras = Partial<{
   where?: string;
   /** Instant Time Comparison */
   instant_time_comparison_range?: string;
+  instant_time_comparison_info?: QueryObjectInstantTimeComparisonInfo;
 }>;
 
 export type ResidualQueryObjectData = {
@@ -156,9 +157,6 @@ export interface QueryObject
   series_columns?: QueryFormColumn[];
   series_limit?: number;
   series_limit_metric?: Maybe<QueryFormMetric>;
-
-  /** Instant Time Comparison */
-  instant_time_comparison_info?: QueryObjectInstantTimeComparisonInfo;
 }
 
 export interface QueryContext {
diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
index 9e93c268a4..f892d2fe94 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
@@ -183,8 +183,8 @@ const buildQuery: BuildQuery<TableChartFormData> = (
 
     // Customize the query for time comparison
     if (canUseTimeComparison) {
-      queryObject = {
-        ...queryObject,
+      queryObject.extras = {
+        ...queryObject.extras,
         instant_time_comparison_info: {
           range: timeComparison,
           filter:
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index 34731af571..b5563a3446 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -1008,6 +1008,17 @@ class ChartDataExtrasSchema(Schema):
         },
         allow_none=True,
     )
+    instant_time_comparison_info = fields.Nested(
+        # TODO: The instant_time_comparison_range must be deleted in favor of this one
+        # once we have the DATEDIFF in place. Keeping for backward compatibility in the
+        # meantime.
+        InstantTimeComparisonInfoSchema,
+        metadata={
+            "description": "Extra parameters to use instant time comparison"
+            " with JOINs using a single query"
+        },
+        allow_none=True,
+    )
 
 
 class AnnotationLayerSchema(Schema):
@@ -1360,14 +1371,6 @@ class ChartDataQueryObjectSchema(Schema):
         fields.String(),
         allow_none=True,
     )
-    instant_time_comparison_info = fields.Nested(
-        InstantTimeComparisonInfoSchema,
-        metadata={
-            "description": "Extra parameters to use instant time comparison"
-            " with JOINs using a single query"
-        },
-        allow_none=True,
-    )
 
 
 class ChartDataQueryContextSchema(Schema):
diff --git a/superset/common/query_object.py b/superset/common/query_object.py
index 77f3a08ce8..5109c465e0 100644
--- a/superset/common/query_object.py
+++ b/superset/common/query_object.py
@@ -107,7 +107,6 @@ class QueryObject:  # pylint: disable=too-many-instance-attributes
     time_shift: str | None
     time_range: str | None
     to_dttm: datetime | None
-    instant_time_comparison_info: dict[str, Any] | None
 
     def __init__(  # pylint: disable=too-many-locals
         self,
@@ -133,7 +132,6 @@ class QueryObject:  # pylint: disable=too-many-instance-attributes
         series_limit_metric: Metric | None = None,
         time_range: str | None = None,
         time_shift: str | None = None,
-        instant_time_comparison_info: dict[str, Any] | None = None,
         **kwargs: Any,
     ):
         self._set_annotation_layers(annotation_layers)
@@ -163,7 +161,6 @@ class QueryObject:  # pylint: disable=too-many-instance-attributes
         self.time_offsets = kwargs.get("time_offsets", [])
         self.inner_from_dttm = kwargs.get("inner_from_dttm")
         self.inner_to_dttm = kwargs.get("inner_to_dttm")
-        self.instant_time_comparison_info = instant_time_comparison_info
         self._rename_deprecated_fields(kwargs)
         self._move_deprecated_extra_fields(kwargs)
 
@@ -338,7 +335,6 @@ class QueryObject:  # pylint: disable=too-many-instance-attributes
             "series_limit_metric": self.series_limit_metric,
             "to_dttm": self.to_dttm,
             "time_shift": self.time_shift,
-            "instant_time_comparison_info": self.instant_time_comparison_info,
         }
         return query_object_dict
 
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index b6e14ce62e..6259628d5b 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -1515,14 +1515,17 @@ class SqlaTable(
         mutate: bool = True,
     ) -> QueryStringExtended:
         # So we don't mutate the original query_obj
-        query_obj_clone = copy.copy(query_obj)
-        instant_time_comparison_info = query_obj.get("instant_time_comparison_info")
-        query_obj_clone.pop("instant_time_comparison_info", None)
-        sqlaq = self.get_sqla_query(**query_obj_clone)
+        sqlaq = self.get_sqla_query(**query_obj)
         sql = self.database.compile_sqla_query(sqlaq.sqla_query)
         sql = self._apply_cte(sql, sqlaq.cte)
         sql = sqlparse.format(sql, reindent=True)
 
+        query_obj_clone = copy.copy(query_obj)
+        query_object_extras: dict[str, Any] = query_obj.get("extras", {})
+        instant_time_comparison_info = query_object_extras.get(
+            "instant_time_comparison_info", {}
+        )
+
         if mutate:
             sql = self.mutate_query_from_config(sql)
 
diff --git a/tests/unit_tests/connectors/test_models.py b/tests/unit_tests/connectors/test_models.py
index cf179c9dfa..1a176f4860 100644
--- a/tests/unit_tests/connectors/test_models.py
+++ b/tests/unit_tests/connectors/test_models.py
@@ -73,7 +73,13 @@ class TestInstantTimeComparisonQueryGeneration:
         return {
             "apply_fetch_values_predicate": False,
             "columns": ["name"],
-            "extras": {"having": "", "where": ""},
+            "extras": {
+                "having": "",
+                "where": "",
+                "instant_time_comparison_info": {
+                    "range": "y",
+                },
+            },
             "filter": [
                 {"op": "TEMPORAL_RANGE", "val": "1984-01-01 : 2024-02-14", "col": "ds"}
             ],
@@ -132,9 +138,6 @@ class TestInstantTimeComparisonQueryGeneration:
                     "sqlExpression": None,
                 },
             ],
-            "instant_time_comparison_info": {
-                "range": "y",
-            },
         }
 
     @with_feature_flags(CHART_PLUGINS_EXPERIMENTAL=True)
@@ -256,7 +259,7 @@ class TestInstantTimeComparisonQueryGeneration:
     def test_creates_query_without_time_comparison(session: Session):
         table = TestInstantTimeComparisonQueryGeneration.base_setup(session)
         query_obj = TestInstantTimeComparisonQueryGeneration.generate_base_query_obj()
-        query_obj["instant_time_comparison_info"] = None
+        query_obj["extras"]["instant_time_comparison_info"] = None
         str = table.get_query_str_extended(query_obj)
         expected_str = """
             SELECT name AS name,
@@ -279,7 +282,7 @@ class TestInstantTimeComparisonQueryGeneration:
     def test_creates_time_comparison_query_custom_filters(session: Session):
         table = TestInstantTimeComparisonQueryGeneration.base_setup(session)
         query_obj = TestInstantTimeComparisonQueryGeneration.generate_base_query_obj()
-        query_obj["instant_time_comparison_info"] = {
+        query_obj["extras"]["instant_time_comparison_info"] = {
             "range": "c",
             "filter": {
                 "op": "TEMPORAL_RANGE",
diff --git a/tests/unit_tests/queries/query_object_test.py b/tests/unit_tests/queries/query_object_test.py
index f90ab8255d..81a654653f 100644
--- a/tests/unit_tests/queries/query_object_test.py
+++ b/tests/unit_tests/queries/query_object_test.py
@@ -47,7 +47,6 @@ def test_default_query_object_to_dict():
         "granularity": None,
         "inner_from_dttm": None,
         "inner_to_dttm": None,
-        "instant_time_comparison_info": None,
         "is_rowcount": False,
         "is_timeseries": False,
         "metrics": None,