Skip to content

Commit

Permalink
[gen4] More aggregation support (#10332)
Browse files Browse the repository at this point in the history 
* refactor: move HAVING with aggregation to QP

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: add aggregation expressions needed by HAVING

Signed-off-by: Andres Taylor <andres@planetscale.com>

* test&refactor: add integration test and clean up types

Signed-off-by: Andres Taylor <andres@planetscale.com>

* refactor: use sort.Slice to avoid extra types

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: change the string representation of offset for better clarity

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: handle columns with not in grouping or aggregations better

Signed-off-by: Andres Taylor <andres@planetscale.com>

* test: update test expectations

Signed-off-by: Andres Taylor <andres@planetscale.com>

* test refactor: move out test cases that pass from unsupported_cases

Signed-off-by: Andres Taylor <andres@planetscale.com>
  • Loading branch information
@systay
systay committed May 19, 2022
1 parent ef9b95a commit 165dee2
Show file tree
Hide file tree
Showing 19 changed files with 875 additions and 417 deletions.
21 changes: 9 additions & 12 deletions go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ func TestEqualFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a = \"5\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a = 5.00", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a, val1 from aggr_test group by val1 having a = 1.00", `[[INT64(1) VARCHAR("a")] [INT64(1) VARCHAR("b")] [INT64(1) VARCHAR("c")] [INT64(1) VARCHAR("d")] [INT64(1) VARCHAR("e")]]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) = 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) = 5", `[[INT64(1)]]`)
})
}
}
Expand Down Expand Up @@ -179,6 +178,9 @@ func TestAggrOnJoin(t *testing.T) {

mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ a.val1, count(*) as leCount from aggr_test a join t3 t on a.val2 = t.id7 group by a.val1 having leCount = 4",
`[[VARCHAR("a") INT64(4)]]`)

mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ a.val1 from aggr_test a join t3 t on a.val2 = t.id7 group by a.val1 having count(*) = 4",
`[[VARCHAR("a")]]`)
}

func TestNotEqualFilterOnScatter(t *testing.T) {
Expand All @@ -200,8 +202,7 @@ func TestNotEqualFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a != \"1\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a != \"5\"", `[]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a != 5.00", `[]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) != 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) != 5", `[]`)
})
}
}
Expand All @@ -224,8 +225,7 @@ func TestLessFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a < \"10\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a < \"5\"", `[]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a < 6.00", `[[INT64(5)]]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) < 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) < 5", `[]`)
})
}
}
Expand All @@ -249,8 +249,7 @@ func TestLessEqualFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a <= \"10\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a <= \"5\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a <= 5.00", `[[INT64(5)]]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) <= 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) <= 5", `[[INT64(1)]]`)
})
}
}
Expand All @@ -274,8 +273,7 @@ func TestGreaterFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a > \"1\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a > \"5\"", `[]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a > 4.00", `[[INT64(5)]]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) > 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) > 5", `[]`)
})
}
}
Expand All @@ -299,8 +297,7 @@ func TestGreaterEqualFilterOnScatter(t *testing.T) {
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a >= \"1\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a >= \"5\"", `[[INT64(5)]]`)
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(*) as a from aggr_test having a >= 5.00", `[[INT64(5)]]`)

utils.AssertContainsError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) >= 5", `expr cannot be translated, not supported`) // will fail since `count(*)` is a FuncExpr
mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ 1 from aggr_test having count(*) >= 5", `[[INT64(1)]]`)
})
}
}
Expand Down
10 changes: 7 additions & 3 deletions go/vt/sqlparser/ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2353,8 +2353,12 @@ type (
JSONVal Expr
}

// Offset is another AST type that is used during planning and never produced by the parser
Offset int
// Offset is an AST type that is used during planning and never produced by the parser
// it is the column offset from the incoming result stream
Offset struct {
V int
Original string
}

// JSONArrayExpr represents JSON_ARRAY()
// More information on https://dev.mysql.com/doc/refman/8.0/en/json-creation-functions.html#function_json-array
Expand Down Expand Up @@ -2597,7 +2601,7 @@ func (*ExtractedSubquery) iExpr() {}
func (*TrimFuncExpr) iExpr() {}
func (*JSONSchemaValidFuncExpr) iExpr() {}
func (*JSONSchemaValidationReportFuncExpr) iExpr() {}
func (Offset) iExpr() {}
func (*Offset) iExpr() {}
func (*JSONPrettyExpr) iExpr() {}
func (*JSONStorageFreeExpr) iExpr() {}
func (*JSONStorageSizeExpr) iExpr() {}
Expand Down
21 changes: 15 additions & 6 deletions go/vt/sqlparser/ast_clone.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 21 additions & 9 deletions go/vt/sqlparser/ast_equals.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 9 additions & 4 deletions go/vt/sqlparser/ast_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2144,8 +2144,13 @@ func (node *JtOnResponse) Format(buf *TrackedBuffer) {
}

// Format formats the node.
func (node Offset) Format(buf *TrackedBuffer) {
buf.astPrintf(node, "[%d]", int(node))
// Using capital letter for this function as an indicator that it's not a normal function call ¯\_(ツ)_/¯
func (node *Offset) Format(buf *TrackedBuffer) {
if node.Original == "" {
buf.astPrintf(node, "OFFSET(%d)", node.V)
} else {
buf.astPrintf(node, "OFFSET(%d, '%s')", node.V, node.Original)
}
}

// Format formats the node.
Expand All @@ -2160,7 +2165,7 @@ func (node *JSONSchemaValidationReportFuncExpr) Format(buf *TrackedBuffer) {

// Format formats the node.
func (node *JSONArrayExpr) Format(buf *TrackedBuffer) {
//buf.astPrintf(node,"%s(,"node.Name.Lowered())
// buf.astPrintf(node,"%s(,"node.Name.Lowered())
buf.literal("json_array(")
if len(node.Params) > 0 {
var prefix string
Expand All @@ -2174,7 +2179,7 @@ func (node *JSONArrayExpr) Format(buf *TrackedBuffer) {

// Format formats the node.
func (node *JSONObjectExpr) Format(buf *TrackedBuffer) {
//buf.astPrintf(node,"%s(,"node.Name.Lowered())
// buf.astPrintf(node,"%s(,"node.Name.Lowered())
buf.literal("json_object(")
if len(node.Params) > 0 {
for i, p := range node.Params {
Expand Down
21 changes: 15 additions & 6 deletions go/vt/sqlparser/ast_format_fast.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,10 @@ func NewColIdentWithAt(str string, at AtCount) ColIdent {
}
}

func NewOffset(v int, original Expr) *Offset {
return &Offset{V: v, Original: String(original)}
}

// IsEmpty returns true if the name is empty.
func (node ColIdent) IsEmpty() bool {
return node.val == ""
Expand Down
57 changes: 30 additions & 27 deletions go/vt/sqlparser/ast_rewrite.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 165dee2

Please sign in to comment.