From 67a79602f910c99cbb44eec2640d93347c253344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Jerman?= Date: Fri, 16 Jul 2021 13:03:36 +0200 Subject: [PATCH] Imply group column kind based on context --- pkg/report/step_group.go | 24 ++-- store/rdbms/compose_record_datasource.go | 125 +++++++++--------- .../integration/report_grouping_base.json | 8 +- 3 files changed, 80 insertions(+), 77 deletions(-) diff --git a/pkg/report/step_group.go b/pkg/report/step_group.go index 282baf12b..ccd05038e 100644 --- a/pkg/report/step_group.go +++ b/pkg/report/step_group.go @@ -18,7 +18,7 @@ type ( } GroupDefinition struct { - Groups []*GroupColumn `json:"groups"` + Keys []*GroupKey `json:"keys"` Columns []*GroupColumn `json:"columns"` Rows *RowDefinition `json:"rows,omitempty"` } @@ -29,18 +29,26 @@ type ( GroupDefinition } + GroupKey struct { + // Name defines the alias for the new column + Name string `json:"name"` + // Label defines the user friendly name for the column + Label string `json:"label"` + // Column defines what column to use when defining the group + Column string `json:"column"` + // Group defines the grouping function to apply + Group string `json:"group"` + } + GroupColumn struct { // Name defines the alias for the new column Name string `json:"name"` // Label defines the user friendly name for the column Label string `json:"label"` - // Expr defines the expression to transform the column - Expr string `json:"expr"` + // Column defines the column to produce the aggregated data + Column string `json:"column"` // Aggregate defines the aggregation function to apply Aggregate string `json:"aggregate"` - - // @todo imply from context - Kind string `json:"kind"` } ) @@ -76,12 +84,12 @@ func (j *stepGroup) Validate() error { case j.def.Source == "": return errors.New(pfx + "groupping dimension not defined") - case len(j.def.Groups) == 0: + case len(j.def.Keys) == 0: return errors.New(pfx + "no group defined") } // columns... - for i, g := range j.def.Groups { + for i, g := range j.def.Keys { if g.Name == "" { return fmt.Errorf("%sgroup key alias missing for group: %d", pfx, i) } diff --git a/store/rdbms/compose_record_datasource.go b/store/rdbms/compose_record_datasource.go index 212e96c95..5468eb98e 100644 --- a/store/rdbms/compose_record_datasource.go +++ b/store/rdbms/compose_record_datasource.go @@ -21,7 +21,7 @@ type ( module *types.Module // @todo use these - supportedAggregationFunctions map[string]bool + supportedAggregationFunctions map[string]string supportedFilterFunctions map[string]bool store *Store @@ -32,10 +32,22 @@ type ( cols report.FrameColumnSet qBuilder squirrel.SelectBuilder nestLevel int - levelColumns map[string]bool + levelColumns map[string]string } ) +var ( + supportedAggregationFunctions = slice.ToStringBoolMap([]string{ + "COUNT", + "SUM", + "MAX", + "MIN", + "AVG", + }) + + // supportedGroupingFunctions = ... +) + func ComposeRecordDatasourceBuilder(s *Store, module *types.Module, ld *report.LoadStepDefinition) (report.Datasource, error) { var err error @@ -44,26 +56,7 @@ func ComposeRecordDatasourceBuilder(s *Store, module *types.Module, ld *report.L module: module, store: s, cols: ld.Columns, - levelColumns: make(map[string]bool), - - supportedAggregationFunctions: slice.ToStringBoolMap([]string{ - "COUNT", - "SUM", - "MAX", - "MIN", - "AVG", - }), - - supportedFilterFunctions: slice.ToStringBoolMap([]string{ - "CONCAT", - "QUARTER", - "YEAR", - "DATE", - "NOW", - "DATE_ADD", - "DATE_SUB", - "DATE_FORMAT", - }), + levelColumns: make(map[string]string), } r.qBuilder, err = r.baseQuery(ld.Rows) @@ -95,33 +88,27 @@ func (r *recordDatasource) Group(d report.GroupDefinition, name string) (bool, e r.name = name }() + var ( + q = squirrel.Select() + auxKind = "" + ok = false + ) + cls := r.levelColumns - r.levelColumns = make(map[string]bool) + r.levelColumns = make(map[string]string) gCols := make(report.FrameColumnSet, 0, 10) - q := squirrel.Select() - - // @todo allow some transformation functions within the agg. functions - parser := ql.NewParser() - parser.OnFunction = r.stdAggregationHandler - parser.OnIdent = func(i ql.Ident) (ql.Ident, error) { - if !cls[i.Value] { - return i, fmt.Errorf("column %s does not exist on level %d", i.Value, r.nestLevel) + for _, g := range d.Keys { + auxKind, ok = cls[g.Column] + if !ok { + return false, fmt.Errorf("column %s does not exist on level %d", g.Column, r.nestLevel) } - i.Value = fmt.Sprintf("l%d.%s", r.nestLevel, i.Value) - return i, nil - } + // @todo... + // if g.Group != "" {...} - for _, g := range d.Groups { - e, err := parser.ParseExpression(g.Expr) - if err != nil { - return false, err - } - - // @todo imply based on context - c := report.MakeColumnOfKind(g.Kind) + c := report.MakeColumnOfKind(auxKind) c.Name = g.Name c.Label = g.Label if c.Label == "" { @@ -129,43 +116,51 @@ func (r *recordDatasource) Group(d report.GroupDefinition, name string) (bool, e } gCols = append(gCols, c) - r.levelColumns[g.Name] = true - q = q.Column(fmt.Sprintf("(%s) as `%s`", e.String(), g.Name)). - GroupBy(e.String()) + r.levelColumns[g.Name] = auxKind + q = q.Column(fmt.Sprintf("%s as `%s`", g.Column, g.Name)). + GroupBy(g.Column) } - var e ql.ASTNode - var err error + var aggregate string for _, c := range d.Columns { - if c.Aggregate != "" { - e, err = parser.ParseExpression(fmt.Sprintf("%s(%s)", c.Aggregate, c.Expr)) - if err != nil { - return false, err + aggregate = strings.ToUpper(c.Aggregate) + + if c.Column == "" { + if c.Aggregate == "" { + return false, fmt.Errorf("the aggregation function is required when the column is omitted") } } else { - e, err = parser.ParseExpression(c.Expr) - if err != nil { - return false, err + auxKind, ok = cls[c.Column] + if !ok { + return false, fmt.Errorf("column %s does not exist on level %d", c.Column, r.nestLevel) } } - var col *report.FrameColumn - if c.Kind != "" { - col = report.MakeColumnOfKind(c.Kind) - } else { - // @todo imply based on context - col = report.MakeColumnOfKind("Number") + qParam := c.Column + if c.Aggregate != "" { + if !supportedAggregationFunctions[aggregate] { + return false, fmt.Errorf("aggregation function not supported: %s", c.Aggregate) + } + + // when an aggregation function is defined, the output is always numeric + auxKind = "Number" + + qParam = fmt.Sprintf("%s(%s)", aggregate, c.Column) + } else if qParam == "" { + qParam = "*" } + + col := report.MakeColumnOfKind(auxKind) col.Name = c.Name col.Label = c.Label if col.Label == "" { col.Label = col.Name } gCols = append(gCols, col) - r.levelColumns[c.Name] = true + r.levelColumns[c.Name] = auxKind q = q. - Column(fmt.Sprintf("%s as `%s`", e.String(), c.Name)) + Column(fmt.Sprintf("%s as `%s`", qParam, c.Name)) } if d.Rows != nil { @@ -367,14 +362,14 @@ func (r *recordDatasource) baseQuery(f *report.RowDefinition) (sqb squirrel.Sele report = report.LeftJoin(strings.ReplaceAll(joinTpl, "%s", f.Name)). Column(f.Name + ".value as " + f.Name) - r.levelColumns[f.Name] = true + r.levelColumns[f.Name] = f.Kind } if f != nil { // @todo functions and function validation parser := ql.NewParser() parser.OnIdent = func(i ql.Ident) (ql.Ident, error) { - if !r.levelColumns[i.Value] { + if _, ok := r.levelColumns[i.Value]; !ok { return i, fmt.Errorf("column %s does not exist on level %d", i.Value, r.nestLevel) } @@ -456,7 +451,7 @@ func (b *recordDatasource) Cast(row sqlx.ColScanner, out *report.Frame) error { // Identifiers should be names of the fields (physical table columns OR json fields, defined in module) func (b *recordDatasource) stdAggregationHandler(f ql.Function) (ql.ASTNode, error) { - if !b.supportedAggregationFunctions[strings.ToUpper(f.Name)] { + if !supportedAggregationFunctions[strings.ToUpper(f.Name)] { return f, fmt.Errorf("unsupported aggregate function %q", f.Name) } diff --git a/tests/reporter/testdata/integration/report_grouping_base.json b/tests/reporter/testdata/integration/report_grouping_base.json index 22d75c889..57ca0c713 100644 --- a/tests/reporter/testdata/integration/report_grouping_base.json +++ b/tests/reporter/testdata/integration/report_grouping_base.json @@ -18,12 +18,12 @@ { "group": { "name": "grouped", "source": "users", - "groups": [ - { "name": "by_name", "expr": "first_name", "kind": "String" } + "keys": [ + { "name": "by_name", "column": "first_name" } ], "columns": [ - { "name": "count", "aggregate": "count", "expr": "*" , "kind": "Number"}, - { "name": "total", "aggregate": "sum", "expr": "number_of_numbers", "kind": "Number" } + { "name": "count", "aggregate": "count" }, + { "name": "total", "aggregate": "sum", "column": "number_of_numbers" } ] }} ]