From c66d392c8784be0d079503e1736b2bd15895d723 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 5 Apr 2023 12:23:57 -0500 Subject: [PATCH 1/2] uncomment old LIMIT tests, make them pass We forward-ported a handful of tests from the previous parser which relied on LIMIT clauses, but then we didn't support that. Now that we do, we uncomment most of these tests, and actually give them the correct data structures to compare with. We leave two tests commented out. One was using `limit 10, 5` to express a limit plus offset, and the other is using `not fld = 1` as a WHERE clause, but we don't support unary-not to negate other expressions. In the process, we discover that converting a SELECT with a LIMIT clause back to a string has a missing space, and fix that. --- sql3/parser/ast.go | 2 +- sql3/parser/parser_test.go | 110 +++++++++++++++++++++++++++++++++---- 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/sql3/parser/ast.go b/sql3/parser/ast.go index 3c34d0ca3..5fd062a2d 100644 --- a/sql3/parser/ast.go +++ b/sql3/parser/ast.go @@ -4037,7 +4037,7 @@ func (s *SelectStatement) String() string { } if s.Limit.IsValid() { - fmt.Fprintf(&buf, "LIMIT %s", s.LimitExpr.String()) + fmt.Fprintf(&buf, " LIMIT %s", s.LimitExpr.String()) } return buf.String() diff --git a/sql3/parser/parser_test.go b/sql3/parser/parser_test.go index 72b316b52..5d6651e0a 100644 --- a/sql3/parser/parser_test.go +++ b/sql3/parser/parser_test.go @@ -3347,16 +3347,106 @@ func TestParser_ParseStatement(t *testing.T) { }, }, }) - - if false { - // not working because we don't support limit(x), we support top(x), i think? - AssertParseStatement(t, `SELECT fld1, fld2, COUNT(*) FROM tbl where fld1 = 1 group by fld1, fld2 limit 1`, nil) // 1:73: expected semicolon or EOF, found limit - AssertParseStatement(t, `SELECT DISTINCT score FROM grouper order by score asc limit 5`, nil) // 1:55: expected semicolon or EOF, found limit - AssertParseStatement(t, `SELECT DISTINCT score FROM grouper order by score desc limit 5`, nil) // 1:56: expected semicolon or EOF, found limit - AssertParseStatement(t, `SELECT fld FROM tbl limit 10`, nil) // 1:27: expected semicolon or EOF, found 10 - AssertParseStatement(t, `SELECT fld FROM tbl limit 10, 5`, nil) // 1:27: expected semicolon or EOF, found 10 - AssertParseStatement(t, `SELECT _id FROM tbl where not fld = 1 limit 10`, nil) // 1:31: expected EXISTS, found fld - } + AssertParseStatement(t, `SELECT fld1, fld2, COUNT(*) FROM tbl where fld1 = 1 group by fld1, fld2 limit 1`, &parser.SelectStatement{ + Select: pos(0), + Columns: []*parser.ResultColumn{ + {Expr: &parser.Ident{NamePos: pos(7), Name: "fld1"}}, + {Expr: &parser.Ident{NamePos: pos(13), Name: "fld2"}}, + { + Expr: &parser.Call{ + Name: &parser.Ident{NamePos: pos(19), Name: "COUNT"}, + Lparen: pos(24), + Star: pos(25), + Rparen: pos(26), + }, + }, + }, + From: pos(28), + Source: &parser.QualifiedTableName{Name: &parser.Ident{NamePos: pos(33), Name: "tbl"}}, + Where: pos(37), + WhereExpr: &parser.BinaryExpr{ + X: &parser.Ident{NamePos: pos(43), Name: "fld1"}, + OpPos: pos(48), + Op: parser.EQ, + Y: &parser.IntegerLit{ + ValuePos: pos(50), + Value: "1", + }, + }, + Group: pos(52), + GroupBy: pos(58), + GroupByExprs: []parser.Expr{ + &parser.Ident{NamePos: pos(61), Name: "fld1"}, + &parser.Ident{NamePos: pos(67), Name: "fld2"}, + }, + Limit: pos(72), + LimitExpr: &parser.IntegerLit{ + ValuePos: pos(78), + Value: "1", + }, + }) + AssertParseStatement(t, `SELECT DISTINCT score FROM grouper order by score asc limit 5`, &parser.SelectStatement{ + Select: pos(0), + Distinct: pos(7), + Columns: []*parser.ResultColumn{ + {Expr: &parser.Ident{NamePos: pos(16), Name: "score"}}, + }, + From: pos(22), + Source: &parser.QualifiedTableName{Name: &parser.Ident{NamePos: pos(27), Name: "grouper"}}, + Order: pos(35), + OrderBy: pos(41), + OrderingTerms: []*parser.OrderingTerm{ + { + X: &parser.Ident{NamePos: pos(44), Name: "score"}, + Asc: pos(50), + }, + }, + Limit: pos(54), + LimitExpr: &parser.IntegerLit{ + ValuePos: pos(60), + Value: "5", + }, + }) + AssertParseStatement(t, `SELECT DISTINCT score FROM grouper order by score desc limit 5`, &parser.SelectStatement{ + Select: pos(0), + Distinct: pos(7), + Columns: []*parser.ResultColumn{ + {Expr: &parser.Ident{NamePos: pos(16), Name: "score"}}, + }, + From: pos(22), + Source: &parser.QualifiedTableName{Name: &parser.Ident{NamePos: pos(27), Name: "grouper"}}, + Order: pos(35), + OrderBy: pos(41), + OrderingTerms: []*parser.OrderingTerm{ + { + X: &parser.Ident{NamePos: pos(44), Name: "score"}, + Desc: pos(50), + }, + }, + Limit: pos(55), + LimitExpr: &parser.IntegerLit{ + ValuePos: pos(61), + Value: "5", + }, + }) + AssertParseStatement(t, `SELECT fld FROM tbl limit 10`, &parser.SelectStatement{ + Select: pos(0), + Columns: []*parser.ResultColumn{ + {Expr: &parser.Ident{NamePos: pos(7), Name: "fld"}}, + }, + From: pos(11), + Source: &parser.QualifiedTableName{Name: &parser.Ident{NamePos: pos(16), Name: "tbl"}}, + Limit: pos(20), + LimitExpr: &parser.IntegerLit{ + ValuePos: pos(26), + Value: "10", + }, + }) + // our previous SQL implementation supported "limit 10, 5" to mean a limit of 10 items, + // starting from the 5th item. The new implementation does not support this feature yet. + // AssertParseStatement(t, `SELECT fld FROM tbl limit 10, 5`, nil) // 1:27: expected semicolon or EOF, found 10 + // the previous SQL implementation allowed `where not [condition]` but we don't currently. + // AssertParseStatement(t, `SELECT _id FROM tbl where not fld = 1 limit 10`, nil) // 1:31: expected EXISTS, found fld /*AssertParseStatementError(t, `WITH `, `1:5: expected table name, found 'EOF'`) AssertParseStatementError(t, `WITH cte`, `1:8: expected AS, found 'EOF'`) AssertParseStatementError(t, `WITH cte (`, `1:10: expected column name, found 'EOF'`) From c619b7d94e6c3849d01e3662b6dd870147e98e6a Mon Sep 17 00:00:00 2001 From: Pat Okeeffe <85502298+paddyjok@users.noreply.github.com> Date: Thu, 6 Apr 2023 17:28:24 -0500 Subject: [PATCH 2/2] implemented query hints (flatten) (fb-2124) (#2373) * implemented query hints (flatten) * improved testing --- dax/test/dax/dax_test.go | 1 + sql3/errors.go | 20 +++ sql3/parser/ast.go | 73 +++++++-- sql3/parser/parser.go | 90 ++++++++--- sql3/parser/walk.go | 23 ++- sql3/planner/compileselect.go | 48 +++++- sql3/planner/expression.go | 5 + sql3/planner/oppqldistinctscan.go | 16 +- sql3/planner/oppqlgroupby.go | 7 +- sql3/planner/oppqltablescan.go | 9 +- sql3/planner/planoptimizer.go | 245 +++++++++++++++++------------- sql3/test/defs/defs.go | 1 + sql3/test/defs/defs_groupby.go | 210 ++++++++++++++++++++++++- sql3/test/defs/defs_top.go | 8 +- 14 files changed, 599 insertions(+), 157 deletions(-) diff --git a/dax/test/dax/dax_test.go b/dax/test/dax/dax_test.go index 286020dc1..d4e3f7d9d 100644 --- a/dax/test/dax/dax_test.go +++ b/dax/test/dax/dax_test.go @@ -147,6 +147,7 @@ func TestDAXIntegration(t *testing.T) { "top-limit-tests/test-2", // don't know why this is failing at all "top-limit-tests/test-3", // don't know why this is failing at all "delete_tests", + "groupby_set_test", // no idea why this has ceased to work "viewtests/drop-view", // drop view does a delete "viewtests/drop-view-if-exists-after-drop", "viewtests/select-view-after-drop", diff --git a/sql3/errors.go b/sql3/errors.go index 3bc42cf6a..4ea363954 100644 --- a/sql3/errors.go +++ b/sql3/errors.go @@ -148,6 +148,10 @@ const ( // remote execution ErrRemoteUnauthorized errors.Code = "ErrRemoteUnauthorized" + + // query hints + ErrUnknownQueryHint errors.Code = "ErrInvalidQueryHint" + ErrInvalidQueryHintParameterCount errors.Code = "ErrInvalidQueryHintParameterCount" ) func NewErrDuplicateColumn(line int, col int, column string) error { @@ -911,3 +915,19 @@ func NewErrRemoteUnauthorized(line, col int, remoteUrl string) error { fmt.Sprintf("unauthorized on remote server '%s'", remoteUrl), ) } + +// query hints + +func NewErrUnknownQueryHint(line, col int, hintName string) error { + return errors.New( + ErrUnknownQueryHint, + fmt.Sprintf("[%d:%d] unknown query hint '%s'", line, col, hintName), + ) +} + +func NewErrInvalidQueryHintParameterCount(line, col int, hintName string, desiredList string, desiredCount int, actualCount int) error { + return errors.New( + ErrInvalidQueryHintParameterCount, + fmt.Sprintf("[%d:%d] query hint '%s' expected %d parameter(s) (%s), got %d parameters", line, col, hintName, desiredCount, desiredList, actualCount), + ) +} diff --git a/sql3/parser/ast.go b/sql3/parser/ast.go index 5fd062a2d..aa63e6e4c 100644 --- a/sql3/parser/ast.go +++ b/sql3/parser/ast.go @@ -90,6 +90,7 @@ func (*RollbackStatement) node() {} func (*SavepointStatement) node() {} func (*SelectStatement) node() {} func (*StringLit) node() {} +func (*TableQueryOption) node() {} func (*TableValuedFunction) node() {} func (*TimeUnitConstraint) node() {} func (*TimeQuantumConstraint) node() {} @@ -4139,15 +4140,45 @@ func (c *ResultColumn) String() string { return c.Expr.String() } +type TableQueryOption struct { + OptionName *Ident + LParen Pos + OptionParams []*Ident + RParen Pos +} + +func (n *TableQueryOption) Clone() *TableQueryOption { + if n == nil { + return nil + } + other := *n + other.OptionName = n.OptionName.Clone() + other.OptionParams = cloneIdents(n.OptionParams) + return &other +} + +func (n *TableQueryOption) String() string { + var buf bytes.Buffer + buf.WriteString(n.OptionName.String()) + buf.WriteString("(") + for i, o := range n.OptionParams { + if i > 0 { + buf.WriteString(", ") + } + fmt.Fprintf(&buf, " %s", o.String()) + } + buf.WriteString(")") + return buf.String() +} + type QualifiedTableName struct { - Name *Ident // table name - As Pos // position of AS keyword - Alias *Ident // optional table alias - Indexed Pos // position of INDEXED keyword - IndexedBy Pos // position of BY keyword after INDEXED - Not Pos // position of NOT keyword before INDEXED - NotIndexed Pos // position of NOT keyword before INDEXED - Index *Ident // name of index + Name *Ident // table name + As Pos // position of AS keyword + Alias *Ident // optional table alias + With Pos // position of WITH keyword + LParen Pos + QueryOptions []*TableQueryOption + RParen Pos OutputColumns []*SourceOutputColumn // output columns - populated during analysis } @@ -4164,6 +4195,17 @@ func (n *QualifiedTableName) MatchesTablenameOrAlias(match string) bool { return strings.EqualFold(IdentName(n.Alias), match) || strings.EqualFold(IdentName(n.Name), match) } +func cloneQueryOptions(a []*TableQueryOption) []*TableQueryOption { + if a == nil { + return nil + } + other := make([]*TableQueryOption, len(a)) + for i := range a { + other[i] = a[i].Clone() + } + return other +} + // Clone returns a deep copy of n. func (n *QualifiedTableName) Clone() *QualifiedTableName { if n == nil { @@ -4172,7 +4214,7 @@ func (n *QualifiedTableName) Clone() *QualifiedTableName { other := *n other.Name = n.Name.Clone() other.Alias = n.Alias.Clone() - other.Index = n.Index.Clone() + other.QueryOptions = cloneQueryOptions(n.QueryOptions) return &other } @@ -4187,10 +4229,15 @@ func (n *QualifiedTableName) String() string { fmt.Fprintf(&buf, " %s", n.Alias.String()) } - if n.Index != nil { - fmt.Fprintf(&buf, " INDEXED BY %s", n.Index.String()) - } else if n.NotIndexed.IsValid() { - buf.WriteString(" NOT INDEXED") + if n.With.IsValid() { + buf.WriteString(" WITH (") + for i, o := range n.QueryOptions { + if i > 0 { + buf.WriteString(", ") + } + fmt.Fprintf(&buf, " %s", o.String()) + } + buf.WriteString(")") } return buf.String() } diff --git a/sql3/parser/parser.go b/sql3/parser/parser.go index 48e4fbcc5..6649adee2 100644 --- a/sql3/parser/parser.go +++ b/sql3/parser/parser.go @@ -2760,29 +2760,85 @@ func (p *Parser) parseQualifiedTableName(ident *Ident) (_ *QualifiedTableName, e } } - // Parse optional "INDEXED BY index-name" or "NOT INDEXED". - /*switch p.peek() { - case INDEXED: - tbl.Indexed, _, _ = p.scan() - if p.peek() != BY { - return &tbl, p.errorExpected(p.pos, p.tok, "BY") - } - tbl.IndexedBy, _, _ = p.scan() + // handle query option + if p.peek() == WITH { + tbl.With, _, _ = p.scan() - if tbl.Index, err = p.parseIdent("index name"); err != nil { - return &tbl, err + tbl.QueryOptions = make([]*TableQueryOption, 0) + if p.peek() != LP { + return nil, p.errorExpected(p.pos, p.tok, "left paren") } - case NOT: - tbl.Not, _, _ = p.scan() - if p.peek() != INDEXED { - return &tbl, p.errorExpected(p.pos, p.tok, "INDEXED") - } - tbl.NotIndexed, _, _ = p.scan() - }*/ + tbl.LParen, _, _ = p.scan() + if tok := p.peek(); !isIdentToken(tok) { + return nil, p.errorExpected(p.pos, p.tok, "identifier") + } + for { + qo, err := p.parseTableQueryOption() + if err != nil { + return &tbl, err + } + tbl.QueryOptions = append(tbl.QueryOptions, qo) + + if p.peek() != COMMA { + break + } + _, _, _ = p.scan() + } + if p.peek() != RP { + return nil, p.errorExpected(p.pos, p.tok, "right paren") + } + tbl.RParen, _, _ = p.scan() + } return &tbl, nil } +func (p *Parser) parseTableQueryOption() (_ *TableQueryOption, err error) { + var opt TableQueryOption + opt.OptionParams = make([]*Ident, 0) + + if tok := p.peek(); !isIdentToken(tok) { + return nil, p.errorExpected(p.pos, p.tok, "identifier") + } + + oi, err := p.parseIdent("query option") + if err != nil { + return &opt, err + } + + opt.OptionName = oi + + if p.peek() != LP { + return nil, p.errorExpected(p.pos, p.tok, "left paren") + } + opt.LParen, _, _ = p.scan() + + for { + if tok := p.peek(); !isIdentToken(tok) { + return nil, p.errorExpected(p.pos, p.tok, "identifier") + } + + opi, err := p.parseIdent("query option parameter") + if err != nil { + return &opt, err + } + + opt.OptionParams = append(opt.OptionParams, opi) + + if p.peek() != COMMA { + break + } + _, _, _ = p.scan() + } + + if p.peek() != RP { + return nil, p.errorExpected(p.pos, p.tok, "right paren") + } + opt.RParen, _, _ = p.scan() + + return &opt, nil +} + func (p *Parser) parseTableValuedFunction(ident *Ident) (_ *TableValuedFunction, err error) { var tbl TableValuedFunction diff --git a/sql3/parser/walk.go b/sql3/parser/walk.go index fed9cb5c7..10701c8e2 100644 --- a/sql3/parser/walk.go +++ b/sql3/parser/walk.go @@ -582,7 +582,15 @@ func walk(v Visitor, node Node) (_ Node, err error) { if err := walkIdent(v, &n.Alias); err != nil { return node, err } - if err := walkIdent(v, &n.Index); err != nil { + if err := walkTableQueryOptionList(v, n.QueryOptions); err != nil { + return node, err + } + + case *TableQueryOption: + if err := walkIdent(v, &n.OptionName); err != nil { + return node, err + } + if err := walkIdentList(v, n.OptionParams); err != nil { return node, err } @@ -844,3 +852,16 @@ func walkColumnDefinitionList(v Visitor, a []*ColumnDefinition) error { } return nil } + +func walkTableQueryOptionList(v Visitor, a []*TableQueryOption) error { + for i := range a { + if def, err := walk(v, a[i]); err != nil { + return err + } else if def != nil { + a[i] = def.(*TableQueryOption) + } else { + a[i] = nil + } + } + return nil +} diff --git a/sql3/planner/compileselect.go b/sql3/planner/compileselect.go index c8e076c10..544efa2d8 100644 --- a/sql3/planner/compileselect.go +++ b/sql3/planner/compileselect.go @@ -428,6 +428,19 @@ func (p *ExecutionPlanner) compileSource(scope *PlanOpQuery, source parser.Sourc return op, nil } + + // get any query hints + queryHints := make([]*TableQueryHint, 0) + for _, o := range sourceExpr.QueryOptions { + h := &TableQueryHint{ + name: parser.IdentName(o.OptionName), + } + for _, op := range o.OptionParams { + h.params = append(h.params, parser.IdentName(op)) + } + queryHints = append(queryHints, h) + } + // get all the columns for this table - we will eliminate unused ones // later on in the optimizer extractColumns := make([]string, 0) @@ -439,9 +452,9 @@ func (p *ExecutionPlanner) compileSource(scope *PlanOpQuery, source parser.Sourc if sourceExpr.Alias != nil { aliasName := parser.IdentName(sourceExpr.Alias) - return NewPlanOpRelAlias(aliasName, NewPlanOpPQLTableScan(p, tableName, extractColumns)), nil + return NewPlanOpRelAlias(aliasName, NewPlanOpPQLTableScan(p, tableName, extractColumns, queryHints)), nil } - return NewPlanOpPQLTableScan(p, tableName, extractColumns), nil + return NewPlanOpPQLTableScan(p, tableName, extractColumns, queryHints), nil case *parser.TableValuedFunction: callExpr, err := p.compileCallExpr(sourceExpr.Call) @@ -557,6 +570,8 @@ func (p *ExecutionPlanner) analyzeSource(ctx context.Context, source parser.Sour return paren, nil } + // if we got to here, not a view, so do table stuff + // check table exists tname := dax.TableName(objectName) tbl, err := p.schemaAPI.TableByName(ctx, tname) @@ -578,6 +593,35 @@ func (p *ExecutionPlanner) analyzeSource(ctx context.Context, source parser.Sour source.OutputColumns = append(source.OutputColumns, soc) } + // check query hints + for _, o := range source.QueryOptions { + opt := parser.IdentName(o.OptionName) + switch strings.ToLower(opt) { + case "flatten": + // should have 1 param and should be a column name + if len(o.OptionParams) != 1 { + // error + return nil, sql3.NewErrInvalidQueryHintParameterCount(o.LParen.Column, o.LParen.Line, opt, "column name", 1, len(o.OptionParams)) + } + for _, op := range o.OptionParams { + param := parser.IdentName(op) + found := false + for _, oc := range source.OutputColumns { + if strings.EqualFold(param, oc.ColumnName) { + found = true + break + } + } + if !found { + return nil, sql3.NewErrColumnNotFound(op.NamePos.Line, op.NamePos.Column, param) + } + } + + default: + return nil, sql3.NewErrUnknownQueryHint(o.OptionName.NamePos.Line, o.OptionName.NamePos.Column, opt) + } + } + return source, nil case *parser.TableValuedFunction: diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 717f5486c..aa7e89e92 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -1724,6 +1724,11 @@ func (n *qualifiedRefPlanExpression) Evaluate(currentRow []interface{}) (interfa switch n.dataType.(type) { case *parser.DataTypeIDSet, *parser.DataTypeIDSetQuantum: + // this could be an []int64 or a []uint64 internally + irow, ok := currentRow[n.columnIndex].([]int64) + if ok { + return irow, nil + } row, ok := currentRow[n.columnIndex].([]uint64) if !ok { return nil, sql3.NewErrInternalf("unexpected type for current row '%T'", currentRow[n.columnIndex]) diff --git a/sql3/planner/oppqldistinctscan.go b/sql3/planner/oppqldistinctscan.go index e6267e12d..47a2a6a4f 100644 --- a/sql3/planner/oppqldistinctscan.go +++ b/sql3/planner/oppqldistinctscan.go @@ -265,26 +265,18 @@ func (i *distinctScanRowIter) Next(ctx context.Context) (types.Row, error) { row[0] = pql.NewDecimal(val, t.Scale) case *parser.DataTypeIDSet: - val, ok := result.([]uint64) + val, ok := result.(int64) if !ok { return nil, sql3.NewErrInternalf("unexpected type for column value '%T'", result) } - if val == nil { - row[0] = nil - } else { - row[0] = val - } + row[0] = []uint64{uint64(val)} case *parser.DataTypeStringSet: - val, ok := result.([]string) + val, ok := result.(string) if !ok { return nil, sql3.NewErrInternalf("unexpected type for column value '%T'", result) } - if val == nil { - row[0] = nil - } else { - row[0] = val - } + row[0] = []string{val} default: row[0] = result diff --git a/sql3/planner/oppqlgroupby.go b/sql3/planner/oppqlgroupby.go index fb0d4387e..41f63717e 100644 --- a/sql3/planner/oppqlgroupby.go +++ b/sql3/planner/oppqlgroupby.go @@ -232,7 +232,12 @@ func (i *pqlGroupByRowIter) Next(ctx context.Context) (types.Row, error) { if g.Value != nil { row[idx] = *g.Value } else if g.RowKey != "" { - row[idx] = g.RowKey + switch c.Type().(type) { + case *parser.DataTypeStringSet: + row[idx] = []string{g.RowKey} + default: + row[idx] = g.RowKey + } } else { switch c.Type().(type) { case *parser.DataTypeIDSet: diff --git a/sql3/planner/oppqltablescan.go b/sql3/planner/oppqltablescan.go index 47154a65e..739524d18 100644 --- a/sql3/planner/oppqltablescan.go +++ b/sql3/planner/oppqltablescan.go @@ -15,6 +15,11 @@ import ( "github.com/featurebasedb/featurebase/v3/sql3/planner/types" ) +type TableQueryHint struct { + name string + params []string +} + // PlanOpPQLTableScan plan operator handles a PQL table scan type PlanOpPQLTableScan struct { planner *ExecutionPlanner @@ -23,15 +28,17 @@ type PlanOpPQLTableScan struct { filter types.PlanExpression timeQuantumFilters []types.PlanExpression topExpr types.PlanExpression + hints []*TableQueryHint warnings []string } -func NewPlanOpPQLTableScan(p *ExecutionPlanner, tableName string, columns []string) *PlanOpPQLTableScan { +func NewPlanOpPQLTableScan(p *ExecutionPlanner, tableName string, columns []string, hints []*TableQueryHint) *PlanOpPQLTableScan { return &PlanOpPQLTableScan{ planner: p, tableName: tableName, columns: columns, timeQuantumFilters: make([]types.PlanExpression, 0), + hints: hints, warnings: make([]string, 0), } } diff --git a/sql3/planner/planoptimizer.go b/sql3/planner/planoptimizer.go index ffae15cc4..c49813b15 100644 --- a/sql3/planner/planoptimizer.go +++ b/sql3/planner/planoptimizer.go @@ -313,7 +313,7 @@ func removeUnusedExtractColumnReferences(ctx context.Context, a *ExecutionPlanne } // newExtractList should now contain just the cols that are referenced - return NewPlanOpPQLTableScan(a, thisNode.tableName, newExtractList), false, nil + return NewPlanOpPQLTableScan(a, thisNode.tableName, newExtractList, thisNode.hints), false, nil default: return thisNode, true, nil @@ -792,47 +792,63 @@ func tryToReplaceDistinctWithPQLDistinct(ctx context.Context, a *ExecutionPlanne tables := getTableScanOperators(ctx, a, n, scope) //only do this if we have one TableScanOperator - if len(tables) == 1 { - replacedWithDistinct := false - // replace the scan with the distinct scan - return TransformPlanOp(n, func(node types.PlanOperator) (types.PlanOperator, bool, error) { - switch thisNode := node.(type) { - case *PlanOpDistinct: - if replacedWithDistinct { - return thisNode.ChildOp, false, nil - } - return thisNode, true, nil + if len(tables) != 1 { + return n, true, nil + } + replacedWithDistinct := false + // replace the scan with the distinct scan + return TransformPlanOp(n, func(node types.PlanOperator) (types.PlanOperator, bool, error) { + switch thisNode := node.(type) { + case *PlanOpDistinct: + if replacedWithDistinct { + return thisNode.ChildOp, false, nil + } + return thisNode, true, nil - case *PlanOpPQLTableScan: - // bail if there is more than one output column - if len(thisNode.columns) != 1 { - return thisNode, true, nil - } - - // make sure it's not the _id column - if strings.EqualFold(thisNode.columns[0], string(dax.PrimaryKeyFieldName)) { - return thisNode, true, nil - } - - // make sure it's not a set type - s := thisNode.Schema() - switch s[0].Type.(type) { - case *parser.DataTypeIDSet, *parser.DataTypeStringSet: - return thisNode, true, nil - } - - newOp, err := NewPlanOpPQLDistinctScan(a, thisNode.tableName, thisNode.columns[0]) - if err != nil { - return nil, false, err - } - replacedWithDistinct = true - return newOp, false, nil - default: + case *PlanOpPQLTableScan: + // bail if there is more than one output column + if len(thisNode.columns) != 1 { return thisNode, true, nil } - }) - } - return n, true, nil + + // make sure it's not the _id column + if strings.EqualFold(thisNode.columns[0], string(dax.PrimaryKeyFieldName)) { + return thisNode, true, nil + } + + // if it is a set type, check to see if we have query hint that tells us to flatten on this column + s := thisNode.Schema() + switch s[0].Type.(type) { + case *parser.DataTypeIDSet, *parser.DataTypeStringSet: + found := false + for _, h := range thisNode.hints { + if strings.EqualFold("flatten", h.name) { + for _, hp := range h.params { + if strings.EqualFold(s[0].ColumnName, hp) { + found = true + break + } + } + if found { + break + } + } + } + if !found { + return thisNode, true, nil + } + } + + newOp, err := NewPlanOpPQLDistinctScan(a, thisNode.tableName, thisNode.columns[0]) + if err != nil { + return nil, false, err + } + replacedWithDistinct = true + return newOp, false, nil + default: + return thisNode, true, nil + } + }) } func tryToReplaceConstRowDeleteWithFilteredDelete(ctx context.Context, a *ExecutionPlanner, n types.PlanOperator, scope *OptimizerScope) (types.PlanOperator, bool, error) { @@ -871,71 +887,94 @@ func tryToReplaceGroupByWithPQLGroupBy(ctx context.Context, a *ExecutionPlanner, tables := getTableScanOperators(ctx, a, n, scope) //only do this if we have one TableScanOperator - if len(tables) == 1 { - return TransformPlanOp(n, func(node types.PlanOperator) (types.PlanOperator, bool, error) { - switch n := node.(type) { - case *PlanOpGroupBy: - //table scan - table := tables[0] - //only do this if we have group by expressions - if len(n.GroupByExprs) > 0 { - pkType, err := table.PrimaryKeyType() - if err != nil { - return n, true, err - } - - //use a multi group by if more than 1 aggregate - if len(n.Aggregates) > 1 { - ops := make([]*PlanOpPQLGroupBy, 0) - for _, agg := range n.Aggregates { - aggregable, ok := agg.(types.Aggregable) - if !ok { - return n, false, sql3.NewErrInternalf("unexpected aggregate function arg type '%T'", agg) - } - - // if it's a count(*) on a pql table scan, so add the arg - star, ok := agg.(*countStarPlanExpression) - if ok { - newChildren := []types.PlanExpression{newQualifiedRefPlanExpression(table.tableName, string(dax.PrimaryKeyFieldName), 0, pkType)} - newAgg, err := star.WithChildren(newChildren...) - if err != nil { - return n, true, err - } - aggregable = newAgg.(types.Aggregable) - } - - ops = append(ops, NewPlanOpPQLGroupBy(a, table.tableName, n.GroupByExprs, table.filter, aggregable)) - } - newOp := NewPlanOpPQLMultiGroupBy(a, ops, n.GroupByExprs) - newOp.AddWarning(fmt.Sprintf("Multiple (%d) aggregates referenced in select list will result in multiple group by aggregate queries being executed.", len(ops))) - return newOp, false, nil - } - //only one aggregate - aggregable, ok := n.Aggregates[0].(types.Aggregable) - if !ok { - return n, false, sql3.NewErrInternalf("unexpected aggregate function arg type '%T'", n.Aggregates[0]) - } - - // if it's a count(*) on a pql table scan, so add the arg - star, ok := aggregable.(*countStarPlanExpression) - if ok { - newChildren := []types.PlanExpression{newQualifiedRefPlanExpression(table.tableName, string(dax.PrimaryKeyFieldName), 0, pkType)} - newAgg, err := star.WithChildren(newChildren...) - if err != nil { - return n, true, err - } - aggregable = newAgg.(types.Aggregable) - } - newOp := NewPlanOpPQLGroupBy(a, table.tableName, n.GroupByExprs, table.filter, aggregable) - return newOp, false, nil - } - return n, true, nil - default: - return n, true, nil - } - }) + if len(tables) != 1 { + return n, true, nil } - return n, true, nil + + return TransformPlanOp(n, func(node types.PlanOperator) (types.PlanOperator, bool, error) { + switch thisNode := node.(type) { + case *PlanOpGroupBy: + + //only do this if we have group by expressions + if len(thisNode.GroupByExprs) == 0 { + return thisNode, true, nil + } + + // get the table + table := tables[0] + + // if we are grouping on set columns, see if we have any flatten query hints + for _, gbc := range thisNode.GroupByExprs { + gbcRef, ok := gbc.(*qualifiedRefPlanExpression) + if !ok { + // don't need to stop the world here + break + } + switch gbcRef.Type().(type) { + case *parser.DataTypeIDSet, *parser.DataTypeStringSet: + // we are grouping on a set, so see if we have any flatten hints, + // if we do, we can continue the transform + found := false + for _, h := range table.hints { + if strings.EqualFold("flatten", h.name) { + for _, hp := range h.params { + if strings.EqualFold(gbcRef.columnName, hp) { + found = true + break + } + } + if found { + break + } + } + } + if !found { + return thisNode, true, nil + } + } + } + + // get the type of the _id column for this table + pkType, err := table.PrimaryKeyType() + if err != nil { + return thisNode, true, err + } + + // for each of the aggregates, go make a PlanOpPQLGroupBy operator + ops := make([]*PlanOpPQLGroupBy, 0) + for _, agg := range thisNode.Aggregates { + aggregable, ok := agg.(types.Aggregable) + if !ok { + return thisNode, false, sql3.NewErrInternalf("unexpected aggregate function arg type '%T'", agg) + } + + // if it's a count(*) on a pql table scan, so add the arg + star, ok := agg.(*countStarPlanExpression) + if ok { + newChildren := []types.PlanExpression{newQualifiedRefPlanExpression(table.tableName, string(dax.PrimaryKeyFieldName), 0, pkType)} + newAgg, err := star.WithChildren(newChildren...) + if err != nil { + return thisNode, true, err + } + aggregable = newAgg.(types.Aggregable) + } + + ops = append(ops, NewPlanOpPQLGroupBy(a, table.tableName, thisNode.GroupByExprs, table.filter, aggregable)) + } + + // use a multi group by if more than 1 aggregate + if len(thisNode.Aggregates) > 1 { + newOp := NewPlanOpPQLMultiGroupBy(a, ops, thisNode.GroupByExprs) + return newOp, false, nil + } + + // else only one aggregate + return ops[0], false, nil + + default: + return thisNode, true, nil + } + }) } func pushdownPQLTop(ctx context.Context, a *ExecutionPlanner, n types.PlanOperator, scope *OptimizerScope) (types.PlanOperator, bool, error) { diff --git a/sql3/test/defs/defs.go b/sql3/test/defs/defs.go index 5412758db..f72acec2a 100644 --- a/sql3/test/defs/defs.go +++ b/sql3/test/defs/defs.go @@ -184,6 +184,7 @@ var TableTests []TableTest = []TableTest{ // groupby tests groupByTests, + groupBySetDistinctTests, // create table tests createTable, diff --git a/sql3/test/defs/defs_groupby.go b/sql3/test/defs/defs_groupby.go index 215ae63a0..76d2e2383 100644 --- a/sql3/test/defs/defs_groupby.go +++ b/sql3/test/defs/defs_groupby.go @@ -232,9 +232,10 @@ var groupByTests = TableTest{ hdr("is1", fldTypeIDSet), ), ExpRows: rows( - row(int64(5), []int64{1}), - row(int64(4), []int64{2}), - row(int64(4), []int64{3}), + row(int64(2), []int64{1, 2}), + row(int64(2), []int64{1, 3}), + row(int64(1), []int64{2, 3}), + row(int64(1), []int64{1, 2, 3}), ), Compare: CompareExactOrdered, }, @@ -257,3 +258,206 @@ var groupByTests = TableTest{ }, }, } + +// groupby/distinct with sets tests +var groupBySetDistinctTests = TableTest{ + Table: tbl( + "groupby_set_test", + srcHdrs( + srcHdr("_id", fldTypeID), + srcHdr("ids1", fldTypeIDSet), + srcHdr("ss1", fldTypeStringSet), + ), + srcRows( + srcRow(int64(1), []int64{1, 2}, []string{"a", "b"}), + srcRow(int64(2), []int64{3, 4}, []string{"d", "e"}), + srcRow(int64(3), []int64{1, 4}, []string{"a", "d"}), + srcRow(int64(4), []int64{3, 2}, []string{"c", "b"}), + srcRow(int64(5), []int64{3, 2}, []string{"c", "b"}), + ), + ), + SQLTests: []SQLTest{ + { + SQLs: sqls( + "select distinct ids1 from groupby_set_test with (flatter(foo))", + ), + ExpErr: "unknown query hint 'flatter'", + }, + { + SQLs: sqls( + "select distinct ids1 from groupby_set_test with (flatten(foo))", + ), + ExpErr: "column 'foo' not found", + }, + { + SQLs: sqls( + "select distinct ids1 from groupby_set_test with (flatten(foo, bar))", + ), + ExpErr: "query hint 'flatten' expected 1 parameter(s) (column name), got 2 parameters", + }, + { + SQLs: sqls( + "select distinct ids1 from groupby_set_test", + ), + ExpHdrs: hdrs( + hdr("ids1", fldTypeIDSet), + ), + ExpRows: rows( + row([]int64{1, 2}), + row([]int64{3, 4}), + row([]int64{1, 4}), + row([]int64{2, 3}), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select distinct ids1 from groupby_set_test with (flatten(ids1))", + ), + ExpHdrs: hdrs( + hdr("ids1", fldTypeIDSet), + ), + ExpRows: rows( + row([]int64{1}), + row([]int64{2}), + row([]int64{3}), + row([]int64{4}), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select distinct ids1, ss1 from groupby_set_test", + ), + ExpHdrs: hdrs( + hdr("ids1", fldTypeIDSet), + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row([]int64{1, 2}, []string{"a", "b"}), + row([]int64{3, 4}, []string{"d", "e"}), + row([]int64{1, 4}, []string{"a", "d"}), + row([]int64{2, 3}, []string{"b", "c"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + SQLs: sqls( + "select distinct ids1, ss1 from groupby_set_test with (flatten(ids1))", + ), + ExpHdrs: hdrs( + hdr("ids1", fldTypeIDSet), + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row([]int64{1, 2}, []string{"a", "b"}), + row([]int64{3, 4}, []string{"d", "e"}), + row([]int64{1, 4}, []string{"a", "d"}), + row([]int64{2, 3}, []string{"b", "c"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + SQLs: sqls( + "select count(*), ids1 from groupby_set_test group by ids1", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("ids1", fldTypeIDSet), + ), + ExpRows: rows( + row(int64(1), []int64{1, 2}), + row(int64(1), []int64{3, 4}), + row(int64(1), []int64{1, 4}), + row(int64(2), []int64{2, 3}), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select count(*), ids1 from groupby_set_test with (flatten(ids1)) group by ids1", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("ids1", fldTypeIDSet), + ), + ExpRows: rows( + row(int64(2), []int64{1}), + row(int64(3), []int64{2}), + row(int64(3), []int64{3}), + row(int64(2), []int64{4}), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select distinct ss1 from groupby_set_test", + ), + ExpHdrs: hdrs( + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row([]string{"a", "b"}), + row([]string{"d", "e"}), + row([]string{"a", "d"}), + row([]string{"b", "c"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + SQLs: sqls( + "select distinct ss1 from groupby_set_test with (flatten(ss1))", + ), + ExpHdrs: hdrs( + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row([]string{"a"}), + row([]string{"b"}), + row([]string{"c"}), + row([]string{"d"}), + row([]string{"e"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + SQLs: sqls( + "select count(*), ss1 from groupby_set_test group by ss1", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row(int64(1), []string{"a", "b"}), + row(int64(1), []string{"d", "e"}), + row(int64(1), []string{"a", "d"}), + row(int64(2), []string{"b", "c"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + { + SQLs: sqls( + "select count(*), ss1 from groupby_set_test with (flatten(ss1)) group by ss1", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + hdr("ss1", fldTypeStringSet), + ), + ExpRows: rows( + row(int64(2), []string{"a"}), + row(int64(3), []string{"b"}), + row(int64(2), []string{"c"}), + row(int64(2), []string{"d"}), + row(int64(1), []string{"e"}), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + }, +} diff --git a/sql3/test/defs/defs_top.go b/sql3/test/defs/defs_top.go index 324cc9c00..ad64302ea 100644 --- a/sql3/test/defs/defs_top.go +++ b/sql3/test/defs/defs_top.go @@ -67,8 +67,8 @@ var topLimitTests = TableTest{ hdr("skills", fldTypeStringSet), ), ExpRows: rows( - row(int64(1), string("Marketing Manager")), - row(int64(1), string("Software Engineer I")), + row(int64(1), []string{"Marketing Manager"}), + row(int64(1), []string{"Software Engineer I"}), ), Compare: CompareExactUnordered, SortStringKeys: true, @@ -82,8 +82,8 @@ var topLimitTests = TableTest{ hdr("skills", fldTypeStringSet), ), ExpRows: rows( - row(int64(1), string("Marketing Manager")), - row(int64(1), string("Software Engineer I")), + row(int64(1), []string{"Marketing Manager"}), + row(int64(1), []string{"Software Engineer I"}), ), Compare: CompareExactUnordered, SortStringKeys: true,