Adding support for defining filters on measures

runtime/queries/metricsview.go

@@ -293,7 +293,7 @@ func buildFilterClauseForCondition(mv *runtimev1.MetricsViewSpec, cond *runtimev
 	return fmt.Sprintf("AND (%s) ", condsClause), args, nil
 }
 
-func columnIdentifierExpression(mv *runtimev1.MetricsViewSpec, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, name string) (string, bool) {
+func columnIdentifierExpression(mv *runtimev1.MetricsViewSpec, aliases []*runtimev1.MetricsViewComparisonMeasureAlias, name string, dialect drivers.Dialect) (string, bool) {
 	// check if identifier is a dimension
 	for _, dim := range mv.Dimensions {
 		if dim.Name == name {
@@ -305,8 +305,12 @@ func columnIdentifierExpression(mv *runtimev1.MetricsViewSpec, aliases []*runtim
 	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:
+			case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_UNSPECIFIED,
+				runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_BASE_VALUE:
+				// using `measure_0` as is causing ambiguity error in duckdb
+				if dialect == drivers.DialectDuckDB {
+					return "base." + safeName(alias.Name), true
+				}
 				return safeName(alias.Name), true
 			case runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_COMPARISON_VALUE:
 				return safeName(alias.Name + "__previous"), true
@@ -351,7 +355,7 @@ func buildExpression(mv *runtimev1.MetricsViewSpec, expr *runtimev1.Expression,
 		return "?", []any{arg}, nil
 
 	case *runtimev1.Expression_Ident:
-		expr, isIdent := columnIdentifierExpression(mv, aliases, e.Ident)
+		expr, isIdent := columnIdentifierExpression(mv, aliases, e.Ident, dialect)
 		if !isIdent {
 			return "", emptyArg, fmt.Errorf("unknown column filter: %s", e.Ident)
 		}

runtime/queries/metricsview_aggregation_test.go

@@ -14,11 +14,11 @@ import (
 )
 
 func TestMetricsViewAggregation_measure_filters(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)

runtime/queries/metricsview_comparison_toplist.go

@@ -459,7 +459,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 		var labelTuple string
 		if dialect != drivers.DialectDruid {
 			columnsTuple = fmt.Sprintf(
-				"base.%[1]s, comparison.%[1]s AS %[2]s, base.%[1]s - comparison.%[1]s AS %[3]s, (base.%[1]s - comparison.%[1]s)/comparison.%[1]s::DOUBLE AS %[4]s",
+				"base.%[1]s AS %[1]s, comparison.%[1]s AS %[2]s, base.%[1]s - comparison.%[1]s AS %[3]s, (base.%[1]s - comparison.%[1]s)/comparison.%[1]s::DOUBLE AS %[4]s",
 				safeName(m.Name),
 				safeName(m.Name+"__previous"),
 				safeName(m.Name+"__delta_abs"),
@@ -475,8 +475,11 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 			)
 		} 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)))",
+				"ANY_VALUE(base.%[1]s) AS %[1]s, ANY_VALUE(comparison.%[1]s) AS %[2]s, ANY_VALUE(base.%[1]s - comparison.%[1]s) AS %[3]s, ANY_VALUE(SAFE_DIVIDE(base.%[1]s - comparison.%[1]s, CAST(comparison.%[1]s AS DOUBLE))) AS %[4]s",
 				safeName(m.Name),
+				safeName(m.Name+"__previous"),
+				safeName(m.Name+"__delta_abs"),
+				safeName(m.Name+"__delta_rel"),
 			)
 			labelTuple = fmt.Sprintf(
 				"ANY_VALUE(base.%[1]s) AS %[2]s, ANY_VALUE(comparison.%[1]s) AS %[3]s, ANY_VALUE(base.%[1]s - comparison.%[1]s) AS %[4]s, ANY_VALUE(SAFE_DIVIDE(base.%[1]s - comparison.%[1]s, CAST(comparison.%[1]s AS DOUBLE))) AS %[5]s",
@@ -550,14 +553,13 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 		args = append(args, clauseArgs...)
 	}
 
-	outerWhereClause := ""
+	havingClause := "1=1"
 	if q.Having != nil {
 		var havingClauseArgs []any
-		outerWhereClause, havingClauseArgs, err = buildExpression(mv, q.Having, q.Aliases, dialect)
+		havingClause, havingClauseArgs, err = buildExpression(mv, q.Having, q.Aliases, dialect)
 		if err != nil {
 			return "", nil, err
 		}
-		outerWhereClause = "WHERE " + outerWhereClause
 		args = append(args, havingClauseArgs...)
 	}
 
@@ -694,7 +696,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 			) comparison
 		ON
 				base.%[2]s = comparison.%[2]s OR (base.%[2]s is null and comparison.%[2]s is null)
-		%[16]s
+		WHERE %[16]s
 		ORDER BY
 			%[6]s
 		%[7]s
@@ -716,7 +718,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 			comparisonLimitClause, // 13
 			unnestClause,          // 14
 			groupByCol,            // 15
-			outerWhereClause,      // 16
+			havingClause,          // 16
 		)
 	} else {
 		/*
@@ -776,8 +778,8 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 					SELECT %[1]s FROM %[3]s WHERE %[5]s AND %[2]s IN (SELECT %[2]s FROM %[11]s) GROUP BY %[2]s %[10]s
 				)
 				SELECT %[11]s.%[2]s AS %[14]s, %[9]s FROM %[11]s LEFT JOIN %[12]s ON base.%[2]s = comparison.%[2]s
-        %[15]s
 				GROUP BY 1
+        HAVING %[15]s
 				ORDER BY %[6]s
 				%[7]s
 				OFFSET %[8]d
@@ -796,7 +798,7 @@ func (q *MetricsViewComparison) buildMetricsComparisonTopListSQL(mv *runtimev1.M
 			rightSubQueryAlias,  // 12
 			subQueryOrderClause, // 13
 			finalDimName,        // 14
-			outerWhereClause,    // 15
+			havingClause,        // 15
 		)
 	}
 

runtime/queries/metricsview_comparison_toplist_test.go

@@ -19,12 +19,18 @@ import (
 	_ "github.com/rilldata/rill/runtime/drivers/duckdb"
 )
 
+// replace with "ad_bids_druid" to test on druid
+const AdBidsProject = "ad_bids"
+
+// replace with "timestamp" to test on druid
+const AdBidsTimestamp = "timestamp"
+
 func TestMetricsViewsComparison_dim_order_comparison_toplist_vs_general_toplist(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)
@@ -116,11 +122,11 @@ func TestMetricsViewsComparison_dim_order_comparison_toplist_vs_general_toplist(
 }
 
 func TestMetricsViewsComparison_dim_order(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)
@@ -168,11 +174,11 @@ func TestMetricsViewsComparison_dim_order(t *testing.T) {
 }
 
 func TestMetricsViewsComparison_measure_order(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)
@@ -220,11 +226,11 @@ func TestMetricsViewsComparison_measure_order(t *testing.T) {
 }
 
 func TestMetricsViewsComparison_measure_filters(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)
@@ -289,11 +295,11 @@ func TestMetricsViewsComparison_measure_filters(t *testing.T) {
 }
 
 func TestMetricsViewsComparison_measure_filters_with_compare_no_alias(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)
@@ -356,12 +362,92 @@ func TestMetricsViewsComparison_measure_filters_with_compare_no_alias(t *testing
 	require.ErrorContains(t, err, "unknown column filter: measure_1__delta_rel")
 }
 
+func TestMetricsViewsComparison_measure_filters_with_compare_base_measure(t *testing.T) {
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
+
+	ctr := &queries.ColumnTimeRange{
+		TableName:  "ad_bids",
+		ColumnName: AdBidsTimestamp,
+	}
+	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
+	require.NoError(t, err)
+	diff := ctr.Result.Max.AsTime().Sub(ctr.Result.Min.AsTime())
+	maxTime := ctr.Result.Min.AsTime().Add(diff / 2)
+
+	ctrl, err := rt.Controller(context.Background(), instanceID)
+	require.NoError(t, err)
+	r, err := ctrl.Get(context.Background(), &runtimev1.ResourceName{Kind: runtime.ResourceKindMetricsView, Name: "ad_bids_metrics"}, false)
+	require.NoError(t, err)
+	mv := r.GetMetricsView()
+
+	q := &queries.MetricsViewComparison{
+		MetricsViewName: "ad_bids_metrics",
+		DimensionName:   "dom",
+		Measures: []*runtimev1.MetricsViewAggregationMeasure{
+			{
+				Name: "measure_1",
+			},
+		},
+		MetricsView: mv.Spec,
+		TimeRange: &runtimev1.TimeRange{
+			Start: ctr.Result.Min,
+			End:   timestamppb.New(maxTime),
+		},
+		ComparisonTimeRange: &runtimev1.TimeRange{
+			Start: timestamppb.New(maxTime),
+			End:   ctr.Result.Max,
+		},
+		Sort: []*runtimev1.MetricsViewComparisonSort{
+			{
+				Name:     "dom",
+				SortType: runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_UNSPECIFIED,
+				Desc:     true,
+			},
+		},
+		Having: &runtimev1.Expression{
+			Expression: &runtimev1.Expression_Cond{
+				Cond: &runtimev1.Condition{
+					Op: runtimev1.Operation_OPERATION_GT,
+					Exprs: []*runtimev1.Expression{
+						{
+							Expression: &runtimev1.Expression_Ident{
+								Ident: "measure_1",
+							},
+						},
+						{
+							Expression: &runtimev1.Expression_Val{
+								Val: structpb.NewNumberValue(3.25),
+							},
+						},
+					},
+				},
+			},
+		},
+		Aliases: []*runtimev1.MetricsViewComparisonMeasureAlias{
+			{
+				Name:  "measure_1",
+				Type:  runtimev1.MetricsViewComparisonMeasureType_METRICS_VIEW_COMPARISON_MEASURE_TYPE_BASE_VALUE,
+				Alias: "measure_1",
+			},
+		},
+		Limit: 250,
+	}
+
+	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())
+}
+
 func TestMetricsViewsComparison_measure_filters_with_compare_aliases(t *testing.T) {
-	rt, instanceID := testruntime.NewInstanceForProject(t, "ad_bids")
+	rt, instanceID := testruntime.NewInstanceForProject(t, AdBidsProject)
 
 	ctr := &queries.ColumnTimeRange{
 		TableName:  "ad_bids",
-		ColumnName: "timestamp",
+		ColumnName: AdBidsTimestamp,
 	}
 	err := ctr.Resolve(context.Background(), rt, instanceID, 0)
 	require.NoError(t, err)

runtime/queries/metricsview_timeseries.go

@@ -320,7 +320,7 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore
 		sql = q.buildDuckDBSQL(mv, tsAlias, selectCols, whereClause, havingClause, timezone)
 	case drivers.DialectDruid:
 		args = append([]any{timezone}, args...)
-		sql = q.buildDruidSQL(args, mv, tsAlias, selectCols, havingClause, whereClause)
+		sql = q.buildDruidSQL(args, mv, tsAlias, selectCols, whereClause, havingClause)
 	default:
 		return "", "", nil, fmt.Errorf("not available for dialect '%s'", olap.Dialect())
 	}