Skip to content

Commit

Permalink
[Dashboard] Support arbitrary non-aggregate expressions in dimensions (
Browse files Browse the repository at this point in the history
…#3702)

* Adding support for dimension expression parsing

* Fix aggregation and toplist for dimension expressions

* Update toplist API

* Integrate with new filter expression

* Add support for unnest

* Update for druid

* Fix tests

* Adding validation for aggreagates

* Add more tests

* Fix aggreagation query

* PR comments

* Fix escaping issues

* More PR comments
  • Loading branch information
AdityaHegde authored Dec 22, 2023
1 parent ada51f7 commit 018ae8a
Show file tree
Hide file tree
Showing 21 changed files with 622 additions and 307 deletions.
14 changes: 12 additions & 2 deletions proto/gen/rill/runtime/v1/resources.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions proto/gen/rill/runtime/v1/resources.pb.validate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions proto/gen/rill/runtime/v1/runtime.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,8 @@ definitions:
type: string
column:
type: string
expression:
type: string
label:
type: string
description:
Expand Down
1 change: 1 addition & 0 deletions proto/rill/runtime/v1/resources.proto
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ message MetricsViewSpec {
message DimensionV2 {
string name = 1;
string column = 2;
string expression = 6;
string label = 3;
string description = 4;
bool unnest = 5;
Expand Down
18 changes: 7 additions & 11 deletions runtime/compilers/rillv1/parse_metrics_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type MetricsViewYAML struct {
Name string
Label string
Column string
Expression string
Property string // For backwards compatibility
Description string
Ignore bool `yaml:"ignore"`
Expand Down Expand Up @@ -213,7 +214,7 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
}

names := make(map[string]bool)
columns := make(map[string]bool)

for i, dim := range tmp.Dimensions {
if dim == nil || dim.Ignore {
continue
Expand All @@ -233,17 +234,15 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
}
}

if (dim.Column == "" && dim.Expression == "") || (dim.Column != "" && dim.Expression != "") {
return fmt.Errorf("exactly one of column or expression should be set for dimension: %q", dim.Name)
}

lower := strings.ToLower(dim.Name)
if ok := names[lower]; ok {
return fmt.Errorf("found duplicate dimension or measure name %q", dim.Name)
}
names[lower] = true

lower = strings.ToLower(dim.Column)
if ok := columns[lower]; ok {
return fmt.Errorf("found duplicate dimension column name %q", dim.Column)
}
columns[lower] = true
}

measureCount := 0
Expand All @@ -265,10 +264,6 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
}
names[lower] = true

if ok := columns[lower]; ok {
return fmt.Errorf("measure name %q coincides with a dimension column name", measure.Name)
}

if measure.FormatPreset != "" && measure.FormatD3 != "" {
return fmt.Errorf(`cannot set both "format_preset" and "format_d3" for a measure`)
}
Expand Down Expand Up @@ -424,6 +419,7 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
spec.Dimensions = append(spec.Dimensions, &runtimev1.MetricsViewSpec_DimensionV2{
Name: dim.Name,
Column: dim.Column,
Expression: dim.Expression,
Label: dim.Label,
Description: dim.Description,
Unnest: dim.Unnest,
Expand Down
12 changes: 8 additions & 4 deletions runtime/compilers/rillv1/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ materialize: true
model: m2
dimensions:
- name: a
column: a
measures:
- name: b
expression: count(*)
Expand Down Expand Up @@ -174,7 +175,7 @@ SELECT * FROM {{ ref "m2" }}
MetricsViewSpec: &runtimev1.MetricsViewSpec{
Table: "m2",
Dimensions: []*runtimev1.MetricsViewSpec_DimensionV2{
{Name: "a"},
{Name: "a", Column: "a"},
},
Measures: []*runtimev1.MetricsViewSpec_MeasureV2{
{Name: "b", Expression: "count(*)"},
Expand Down Expand Up @@ -841,6 +842,7 @@ dashboards:
table: t1
dimensions:
- name: a
column: a
measures:
- name: b
expression: count(*)
Expand All @@ -850,6 +852,7 @@ measures:
table: t2
dimensions:
- name: a
column: a
measures:
- name: b
expression: count(*)
Expand All @@ -868,7 +871,7 @@ security:
MetricsViewSpec: &runtimev1.MetricsViewSpec{
Table: "t1",
Dimensions: []*runtimev1.MetricsViewSpec_DimensionV2{
{Name: "a"},
{Name: "a", Column: "a"},
},
Measures: []*runtimev1.MetricsViewSpec_MeasureV2{
{Name: "b", Expression: "count(*)"},
Expand All @@ -887,7 +890,7 @@ security:
MetricsViewSpec: &runtimev1.MetricsViewSpec{
Table: "t2",
Dimensions: []*runtimev1.MetricsViewSpec_DimensionV2{
{Name: "a"},
{Name: "a", Column: "a"},
},
Measures: []*runtimev1.MetricsViewSpec_MeasureV2{
{Name: "b", Expression: "count(*)"},
Expand Down Expand Up @@ -971,6 +974,7 @@ func TestMetricsViewAvoidSelfCyclicRef(t *testing.T) {
model: d1
dimensions:
- name: a
column: a
measures:
- name: b
expression: count(*)
Expand All @@ -985,7 +989,7 @@ measures:
MetricsViewSpec: &runtimev1.MetricsViewSpec{
Table: "d1",
Dimensions: []*runtimev1.MetricsViewSpec_DimensionV2{
{Name: "a"},
{Name: "a", Column: "a"},
},
Measures: []*runtimev1.MetricsViewSpec_MeasureV2{
{Name: "b", Expression: "count(*)"},
Expand Down
9 changes: 2 additions & 7 deletions runtime/queries/column_timeseries.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,8 @@ func (q *ColumnTimeseries) Resolve(ctx context.Context, rt *runtime.Runtime, ins
}

return olap.WithConnection(ctx, priority, false, false, func(ctx context.Context, ensuredCtx context.Context, _ *sql.Conn) error {
filter, args, err := buildFilterClauseForMetricsViewFilter(q.MetricsView, q.MetricsViewFilter, olap.Dialect(), q.MetricsViewPolicy)
if err != nil {
return err
}
if filter != "" {
filter = "WHERE 1=1 " + filter
}
filter := ""
var args []any

measures := normaliseMeasures(q.Measures, q.Pixels != 0)
dateTruncSpecifier := convertToDateTruncSpecifier(timeRange.Interval)
Expand Down
Loading

1 comment on commit 018ae8a

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.