This is an automated email from the ASF dual-hosted git repository. arivero pushed a commit to branch table-time-comparison-offset in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5819e3a527c67c4357db0cef5e2e8b0d47624d37 Author: Antonio Rivero <[email protected]> AuthorDate: Wed Apr 24 00:01:10 2024 +0200 Table with Time Comparison: - Support textual columns in time_offsets comparisons - Because textual columns have no time grain nor they use xaxis labels all the time, the time_grain and xaxis are not required for the comparison queries that use textual columns only - Remove time grain from time_comparison controls - Stop passing the temporal column if none is inteded to be used --- .../src/sections/timeComparison.tsx | 38 +-------------- .../plugins/plugin-chart-table/src/buildQuery.ts | 22 ++------- superset/common/query_context_processor.py | 57 ++++++++++++++-------- superset/common/utils/time_range_utils.py | 8 ++- 4 files changed, 47 insertions(+), 78 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx index eb8ac80033..f1a4aad6ac 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx @@ -16,16 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { - t, - ComparisonType, - ensureIsArray, - isAdhocColumn, - isPhysicalColumn, -} from '@superset-ui/core'; +import { t, ComparisonType } from '@superset-ui/core'; import { ControlPanelSectionConfig } from '../types'; -import { sharedControls } from '../shared-controls'; export const timeComparisonControls: ControlPanelSectionConfig = { label: t('Time Comparison'), @@ -66,35 +59,6 @@ export const timeComparisonControls: ControlPanelSectionConfig = { }, }, ], - [ - { - name: 'time_comparison_grain_sqla', - config: { - ...sharedControls.time_grain_sqla, - visibility: ({ controls }) => { - // So it doesn't collide with any existing time_grain_sqla control - const dttmLookup = Object.fromEntries( - ensureIsArray(controls?.groupby?.options).map(option => [ - option.column_name, - option.is_dttm, - ]), - ); - - return !ensureIsArray(controls?.groupby.value) - .map(selection => { - if (isAdhocColumn(selection)) { - return true; - } - if (isPhysicalColumn(selection)) { - return !!dttmLookup[selection]; - } - return false; - }) - .some(Boolean); - }, - }, - }, - ], [ { name: 'comparison_type', diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 0cbc9a14f9..321eaf5d96 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -82,11 +82,13 @@ const buildQuery: BuildQuery<TableChartFormData> = ( return buildQueryContext(formDataCopy, baseQueryObject => { let { metrics, orderby = [], columns = [] } = baseQueryObject; + const { extras = {} } = baseQueryObject; let postProcessing: PostProcessingRule[] = []; const timeOffsets = ensureIsArray( isTimeComparison(formData, baseQueryObject) ? formData.time_compare : [], ); - const timeCompareGrainSqla = formData.time_comparison_grain_sqla; + let temporalColumAdded = false; + let temporalColum = null; if (queryMode === QueryMode.Aggregate) { metrics = metrics || []; @@ -133,8 +135,6 @@ const buildQuery: BuildQuery<TableChartFormData> = ( const temporalColumnsLookup = formData?.temporal_columns_lookup; // Filter out the column if needed and prepare the temporal column object - let temporalColumAdded = false; - let temporalColum = null; columns = columns.filter(col => { const shouldBeAdded = @@ -160,19 +160,6 @@ const buildQuery: BuildQuery<TableChartFormData> = ( if (temporalColum) { columns = [temporalColum, ...columns]; } - - if (!temporalColumAdded && !isEmpty(timeOffsets)) { - columns = [ - { - timeGrain: timeCompareGrainSqla || 'P1Y', // Group by year by default - columnType: 'BASE_AXIS', - sqlExpression: baseQueryObject.filters?.[0]?.col.toString() || '', - label: baseQueryObject.filters?.[0]?.col.toString() || '', - expressionType: 'SQL', - } as AdhocColumn, - ...columns, - ]; - } } const moreProps: Partial<QueryObject> = {}; @@ -187,6 +174,7 @@ const buildQuery: BuildQuery<TableChartFormData> = ( let queryObject = { ...baseQueryObject, columns, + extras: !isEmpty(timeOffsets) && !temporalColum ? {} : extras, orderby, metrics, post_processing: postProcessing, @@ -223,7 +211,7 @@ const buildQuery: BuildQuery<TableChartFormData> = ( columns: !isEmpty(timeOffsets) ? [ { - timeGrain: timeCompareGrainSqla || 'P1Y', // Group by year by default + timeGrain: 'P1Y', // Group by year by default columnType: 'BASE_AXIS', sqlExpression: baseQueryObject.filters?.[0]?.col.toString() || '', diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py index 55c80386a3..5efa0caae0 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -19,7 +19,7 @@ from __future__ import annotations import copy import logging import re -from typing import Any, ClassVar, TYPE_CHECKING, TypedDict +from typing import Any, cast, ClassVar, TYPE_CHECKING, TypedDict import numpy as np import pandas as pd @@ -55,6 +55,7 @@ from superset.utils.core import ( DateColumn, DTTM_ALIAS, error_msg_from_exception, + FilterOperator, get_base_axis_labels, get_column_names_from_columns, get_column_names_from_metrics, @@ -355,7 +356,7 @@ class QueryContextProcessor: axis=1, ) - def processing_time_offsets( # pylint: disable=too-many-locals,too-many-statements + def processing_time_offsets( # pylint: disable=too-many-locals,too-many-statements,too-many-branches self, df: pd.DataFrame, query_object: QueryObject, @@ -379,18 +380,18 @@ class QueryContextProcessor: columns = df.columns time_grain = self.get_time_grain(query_object) - if not time_grain: - raise QueryObjectValidationError( - _("Time Grain must be specified when using Time Shift.") - ) - join_column_producer = config["TIME_GRAIN_JOIN_COLUMN_PRODUCERS"].get( time_grain ) use_aggregated_join_column = ( join_column_producer or time_grain in AGGREGATED_JOIN_GRAINS ) + if use_aggregated_join_column: + if not time_grain: + raise QueryObjectValidationError( + _("Time Grain must be specified when using Time Shift.") + ) self.add_aggregated_join_column(df, time_grain, join_column_producer) # skips the first column which is the temporal column # because we'll use the aggregated join columns instead @@ -428,6 +429,28 @@ class QueryContextProcessor: query_object_clone.inner_to_dttm = outer_to_dttm query_object_clone.time_offsets = [] query_object_clone.post_processing = [] + # Get time offset index + index = (get_base_axis_labels(query_object.columns) or [DTTM_ALIAS])[0] + # The comparison is not using a temporal column so we need to modify + # the temporal filter so we run the query with the correct time range + if not dataframe_utils.is_datetime_series(df.get(index)): + # Lets find the first temporal filter in the filters array and change + # its val to be the result of get_since_until with the offset + for flt in query_object_clone.filter: + if flt.get( + "op" + ) == FilterOperator.TEMPORAL_RANGE.value and isinstance( + flt.get("val"), str + ): + time_range = cast(str, flt.get("val")) + ( + new_outer_from_dttm, + new_outer_to_dttm, + ) = get_since_until_from_time_range( + time_range=time_range, + time_shift=offset, + ) + flt["val"] = f"{new_outer_from_dttm} : {new_outer_to_dttm}" query_object_clone.filter = [ flt for flt in query_object_clone.filter @@ -488,21 +511,17 @@ class QueryContextProcessor: offset_metrics_df = offset_metrics_df.rename(columns=metrics_mapping) # 3. set time offset for index - index = (get_base_axis_labels(query_object.columns) or [DTTM_ALIAS])[0] - if not dataframe_utils.is_datetime_series(offset_metrics_df.get(index)): - raise QueryObjectValidationError( - _( - "A time column must be specified " - "when using a Time Comparison." - ) + if dataframe_utils.is_datetime_series(offset_metrics_df.get(index)): + # modifies temporal column using offset + offset_metrics_df[index] = offset_metrics_df[index] - DateOffset( + **normalize_time_delta(offset) ) - # modifies temporal column using offset - offset_metrics_df[index] = offset_metrics_df[index] - DateOffset( - **normalize_time_delta(offset) - ) - if use_aggregated_join_column: + if not time_grain: + raise QueryObjectValidationError( + _("Time Grain must be specified when using Time Shift.") + ) self.add_aggregated_join_column( offset_metrics_df, time_grain, join_column_producer ) diff --git a/superset/common/utils/time_range_utils.py b/superset/common/utils/time_range_utils.py index 2ceb9f766e..4969988657 100644 --- a/superset/common/utils/time_range_utils.py +++ b/superset/common/utils/time_range_utils.py @@ -21,7 +21,7 @@ from typing import Any, cast from superset import app from superset.common.query_object import QueryObject -from superset.utils.core import FilterOperator, get_xaxis_label +from superset.utils.core import FilterOperator from superset.utils.date_parser import get_since_until @@ -66,10 +66,8 @@ def get_since_until_from_query_object( time_range = None for flt in query_object.filter: - if ( - flt.get("op") == FilterOperator.TEMPORAL_RANGE.value - and flt.get("col") == get_xaxis_label(query_object.columns) - and isinstance(flt.get("val"), str) + if flt.get("op") == FilterOperator.TEMPORAL_RANGE.value and isinstance( + flt.get("val"), str ): time_range = cast(str, flt.get("val"))
