From eca3168d63d3c028663a87b738bc1e6b09a2a05c Mon Sep 17 00:00:00 2001 From: pokeeffe-molecula <85502298+pokeeffe-molecula@users.noreply.github.com> Date: Tue, 13 Dec 2022 17:43:37 -0600 Subject: [PATCH] implement having; create view experiment (#2357) --- sql3/errors.go | 10 ++- sql3/parser/ast_test.go | 5 +- sql3/parser/parser.go | 28 ++++---- sql3/parser/parser_test.go | 20 +++--- sql3/planner/compileselect.go | 119 ++++++++++++++++++++++++++++--- sql3/planner/executionplanner.go | 15 ---- sql3/planner/expression.go | 10 ++- sql3/planner/opfilter.go | 3 +- sql3/planner/ophaving.go | 95 ++++++++++++++++++++++++ sql3/planner/opquery.go | 3 - sql3/planner/planoptimizer.go | 96 ++++++++++++++++++++++++- sql3/test/defs/defs.go | 1 + sql3/test/defs/defs_having.go | 43 +++++++++++ 13 files changed, 384 insertions(+), 64 deletions(-) create mode 100644 sql3/planner/ophaving.go create mode 100644 sql3/test/defs/defs_having.go diff --git a/sql3/errors.go b/sql3/errors.go index ac5816e35..bffc5d433 100644 --- a/sql3/errors.go +++ b/sql3/errors.go @@ -58,7 +58,8 @@ const ( ErrTypeAssignmentIncompatible errors.Code = "ErrTypeAssignmentIncompatible" - ErrInvalidUngroupedColumnReference errors.Code = "ErrInvalidUngroupedColumnReference" + ErrInvalidUngroupedColumnReference errors.Code = "ErrInvalidUngroupedColumnReference" + ErrInvalidUngroupedColumnReferenceInHaving errors.Code = "ErrInvalidUngroupedColumnReferenceInHaving" ErrInvalidTimeUnit errors.Code = "ErrInvalidTimeUnit" ErrInvalidTimeEpoch errors.Code = "ErrInvalidTimeEpoch" @@ -184,6 +185,13 @@ func NewErrInvalidUngroupedColumnReference(line, col int, column string) error { ) } +func NewErrInvalidUngroupedColumnReferenceInHaving(line, col int, column string) error { + return errors.New( + ErrInvalidUngroupedColumnReferenceInHaving, + fmt.Sprintf("[%d:%d] column '%s' invalid in the having clause because it is not contained in an aggregate or the GROUP BY clause", line, col, column), + ) +} + func NewErrInvalidCast(line, col int, from, to string) error { return errors.New( ErrInvalidCast, diff --git a/sql3/parser/ast_test.go b/sql3/parser/ast_test.go index f54ffe95e..f4e0463cc 100644 --- a/sql3/parser/ast_test.go +++ b/sql3/parser/ast_test.go @@ -393,7 +393,6 @@ func TestCreateFunctionStatement_String(t *testing.T) { } func TestCreateViewStatement_String(t *testing.T) { - t.Skip("CREATE VIEW is currently disabled in the parser") AssertStatementStringer(t, &parser.CreateViewStatement{ Name: &parser.Ident{Name: "vw"}, Columns: []*parser.Ident{ @@ -403,7 +402,7 @@ func TestCreateViewStatement_String(t *testing.T) { Select: &parser.SelectStatement{ Columns: []*parser.ResultColumn{{Star: pos(0)}}, }, - }, `CREATE VIEW "vw" ("x", "y") AS SELECT *`) + }, `CREATE VIEW vw (x, y) AS SELECT *`) AssertStatementStringer(t, &parser.CreateViewStatement{ IfNotExists: pos(0), @@ -411,7 +410,7 @@ func TestCreateViewStatement_String(t *testing.T) { Select: &parser.SelectStatement{ Columns: []*parser.ResultColumn{{Star: pos(0)}}, }, - }, `CREATE VIEW IF NOT EXISTS "vw" AS SELECT *`) + }, `CREATE VIEW IF NOT EXISTS vw AS SELECT *`) } func TestDeleteStatement_String(t *testing.T) { diff --git a/sql3/parser/parser.go b/sql3/parser/parser.go index 763d7e6ab..3077bb502 100644 --- a/sql3/parser/parser.go +++ b/sql3/parser/parser.go @@ -326,14 +326,14 @@ func (p *Parser) parseCreateStatement() (Statement, error) { switch p.peek() { case TABLE: return p.parseCreateTableStatement(pos) - /* case VIEW: - return p.parseCreateViewStatement(pos) - case INDEX, UNIQUE: - return p.parseCreateIndexStatement(pos)*/ + case VIEW: + return p.parseCreateViewStatement(pos) + /*case INDEX, UNIQUE: + return p.parseCreateIndexStatement(pos)*/ case FUNCTION: return p.parseCreateFunctionStatement(pos) default: - return nil, p.errorExpected(pos, tok, "TABLE") + return nil, p.errorExpected(pos, tok, "TABLE, VIEW or FUNCTION") } } @@ -344,14 +344,14 @@ func (p *Parser) parseDropStatement() (Statement, error) { switch p.peek() { case TABLE: return p.parseDropTableStatement(pos) - /* case VIEW: - return p.parseDropViewStatement(pos) - case INDEX: - return p.parseDropIndexStatement(pos)*/ + case VIEW: + return p.parseDropViewStatement(pos) + /* case INDEX: + return p.parseDropIndexStatement(pos)*/ case FUNCTION: return p.parseDropFunctionStatement(pos) default: - return nil, p.errorExpected(pos, tok, "TABLE") + return nil, p.errorExpected(pos, tok, "TABLE, VIEW or FUNCTION") } } @@ -1045,7 +1045,7 @@ func (p *Parser) parseDropTableStatement(dropPos Pos) (_ *DropTableStatement, er return &stmt, nil } -/*func (p *Parser) parseCreateViewStatement(createPos Pos) (_ *CreateViewStatement, err error) { +func (p *Parser) parseCreateViewStatement(createPos Pos) (_ *CreateViewStatement, err error) { assert(p.peek() == VIEW) var stmt CreateViewStatement @@ -1100,9 +1100,9 @@ func (p *Parser) parseDropTableStatement(dropPos Pos) (_ *DropTableStatement, er return &stmt, err } return &stmt, nil -}*/ +} -/*func (p *Parser) parseDropViewStatement(dropPos Pos) (_ *DropViewStatement, err error) { +func (p *Parser) parseDropViewStatement(dropPos Pos) (_ *DropViewStatement, err error) { assert(p.peek() == VIEW) var stmt DropViewStatement @@ -1123,7 +1123,7 @@ func (p *Parser) parseDropTableStatement(dropPos Pos) (_ *DropTableStatement, er } return &stmt, nil -}*/ +} /*func (p *Parser) parseCreateIndexStatement(createPos Pos) (_ *CreateIndexStatement, err error) { assert(p.peek() == INDEX || p.peek() == UNIQUE) diff --git a/sql3/parser/parser_test.go b/sql3/parser/parser_test.go index 4e31f4613..b66cdf529 100644 --- a/sql3/parser/parser_test.go +++ b/sql3/parser/parser_test.go @@ -1565,13 +1565,13 @@ func TestParser_ParseStatement(t *testing.T) { AssertParseStatementError(t, `DROP TABLE IF EXISTS`, `1:20: expected table name, found 'EOF'`) }) - /*t.Run("CreateView", func(t *testing.T) { + t.Run("CreateView", func(t *testing.T) { AssertParseStatement(t, `CREATE VIEW vw (col1, col2) AS SELECT x, y`, &parser.CreateViewStatement{ Create: pos(0), View: pos(7), Name: &parser.Ident{NamePos: pos(12), Name: "vw"}, Lparen: pos(15), - Columns: []*sql.Ident{ + Columns: []*parser.Ident{ {NamePos: pos(16), Name: "col1"}, {NamePos: pos(22), Name: "col2"}, }, @@ -1579,7 +1579,7 @@ func TestParser_ParseStatement(t *testing.T) { As: pos(28), Select: &parser.SelectStatement{ Select: pos(31), - Columns: []*sql.ResultColumn{ + Columns: []*parser.ResultColumn{ {Expr: &parser.Ident{NamePos: pos(38), Name: "x"}}, {Expr: &parser.Ident{NamePos: pos(41), Name: "y"}}, }, @@ -1592,7 +1592,7 @@ func TestParser_ParseStatement(t *testing.T) { As: pos(15), Select: &parser.SelectStatement{ Select: pos(18), - Columns: []*sql.ResultColumn{ + Columns: []*parser.ResultColumn{ {Expr: &parser.Ident{NamePos: pos(25), Name: "x"}}, }, }, @@ -1607,7 +1607,7 @@ func TestParser_ParseStatement(t *testing.T) { As: pos(29), Select: &parser.SelectStatement{ Select: pos(32), - Columns: []*sql.ResultColumn{ + Columns: []*parser.ResultColumn{ {Expr: &parser.Ident{NamePos: pos(39), Name: "x"}}, }, }, @@ -1618,11 +1618,11 @@ func TestParser_ParseStatement(t *testing.T) { AssertParseStatementError(t, `CREATE VIEW vw`, `1:14: expected AS, found 'EOF'`) AssertParseStatementError(t, `CREATE VIEW vw (`, `1:16: expected column name, found 'EOF'`) AssertParseStatementError(t, `CREATE VIEW vw (x`, `1:17: expected comma or right paren, found 'EOF'`) - AssertParseStatementError(t, `CREATE VIEW vw AS`, `1:17: expected SELECT or VALUES, found 'EOF'`) + AssertParseStatementError(t, `CREATE VIEW vw AS`, `1:17: expected SELECT, found 'EOF'`) AssertParseStatementError(t, `CREATE VIEW vw AS SELECT`, `1:24: expected expression, found 'EOF'`) - })*/ + }) - /*t.Run("DropView", func(t *testing.T) { + t.Run("DropView", func(t *testing.T) { AssertParseStatement(t, `DROP VIEW vw`, &parser.DropViewStatement{ Drop: pos(0), View: pos(5), @@ -1635,11 +1635,11 @@ func TestParser_ParseStatement(t *testing.T) { IfExists: pos(13), Name: &parser.Ident{NamePos: pos(20), Name: "vw"}, }) - AssertParseStatementError(t, `DROP`, `1:1: expected TABLE, VIEW, INDEX, or TRIGGER`) + AssertParseStatementError(t, `DROP`, `1:1: expected TABLE, VIEW or FUNCTION`) AssertParseStatementError(t, `DROP VIEW`, `1:9: expected view name, found 'EOF'`) AssertParseStatementError(t, `DROP VIEW IF`, `1:12: expected EXISTS, found 'EOF'`) AssertParseStatementError(t, `DROP VIEW IF EXISTS`, `1:19: expected view name, found 'EOF'`) - })*/ + }) /*t.Run("CreateIndex", func(t *testing.T) { AssertParseStatement(t, `CREATE INDEX idx ON tbl (x ASC, y DESC, z)`, &parser.CreateIndexStatement{ diff --git a/sql3/planner/compileselect.go b/sql3/planner/compileselect.go index 0e5f2d777..a2de2eb14 100644 --- a/sql3/planner/compileselect.go +++ b/sql3/planner/compileselect.go @@ -19,6 +19,8 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, query := NewPlanOpQuery(p, NewPlanOpNullTable(), p.sql) p.scopeStack.push(query) + aggregates := make([]types.PlanExpression, 0) + // handle projections projections := make([]types.PlanExpression, 0) for _, c := range stmt.Columns { @@ -30,6 +32,7 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, planExpr = newAliasPlanExpression(c.Alias.Name, planExpr) } projections = append(projections, planExpr) + aggregates = p.gatherExprAggregates(planExpr, aggregates) } // group by clause. @@ -44,10 +47,6 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, } var err error - if stmt.Having.IsValid() { - query.AddWarning("HAVING is not yet supported") - } - // handle distinct if stmt.Distinct.IsValid() { query.AddWarning("DISTINCT not yet implemented") @@ -67,14 +66,82 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, // if we did have a where, insert the filter op if where != nil { + aggregates = p.gatherExprAggregates(where, aggregates) source = NewPlanOpFilter(p, where, source) } + // handle the having clause + having, err := p.compileExpr(stmt.HavingExpr) + if err != nil { + return nil, err + } + + if having != nil { + // gather aggregates + aggregates = p.gatherExprAggregates(having, aggregates) + + // make sure that any references are columns in the group by list, or in an aggregate + + // make a list of group by expresssions + aggregateAndGroupByExprs := make([]types.PlanExpression, 0) + aggregateAndGroupByExprs = append(aggregateAndGroupByExprs, groupByExprs...) + // add to that the refs used by all the aggregates.. + for _, agg := range aggregates { + InspectExpression(agg, func(expr types.PlanExpression) bool { + switch ex := expr.(type) { + case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *percentilePlanExpression: + ch := ex.Children() + // first arg is always the ref + aggregateAndGroupByExprs = append(aggregateAndGroupByExprs, ch[0]) + return false + } + return true + }) + } + + // inspect the having expression, build a list of references that are not + // part of an aggregate + havingReferences := make([]*qualifiedRefPlanExpression, 0) + InspectExpression(having, func(expr types.PlanExpression) bool { + switch ex := expr.(type) { + case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *percentilePlanExpression: + return false + case *qualifiedRefPlanExpression: + havingReferences = append(havingReferences, ex) + return false + } + return true + }) + + // check the list of references against the aggregate and group by expressions + for _, nae := range havingReferences { + found := false + for _, pe := range aggregateAndGroupByExprs { + gbe, ok := pe.(*qualifiedRefPlanExpression) + if !ok { + continue + } + if strings.EqualFold(nae.columnName, gbe.columnName) && + strings.EqualFold(nae.tableName, gbe.tableName) { + found = true + break + } + } + if !found { + return nil, sql3.NewErrInvalidUngroupedColumnReferenceInHaving(0, 0, nae.columnName) + } + } + } + // do we have straight projection or a group by? var compiledOp types.PlanOperator - if len(query.aggregates) > 0 { + if len(aggregates) > 0 { //check that any projections that are not aggregates are in the group by list - var nonAggregateReferences []*qualifiedRefPlanExpression + nonAggregateReferences := make([]*qualifiedRefPlanExpression, 0) for _, expr := range projections { InspectExpression(expr, func(expr types.PlanExpression) bool { switch ex := expr.(type) { @@ -108,8 +175,12 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, return nil, sql3.NewErrInvalidUngroupedColumnReference(0, 0, nae.columnName) } } - - compiledOp = NewPlanOpProjection(projections, NewPlanOpGroupBy(query.aggregates, groupByExprs, source)) + var groupByOp types.PlanOperator + groupByOp = NewPlanOpGroupBy(aggregates, groupByExprs, source) + if having != nil { + groupByOp = NewPlanOpHaving(p, having, groupByOp) + } + compiledOp = NewPlanOpProjection(projections, groupByOp) } else { compiledOp = NewPlanOpProjection(projections, source) } @@ -166,6 +237,32 @@ func (p *ExecutionPlanner) compileSelectStatement(stmt *parser.SelectStatement, return query.WithChildren(children...) } +func (p *ExecutionPlanner) gatherExprAggregates(expr types.PlanExpression, aggregates []types.PlanExpression) []types.PlanExpression { + result := aggregates + InspectExpression(expr, func(expr types.PlanExpression) bool { + switch ex := expr.(type) { + case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *percentilePlanExpression: + found := false + for _, ag := range result { + //compare based on string representation + if strings.EqualFold(ag.String(), ex.String()) { + found = true + break + } + } + if !found { + result = append(result, ex) + } + // return false because thats as far down we want to inspect + return false + } + return true + }) + return result +} + func (p *ExecutionPlanner) compileSelectSource(scope *PlanOpQuery, source parser.Source) (types.PlanOperator, error) { if source == nil { return NewPlanOpNullTable(), nil @@ -411,6 +508,12 @@ func (p *ExecutionPlanner) analyzeSelectStatement(stmt *parser.SelectStatement) stmt.TopExpr = expr } + expr, err = p.analyzeExpression(stmt.HavingExpr, stmt) + if err != nil { + return err + } + stmt.HavingExpr = expr + expr, err = p.analyzeExpression(stmt.WhereExpr, stmt) if err != nil { return err diff --git a/sql3/planner/executionplanner.go b/sql3/planner/executionplanner.go index d8c7e40cf..26904cee0 100644 --- a/sql3/planner/executionplanner.go +++ b/sql3/planner/executionplanner.go @@ -126,21 +126,6 @@ func (p *ExecutionPlanner) checkAccess(ctx context.Context, objectName string, _ return nil } -// convenience function that allows the planner to keep track of aggregates so we can -// use them during optimization -func (p *ExecutionPlanner) addAggregate(agg types.PlanExpression) error { - table := p.scopeStack.read() - if table == nil { - return sql3.NewErrInternalf("unexpected symbol table state") - } - - switch s := table.scope.(type) { - case *PlanOpQuery: - s.aggregates = append(s.aggregates, agg) - } - return nil -} - // addReference is a convenience function that allows the planner to keep track // of references so we can use them during optimization. func (p *ExecutionPlanner) addReference(ref *qualifiedRefPlanExpression) error { diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 8caa9b2cc..1bfc1648a 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -343,6 +343,10 @@ func (n *binOpPlanExpression) Evaluate(currentRow []interface{}) (interface{}, e return nl != nr, nil case parser.EQ: return nl == nr, nil + case parser.AND: + return nl && nr, nil + case parser.OR: + return nl || nr, nil default: return nil, sql3.NewErrInternalf("unhandled operator %d", n.op) @@ -2704,32 +2708,26 @@ func (p *ExecutionPlanner) compileCallExpr(expr *parser.Call) (_ types.PlanExpre } else { agg = newCountPlanExpression(args[0], expr.ResultDataType) } - p.addAggregate(agg) return agg, nil case "SUM": agg := newSumPlanExpression(args[0], expr.ResultDataType) - p.addAggregate(agg) return agg, nil case "AVG": agg := newAvgPlanExpression(args[0], expr.ResultDataType) - p.addAggregate(agg) return agg, nil case "PERCENTILE": agg := newPercentilePlanExpression(args[0], args[1], expr.ResultDataType) - p.addAggregate(agg) return agg, nil case "MIN": agg := newMinPlanExpression(args[0], expr.ResultDataType) - p.addAggregate(agg) return agg, nil case "MAX": agg := newMaxPlanExpression(args[0], expr.ResultDataType) - p.addAggregate(agg) return agg, nil default: diff --git a/sql3/planner/opfilter.go b/sql3/planner/opfilter.go index 808078d82..1f37245ee 100644 --- a/sql3/planner/opfilter.go +++ b/sql3/planner/opfilter.go @@ -91,8 +91,7 @@ func (p *PlanOpFilter) WithUpdatedExpressions(exprs ...types.PlanExpression) (ty if len(exprs) != 1 { return nil, sql3.NewErrInternalf("unexpected number of exprs '%d'", len(exprs)) } - p.Predicate = exprs[0] - return p, nil + return NewPlanOpFilter(p.planner, exprs[0], p.ChildOp), nil } type filterIterator struct { diff --git a/sql3/planner/ophaving.go b/sql3/planner/ophaving.go new file mode 100644 index 000000000..44fa57247 --- /dev/null +++ b/sql3/planner/ophaving.go @@ -0,0 +1,95 @@ +// Copyright 2022 Molecula Corp. All rights reserved. + +package planner + +import ( + "context" + "fmt" + + "github.com/molecula/featurebase/v3/sql3" + "github.com/molecula/featurebase/v3/sql3/planner/types" +) + +// PlanOpHaving is a filter operator for the HAVING clause +type PlanOpHaving struct { + planner *ExecutionPlanner + ChildOp types.PlanOperator + Predicate types.PlanExpression + + warnings []string +} + +func NewPlanOpHaving(planner *ExecutionPlanner, predicate types.PlanExpression, child types.PlanOperator) *PlanOpHaving { + return &PlanOpHaving{ + planner: planner, + Predicate: predicate, + ChildOp: child, + warnings: make([]string, 0), + } +} + +func (p *PlanOpHaving) Schema() types.Schema { + return p.ChildOp.Schema() +} + +func (p *PlanOpHaving) Iterator(ctx context.Context, row types.Row) (types.RowIterator, error) { + i, err := p.ChildOp.Iterator(ctx, row) + if err != nil { + return nil, err + } + return newFilterIterator(ctx, p.Predicate, i), nil +} + +func (p *PlanOpHaving) WithChildren(children ...types.PlanOperator) (types.PlanOperator, error) { + if len(children) != 1 { + return nil, sql3.NewErrInternalf("unexpected number of children '%d'", len(children)) + } + return NewPlanOpHaving(p.planner, p.Predicate, children[0]), nil +} + +func (p *PlanOpHaving) Children() []types.PlanOperator { + return []types.PlanOperator{ + p.ChildOp, + } +} + +func (p *PlanOpHaving) Plan() map[string]interface{} { + result := make(map[string]interface{}) + result["_op"] = fmt.Sprintf("%T", p) + ps := make([]string, 0) + for _, e := range p.Schema() { + ps = append(ps, fmt.Sprintf("'%s', '%s', '%s'", e.ColumnName, e.RelationName, e.Type.TypeDescription())) + } + result["_schema"] = ps + result["predicate"] = p.Predicate.Plan() + result["child"] = p.ChildOp.Plan() + return result +} + +func (p *PlanOpHaving) String() string { + return "" +} + +func (p *PlanOpHaving) AddWarning(warning string) { + p.warnings = append(p.warnings, warning) +} + +func (p *PlanOpHaving) Warnings() []string { + return p.warnings +} + +func (p *PlanOpHaving) Expressions() []types.PlanExpression { + if p.Predicate != nil { + return []types.PlanExpression{ + p.Predicate, + } + } + return []types.PlanExpression{} +} + +func (p *PlanOpHaving) WithUpdatedExpressions(exprs ...types.PlanExpression) (types.PlanOperator, error) { + if len(exprs) != 1 { + return nil, sql3.NewErrInternalf("unexpected number of exprs '%d'", len(exprs)) + } + return NewPlanOpHaving(p.planner, exprs[0], p.ChildOp), nil +} diff --git a/sql3/planner/opquery.go b/sql3/planner/opquery.go index 3bda9390e..3df19de3a 100644 --- a/sql3/planner/opquery.go +++ b/sql3/planner/opquery.go @@ -18,9 +18,6 @@ type PlanOpQuery struct { ChildOp types.PlanOperator - // the list of aggregate terms - aggregates []types.PlanExpression - // all the identifiers that are referenced referenceList []*qualifiedRefPlanExpression diff --git a/sql3/planner/planoptimizer.go b/sql3/planner/planoptimizer.go index 227bdd4f8..fa04b97a6 100644 --- a/sql3/planner/planoptimizer.go +++ b/sql3/planner/planoptimizer.go @@ -23,6 +23,9 @@ type OptimizerFunc func(context.Context, *ExecutionPlanner, types.PlanOperator, // a list of optimzer rules; order can be important important var optimizerFunctions = []OptimizerFunc{ + // fix expression references for having + fixHavingReferences, + // push down filter predicates as far as possible, pushdownFilters, @@ -716,7 +719,7 @@ func fixProjectionReferences(ctx context.Context, a *ExecutionPlanner, n types.P case *PlanOpProjection: switch childOp := thisNode.ChildOp.(type) { - case *PlanOpGroupBy, *PlanOpPQLGroupBy, *PlanOpPQLMultiAggregate, *PlanOpPQLMultiGroupBy: + case *PlanOpGroupBy, *PlanOpHaving, *PlanOpPQLGroupBy, *PlanOpPQLMultiAggregate, *PlanOpPQLMultiGroupBy: childSchema := childOp.Schema() for idx, pj := range thisNode.Projections { @@ -724,7 +727,7 @@ func fixProjectionReferences(ctx context.Context, a *ExecutionPlanner, n types.P switch thisAggregate := e.(type) { case types.Aggregable: // if we have a Aggregable, the AggExpression() will be a qualified ref - // given we are in the context of a PlanOpProjection with a PlanOpGroupBy + // given we are in the context of a PlanOpProjection with a PlanOpGroupBy/Having // we can use the ordinal position of the projection as the column index for idx, sc := range childSchema { if strings.EqualFold(thisAggregate.String(), sc.ColumnName) { @@ -865,6 +868,29 @@ func fixFieldRefs(ctx context.Context, a *ExecutionPlanner, n types.PlanOperator }) } +func fixHavingReferences(ctx context.Context, a *ExecutionPlanner, n types.PlanOperator, scope *OptimizerScope) (types.PlanOperator, bool, error) { + return TransformPlanOp(n, func(node types.PlanOperator) (types.PlanOperator, bool, error) { + switch thisNode := node.(type) { + case *PlanOpHaving: + // fix references for the expressions referenced in the having predicate expression + schema := thisNode.Schema() + expressions := thisNode.Expressions() + fixed, same, err := fixFieldRefIndexesOnExpressionsForHaving(ctx, scope, a, schema, expressions...) + if err != nil { + return nil, true, err + } + newNode, err := thisNode.WithUpdatedExpressions(fixed...) + if err != nil { + return nil, true, err + } + return newNode, same, nil + + default: + return node, true, nil + } + }) +} + // hasTop inspects a plan op tree and returns true (or error) if there are Top // operators. func hasTop(ctx context.Context, a *ExecutionPlanner, n types.PlanOperator, scope *OptimizerScope) (bool, error) { @@ -1004,3 +1030,69 @@ func fixFieldRefIndexes(ctx context.Context, scope *OptimizerScope, a *Execution return true }) } + +// for a list of expressions and an operator schema, fix the references for any qualifiedRef expressions +func fixFieldRefIndexesOnExpressionsForHaving(ctx context.Context, scope *OptimizerScope, a *ExecutionPlanner, schema types.Schema, expressions ...types.PlanExpression) ([]types.PlanExpression, bool, error) { + var result []types.PlanExpression + var res types.PlanExpression + var same bool + var err error + for i := range expressions { + e := expressions[i] + res, same, err = fixFieldRefIndexesForHaving(ctx, scope, a, schema, e) + if err != nil { + return nil, true, err + } + if !same { + if result == nil { + result = make([]types.PlanExpression, len(expressions)) + copy(result, expressions) + } + result[i] = res + } + } + if len(result) > 0 { + return result, false, nil + } + return expressions, true, nil +} + +func fixFieldRefIndexesForHaving(ctx context.Context, scope *OptimizerScope, a *ExecutionPlanner, schema types.Schema, exp types.PlanExpression) (types.PlanExpression, bool, error) { + return TransformExpr(exp, func(e types.PlanExpression) (types.PlanExpression, bool, error) { + switch typedExpr := e.(type) { + case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *percentilePlanExpression: + for i, col := range schema { + if strings.EqualFold(typedExpr.String(), col.ColumnName) { + e := newQualifiedRefPlanExpression("", "", i, typedExpr.Type()) + return e, false, nil + } + } + return nil, true, sql3.NewErrColumnNotFound(0, 0, typedExpr.String()) + + case *qualifiedRefPlanExpression: + for i, col := range schema { + newIndex := i + if matchesSchema(typedExpr, col) { + if newIndex != typedExpr.columnIndex { + // update the column index + return newQualifiedRefPlanExpression(typedExpr.tableName, typedExpr.columnName, newIndex, typedExpr.dataType), false, nil + } + return e, true, nil + } + } + return nil, true, sql3.NewErrColumnNotFound(0, 0, typedExpr.Name()) + } + return e, true, nil + }, func(parentExpr, childExpr types.PlanExpression) bool { + switch parentExpr.(type) { + case *sumPlanExpression, *countPlanExpression, *countDistinctPlanExpression, + *avgPlanExpression, *minPlanExpression, *maxPlanExpression, + *percentilePlanExpression: + return false + default: + return true + } + }) +} diff --git a/sql3/test/defs/defs.go b/sql3/test/defs/defs.go index c6c8d0d69..31bb62f85 100644 --- a/sql3/test/defs/defs.go +++ b/sql3/test/defs/defs.go @@ -15,6 +15,7 @@ var TableTests []TableTest = []TableTest{ selectTests, selectKeyedTests, + selectHavingTests, orderByTests, topTests, diff --git a/sql3/test/defs/defs_having.go b/sql3/test/defs/defs_having.go new file mode 100644 index 000000000..a9aa2abec --- /dev/null +++ b/sql3/test/defs/defs_having.go @@ -0,0 +1,43 @@ +package defs + +var selectHavingTests = TableTest{ + name: "select-having", + Table: tbl( + "having_test", + srcHdrs( + srcHdr("_id", fldTypeID), + srcHdr("an_int", fldTypeInt, "min 0", "max 100"), + srcHdr("an_id_set", fldTypeIDSet), + srcHdr("an_id", fldTypeID), + srcHdr("a_string", fldTypeString), + srcHdr("a_string_set", fldTypeStringSet), + srcHdr("a_decimal", fldTypeDecimal2), + ), + srcRows( + srcRow(int64(1), int64(11), []int64{11, 12, 13}, int64(101), "str1", []string{"a1", "b1", "c1"}, float64(123.45)), + 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)), + ), + ), + SQLTests: []SQLTest{ + { + name: "select-having", + SQLs: sqls( + "select count(*), an_int from having_test group by an_int having count(*) > 0", + ), + 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)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + }, +}