This is an automated email from the ASF dual-hosted git repository. amaan pushed a commit to branch fix/ag-grid-and-filter-conditions in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9569f649a89c84fe0fc8393e23f45c8f4e465e5f Author: amaannawab923 <[email protected]> AuthorDate: Wed Mar 4 02:08:51 2026 +0530 fix(ag-grid-table): fix AND filter conditions not applied - Prevent saved filters from re-injecting on every re-render - Resolve saved metric and calculated column labels to SQL in WHERE/HAVING clauses --- .../src/AgGridTableChart.tsx | 3 + .../plugin-chart-ag-grid-table/src/buildQuery.ts | 77 ++++++++++++++++++++-- .../src/transformProps.ts | 39 ++++++++++- .../plugin-chart-ag-grid-table/src/types.ts | 1 + 4 files changed, 113 insertions(+), 7 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx index 79eee9a91ff..8018a5c5b97 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx @@ -85,6 +85,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( width, onChartStateChange, chartState, + metricSqlExpressions, } = props; const [searchOptions, setSearchOptions] = useState<SearchOption[]>([]); @@ -187,6 +188,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( lastFilteredColumn: completeFilterState.lastFilteredColumn, lastFilteredInputPosition: completeFilterState.inputPosition, currentPage: 0, // Reset to first page when filtering + metricSqlExpressions, }; updateTableOwnState(setDataMask, modifiedOwnState); @@ -197,6 +199,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( serverPaginationData, onChartStateChange, chartState, + metricSqlExpressions, ], ); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts index 8122c86f825..f34a268ccb9 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts @@ -482,12 +482,76 @@ const buildQuery: BuildQuery<TableChartFormData> = ( }; } - // Add AG Grid complex WHERE clause from ownState (non-metric filters) + // Map metric/column labels to SQL expressions for WHERE/HAVING resolution + const sqlExpressionMap: Record<string, string> = {}; + (metrics || []).forEach((m: QueryFormMetric) => { + if (typeof m === 'object' && 'expressionType' in m) { + const label = getMetricLabel(m); + if (m.expressionType === 'SQL' && m.sqlExpression) { + sqlExpressionMap[label] = m.sqlExpression; + } else if ( + m.expressionType === 'SIMPLE' && + m.aggregate && + m.column?.column_name + ) { + sqlExpressionMap[label] = + `${m.aggregate}(${m.column.column_name})`; + } + } + }); + // Map dimension columns with custom SQL expressions + (columns || []).forEach((col: QueryFormColumn) => { + if (typeof col === 'object' && 'sqlExpression' in col) { + const label = getColumnLabel(col); + if (col.sqlExpression) { + sqlExpressionMap[label] = col.sqlExpression; + } + } + }); + // Merge datasource-level saved metrics and calculated columns + if (ownState.metricSqlExpressions) { + Object.entries( + ownState.metricSqlExpressions as Record<string, string>, + ).forEach(([label, expression]) => { + if (!sqlExpressionMap[label]) { + sqlExpressionMap[label] = expression; + } + }); + } + + const resolveLabelsToSQL = (clause: string): string => { + let resolved = clause; + // Sort by label length descending to prevent substring false positives + const sortedEntries = Object.entries(sqlExpressionMap).sort( + ([a], [b]) => b.length - a.length, + ); + sortedEntries.forEach(([label, expression]) => { + if (resolved.includes(label)) { + const escapedLabel = label.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // Wrap complex expressions in parentheses for valid SQL + const isExpression = + expression.includes('(') || + expression.toUpperCase().includes('CASE') || + expression.includes('\n'); + const wrappedExpression = isExpression + ? `(${expression})` + : `"${expression}"`; + resolved = resolved.replace( + new RegExp(`\\b${escapedLabel}\\b`, 'g'), + wrappedExpression, + ); + } + }); + return resolved; + }; + + // Resolve and apply AG Grid WHERE clause if (ownState.agGridComplexWhere && ownState.agGridComplexWhere.trim()) { + const resolvedWhere = resolveLabelsToSQL(ownState.agGridComplexWhere); const existingWhere = queryObject.extras?.where; const combinedWhere = existingWhere - ? `${existingWhere} AND ${ownState.agGridComplexWhere}` - : ownState.agGridComplexWhere; + ? `${existingWhere} AND ${resolvedWhere}` + : resolvedWhere; queryObject = { ...queryObject, @@ -498,12 +562,13 @@ const buildQuery: BuildQuery<TableChartFormData> = ( }; } - // Add AG Grid HAVING clause from ownState (metric filters only) + // Resolve and apply AG Grid HAVING clause if (ownState.agGridHavingClause && ownState.agGridHavingClause.trim()) { + const resolvedHaving = resolveLabelsToSQL(ownState.agGridHavingClause); const existingHaving = queryObject.extras?.having; const combinedHaving = existingHaving - ? `${existingHaving} AND ${ownState.agGridHavingClause}` - : ownState.agGridHavingClause; + ? `${existingHaving} AND ${resolvedHaving}` + : resolvedHaving; queryObject = { ...queryObject, diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts index c22b6ff46c4..54c45da1211 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts @@ -34,6 +34,8 @@ import { SMART_DATE_ID, TimeFormats, TimeFormatter, + AgGridChartState, + AgGridFilterModel, } from '@superset-ui/core'; import { GenericDataType } from '@apache-superset/core/api/core'; import { isEmpty, isEqual, merge } from 'lodash'; @@ -456,6 +458,9 @@ const getPageSize = ( return numRecords * numColumns > 5000 ? 200 : 0; }; +// Tracks slice_ids that have already applied their saved chartState filter on mount +const savedFilterAppliedSet = new Set<number>(); + const transformProps = ( chartProps: TableChartProps, ): AgGridTableChartTransformedProps => { @@ -710,6 +715,37 @@ const transformProps = ( : totalQuery?.data[0] : undefined; + // Map saved metric/calculated column labels to their SQL expressions for filter resolution + const metricSqlExpressions: Record<string, string> = {}; + chartProps.datasource.metrics.forEach(metric => { + if (metric.metric_name && metric.expression) { + metricSqlExpressions[metric.metric_name] = metric.expression; + } + }); + chartProps.datasource.columns.forEach(col => { + if (col.column_name && col.expression) { + metricSqlExpressions[col.column_name] = col.expression; + if (col.verbose_name && col.verbose_name !== col.column_name) { + metricSqlExpressions[col.verbose_name] = col.expression; + } + } + }); + + // Strip saved filter from chartState after initial application to prevent re-injection + let chartState = serverPaginationData?.chartState as + | AgGridChartState + | undefined; + const chartStateHasFilter = !!( + chartState?.filterModel && + Object.keys(chartState.filterModel).length > 0 + ); + + if (chartStateHasFilter && savedFilterAppliedSet.has(slice_id)) { + chartState = { ...chartState, filterModel: {} as AgGridFilterModel }; + } else if (chartStateHasFilter) { + savedFilterAppliedSet.add(slice_id); + } + return { height, width, @@ -742,7 +778,8 @@ const transformProps = ( basicColorColumnFormatters, basicColorFormatters, formData, - chartState: serverPaginationData?.chartState, + metricSqlExpressions, + chartState, onChartStateChange, }; }; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts index d97f36b267f..e3d6bf07079 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts @@ -128,6 +128,7 @@ export interface AgGridTableChartTransformedProps< basicColorFormatters?: { [Key: string]: BasicColorFormatterType }[]; basicColorColumnFormatters?: { [Key: string]: BasicColorFormatterType }[]; formData: TableChartFormData; + metricSqlExpressions: Record<string, string>; onChartStateChange?: (chartState: JsonObject) => void; chartState?: AgGridChartState; }
