diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 33937e6e0..3e585910c 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -538,7 +538,7 @@ func (n *binOpPlanExpression) Evaluate(currentRow []interface{}) (interface{}, e return nil, sql3.NewErrInternalf("unhandled operator %d", n.op) } } - return nil, sql3.NewErrInternalf("unexpected type conversion error '%t', '%t'", nlok, nrok) + return nil, sql3.NewErrInternalf("unexpected type conversion error '%T', '%T'", coercedLhs, coercedRhs) case *parser.DataTypeTimestamp: //if either side is nil, return nil diff --git a/sql3/planner/expressionagg.go b/sql3/planner/expressionagg.go index e1098c1f8..eab7f098d 100644 --- a/sql3/planner/expressionagg.go +++ b/sql3/planner/expressionagg.go @@ -122,6 +122,9 @@ func (n *countStarPlanExpression) Plan() map[string]interface{} { result["_expr"] = fmt.Sprintf("%T", n) result["description"] = n.String() result["dataType"] = n.Type().TypeDescription() + if n.arg != nil { + result["arg"] = n.arg.Plan() + } return result } diff --git a/sql3/planner/oppqlgroupby.go b/sql3/planner/oppqlgroupby.go index 5687a9e73..fb0d4387e 100644 --- a/sql3/planner/oppqlgroupby.go +++ b/sql3/planner/oppqlgroupby.go @@ -244,13 +244,27 @@ func (i *pqlGroupByRowIter) Next(ctx context.Context) (types.Row, error) { } //now populate the aggregate value aggIdx := len(i.groupByColumns) - switch i.aggregate.(type) { + switch agg := i.aggregate.(type) { case *countPlanExpression, *countStarPlanExpression: row[aggIdx] = int64(group.Count) - case *countDistinctPlanExpression, *sumPlanExpression: + case *countDistinctPlanExpression: row[aggIdx] = int64(group.Agg) + case *sumPlanExpression: + switch ty := agg.Type().(type) { + case *parser.DataTypeDecimal: + if group.DecimalAgg == nil { + row[aggIdx] = pql.NewDecimal(int64(group.Agg), ty.Scale) + } else { + row[aggIdx] = *group.DecimalAgg + } + case *parser.DataTypeInt: + row[aggIdx] = int64(group.Agg) + default: + return nil, sql3.NewErrInternalf("unhandled sum return type '%T'", ty) + } + case *avgPlanExpression: if group.DecimalAgg == nil { average := float64(group.Agg) / float64(group.Count) diff --git a/sql3/planner/planoptimizer.go b/sql3/planner/planoptimizer.go index 114c29922..039930582 100644 --- a/sql3/planner/planoptimizer.go +++ b/sql3/planner/planoptimizer.go @@ -1355,7 +1355,7 @@ func fixFieldRefIndexesForHaving(ctx context.Context, scope *OptimizerScope, a * return TransformExpr(exp, func(e types.PlanExpression) (types.PlanExpression, bool, error) { switch typedExpr := e.(type) { case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, - *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, *countStarPlanExpression, *percentilePlanExpression: for i, col := range schema { if strings.EqualFold(typedExpr.String(), col.ColumnName) { diff --git a/sql3/test/defs/defs_having.go b/sql3/test/defs/defs_having.go index a9aa2abec..3b282ce52 100644 --- a/sql3/test/defs/defs_having.go +++ b/sql3/test/defs/defs_having.go @@ -1,5 +1,7 @@ package defs +import "github.com/featurebasedb/featurebase/v3/pql" + var selectHavingTests = TableTest{ name: "select-having", Table: tbl( @@ -18,20 +20,20 @@ var selectHavingTests = TableTest{ srcRow(int64(2), int64(22), []int64{21, 22, 23}, int64(201), "str2", []string{"a2", "b2", "c2"}, float64(234.56)), srcRow(int64(3), int64(33), []int64{31, 32, 33}, int64(301), "str3", []string{"a3", "b3", "c3"}, float64(345.67)), srcRow(int64(4), int64(44), []int64{41, 42, 43}, int64(401), "str4", []string{"a4", "b4", "c4"}, float64(456.78)), + srcRow(int64(5), int64(11), []int64{11, 12, 13}, int64(101), "str1", []string{"a5", "b5", "c5"}, float64(567.89)), ), ), SQLTests: []SQLTest{ { - name: "select-having", + name: "countfieldincluded", SQLs: sqls( - "select count(*), an_int from having_test group by an_int having count(*) > 0", + "select count(an_int), an_int from having_test group by an_int having count(an_int) = 1", ), ExpHdrs: hdrs( hdr("", fldTypeInt), hdr("an_int", fldTypeInt), ), ExpRows: rows( - row(int64(1), int64(11)), row(int64(1), int64(22)), row(int64(1), int64(33)), row(int64(1), int64(44)), @@ -39,5 +41,99 @@ var selectHavingTests = TableTest{ Compare: CompareExactUnordered, SortStringKeys: true, }, + { + name: "countfieldnotincluded", + SQLs: sqls( + "select an_int from having_test group by an_int having count(an_int) = 1", + ), + ExpHdrs: hdrs( + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row(int64(22)), + row(int64(33)), + row(int64(44)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + name: "countstarincluded", + SQLs: sqls( + "select count(*), an_int from having_test group by an_int having count(*) > 1", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row(int64(2), int64(11)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + name: "countstarnotincluded", + SQLs: sqls( + "select an_int from having_test group by an_int having count(*) > 1", + ), + ExpHdrs: hdrs( + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row( + int64(11), + ), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + name: "sum-dec", + SQLs: sqls( + "select sum(a_decimal), an_int from having_test group by an_int having sum(a_decimal) < 250.00", + ), + ExpHdrs: hdrs( + hdr("", fldTypeDecimal2), + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row(pql.NewDecimal(23456, 2), int64(22)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + name: "sum-int", + SQLs: sqls( + "select sum(an_int), an_int from having_test group by an_int having sum(an_int) < 25", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row(int64(22), int64(11)), + row(int64(22), int64(22)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + // Fails in DAX because the string isn't translated. + // { + // name: "string", + // SQLs: sqls( + // "select a_string, count(*) from having_test group by a_string having count(*) > 1", + // ), + // ExpHdrs: hdrs( + // hdr("a_string", fldTypeString), + // hdr("", fldTypeInt), + // ), + // ExpRows: rows( + // row(string("str1"), int64(2)), + // ), + // Compare: CompareExactUnordered, + // SortStringKeys: true, + // }, }, }