Skip to content

Commit

Permalink
Adding some tests for having clauses
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaHegde committed Dec 13, 2023
1 parent 171a7f2 commit 9d9b016
Show file tree
Hide file tree
Showing 14 changed files with 280 additions and 227 deletions.
2 changes: 0 additions & 2 deletions proto/rill/runtime/v1/queries.proto
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,6 @@ message MetricsViewTotalsRequest {
google.protobuf.Timestamp time_start = 4;
google.protobuf.Timestamp time_end = 5;
Expression where = 7;
Expression having = 10;
int32 priority = 8;
}

Expand All @@ -445,7 +444,6 @@ message MetricsViewRowsRequest {
google.protobuf.Timestamp time_end = 4;
TimeGrain time_granularity = 10;
Expression where = 5;
Expression having = 12;
repeated MetricsViewSort sort = 6;
int32 limit = 7 [(validate.rules).int32.gte = 0];
int64 offset = 8 [(validate.rules).int64.gte = 0];
Expand Down
2 changes: 1 addition & 1 deletion runtime/queries/metricsview.go
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ func duckDBCopyExport(ctx context.Context, w io.Writer, opts *runtime.ExportOpti

func (q *MetricsViewRows) generateFilename(mv *runtimev1.MetricsViewSpec) string {
filename := strings.ReplaceAll(mv.Table, `"`, `_`)
if q.TimeStart != nil || q.TimeEnd != nil || q.Where != nil || q.Having != nil {
if q.TimeStart != nil || q.TimeEnd != nil || q.Where != nil {
filename += "_filtered"
}
return filename
Expand Down
1 change: 0 additions & 1 deletion runtime/queries/metricsview_rows.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type MetricsViewRows struct {
TimeEnd *timestamppb.Timestamp `json:"time_end,omitempty"`
TimeGranularity runtimev1.TimeGrain `json:"time_granularity,omitempty"`
Where *runtimev1.Expression `json:"where,omitempty"`
Having *runtimev1.Expression `json:"having,omitempty"`
Sort []*runtimev1.MetricsViewSort `json:"sort,omitempty"`
Limit *int64 `json:"limit,omitempty"`
Offset int64 `json:"offset,omitempty"`
Expand Down
39 changes: 31 additions & 8 deletions runtime/queries/metricsview_timeseries.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func (q *MetricsViewTimeSeries) Resolve(ctx context.Context, rt *runtime.Runtime
return fmt.Errorf("error building query: %w", err)
}

fmt.Println(sql, args)
rows, err := olap.Execute(ctx, &drivers.Statement{
Query: sql,
Args: args,
Expand Down Expand Up @@ -264,10 +265,12 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore
return "", "", nil, err
}

measureAliases := map[string]identifier{}
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"
Expand All @@ -290,6 +293,16 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore
args = append(args, clauseArgs...)
}

havingClause := ""
if q.Having != nil {
clause, clauseArgs, err := buildFromExpression(q.Having, measureAliases, olap.Dialect())
if err != nil {
return "", "", nil, err
}
havingClause = " HAVING " + clause
args = append(args, clauseArgs...)
}

tsAlias := tempName("_ts_")
timezone := "UTC"
if q.TimeZone != "" {
Expand All @@ -299,18 +312,18 @@ func (q *MetricsViewTimeSeries) buildMetricsTimeseriesSQL(olap drivers.OLAPStore
var sql string
switch olap.Dialect() {
case drivers.DialectDuckDB:
sql = q.buildDuckDBSQL(mv, tsAlias, selectCols, whereClause, timezone)
sql = q.buildDuckDBSQL(mv, tsAlias, selectCols, whereClause, havingClause, timezone)
case drivers.DialectDruid:
args = append([]any{timezone}, args...)
sql = q.buildDruidSQL(args, mv, tsAlias, selectCols, whereClause)
sql = q.buildDruidSQL(args, mv, tsAlias, selectCols, havingClause, whereClause)
default:
return "", "", nil, fmt.Errorf("not available for dialect '%s'", olap.Dialect())
}

return sql, tsAlias, args, nil
}

func (q *MetricsViewTimeSeries) buildDruidSQL(args []any, mv *runtimev1.MetricsViewSpec, tsAlias string, selectCols []string, whereClause string) string {
func (q *MetricsViewTimeSeries) buildDruidSQL(args []any, mv *runtimev1.MetricsViewSpec, tsAlias string, selectCols []string, whereClause, havingClause string) string {
tsSpecifier := convertToDruidTimeFloorSpecifier(q.TimeGranularity)

timeClause := fmt.Sprintf("time_floor(%s, '%s', null, CAST(? AS VARCHAR))", safeName(mv.TimeDimension), tsSpecifier)
Expand All @@ -323,18 +336,19 @@ func (q *MetricsViewTimeSeries) buildDruidSQL(args []any, mv *runtimev1.MetricsV
}

sql := fmt.Sprintf(
`SELECT %s AS %s, %s FROM %s WHERE %s GROUP BY 1 ORDER BY 1`,
`SELECT %s AS %s, %s FROM %s WHERE %s GROUP BY 1 %s ORDER BY 1`,
timeClause,
tsAlias,
strings.Join(selectCols, ", "),
safeName(mv.Table),
whereClause,
havingClause,
)

return sql
}

func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, tsAlias string, selectCols []string, whereClause, timezone string) string {
func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, tsAlias string, selectCols []string, whereClause, havingClause, timezone string) string {
dateTruncSpecifier := convertToDateTruncSpecifier(q.TimeGranularity)

shift := "" // shift to accommodate FirstDayOfWeek or FirstMonthOfYear
Expand All @@ -358,14 +372,17 @@ func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, ts
%[4]s
FROM %[5]s
WHERE %[6]s
GROUP BY 1 ORDER BY 1`,
GROUP BY 1
%[8]s
ORDER BY 1`,
dateTruncSpecifier, // 1
safeName(mv.TimeDimension), // 2
tsAlias, // 3
strings.Join(selectCols, ", "), // 4
safeName(mv.Table), // 5
whereClause, // 6
timezone, // 7
havingClause, // 8
)
} else { // date_trunc is faster than time_bucket for year, month, week
sql = fmt.Sprintf(
Expand All @@ -375,14 +392,17 @@ func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, ts
%[4]s
FROM %[5]s
WHERE %[6]s
GROUP BY 1 ORDER BY 1`,
GROUP BY 1
%[8]s
ORDER BY 1`,
dateTruncSpecifier, // 1
safeName(mv.TimeDimension), // 2
tsAlias, // 3
strings.Join(selectCols, ", "), // 4
safeName(mv.Table), // 5
whereClause, // 6
timezone, // 7
havingClause, // 8
)
}
} else {
Expand All @@ -393,7 +413,9 @@ func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, ts
%[4]s
FROM %[5]s
WHERE %[6]s
GROUP BY 1 ORDER BY 1`,
GROUP BY 1
%[9]s
ORDER BY 1`,
dateTruncSpecifier, // 1
safeName(mv.TimeDimension), // 2
tsAlias, // 3
Expand All @@ -402,6 +424,7 @@ func (q *MetricsViewTimeSeries) buildDuckDBSQL(mv *runtimev1.MetricsViewSpec, ts
whereClause, // 6
timezone, // 7
shift, // 8
havingClause, // 9
)
}

Expand Down
110 changes: 78 additions & 32 deletions runtime/queries/metricsview_timeseries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,10 @@ func TestMetricsViewTimeSeries_DayLightSavingsBackwards_Sparse_Daily(t *testing.

q := &queries.MetricsViewTimeSeries{
MeasureNames: []string{"total_records"},
Filter: &runtimev1.MetricsViewFilter{
Include: []*runtimev1.MetricsViewFilter_Cond{
{
Name: "label",
In: []*structpb.Value{toStructpbValue(t, "sparse_day")},
},
},
},
Where: testruntime.FilterInClause(
testruntime.FilterColumn("label"),
[]*runtimev1.Expression{testruntime.FilterValue(toStructpbValue(t, "sparse_day"))},
),
MetricsViewName: "timeseries_dst_backwards",
MetricsView: mv.Spec,
TimeStart: parseTime(t, "2023-11-03T04:00:00.000Z"),
Expand Down Expand Up @@ -474,14 +470,10 @@ func TestMetricsViewTimeSeries_DayLightSavingsBackwards_Sparse_Hourly(t *testing

q := &queries.MetricsViewTimeSeries{
MeasureNames: []string{"total_records"},
Filter: &runtimev1.MetricsViewFilter{
Include: []*runtimev1.MetricsViewFilter_Cond{
{
Name: "label",
In: []*structpb.Value{toStructpbValue(t, "sparse_hour")},
},
},
},
Where: testruntime.FilterInClause(
testruntime.FilterColumn("label"),
[]*runtimev1.Expression{testruntime.FilterValue(toStructpbValue(t, "sparse_hour"))},
),
MetricsViewName: "timeseries_dst_backwards",
MetricsView: mv.Spec,
TimeStart: parseTime(t, "2023-11-05T03:00:00.000Z"),
Expand Down Expand Up @@ -591,14 +583,10 @@ func TestMetricsViewTimeSeries_DayLightSavingsForwards_Sparse_Daily(t *testing.T

q := &queries.MetricsViewTimeSeries{
MeasureNames: []string{"total_records"},
Filter: &runtimev1.MetricsViewFilter{
Include: []*runtimev1.MetricsViewFilter_Cond{
{
Name: "label",
In: []*structpb.Value{toStructpbValue(t, "sparse_day")},
},
},
},
Where: testruntime.FilterInClause(
testruntime.FilterColumn("label"),
[]*runtimev1.Expression{testruntime.FilterValue(toStructpbValue(t, "sparse_day"))},
),
MetricsViewName: "timeseries_dst_forwards",
MetricsView: mv.Spec,
TimeStart: parseTime(t, "2023-03-10T05:00:00.000Z"),
Expand Down Expand Up @@ -673,14 +661,10 @@ func TestMetricsViewTimeSeries_DayLightSavingsForwards_Sparse_Hourly(t *testing.

q := &queries.MetricsViewTimeSeries{
MeasureNames: []string{"total_records"},
Filter: &runtimev1.MetricsViewFilter{
Include: []*runtimev1.MetricsViewFilter_Cond{
{
Name: "label",
In: []*structpb.Value{toStructpbValue(t, "sparse_hour")},
},
},
},
Where: testruntime.FilterInClause(
testruntime.FilterColumn("label"),
[]*runtimev1.Expression{testruntime.FilterValue(toStructpbValue(t, "sparse_hour"))},
),
MetricsViewName: "timeseries_dst_forwards",
MetricsView: mv.Spec,
TimeStart: parseTime(t, "2023-03-12T04:00:00.000Z"),
Expand Down Expand Up @@ -711,6 +695,68 @@ func TestMetricsViewTimeSeries_DayLightSavingsForwards_Sparse_Hourly(t *testing.
require.Nil(t, q.Result.Data[i].Records.AsMap()["total_records"])
}

func TestMetricsViewTimeSeries_having_clause(t *testing.T) {
rt, instanceID := testruntime.NewInstanceForProject(t, "timeseries")

ctrl, err := rt.Controller(context.Background(), instanceID)
require.NoError(t, err)
r, err := ctrl.Get(context.Background(), &runtimev1.ResourceName{Kind: runtime.ResourceKindMetricsView, Name: "timeseries_gaps"}, false)
require.NoError(t, err)
mv := r.GetMetricsView()

q := &queries.MetricsViewTimeSeries{
MeasureNames: []string{"sum_imps"},
MetricsViewName: "timeseries_gaps",
MetricsView: mv.Spec,
TimeStart: parseTime(t, "2019-01-01T00:00:00Z"),
TimeEnd: parseTime(t, "2019-01-07T00:00:00Z"),
TimeGranularity: runtimev1.TimeGrain_TIME_GRAIN_DAY,
Having: &runtimev1.Expression{
Expression: &runtimev1.Expression_Cond{
Cond: &runtimev1.Condition{
Op: runtimev1.Operation_OPERATION_LTE,
Exprs: []*runtimev1.Expression{
{
Expression: &runtimev1.Expression_Ident{
Ident: "sum_imps",
},
},
{
Expression: &runtimev1.Expression_Val{
Val: structpb.NewNumberValue(3),
},
},
},
},
},
},
Limit: 250,
}
err = q.Resolve(context.Background(), rt, instanceID, 0)
require.NoError(t, err)
require.NotEmpty(t, q.Result)
rows := q.Result.Data
require.Len(t, rows, 6)
i := 0
require.Equal(t, parseTime(t, "2019-01-01T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.NotNil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
i++
require.Equal(t, parseTime(t, "2019-01-02T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.Nil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
i++
require.Equal(t, parseTime(t, "2019-01-03T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.Nil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
i++
require.Equal(t, parseTime(t, "2019-01-04T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.Nil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
i++
require.Equal(t, parseTime(t, "2019-01-05T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.Nil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
i++
require.Equal(t, parseTime(t, "2019-01-06T00:00:00Z").AsTime(), rows[i].Ts.AsTime())
require.NotNil(t, q.Result.Data[i].Records.AsMap()["sum_imps"])
}

func toStructpbValue(t *testing.T, v any) *structpb.Value {
sv, err := structpb.NewValue(v)
require.NoError(t, err)
Expand Down
15 changes: 1 addition & 14 deletions runtime/queries/metricsview_totals.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,10 @@ func (q *MetricsViewTotals) buildMetricsTotalsSQL(mv *runtimev1.MetricsViewSpec,
return "", nil, err
}

measureAliases := map[string]identifier{}
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"
Expand All @@ -139,22 +137,11 @@ func (q *MetricsViewTotals) buildMetricsTotalsSQL(mv *runtimev1.MetricsViewSpec,
args = append(args, clauseArgs...)
}

havingClause := ""
if q.Having != nil {
clause, clauseArgs, err := buildFromExpression(q.Having, dimensionAliases(mv), dialect)
if err != nil {
return "", nil, err
}
havingClause += "HAVING " + clause
args = append(args, clauseArgs...)
}

sql := fmt.Sprintf(
"SELECT %s FROM %q WHERE %s %s",
"SELECT %s FROM %q WHERE %s",
strings.Join(selectCols, ", "),
mv.Table,
whereClause,
havingClause,
)
fmt.Println(sql, args)
return sql, args, nil
Expand Down
1 change: 0 additions & 1 deletion runtime/server/downloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ func (s *Server) downloadHandler(w http.ResponseWriter, req *http.Request) {
TimeStart: r.TimeStart,
TimeEnd: r.TimeEnd,
Where: r.Where,
Having: r.Having,
Sort: r.Sort,
Limit: limitPtr,
TimeZone: r.TimeZone,
Expand Down
1 change: 0 additions & 1 deletion runtime/server/queries_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ func (s *Server) MetricsViewRows(ctx context.Context, req *runtimev1.MetricsView
TimeEnd: req.TimeEnd,
TimeGranularity: req.TimeGranularity,
Where: req.Where,
Having: req.Having,
Sort: req.Sort,
Limit: &limit,
Offset: req.Offset,
Expand Down
12 changes: 6 additions & 6 deletions runtime/server/queries_metrics_comparison_toplist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,9 +618,9 @@ func TestServer_MetricsViewComparison_sort_by_base_filter(t *testing.T) {
Desc: true,
},
},
Where: filterNotInClause(
filterColumn("domain"),
[]*runtimev1.Expression{filterValue(structpb.NewStringValue("yahoo.com"))},
Where: testruntime.FilterNotInClause(
testruntime.FilterColumn("domain"),
[]*runtimev1.Expression{testruntime.FilterValue(structpb.NewStringValue("yahoo.com"))},
),
Exact: true,
})
Expand Down Expand Up @@ -1071,9 +1071,9 @@ func TestServer_MetricsViewComparison_no_comparison_complete_source_sanity_test(
Desc: false,
},
},
Where: filterNotInClause(
filterColumn("pub"),
[]*runtimev1.Expression{filterValue(structpb.NewStringValue("Yahoo"))},
Where: testruntime.FilterNotInClause(
testruntime.FilterColumn("pub"),
[]*runtimev1.Expression{testruntime.FilterValue(structpb.NewStringValue("Yahoo"))},
),
Exact: true,
})
Expand Down
Loading

0 comments on commit 9d9b016

Please sign in to comment.