From a4086afd2a255d51178eb7086d38b19646d95f6a Mon Sep 17 00:00:00 2001 From: Aditya Hegde Date: Fri, 15 Dec 2023 17:29:04 +0530 Subject: [PATCH] refactor filter builder to not use map --- runtime/queries/metricsview.go | 113 ++++++++++-------- runtime/queries/metricsview_aggregation.go | 6 +- .../queries/metricsview_comparison_toplist.go | 57 ++++----- .../metricsview_comparison_toplist_test.go | 9 +- runtime/queries/metricsview_rows.go | 2 +- runtime/queries/metricsview_timeseries.go | 6 +- runtime/queries/metricsview_toplist.go | 6 +- runtime/queries/metricsview_totals.go | 2 +- .../queries_metrics_aggregation_test.go | 2 +- 9 files changed, 100 insertions(+), 103 deletions(-) diff --git a/runtime/queries/metricsview.go b/runtime/queries/metricsview.go index e9da9e4e969..574b8c54e07 100644 --- a/runtime/queries/metricsview.go +++ b/runtime/queries/metricsview.go @@ -26,6 +26,8 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +var emptyMeasureAliases = make([]*runtimev1.MetricsViewComparisonMeasureAlias, 0) + // resolveMeasures returns the selected measures func resolveMeasures(mv *runtimev1.MetricsViewSpec, inlines []*runtimev1.InlineMeasure, selectedNames []string) ([]*runtimev1.MetricsViewSpec_MeasureV2, error) { // Build combined measures @@ -293,28 +295,54 @@ func buildFilterClauseForCondition(mv *runtimev1.MetricsViewSpec, cond *runtimev return fmt.Sprintf("AND (%s) ", condsClause), args, nil } -type columnIdentifier struct { - // expression to use instead of a name for dimension or expression - // EG: measure expression : impressions => "impressions" (would be aliases in query) - // dimension column : publisher => "publisher" - // dimension expression : tld => "regexp_extract(domain, '(.*\\.)?(.*\\.com)', 2)" (needed since tld might not be selected) - expr string - unnest bool -} +func columnIdentifierExpression(mv *runtimev1.MetricsViewSpec, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, name string) (string, bool) { + // check if identifier is a dimension + for _, dim := range mv.Dimensions { + if dim.Name == name { + return safeName(metricsViewDimensionColumn(dim)), true + } + } + + // check if identifier is passed as an alias + for _, alias := range aliases { + if alias.Alias == name { + switch alias.Type { + case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_BASE_VALUE, + runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_UNSPECIFIED: + return safeName(alias.Name), true + case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_COMPARISON_VALUE: + return safeName(alias.Name + "__previous"), true + case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_ABS_DELTA: + return safeName(alias.Name + "__delta_abs"), true + case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_REL_DELTA: + return safeName(alias.Name + "__delta_rel"), true + } + } + } + + // check if identifier is measure but not passed as alias + for _, mes := range mv.Measures { + if mes.Name == name { + return safeName(mes.Name), true + } + } -func newIdentifier(name string) columnIdentifier { - return columnIdentifier{safeName(name), false} + return "", false } -func dimensionAliases(mv *runtimev1.MetricsViewSpec) map[string]columnIdentifier { - aliases := map[string]columnIdentifier{} - for _, dim := range mv.Dimensions { - aliases[dim.Name] = columnIdentifier{safeName(metricsViewDimensionColumn(dim)), dim.Unnest} +func identifierIsUnnest(mv *runtimev1.MetricsViewSpec, expr *runtimev1.Expression) bool { + ident, isIdent := expr.Expression.(*runtimev1.Expression_Ident) + if isIdent { + for _, dim := range mv.Dimensions { + if dim.Name == ident.Ident { + return dim.Unnest + } + } } - return aliases + return false } -func buildExpression(expr *runtimev1.Expression, allowedIdentifiers map[string]columnIdentifier, dialect drivers.Dialect) (string, []any, error) { +func buildExpression(mv *runtimev1.MetricsViewSpec, expr *runtimev1.Expression, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, dialect drivers.Dialect) (string, []any, error) { var emptyArg []any switch e := expr.Expression.(type) { case *runtimev1.Expression_Val: @@ -325,40 +353,40 @@ func buildExpression(expr *runtimev1.Expression, allowedIdentifiers map[string]c return "?", []any{arg}, nil case *runtimev1.Expression_Ident: - col, ok := allowedIdentifiers[e.Ident] - if !ok { + expr, isIdent := columnIdentifierExpression(mv, aliases, e.Ident) + if !isIdent { return "", emptyArg, fmt.Errorf("unknown column filter: %s", e.Ident) } - return col.expr, emptyArg, nil + return expr, emptyArg, nil case *runtimev1.Expression_Cond: - return buildConditionExpression(e.Cond, allowedIdentifiers, dialect) + return buildConditionExpression(mv, e.Cond, aliases, dialect) } return "", emptyArg, nil } -func buildConditionExpression(cond *runtimev1.Condition, allowedIdentifiers map[string]columnIdentifier, dialect drivers.Dialect) (string, []any, error) { +func buildConditionExpression(mv *runtimev1.MetricsViewSpec, cond *runtimev1.Condition, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, dialect drivers.Dialect) (string, []any, error) { switch cond.Op { case runtimev1.Operation_OPERATION_LIKE, runtimev1.Operation_OPERATION_NLIKE: - return buildLikeExpression(cond, allowedIdentifiers, dialect) + return buildLikeExpression(mv, cond, aliases, dialect) case runtimev1.Operation_OPERATION_IN, runtimev1.Operation_OPERATION_NIN: - return buildInExpression(cond, allowedIdentifiers, dialect) + return buildInExpression(mv, cond, aliases, dialect) case runtimev1.Operation_OPERATION_AND: - return buildAndOrExpressions(cond, allowedIdentifiers, dialect, " AND ") + return buildAndOrExpressions(mv, cond, aliases, dialect, " AND ") case runtimev1.Operation_OPERATION_OR: - return buildAndOrExpressions(cond, allowedIdentifiers, dialect, " OR ") + return buildAndOrExpressions(mv, cond, aliases, dialect, " OR ") default: - leftExpr, args, err := buildExpression(cond.Exprs[0], allowedIdentifiers, dialect) + leftExpr, args, err := buildExpression(mv, cond.Exprs[0], aliases, dialect) if err != nil { return "", nil, err } - rightExpr, subArgs, err := buildExpression(cond.Exprs[1], allowedIdentifiers, dialect) + rightExpr, subArgs, err := buildExpression(mv, cond.Exprs[1], aliases, dialect) if err != nil { return "", nil, err } @@ -368,17 +396,17 @@ func buildConditionExpression(cond *runtimev1.Condition, allowedIdentifiers map[ } } -func buildLikeExpression(cond *runtimev1.Condition, allowedIdentifiers map[string]columnIdentifier, dialect drivers.Dialect) (string, []any, error) { +func buildLikeExpression(mv *runtimev1.MetricsViewSpec, cond *runtimev1.Condition, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, dialect drivers.Dialect) (string, []any, error) { if len(cond.Exprs) != 2 { return "", nil, fmt.Errorf("like/not like expression should have exactly 2 sub expressions") } - leftExpr, args, err := buildExpression(cond.Exprs[0], allowedIdentifiers, dialect) + leftExpr, args, err := buildExpression(mv, cond.Exprs[0], aliases, dialect) if err != nil { return "", nil, err } - rightExpr, subArgs, err := buildExpression(cond.Exprs[1], allowedIdentifiers, dialect) + rightExpr, subArgs, err := buildExpression(mv, cond.Exprs[1], aliases, dialect) if err != nil { return "", nil, err } @@ -390,12 +418,7 @@ func buildLikeExpression(cond *runtimev1.Condition, allowedIdentifiers map[strin } // identify if immediate identifier has unnest - unnest := false - ident, isIdent := cond.Exprs[0].Expression.(*runtimev1.Expression_Ident) - if isIdent { - i := allowedIdentifiers[ident.Ident] - unnest = i.unnest - } + unnest := identifierIsUnnest(mv, cond.Exprs[0]) var clause string // Build [NOT] len(list_filter("dim", x -> x ILIKE ?)) > 0 @@ -419,12 +442,12 @@ func buildLikeExpression(cond *runtimev1.Condition, allowedIdentifiers map[strin return clause, args, nil } -func buildInExpression(cond *runtimev1.Condition, allowedIdentifiers map[string]columnIdentifier, dialect drivers.Dialect) (string, []any, error) { +func buildInExpression(mv *runtimev1.MetricsViewSpec, cond *runtimev1.Condition, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, dialect drivers.Dialect) (string, []any, error) { if len(cond.Exprs) <= 1 { return "", nil, fmt.Errorf("in/not in expression should have atleast 2 sub expressions") } - leftExpr, args, err := buildExpression(cond.Exprs[0], allowedIdentifiers, dialect) + leftExpr, args, err := buildExpression(mv, cond.Exprs[0], aliases, dialect) if err != nil { return "", nil, err } @@ -445,7 +468,7 @@ func buildInExpression(cond *runtimev1.Condition, allowedIdentifiers map[string] continue // Handled later using "dim IS [NOT] NULL" clause } } - inVal, subArgs, err := buildExpression(subExpr, allowedIdentifiers, dialect) + inVal, subArgs, err := buildExpression(mv, subExpr, aliases, dialect) if err != nil { return "", nil, err } @@ -454,13 +477,7 @@ func buildInExpression(cond *runtimev1.Condition, allowedIdentifiers map[string] } // identify if immediate identifier has unnest - // TODO: do we need to do a deeper check? - unnest := false - ident, isIndent := cond.Exprs[0].Expression.(*runtimev1.Expression_Ident) - if isIndent { - i := allowedIdentifiers[ident.Ident] - unnest = i.unnest - } + unnest := identifierIsUnnest(mv, cond.Exprs[0]) clauses := make([]string, 0) @@ -498,11 +515,11 @@ func buildInExpression(cond *runtimev1.Condition, allowedIdentifiers map[string] return condsClause, args, nil } -func buildAndOrExpressions(cond *runtimev1.Condition, allowedIdentifiers map[string]columnIdentifier, dialect drivers.Dialect, joiner string) (string, []any, error) { +func buildAndOrExpressions(mv *runtimev1.MetricsViewSpec, cond *runtimev1.Condition, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, dialect drivers.Dialect, joiner string) (string, []any, error) { clauses := make([]string, 0) var args []any for _, expr := range cond.Exprs { - clause, subArgs, err := buildExpression(expr, allowedIdentifiers, dialect) + clause, subArgs, err := buildExpression(mv, expr, aliases, dialect) if err != nil { return "", nil, err } diff --git a/runtime/queries/metricsview_aggregation.go b/runtime/queries/metricsview_aggregation.go index b7702ddf1fa..44c732b14aa 100644 --- a/runtime/queries/metricsview_aggregation.go +++ b/runtime/queries/metricsview_aggregation.go @@ -184,7 +184,6 @@ func (q *MetricsViewAggregation) buildMetricsAggregationSQL(mv *runtimev1.Metric args = append(args, exprArgs...) } - measureAliases := map[string]columnIdentifier{} for _, m := range q.Measures { switch m.BuiltinMeasure { case runtimev1.BuiltinMeasure_BUILTIN_MEASURE_UNSPECIFIED: @@ -207,7 +206,6 @@ func (q *MetricsViewAggregation) buildMetricsAggregationSQL(mv *runtimev1.Metric default: return "", nil, fmt.Errorf("unknown builtin measure '%d'", m.BuiltinMeasure) } - measureAliases[m.Name] = newIdentifier(m.Name) } groupClause := "" @@ -225,7 +223,7 @@ func (q *MetricsViewAggregation) buildMetricsAggregationSQL(mv *runtimev1.Metric whereClause += clause } if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } @@ -240,7 +238,7 @@ func (q *MetricsViewAggregation) buildMetricsAggregationSQL(mv *runtimev1.Metric if q.Having != nil { var havingClauseArgs []any var err error - havingClause, havingClauseArgs, err = buildExpression(q.Having, measureAliases, dialect) + havingClause, havingClauseArgs, err = buildExpression(mv, q.Having, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } diff --git a/runtime/queries/metricsview_comparison_toplist.go b/runtime/queries/metricsview_comparison_toplist.go index 70991c97119..811d42686be 100644 --- a/runtime/queries/metricsview_comparison_toplist.go +++ b/runtime/queries/metricsview_comparison_toplist.go @@ -5,15 +5,17 @@ import ( "encoding/json" "fmt" "io" + "slices" "strings" - // Load IANA time zone data - _ "time/tzdata" runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1" "github.com/rilldata/rill/runtime" "github.com/rilldata/rill/runtime/drivers" "github.com/rilldata/rill/runtime/pkg/pbutil" "google.golang.org/protobuf/types/known/structpb" + + // Load IANA time zone data + _ "time/tzdata" ) type MetricsViewComparison struct { @@ -264,7 +266,6 @@ func (q *MetricsViewComparison) buildMetricsTopListSQL(mv *runtimev1.MetricsView } labelCols = []string{fmt.Sprintf("%s as %s", safeName(dim.Name), dimLabel)} - measureAliases := map[string]columnIdentifier{} for _, m := range q.Measures { switch m.BuiltinMeasure { case runtimev1.BuiltinMeasure_BUILTIN_MEASURE_UNSPECIFIED: @@ -290,11 +291,10 @@ func (q *MetricsViewComparison) buildMetricsTopListSQL(mv *runtimev1.MetricsView default: return "", nil, fmt.Errorf("unknown builtin measure '%d'", m.BuiltinMeasure) } - measureAliases[m.Name] = newIdentifier(m.Name) } if q.Aliases != nil { - err = aliasesToMeasureMap(q.Aliases, measureAliases, false) + err = validateMeasureAliases(q.Aliases, q.Measures, false) if err != nil { return "", nil, err } @@ -313,7 +313,7 @@ func (q *MetricsViewComparison) buildMetricsTopListSQL(mv *runtimev1.MetricsView baseWhereClause += trc if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } @@ -325,7 +325,7 @@ func (q *MetricsViewComparison) buildMetricsTopListSQL(mv *runtimev1.MetricsView havingClause := "" if q.Having != nil { var havingClauseArgs []any - havingClause, havingClauseArgs, err = buildExpression(q.Having, measureAliases, dialect) + havingClause, havingClauseArgs, err = buildExpression(mv, q.Having, q.Aliases, dialect) if err != nil { return "", nil, err } @@ -427,7 +427,6 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M selectCols = append(selectCols, colName) } - measureAliases := map[string]columnIdentifier{} for _, m := range q.Measures { switch m.BuiltinMeasure { case runtimev1.BuiltinMeasure_BUILTIN_MEASURE_UNSPECIFIED: @@ -450,7 +449,6 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M default: return "", nil, fmt.Errorf("unknown builtin measure '%d'", m.BuiltinMeasure) } - measureAliases[m.Name] = newIdentifier(m.Name) } finalSelectCols := []string{} @@ -462,6 +460,8 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M var labelTuple string if dialect != drivers.DialectDruid { if q.Having != nil { + // having clause needs selected columns to either be in group by or be aggregations. + // so adding additional sum() around measure and comparison columns columnsTuple = fmt.Sprintf( "sum(base.%[1]s) as %[1]s, sum(comparison.%[1]s) AS %[2]s, sum(base.%[1]s - comparison.%[1]s) AS %[3]s, sum((base.%[1]s - comparison.%[1]s)/comparison.%[1]s::DOUBLE) AS %[4]s", safeName(m.Name), @@ -486,10 +486,6 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M safeName(labelMap[m.Name]+" (Δ%)"), safeName(labelMap[m.Name]), ) - measureAliases[m.Name] = newIdentifier(m.Name) - measureAliases[m.Name+"__previous"] = newIdentifier(m.Name + "__previous") - measureAliases[m.Name+"__delta_abs"] = newIdentifier(m.Name + "__delta_abs") - measureAliases[m.Name+"__delta_rel"] = newIdentifier(m.Name + "__delta_rel") } else { columnsTuple = fmt.Sprintf( "ANY_VALUE(base.%[1]s), ANY_VALUE(comparison.%[1]s), ANY_VALUE(base.%[1]s - comparison.%[1]s), ANY_VALUE(SAFE_DIVIDE(base.%[1]s - comparison.%[1]s, CAST(comparison.%[1]s AS DOUBLE)))", @@ -512,7 +508,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M } if q.Aliases != nil { - err = aliasesToMeasureMap(q.Aliases, measureAliases, true) + err = validateMeasureAliases(q.Aliases, q.Measures, true) if err != nil { return "", nil, err } @@ -542,7 +538,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M baseWhereClause += trc if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } @@ -558,7 +554,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M comparisonWhereClause += trc if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } @@ -570,7 +566,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M havingClause := "" if q.Having != nil { var havingClauseArgs []any - havingClause, havingClauseArgs, err = buildExpression(q.Having, measureAliases, dialect) + havingClause, havingClauseArgs, err = buildExpression(mv, q.Having, q.Aliases, dialect) if err != nil { return "", nil, err } @@ -1030,27 +1026,22 @@ func isTimeRangeNil(tr *runtimev1.TimeRange) bool { return tr == nil || (tr.Start == nil && tr.End == nil) } -func aliasesToMeasureMap(aliases []*runtimev1.MetricsViewComparisonMeasureAlias, measureAliases map[string]columnIdentifier, hasComparison bool) error { +func validateMeasureAliases(aliases []*runtimev1.MetricsViewComparisonMeasureAlias, measures []*runtimev1.MetricsViewAggregationMeasure, hasComparison bool) error { for _, alias := range aliases { + // We need to make sure the aliases measure is selected. Having query will fail otherwise + if !slices.ContainsFunc(measures, func(measure *runtimev1.MetricsViewAggregationMeasure) bool { + return measure.Name == alias.Name + }) { + return fmt.Errorf("measure alias not selected: %s", alias.Name) + } + switch alias.Type { - case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_BASE_VALUE, - runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_UNSPECIFIED: - measureAliases[alias.Alias] = newIdentifier(alias.Name) - case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_COMPARISON_VALUE: - if !hasComparison { - return fmt.Errorf("comparison not enabled for alias %s", alias.Alias) - } - measureAliases[alias.Alias] = newIdentifier(alias.Name + "__previous") - case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_ABS_DELTA: - if !hasComparison { - return fmt.Errorf("comparison not enabled for alias %s", alias.Alias) - } - measureAliases[alias.Alias] = newIdentifier(alias.Name + "__delta_abs") - case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_REL_DELTA: + case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_COMPARISON_VALUE, + runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_ABS_DELTA, + runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_REL_DELTA: if !hasComparison { return fmt.Errorf("comparison not enabled for alias %s", alias.Alias) } - measureAliases[alias.Alias] = newIdentifier(alias.Name + "__delta_rel") } } return nil diff --git a/runtime/queries/metricsview_comparison_toplist_test.go b/runtime/queries/metricsview_comparison_toplist_test.go index ac04f683b5d..b20a9753f67 100644 --- a/runtime/queries/metricsview_comparison_toplist_test.go +++ b/runtime/queries/metricsview_comparison_toplist_test.go @@ -288,7 +288,7 @@ func TestMetricsViewsComparison_measure_filters(t *testing.T) { require.Equal(t, "instagram.com", q.Result.Rows[2].DimensionValue.GetStringValue()) } -func TestMetricsViewsComparison_measure_filters_with_compare(t *testing.T) { +func TestMetricsViewsComparison_measure_filters_with_compare_no_alias(t *testing.T) { rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids") ctr := &queries.ColumnTimeRange{ @@ -353,12 +353,7 @@ func TestMetricsViewsComparison_measure_filters_with_compare(t *testing.T) { } err = q.Resolve(context.Background(), rt, instanceID, 0) - require.NoError(t, err) - require.NotEmpty(t, q.Result) - require.Len(t, q.Result.Rows, 3) - require.Equal(t, "sports.yahoo.com", q.Result.Rows[0].DimensionValue.GetStringValue()) - require.Equal(t, "news.google.com", q.Result.Rows[1].DimensionValue.GetStringValue()) - require.Equal(t, "instagram.com", q.Result.Rows[2].DimensionValue.GetStringValue()) + require.ErrorContains(t, err, "unknown column filter: measure_1__delta_rel") } func TestMetricsViewsComparison_measure_filters_with_compare_aliases(t *testing.T) { diff --git a/runtime/queries/metricsview_rows.go b/runtime/queries/metricsview_rows.go index bb81c1be34c..d7e3dcdae19 100644 --- a/runtime/queries/metricsview_rows.go +++ b/runtime/queries/metricsview_rows.go @@ -240,7 +240,7 @@ func (q *MetricsViewRows) buildMetricsRowsSQL(mv *runtimev1.MetricsViewSpec, dia } if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } diff --git a/runtime/queries/metricsview_timeseries.go b/runtime/queries/metricsview_timeseries.go index 1dbc5ae67d4..47ea7da9615 100644 --- a/runtime/queries/metricsview_timeseries.go +++ b/runtime/queries/metricsview_timeseries.go @@ -272,12 +272,10 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore return "", "", nil, err } - measureAliases := map[string]columnIdentifier{} selectCols := []string{} for _, m := range ms { expr := fmt.Sprintf(`%s as "%s"`, m.Expression, m.Name) selectCols = append(selectCols, expr) - measureAliases[m.Name] = newIdentifier(m.Name) } whereClause := "1=1" @@ -292,7 +290,7 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore } if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), olap.Dialect()) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, olap.Dialect()) if err != nil { return "", "", nil, err } @@ -302,7 +300,7 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore havingClause := "" if q.Having != nil { - clause, clauseArgs, err := buildExpression(q.Having, measureAliases, olap.Dialect()) + clause, clauseArgs, err := buildExpression(mv, q.Having, emptyMeasureAliases, olap.Dialect()) if err != nil { return "", "", nil, err } diff --git a/runtime/queries/metricsview_toplist.go b/runtime/queries/metricsview_toplist.go index ecba0068019..da6b396976f 100644 --- a/runtime/queries/metricsview_toplist.go +++ b/runtime/queries/metricsview_toplist.go @@ -208,11 +208,9 @@ func (q *MetricsViewToplist) buildMetricsTopListSQL(mv *runtimev1.MetricsViewSpe selectCols = append(selectCols, colName) } - measureAliases := map[string]columnIdentifier{} for _, m := range ms { expr := fmt.Sprintf(`%s as "%s"`, m.Expression, m.Name) selectCols = append(selectCols, expr) - measureAliases[m.Name] = newIdentifier(m.Name) } whereClause := "1=1" @@ -229,7 +227,7 @@ func (q *MetricsViewToplist) buildMetricsTopListSQL(mv *runtimev1.MetricsViewSpe } if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } @@ -240,7 +238,7 @@ func (q *MetricsViewToplist) buildMetricsTopListSQL(mv *runtimev1.MetricsViewSpe havingClause := "" if q.Having != nil { var havingClauseArgs []any - havingClause, havingClauseArgs, err = buildExpression(q.Having, measureAliases, dialect) + havingClause, havingClauseArgs, err = buildExpression(mv, q.Having, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } diff --git a/runtime/queries/metricsview_totals.go b/runtime/queries/metricsview_totals.go index 999945200a7..206f1089b0c 100644 --- a/runtime/queries/metricsview_totals.go +++ b/runtime/queries/metricsview_totals.go @@ -137,7 +137,7 @@ func (q *MetricsViewTotals) buildMetricsTotalsSQL(mv *runtimev1.MetricsViewSpec, } if q.Where != nil { - clause, clauseArgs, err := buildExpression(q.Where, dimensionAliases(mv), dialect) + clause, clauseArgs, err := buildExpression(mv, q.Where, emptyMeasureAliases, dialect) if err != nil { return "", nil, err } diff --git a/runtime/server/queries_metrics_aggregation_test.go b/runtime/server/queries_metrics_aggregation_test.go index e78a141537a..faffd3e7635 100644 --- a/runtime/server/queries_metrics_aggregation_test.go +++ b/runtime/server/queries_metrics_aggregation_test.go @@ -37,7 +37,7 @@ func TestMetricsViewAggregation_Toplist(t *testing.T) { require.Equal(t, "yahoo.com", tr.Data[1].Fields["domain"].GetStringValue()) require.Equal(t, 1.0, tr.Data[1].Fields["measure_2"].GetNumberValue()) - require.Equal(t, 1.0, tr.Data[0].Fields["__count"].GetNumberValue()) + require.Equal(t, 1.0, tr.Data[1].Fields["__count"].GetNumberValue()) } func TestMetricsViewAggregation_Totals(t *testing.T) {