amaannawab923 commented on code in PR #38369:
URL: https://github.com/apache/superset/pull/38369#discussion_r2917216410
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts:
##########
@@ -482,12 +482,77 @@ 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;
+ };
Review Comment:
We need to take that as a chore pr because we need to move a significant
portion of the Ag-Grid logic to the backend. The goal is to completely remove
raw SQL from the frontend .
Currently, raw SQL exists on the frontend only because the backend does not
support the OR operator yet.
If we add this support on the backend and eliminate the need for raw SQL in
the frontend, we can remove all of this complexity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
