mirror of
https://github.com/featurebasedb/featurebase.git
synced 2026-08-28 02:44:59 +00:00
handle count(*) in having correctly (#2274)
* correct reference for `having count(*)` It turns out that `having count(*) ...` was always treating the count(*) as exactly 1. After studying this a lot, I noticed that in fact, we correctly handle other counts. The reason is that there's already code to recognize aggregates in `having` clauses as matching aggregates that are being computed -- but it only covers the other aggregate clause types, not the newly added `countStarPlanExpression` from making `count(*)` work even if there's no `_id` field. We add several corresponding test cases. * fix sum(a_decimal) type conversion Added a test case for this, and also added a fix for it. Underlying issue: qualifiedRefPlanExpression could end up producing an int64 instead of a pql.Decimal, even though it had expected type Decimal. Originally this worked by politely converting an int64 to a pql.Decimal in the Evaluate phase, but this was not ideal; the real question is why it was coming out as an int64 at that step. Showed this to Pat, who spent a while studying it and produced a better fix. * temporarily comment out test which fails in DAX
This commit is contained in:
seebs
GitHub
parent
a479441ea2
commit
b35c240da7
5 changed files with 120 additions and 7 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
// },
|
||||
},
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue