From 588aa626bda1a45d74ae0cb2d21fca43b35a7a4e Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Tue, 18 Aug 2020 09:54:49 +0200 Subject: [PATCH 01/16] added failing end-to-end test for sql_calc_found_rows Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 go/test/endtoend/vtgate/found_rows_test.go diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go new file mode 100644 index 00000000000..a0c077b7a4b --- /dev/null +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -0,0 +1,52 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vtgate + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/test/endtoend/cluster" +) + +func TestFoundRows(t *testing.T) { + t.Skip("failing at the moment") + defer cluster.PanicHandler(t) + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.Nil(t, err) + defer conn.Close() + + exec(t, conn, "insert into t2(id3,id4) values(1,1), (2,2), (3,3), (4,4), (5,5)") + defer exec(t, conn, "delete from t2") + + assertFoundRowsValue(t, conn, "select * from t2", 5) + assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 limit 2", 5) +} + +func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query string, count int) { + exec(t, conn, query) + qr := exec(t, conn, "select found_rows()") + got := fmt.Sprintf("%v", qr.Rows) + want := fmt.Sprintf(`[[UINT64(%d)]]`, count) + require.Equal(t, want, got) +} From c736ea6590f3840e3b44e5d50bd49c72b2da9ad2 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 9 Sep 2020 12:44:48 +0530 Subject: [PATCH 02/16] added sqlcalcfoundrows engine primitive Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/sqlcalfoundrows.go | 79 ++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 go/vt/vtgate/engine/sqlcalfoundrows.go diff --git a/go/vt/vtgate/engine/sqlcalfoundrows.go b/go/vt/vtgate/engine/sqlcalfoundrows.go new file mode 100644 index 00000000000..d992762546b --- /dev/null +++ b/go/vt/vtgate/engine/sqlcalfoundrows.go @@ -0,0 +1,79 @@ +package engine + +import ( + "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/evalengine" +) + +var _ Primitive = (*SQLCalFoundRows)(nil) + +//SQLCalFoundRows is a primitive to execute limit and count query as per their individual plan. +type SQLCalFoundRows struct { + LimitPrimitive Primitive + CountPrimitive Primitive +} + +//RouteType implements the Primitive interface +func (s SQLCalFoundRows) RouteType() string { + return "SQLCalcFoundRows" +} + +//GetKeyspaceName implements the Primitive interface +func (s SQLCalFoundRows) GetKeyspaceName() string { + return s.LimitPrimitive.GetKeyspaceName() +} + +//GetTableName implements the Primitive interface +func (s SQLCalFoundRows) GetTableName() string { + return s.LimitPrimitive.GetTableName() +} + +//Execute implements the Primitive interface +func (s SQLCalFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { + limitQr, err := s.LimitPrimitive.Execute(vcursor, bindVars, wantfields) + if err != nil { + return nil, err + } + countQr, err := s.CountPrimitive.Execute(vcursor, bindVars, false) + if err != nil { + return nil, err + } + if len(countQr.Rows) != 1 || len(countQr.Rows[0]) != 1 { + return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "count query is not a scalar") + } + fr, err := evalengine.ToUint64(countQr.Rows[0][0]) + if err != nil { + return nil, err + } + vcursor.Session().SetFoundRows(fr) + return limitQr, nil +} + +//StreamExecute implements the Primitive interface +func (s SQLCalFoundRows) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { + panic("implement me") +} + +//GetFields implements the Primitive interface +func (s SQLCalFoundRows) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { + return s.LimitPrimitive.GetFields(vcursor, bindVars) +} + +//NeedsTransaction implements the Primitive interface +func (s SQLCalFoundRows) NeedsTransaction() bool { + return s.LimitPrimitive.NeedsTransaction() +} + +//Inputs implements the Primitive interface +func (s SQLCalFoundRows) Inputs() []Primitive { + return []Primitive{s.LimitPrimitive, s.CountPrimitive} +} + +func (s SQLCalFoundRows) description() PrimitiveDescription { + return PrimitiveDescription{ + OperatorType: "SQL_CALC_FOUND_ROWS", + } +} From 4c25a490c3d3e40fe985932b8003c0a01d85e3b8 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 9 Sep 2020 12:48:24 +0530 Subject: [PATCH 03/16] sql_calc_found_rows planning Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/select.go | 25 +++++- .../vtgate/planbuilder/sql_calc_found_rows.go | 87 +++++++++++++++++++ .../planbuilder/testdata/select_cases.txt | 70 +++++++++++++++ 3 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/sql_calc_found_rows.go diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index f4295350152..4468a3c2979 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -97,8 +97,29 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "%v allowed only with dual", sqlparser.String(aExpr)) } } - if sel.SQLCalcFoundRows && sel.Limit != nil { - return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "sql_calc_found_rows not yet fully supported") + if sel.SQLCalcFoundRows { + sel.SQLCalcFoundRows = false + if sel.Limit != nil { + frpb := newPrimitiveBuilder(pb.vschema, pb.jt) + err := frpb.processSelect(sel, outer) + if err != nil { + return err + } + sel.SelectExprs = []sqlparser.SelectExpr{&sqlparser.AliasedExpr{ + Expr: &sqlparser.FuncExpr{ + Name: sqlparser.NewColIdent("count"), + Exprs: []sqlparser.SelectExpr{&sqlparser.StarExpr{}}, + }, + }} + sel.OrderBy = nil + sel.Limit = nil + err = pb.processSelect(sel, outer) + if err != nil { + return err + } + pb.bldr = sqlCalcFoundRows{LimitQuery: frpb.bldr, CountQuery: pb.bldr} + return nil + } } if err := pb.processTableExprs(sel.From); err != nil { diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go new file mode 100644 index 00000000000..bf1edd79b2e --- /dev/null +++ b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go @@ -0,0 +1,87 @@ +package planbuilder + +import ( + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/engine" +) + +var _ builder = (*sqlCalcFoundRows)(nil) + +type sqlCalcFoundRows struct { + LimitQuery, CountQuery builder +} + +func (s sqlCalcFoundRows) Order() int { + panic("implement me") +} + +func (s sqlCalcFoundRows) ResultColumns() []*resultColumn { + panic("implement me") +} + +func (s sqlCalcFoundRows) Reorder(i int) { + panic("implement me") +} + +func (s sqlCalcFoundRows) First() builder { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushFilter(pb *primitiveBuilder, filter sqlparser.Expr, whereType string, origin builder) error { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushSelect(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, origin builder) (rc *resultColumn, colNumber int, err error) { + panic("implement me") +} + +func (s sqlCalcFoundRows) MakeDistinct() error { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushGroupBy(by sqlparser.GroupBy) error { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushOrderBy(by sqlparser.OrderBy) (builder, error) { + panic("implement me") +} + +func (s sqlCalcFoundRows) SetUpperLimit(count sqlparser.Expr) { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushMisc(sel *sqlparser.Select) { + panic("implement me") +} + +func (s sqlCalcFoundRows) Wireup(bldr builder, jt *jointab) error { + err := s.LimitQuery.Wireup(bldr, jt) + if err != nil { + return err + } + return s.CountQuery.Wireup(bldr, jt) +} + +func (s sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { + panic("implement me") +} + +func (s sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (rc *resultColumn, colNumber int) { + panic("implement me") +} + +func (s sqlCalcFoundRows) SupplyWeightString(colNumber int) (weightcolNumber int, err error) { + panic("implement me") +} + +func (s sqlCalcFoundRows) PushLock(lock string) error { + panic("implement me") +} + +func (s sqlCalcFoundRows) Primitive() engine.Primitive { + return engine.SQLCalFoundRows{ + LimitPrimitive: s.LimitQuery.Primitive(), + CountPrimitive: s.CountQuery.Primitive(), + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index 35750f6e37d..e32c4c6adde 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1387,3 +1387,73 @@ ] } } + +# sql_calc_found_rows without limit +"select sql_calc_found_rows * from music where user_id = 1" +{ + "QueryType": "SELECT", + "Original": "select sql_calc_found_rows * from music where user_id = 1", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select * from music where 1 != 1", + "Query": "select * from music where user_id = 1", + "Table": "music", + "Values": [ + 1 + ], + "Vindex": "user_index" + } +} + +# sql_calc_found_rows with limit +"select sql_calc_found_rows * from music limit 100" +{ + "QueryType": "SELECT", + "Original": "select sql_calc_found_rows * from music limit 100", + "Instructions": { + "OperatorType": "SQL_CALC_FOUND_ROWS", + "Inputs": [ + { + "OperatorType": "Limit", + "Count": 100, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select * from music where 1 != 1", + "Query": "select * from music limit :__upper_limit", + "Table": "music" + } + ] + }, + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(0)", + "Distinct": "false", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select count(*) from music where 1 != 1", + "Query": "select count(*) from music", + "Table": "music" + } + ] + } + ] + } +} From 83d4273cb15a2ea03e0446cbbeaaae2cc14c2648 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 9 Sep 2020 12:48:55 +0530 Subject: [PATCH 04/16] added foundRows session action method Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/fake_vcursor_test.go | 8 ++++++++ go/vt/vtgate/engine/primitive.go | 1 + go/vt/vtgate/executor.go | 4 +++- go/vt/vtgate/safe_session.go | 9 +++++---- go/vt/vtgate/vcursor_impl.go | 5 +++++ 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index 04448731c4c..f931a40b5c2 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -53,6 +53,10 @@ type noopVCursor struct { ctx context.Context } +func (t noopVCursor) SetFoundRows(u uint64) { + panic("implement me") +} + func (t noopVCursor) InTransactionAndIsDML() bool { panic("implement me") } @@ -199,6 +203,10 @@ type loggingVCursor struct { resolvedTargetTabletType topodatapb.TabletType } +func (f *loggingVCursor) SetFoundRows(u uint64) { + panic("implement me") +} + func (f *loggingVCursor) InTransactionAndIsDML() bool { return false } diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go index 4e2e40e7ed7..f5bdf6a6fe5 100644 --- a/go/vt/vtgate/engine/primitive.go +++ b/go/vt/vtgate/engine/primitive.go @@ -116,6 +116,7 @@ type ( SetSQLSelectLimit(int64) SetTransactionMode(vtgatepb.TransactionMode) SetWorkload(querypb.ExecuteOptions_Workload) + SetFoundRows(uint64) } // Plan represents the execution strategy for a given query. diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 9356945ef03..110f586c8c9 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -158,7 +158,9 @@ func saveSessionStats(safeSession *SafeSession, stmtType sqlparser.StatementType if err != nil { return } - safeSession.FoundRows = result.RowsAffected + if !safeSession.foundRowsHandled { + safeSession.FoundRows = result.RowsAffected + } if result.InsertID > 0 { safeSession.LastInsertId = result.InsertID } diff --git a/go/vt/vtgate/safe_session.go b/go/vt/vtgate/safe_session.go index c33d0375b6b..1ccd0fdb020 100644 --- a/go/vt/vtgate/safe_session.go +++ b/go/vt/vtgate/safe_session.go @@ -34,10 +34,11 @@ import ( // (the use pattern is 'Find', if not found, then 'AppendOrUpdate', // for a single shard) type SafeSession struct { - mu sync.Mutex - mustRollback bool - autocommitState autocommitState - commitOrder vtgatepb.CommitOrder + mu sync.Mutex + mustRollback bool + autocommitState autocommitState + commitOrder vtgatepb.CommitOrder + foundRowsHandled bool *vtgatepb.Session } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 09ea335ba7d..4aaa7aae770 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -526,6 +526,11 @@ func (vc *vcursorImpl) SysVarSetEnabled() bool { return *sysVarSetEnabled } +func (vc *vcursorImpl) SetFoundRows(foundRows uint64) { + vc.safeSession.FoundRows = foundRows + vc.safeSession.foundRowsHandled = true +} + // ParseDestinationTarget parses destination target string and sets default keyspace if possible. func parseDestinationTarget(targetString string, vschema *vindexes.VSchema) (string, topodatapb.TabletType, key.Destination, error) { destKeyspace, destTabletType, dest, err := topoprotopb.ParseDestination(targetString, defaultTabletType) From cf9f6abc5badc828af4de6c54f82232ebf029292 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 9 Sep 2020 12:49:23 +0530 Subject: [PATCH 05/16] removed test skip Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/found_rows_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index a0c077b7a4b..86c94e5accd 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -28,7 +28,6 @@ import ( ) func TestFoundRows(t *testing.T) { - t.Skip("failing at the moment") defer cluster.PanicHandler(t) ctx := context.Background() conn, err := mysql.Connect(ctx, &vtParams) From e26f979f05c0617a3d1181d8c4cf9fab4e0465fd Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 9 Sep 2020 16:38:10 +0200 Subject: [PATCH 06/16] plan more complicated sql_calc_found_rows queries Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 2 + ...calfoundrows.go => sql_calc_found_rows.go} | 40 +++++++++---- go/vt/vtgate/planbuilder/select.go | 58 +++++++++++++----- .../vtgate/planbuilder/sql_calc_found_rows.go | 59 ++++++++++++------- .../planbuilder/testdata/select_cases.txt | 42 +++++++++++++ 5 files changed, 153 insertions(+), 48 deletions(-) rename go/vt/vtgate/engine/{sqlcalfoundrows.go => sql_calc_found_rows.go} (51%) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index 86c94e5accd..8f8eb0fdc2a 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -40,6 +40,8 @@ func TestFoundRows(t *testing.T) { assertFoundRowsValue(t, conn, "select * from t2", 5) assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 limit 2", 5) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id3 = 4 limit 2", 1) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 4 limit 2", 1) } func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query string, count int) { diff --git a/go/vt/vtgate/engine/sqlcalfoundrows.go b/go/vt/vtgate/engine/sql_calc_found_rows.go similarity index 51% rename from go/vt/vtgate/engine/sqlcalfoundrows.go rename to go/vt/vtgate/engine/sql_calc_found_rows.go index d992762546b..5a07d0eb357 100644 --- a/go/vt/vtgate/engine/sqlcalfoundrows.go +++ b/go/vt/vtgate/engine/sql_calc_found_rows.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package engine import ( @@ -8,31 +24,31 @@ import ( "vitess.io/vitess/go/vt/vtgate/evalengine" ) -var _ Primitive = (*SQLCalFoundRows)(nil) +var _ Primitive = (*SQLCalcFoundRows)(nil) -//SQLCalFoundRows is a primitive to execute limit and count query as per their individual plan. -type SQLCalFoundRows struct { +//SQLCalcFoundRows is a primitive to execute limit and count query as per their individual plan. +type SQLCalcFoundRows struct { LimitPrimitive Primitive CountPrimitive Primitive } //RouteType implements the Primitive interface -func (s SQLCalFoundRows) RouteType() string { +func (s SQLCalcFoundRows) RouteType() string { return "SQLCalcFoundRows" } //GetKeyspaceName implements the Primitive interface -func (s SQLCalFoundRows) GetKeyspaceName() string { +func (s SQLCalcFoundRows) GetKeyspaceName() string { return s.LimitPrimitive.GetKeyspaceName() } //GetTableName implements the Primitive interface -func (s SQLCalFoundRows) GetTableName() string { +func (s SQLCalcFoundRows) GetTableName() string { return s.LimitPrimitive.GetTableName() } //Execute implements the Primitive interface -func (s SQLCalFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { +func (s SQLCalcFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { limitQr, err := s.LimitPrimitive.Execute(vcursor, bindVars, wantfields) if err != nil { return nil, err @@ -53,26 +69,26 @@ func (s SQLCalFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb.B } //StreamExecute implements the Primitive interface -func (s SQLCalFoundRows) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { +func (s SQLCalcFoundRows) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { panic("implement me") } //GetFields implements the Primitive interface -func (s SQLCalFoundRows) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { +func (s SQLCalcFoundRows) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { return s.LimitPrimitive.GetFields(vcursor, bindVars) } //NeedsTransaction implements the Primitive interface -func (s SQLCalFoundRows) NeedsTransaction() bool { +func (s SQLCalcFoundRows) NeedsTransaction() bool { return s.LimitPrimitive.NeedsTransaction() } //Inputs implements the Primitive interface -func (s SQLCalFoundRows) Inputs() []Primitive { +func (s SQLCalcFoundRows) Inputs() []Primitive { return []Primitive{s.LimitPrimitive, s.CountPrimitive} } -func (s SQLCalFoundRows) description() PrimitiveDescription { +func (s SQLCalcFoundRows) description() PrimitiveDescription { return PrimitiveDescription{ OperatorType: "SQL_CALC_FOUND_ROWS", } diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index 4468a3c2979..3e960b751af 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -100,24 +100,11 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) if sel.SQLCalcFoundRows { sel.SQLCalcFoundRows = false if sel.Limit != nil { - frpb := newPrimitiveBuilder(pb.vschema, pb.jt) - err := frpb.processSelect(sel, outer) + builder, err := buildSQLCalcFoundRowsPlan(sel, outer, pb.vschema) if err != nil { return err } - sel.SelectExprs = []sqlparser.SelectExpr{&sqlparser.AliasedExpr{ - Expr: &sqlparser.FuncExpr{ - Name: sqlparser.NewColIdent("count"), - Exprs: []sqlparser.SelectExpr{&sqlparser.StarExpr{}}, - }, - }} - sel.OrderBy = nil - sel.Limit = nil - err = pb.processSelect(sel, outer) - if err != nil { - return err - } - pb.bldr = sqlCalcFoundRows{LimitQuery: frpb.bldr, CountQuery: pb.bldr} + pb.bldr = builder return nil } } @@ -168,6 +155,47 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) return nil } +func buildSQLCalcFoundRowsPlan(sel *sqlparser.Select, outer *symtab, vschema ContextVSchema) (builder, error) { + ljt := newJointab(sqlparser.GetBindvars(sel)) + frpb := newPrimitiveBuilder(vschema, ljt) + err := frpb.processSelect(sel, outer) + if err != nil { + return nil, err + } + + // TODO systay this is a hack + s := sqlparser.String(sel) + statement, err := sqlparser.Parse(s) + if err != nil { + return nil, err + } + sel2 := statement.(*sqlparser.Select) + + sel2.SelectExprs = []sqlparser.SelectExpr{&sqlparser.AliasedExpr{ + Expr: &sqlparser.FuncExpr{ + Name: sqlparser.NewColIdent("count"), + Exprs: []sqlparser.SelectExpr{&sqlparser.StarExpr{}}, + }, + }} + sel2.OrderBy = nil + sel2.Limit = nil + + sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch col := node.(type) { + case *sqlparser.ColName: + col.Metadata = nil + } + return true, nil + }, sel2) + cjt := newJointab(sqlparser.GetBindvars(sel2)) + countpb := newPrimitiveBuilder(vschema, cjt) + err = countpb.processSelect(sel2, outer) + if err != nil { + return nil, err + } + return &sqlCalcFoundRows{LimitQuery: frpb.bldr, CountQuery: countpb.bldr, ljt: ljt, cjt: cjt}, nil +} + func handleDualSelects(sel *sqlparser.Select, vschema ContextVSchema) (engine.Primitive, error) { if !isOnlyDual(sel) { return nil, nil diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go index bf1edd79b2e..cfa01fc2432 100644 --- a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go +++ b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package planbuilder import ( @@ -9,78 +25,79 @@ var _ builder = (*sqlCalcFoundRows)(nil) type sqlCalcFoundRows struct { LimitQuery, CountQuery builder + ljt, cjt *jointab } -func (s sqlCalcFoundRows) Order() int { +func (s *sqlCalcFoundRows) Order() int { panic("implement me") } -func (s sqlCalcFoundRows) ResultColumns() []*resultColumn { +func (s *sqlCalcFoundRows) ResultColumns() []*resultColumn { panic("implement me") } -func (s sqlCalcFoundRows) Reorder(i int) { +func (s *sqlCalcFoundRows) Reorder(i int) { panic("implement me") } -func (s sqlCalcFoundRows) First() builder { +func (s *sqlCalcFoundRows) First() builder { panic("implement me") } -func (s sqlCalcFoundRows) PushFilter(pb *primitiveBuilder, filter sqlparser.Expr, whereType string, origin builder) error { +func (s *sqlCalcFoundRows) PushFilter(pb *primitiveBuilder, filter sqlparser.Expr, whereType string, origin builder) error { panic("implement me") } -func (s sqlCalcFoundRows) PushSelect(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, origin builder) (rc *resultColumn, colNumber int, err error) { +func (s *sqlCalcFoundRows) PushSelect(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, origin builder) (rc *resultColumn, colNumber int, err error) { panic("implement me") } -func (s sqlCalcFoundRows) MakeDistinct() error { +func (s *sqlCalcFoundRows) MakeDistinct() error { panic("implement me") } -func (s sqlCalcFoundRows) PushGroupBy(by sqlparser.GroupBy) error { +func (s *sqlCalcFoundRows) PushGroupBy(by sqlparser.GroupBy) error { panic("implement me") } -func (s sqlCalcFoundRows) PushOrderBy(by sqlparser.OrderBy) (builder, error) { +func (s *sqlCalcFoundRows) PushOrderBy(by sqlparser.OrderBy) (builder, error) { panic("implement me") } -func (s sqlCalcFoundRows) SetUpperLimit(count sqlparser.Expr) { +func (s *sqlCalcFoundRows) SetUpperLimit(count sqlparser.Expr) { panic("implement me") } -func (s sqlCalcFoundRows) PushMisc(sel *sqlparser.Select) { +func (s *sqlCalcFoundRows) PushMisc(sel *sqlparser.Select) { panic("implement me") } -func (s sqlCalcFoundRows) Wireup(bldr builder, jt *jointab) error { - err := s.LimitQuery.Wireup(bldr, jt) +func (s *sqlCalcFoundRows) Wireup(builder, *jointab) error { + err := s.LimitQuery.Wireup(s.LimitQuery, s.ljt) if err != nil { return err } - return s.CountQuery.Wireup(bldr, jt) + return s.CountQuery.Wireup(s.CountQuery, s.cjt) } -func (s sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { - panic("implement me") +func (s *sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { + s.LimitQuery.SupplyVar(from, to, col, varname) } -func (s sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (rc *resultColumn, colNumber int) { +func (s *sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (rc *resultColumn, colNumber int) { panic("implement me") } -func (s sqlCalcFoundRows) SupplyWeightString(colNumber int) (weightcolNumber int, err error) { +func (s *sqlCalcFoundRows) SupplyWeightString(colNumber int) (weightcolNumber int, err error) { panic("implement me") } -func (s sqlCalcFoundRows) PushLock(lock string) error { +func (s *sqlCalcFoundRows) PushLock(lock string) error { panic("implement me") } -func (s sqlCalcFoundRows) Primitive() engine.Primitive { - return engine.SQLCalFoundRows{ +func (s *sqlCalcFoundRows) Primitive() engine.Primitive { + return engine.SQLCalcFoundRows{ LimitPrimitive: s.LimitQuery.Primitive(), CountPrimitive: s.CountQuery.Primitive(), } diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index e32c4c6adde..af81191ac83 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1457,3 +1457,45 @@ ] } } + +# sql_calc_found_rows with SelectEqualUnique plans +"select sql_calc_found_rows * from music where user_id = 1 limit 2" +{ + "QueryType": "SELECT", + "Original": "select sql_calc_found_rows * from music where user_id = 1 limit 2", + "Instructions": { + "OperatorType": "SQL_CALC_FOUND_ROWS", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select * from music where 1 != 1", + "Query": "select * from music where user_id = 1 limit 2", + "Table": "music", + "Values": [ + 1 + ], + "Vindex": "user_index" + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select count(*) from music where 1 != 1", + "Query": "select count(*) from music where user_id = 1", + "Table": "music", + "Values": [ + 1 + ], + "Vindex": "user_index" + } + ] + } +} From 227102832774ae606ad536719ec001ecf75191c7 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 9 Sep 2020 20:46:57 +0530 Subject: [PATCH 07/16] removed tree walk to replace metadata Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/select.go | 7 ------- go/vt/vtgate/planbuilder/sql_calc_found_rows.go | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index 3e960b751af..667ed7b8568 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -180,13 +180,6 @@ func buildSQLCalcFoundRowsPlan(sel *sqlparser.Select, outer *symtab, vschema Con sel2.OrderBy = nil sel2.Limit = nil - sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { - switch col := node.(type) { - case *sqlparser.ColName: - col.Metadata = nil - } - return true, nil - }, sel2) cjt := newJointab(sqlparser.GetBindvars(sel2)) countpb := newPrimitiveBuilder(vschema, cjt) err = countpb.processSelect(sel2, outer) diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go index cfa01fc2432..31e06ea9e76 100644 --- a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go +++ b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go @@ -81,7 +81,7 @@ func (s *sqlCalcFoundRows) Wireup(builder, *jointab) error { } func (s *sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { - s.LimitQuery.SupplyVar(from, to, col, varname) + panic("implement me") } func (s *sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (rc *resultColumn, colNumber int) { From 0c8b49d975c7492c47b43131ae5f46cdf9f772c1 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 10 Sep 2020 11:45:43 +0530 Subject: [PATCH 08/16] use original query to parse for count star query plan Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/builder.go | 2 +- go/vt/vtgate/planbuilder/expr.go | 4 +-- go/vt/vtgate/planbuilder/from.go | 2 +- go/vt/vtgate/planbuilder/select.go | 49 ++++++++++++++--------------- go/vt/vtgate/planbuilder/union.go | 2 +- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 5f5dabc26da..c1f12e64dd6 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -305,7 +305,7 @@ func buildRoutePlan(stmt sqlparser.Statement, vschema ContextVSchema, f func(sta func createInstructionFor(query string, stmt sqlparser.Statement, vschema ContextVSchema) (engine.Primitive, error) { switch stmt := stmt.(type) { case *sqlparser.Select: - return buildRoutePlan(stmt, vschema, buildSelectPlan) + return buildRoutePlan(stmt, vschema, buildSelectPlan(query)) case *sqlparser.Insert: return buildRoutePlan(stmt, vschema, buildInsertPlan) case *sqlparser.Update: diff --git a/go/vt/vtgate/planbuilder/expr.go b/go/vt/vtgate/planbuilder/expr.go index c289b073f29..8c86472fc21 100644 --- a/go/vt/vtgate/planbuilder/expr.go +++ b/go/vt/vtgate/planbuilder/expr.go @@ -104,7 +104,7 @@ func (pb *primitiveBuilder) findOrigin(expr sqlparser.Expr) (pullouts []*pullout spb := newPrimitiveBuilder(pb.vschema, pb.jt) switch stmt := node.Select.(type) { case *sqlparser.Select: - if err := spb.processSelect(stmt, pb.st); err != nil { + if err := spb.processSelect(stmt, pb.st, ""); err != nil { return false, err } case *sqlparser.Union: @@ -230,7 +230,7 @@ func (pb *primitiveBuilder) finalizeUnshardedDMLSubqueries(nodes ...sqlparser.SQ return true, nil } spb := newPrimitiveBuilder(pb.vschema, pb.jt) - if err := spb.processSelect(nodeType, pb.st); err != nil { + if err := spb.processSelect(nodeType, pb.st, ""); err != nil { samePlan = false return false, err } diff --git a/go/vt/vtgate/planbuilder/from.go b/go/vt/vtgate/planbuilder/from.go index 467e83ac752..49699be6786 100644 --- a/go/vt/vtgate/planbuilder/from.go +++ b/go/vt/vtgate/planbuilder/from.go @@ -96,7 +96,7 @@ func (pb *primitiveBuilder) processAliasedTable(tableExpr *sqlparser.AliasedTabl spb := newPrimitiveBuilder(pb.vschema, pb.jt) switch stmt := expr.Select.(type) { case *sqlparser.Select: - if err := spb.processSelect(stmt, nil); err != nil { + if err := spb.processSelect(stmt, nil, ""); err != nil { return err } case *sqlparser.Union: diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index 667ed7b8568..d0243bfaece 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -33,26 +33,27 @@ import ( "vitess.io/vitess/go/vt/vtgate/engine" ) -// buildSelectPlan is the new function to build a Select plan. -func buildSelectPlan(stmt sqlparser.Statement, vschema ContextVSchema) (engine.Primitive, error) { - sel := stmt.(*sqlparser.Select) +func buildSelectPlan(query string) func(sqlparser.Statement, ContextVSchema) (engine.Primitive, error) { + return func(stmt sqlparser.Statement, vschema ContextVSchema) (engine.Primitive, error) { + sel := stmt.(*sqlparser.Select) - p, err := handleDualSelects(sel, vschema) - if err != nil { - return nil, err - } - if p != nil { - return p, nil - } + p, err := handleDualSelects(sel, vschema) + if err != nil { + return nil, err + } + if p != nil { + return p, nil + } - pb := newPrimitiveBuilder(vschema, newJointab(sqlparser.GetBindvars(sel))) - if err := pb.processSelect(sel, nil); err != nil { - return nil, err - } - if err := pb.bldr.Wireup(pb.bldr, pb.jt); err != nil { - return nil, err + pb := newPrimitiveBuilder(vschema, newJointab(sqlparser.GetBindvars(sel))) + if err := pb.processSelect(sel, nil, query); err != nil { + return nil, err + } + if err := pb.bldr.Wireup(pb.bldr, pb.jt); err != nil { + return nil, err + } + return pb.bldr.Primitive(), nil } - return pb.bldr.Primitive(), nil } // processSelect builds a primitive tree for the given query or subquery. @@ -90,7 +91,7 @@ func buildSelectPlan(stmt sqlparser.Statement, vschema ContextVSchema) (engine.P // The LIMIT clause is the last construct of a query. If it cannot be // pushed into a route, then a primitive is created on top of any // of the above trees to make it discard unwanted rows. -func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) error { +func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab, query string) error { // Check and error if there is any locking function present in select expression. for _, expr := range sel.SelectExprs { if aExpr, ok := expr.(*sqlparser.AliasedExpr); ok && sqlparser.IsLockingFunc(aExpr.Expr) { @@ -100,7 +101,7 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) if sel.SQLCalcFoundRows { sel.SQLCalcFoundRows = false if sel.Limit != nil { - builder, err := buildSQLCalcFoundRowsPlan(sel, outer, pb.vschema) + builder, err := buildSQLCalcFoundRowsPlan(query, sel, outer, pb.vschema) if err != nil { return err } @@ -155,17 +156,15 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab) return nil } -func buildSQLCalcFoundRowsPlan(sel *sqlparser.Select, outer *symtab, vschema ContextVSchema) (builder, error) { +func buildSQLCalcFoundRowsPlan(query string, sel *sqlparser.Select, outer *symtab, vschema ContextVSchema) (builder, error) { ljt := newJointab(sqlparser.GetBindvars(sel)) frpb := newPrimitiveBuilder(vschema, ljt) - err := frpb.processSelect(sel, outer) + err := frpb.processSelect(sel, outer, "") if err != nil { return nil, err } - // TODO systay this is a hack - s := sqlparser.String(sel) - statement, err := sqlparser.Parse(s) + statement, err := sqlparser.Parse(query) if err != nil { return nil, err } @@ -182,7 +181,7 @@ func buildSQLCalcFoundRowsPlan(sel *sqlparser.Select, outer *symtab, vschema Con cjt := newJointab(sqlparser.GetBindvars(sel2)) countpb := newPrimitiveBuilder(vschema, cjt) - err = countpb.processSelect(sel2, outer) + err = countpb.processSelect(sel2, outer, "") if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/union.go b/go/vt/vtgate/planbuilder/union.go index 42f692cedb6..88408131a2a 100644 --- a/go/vt/vtgate/planbuilder/union.go +++ b/go/vt/vtgate/planbuilder/union.go @@ -92,7 +92,7 @@ func (pb *primitiveBuilder) processPart(part sqlparser.SelectStatement, outer *s return err } } - return pb.processSelect(part, outer) + return pb.processSelect(part, outer, "") case *sqlparser.ParenSelect: err := pb.processPart(part.Select, outer, true) if err != nil { From 749b7275b483e51a0760780b9b62640c94cb670c Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 10 Sep 2020 13:14:03 +0530 Subject: [PATCH 09/16] support for group by and having with sql_calc_found_rows Signed-off-by: Harshit Gangal Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 5 +- go/vt/vtgate/planbuilder/select.go | 27 ++++++++- .../planbuilder/testdata/select_cases.txt | 58 +++++++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index 8f8eb0fdc2a..dd6b888702f 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -34,14 +34,15 @@ func TestFoundRows(t *testing.T) { require.Nil(t, err) defer conn.Close() - exec(t, conn, "insert into t2(id3,id4) values(1,1), (2,2), (3,3), (4,4), (5,5)") + exec(t, conn, "insert into t2(id3,id4) values(1,2), (2,2), (3,3), (4,3), (5,3)") defer exec(t, conn, "delete from t2") assertFoundRowsValue(t, conn, "select * from t2", 5) assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 limit 2", 5) assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id3 = 4 limit 2", 1) - assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 4 limit 2", 1) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 3 limit 2", 3) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS id4, count(id3) from t2 where id3 = 3 group by id4 limit 1", 1) } func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query string, count int) { diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index d0243bfaece..24bd8dc4e08 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -20,6 +20,8 @@ import ( "errors" "fmt" + "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" @@ -99,6 +101,9 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab, } } if sel.SQLCalcFoundRows { + if outer != nil || query == "" { + return mysql.NewSQLError(mysql.ERCantUseOptionHere, "42000", "Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'") + } sel.SQLCalcFoundRows = false if sel.Limit != nil { builder, err := buildSQLCalcFoundRowsPlan(query, sel, outer, pb.vschema) @@ -170,14 +175,30 @@ func buildSQLCalcFoundRowsPlan(query string, sel *sqlparser.Select, outer *symta } sel2 := statement.(*sqlparser.Select) - sel2.SelectExprs = []sqlparser.SelectExpr{&sqlparser.AliasedExpr{ + sel2.SQLCalcFoundRows = false + sel2.OrderBy = nil + sel2.Limit = nil + + countStartExpr := []sqlparser.SelectExpr{&sqlparser.AliasedExpr{ Expr: &sqlparser.FuncExpr{ Name: sqlparser.NewColIdent("count"), Exprs: []sqlparser.SelectExpr{&sqlparser.StarExpr{}}, }, }} - sel2.OrderBy = nil - sel2.Limit = nil + if sel2.GroupBy == nil && sel2.Having == nil { + sel2.SelectExprs = countStartExpr + } else { + sel3 := &sqlparser.Select{ + SelectExprs: countStartExpr, + From: []sqlparser.TableExpr{ + &sqlparser.AliasedTableExpr{ + Expr: &sqlparser.Subquery{Select: sel2}, + As: sqlparser.NewTableIdent("t"), + }, + }, + } + sel2 = sel3 + } cjt := newJointab(sqlparser.GetBindvars(sel2)) countpb := newPrimitiveBuilder(vschema, cjt) diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index af81191ac83..95b70a3ef3d 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1499,3 +1499,61 @@ ] } } + +# sql_calc_found_rows with group by and having +"select sql_calc_found_rows user_id, count(id) from music group by user_id having count(user_id) = 1 order by user_id limit 2" +{ + "QueryType": "SELECT", + "Original": "select sql_calc_found_rows user_id, count(id) from music group by user_id having count(user_id) = 1 order by user_id limit 2", + "Instructions": { + "OperatorType": "SQL_CALC_FOUND_ROWS", + "Inputs": [ + { + "OperatorType": "Limit", + "Count": 2, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select user_id, count(id) from music where 1 != 1 group by user_id", + "Query": "select user_id, count(id) from music group by user_id having count(user_id) = 1 order by user_id asc limit :__upper_limit", + "Table": "music" + } + ] + }, + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(0)", + "Distinct": "false", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select count(*) from (select user_id, count(id) from music where 1 != 1 group by user_id) as t where 1 != 1", + "Query": "select count(*) from (select user_id, count(id) from music group by user_id having count(user_id) = 1) as t", + "Table": "music" + } + ] + } + ] + } +} + +# sql_calc_found_rows in sub queries +"select * from music where user_id IN (select sql_calc_found_rows * from music limit 10)" +"Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS' (errno 1234) (sqlstate 42000)" + +# sql_calc_found_rows in derived table +"select sql_calc_found_rows * from (select sql_calc_found_rows * from music limit 10) t limit 1" +"Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS' (errno 1234) (sqlstate 42000)" + + From 44f9de5e8787c91c54eda23c31737e4a11bd7583 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 10 Sep 2020 10:18:10 +0200 Subject: [PATCH 10/16] StreamExecute and cleanup Signed-off-by: Andres Taylor --- go/vt/vtgate/engine/sql_calc_found_rows.go | 17 ++- go/vt/vtgate/planbuilder/select.go | 5 + .../vtgate/planbuilder/sql_calc_found_rows.go | 103 +++++++++++------- go/vt/vtgate/safe_session.go | 11 +- 4 files changed, 90 insertions(+), 46 deletions(-) diff --git a/go/vt/vtgate/engine/sql_calc_found_rows.go b/go/vt/vtgate/engine/sql_calc_found_rows.go index 5a07d0eb357..dc3136d4946 100644 --- a/go/vt/vtgate/engine/sql_calc_found_rows.go +++ b/go/vt/vtgate/engine/sql_calc_found_rows.go @@ -70,7 +70,22 @@ func (s SQLCalcFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb. //StreamExecute implements the Primitive interface func (s SQLCalcFoundRows) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { - panic("implement me") + err := s.LimitPrimitive.StreamExecute(vcursor, bindVars, wantfields, callback) + if err != nil { + return err + } + + return s.CountPrimitive.StreamExecute(vcursor, bindVars, wantfields, func(countQr *sqltypes.Result) error { + if len(countQr.Rows) != 1 || len(countQr.Rows[0]) != 1 { + return vterrors.Errorf(vtrpc.Code_INTERNAL, "count query is not a scalar") + } + fr, err := evalengine.ToUint64(countQr.Rows[0][0]) + if err != nil { + return err + } + vcursor.Session().SetFoundRows(fr) + return nil + }) } //GetFields implements the Primitive interface diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index 24bd8dc4e08..90ff1c57068 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -186,8 +186,13 @@ func buildSQLCalcFoundRowsPlan(query string, sel *sqlparser.Select, outer *symta }, }} if sel2.GroupBy == nil && sel2.Having == nil { + // if there is no grouping, we can use the same query and + // just replace the SELECT sub-clause to have a single count(*) sel2.SelectExprs = countStartExpr } else { + // when there is grouping, we have to move the original query into a derived table. + // select id, sum(12) from user group by id => + // select count(*) from (select id, sum(12) from user group by id) t sel3 := &sqlparser.Select{ SelectExprs: countStartExpr, From: []sqlparser.TableExpr{ diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go index 31e06ea9e76..af566272379 100644 --- a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go +++ b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go @@ -17,7 +17,9 @@ limitations under the License. package planbuilder import ( + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" ) @@ -28,77 +30,96 @@ type sqlCalcFoundRows struct { ljt, cjt *jointab } -func (s *sqlCalcFoundRows) Order() int { - panic("implement me") +//Wireup implements the builder interface +func (s *sqlCalcFoundRows) Wireup(builder, *jointab) error { + err := s.LimitQuery.Wireup(s.LimitQuery, s.ljt) + if err != nil { + return err + } + return s.CountQuery.Wireup(s.CountQuery, s.cjt) } -func (s *sqlCalcFoundRows) ResultColumns() []*resultColumn { - panic("implement me") +//Primitive implements the builder interface +func (s *sqlCalcFoundRows) Primitive() engine.Primitive { + return engine.SQLCalcFoundRows{ + LimitPrimitive: s.LimitQuery.Primitive(), + CountPrimitive: s.CountQuery.Primitive(), + } } -func (s *sqlCalcFoundRows) Reorder(i int) { - panic("implement me") +// All the methods below are not implemented. They should not be called on a sqlCalcFoundRows builder + +//Order implements the builder interface +func (s *sqlCalcFoundRows) Order() int { + panic("unreachable") } -func (s *sqlCalcFoundRows) First() builder { - panic("implement me") +//ResultColumns implements the builder interface +func (s *sqlCalcFoundRows) ResultColumns() []*resultColumn { + return s.LimitQuery.ResultColumns() } -func (s *sqlCalcFoundRows) PushFilter(pb *primitiveBuilder, filter sqlparser.Expr, whereType string, origin builder) error { - panic("implement me") +//Reorder implements the builder interface +func (s *sqlCalcFoundRows) Reorder(int) { + panic("unreachable") } -func (s *sqlCalcFoundRows) PushSelect(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, origin builder) (rc *resultColumn, colNumber int, err error) { - panic("implement me") +//First implements the builder interface +func (s *sqlCalcFoundRows) First() builder { + panic("unreachable") } -func (s *sqlCalcFoundRows) MakeDistinct() error { - panic("implement me") +//PushFilter implements the builder interface +func (s *sqlCalcFoundRows) PushFilter(*primitiveBuilder, sqlparser.Expr, string, builder) error { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.PushFilter") } -func (s *sqlCalcFoundRows) PushGroupBy(by sqlparser.GroupBy) error { - panic("implement me") +//PushSelect implements the builder interface +func (s *sqlCalcFoundRows) PushSelect(*primitiveBuilder, *sqlparser.AliasedExpr, builder) (rc *resultColumn, colNumber int, err error) { + return nil, 0, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.PushSelect") } -func (s *sqlCalcFoundRows) PushOrderBy(by sqlparser.OrderBy) (builder, error) { - panic("implement me") +//MakeDistinct implements the builder interface +func (s *sqlCalcFoundRows) MakeDistinct() error { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.MakeDistinct") } -func (s *sqlCalcFoundRows) SetUpperLimit(count sqlparser.Expr) { - panic("implement me") +//PushGroupBy implements the builder interface +func (s *sqlCalcFoundRows) PushGroupBy(sqlparser.GroupBy) error { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.PushGroupBy") } -func (s *sqlCalcFoundRows) PushMisc(sel *sqlparser.Select) { - panic("implement me") +//PushOrderBy implements the builder interface +func (s *sqlCalcFoundRows) PushOrderBy(sqlparser.OrderBy) (builder, error) { + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.PushOrderBy") } -func (s *sqlCalcFoundRows) Wireup(builder, *jointab) error { - err := s.LimitQuery.Wireup(s.LimitQuery, s.ljt) - if err != nil { - return err - } - return s.CountQuery.Wireup(s.CountQuery, s.cjt) +//SetUpperLimit implements the builder interface +func (s *sqlCalcFoundRows) SetUpperLimit(sqlparser.Expr) { + panic("unreachable") } -func (s *sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { - panic("implement me") +//PushMisc implements the builder interface +func (s *sqlCalcFoundRows) PushMisc(*sqlparser.Select) { + panic("unreachable") } -func (s *sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (rc *resultColumn, colNumber int) { - panic("implement me") +//SupplyVar implements the builder interface +func (s *sqlCalcFoundRows) SupplyVar(int, int, *sqlparser.ColName, string) { + panic("unreachable") } -func (s *sqlCalcFoundRows) SupplyWeightString(colNumber int) (weightcolNumber int, err error) { - panic("implement me") +//SupplyCol implements the builder interface +func (s *sqlCalcFoundRows) SupplyCol(*sqlparser.ColName) (rc *resultColumn, colNumber int) { + panic("unreachable") } -func (s *sqlCalcFoundRows) PushLock(lock string) error { - panic("implement me") +//SupplyWeightString implements the builder interface +func (s *sqlCalcFoundRows) SupplyWeightString(int) (weightcolNumber int, err error) { + return 0, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.SupplyWeightString") } -func (s *sqlCalcFoundRows) Primitive() engine.Primitive { - return engine.SQLCalcFoundRows{ - LimitPrimitive: s.LimitQuery.Primitive(), - CountPrimitive: s.CountQuery.Primitive(), - } +//PushLock implements the builder interface +func (s *sqlCalcFoundRows) PushLock(string) error { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unreachable: sqlCalcFoundRows.PushLock") } diff --git a/go/vt/vtgate/safe_session.go b/go/vt/vtgate/safe_session.go index 1ccd0fdb020..2d4259b34ae 100644 --- a/go/vt/vtgate/safe_session.go +++ b/go/vt/vtgate/safe_session.go @@ -34,10 +34,13 @@ import ( // (the use pattern is 'Find', if not found, then 'AppendOrUpdate', // for a single shard) type SafeSession struct { - mu sync.Mutex - mustRollback bool - autocommitState autocommitState - commitOrder vtgatepb.CommitOrder + mu sync.Mutex + mustRollback bool + autocommitState autocommitState + commitOrder vtgatepb.CommitOrder + + // this is a signal that found_rows has already been handles by the primitives, + // and doesn't have to be updated by the executor foundRowsHandled bool *vtgatepb.Session } From c31e8fa5ffb3e6342d5abb0f08e2633630a54390 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 10 Sep 2020 10:49:33 +0200 Subject: [PATCH 11/16] disallow sql_calc_found_rows with union Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/testdata/union_cases.txt | 1 - go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt | 4 ++++ go/vt/vtgate/planbuilder/union.go | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/testdata/union_cases.txt b/go/vt/vtgate/planbuilder/testdata/union_cases.txt index 8a88dc48f0c..0af47b4cae2 100644 --- a/go/vt/vtgate/planbuilder/testdata/union_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/union_cases.txt @@ -279,4 +279,3 @@ ] } } - diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index 7dd4584e1f5..f2b9a155564 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -423,3 +423,7 @@ # insert using select get_lock from table "insert into user(pattern) SELECT GET_LOCK('xyz1', 10)" "unsupported: insert into select" + +# union with SQL_CALC_FOUND_ROWS +"(select sql_calc_found_rows id from user where id = 1 limit 1) union select id from user where id = 1" +"SQL_CALC_FOUND_ROWS not supported with union" diff --git a/go/vt/vtgate/planbuilder/union.go b/go/vt/vtgate/planbuilder/union.go index 88408131a2a..e96178f9331 100644 --- a/go/vt/vtgate/planbuilder/union.go +++ b/go/vt/vtgate/planbuilder/union.go @@ -20,6 +20,9 @@ import ( "errors" "fmt" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/vt/sqlparser" @@ -86,6 +89,9 @@ func (pb *primitiveBuilder) processPart(part sqlparser.SelectStatement, outer *s case *sqlparser.Union: return pb.processUnion(part, outer) case *sqlparser.Select: + if part.SQLCalcFoundRows { + return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "SQL_CALC_FOUND_ROWS not supported with union") + } if !hasParens { err := checkOrderByAndLimit(part) if err != nil { From 7c5cf3d269e5d2173832eeaeabae5961bff94425 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 10 Sep 2020 11:07:50 +0200 Subject: [PATCH 12/16] clean out test data so we don't interfer with other tests Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 6 +++++- go/test/endtoend/vtgate/lookup_test.go | 20 ++++---------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index dd6b888702f..47b45720a5f 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -35,7 +35,7 @@ func TestFoundRows(t *testing.T) { defer conn.Close() exec(t, conn, "insert into t2(id3,id4) values(1,2), (2,2), (3,3), (4,3), (5,3)") - defer exec(t, conn, "delete from t2") + assertMatches(t, conn, "select * from t2_id4_idx", "") assertFoundRowsValue(t, conn, "select * from t2", 5) assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) @@ -43,6 +43,10 @@ func TestFoundRows(t *testing.T) { assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id3 = 4 limit 2", 1) assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 3 limit 2", 3) assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS id4, count(id3) from t2 where id3 = 3 group by id4 limit 1", 1) + + // cleanup test data + exec(t, conn, "delete from t2") + exec(t, conn, "delete from t2_id4_idx") // TODO systay do we really need to do this manually? } func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query string, count int) { diff --git a/go/test/endtoend/vtgate/lookup_test.go b/go/test/endtoend/vtgate/lookup_test.go index 29829410d38..636be7f2d0a 100644 --- a/go/test/endtoend/vtgate/lookup_test.go +++ b/go/test/endtoend/vtgate/lookup_test.go @@ -410,14 +410,8 @@ func TestHashLookupMultiInsertIgnore(t *testing.T) { defer conn2.Close() // DB should start out clean - qr := exec(t, conn, "select count(*) from t2_id4_idx") - if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(0)]]"; got != want { - t.Errorf("select:\n%v want\n%v", got, want) - } - qr = exec(t, conn, "select count(*) from t2") - if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(0)]]"; got != want { - t.Errorf("select:\n%v want\n%v", got, want) - } + assertMatches(t, conn, "select count(*) from t2_id4_idx", "[[INT64(0)]]") + assertMatches(t, conn, "select count(*) from t2", "[[INT64(0)]]") // Try inserting a bunch of ids at once exec(t, conn, "begin") @@ -425,14 +419,8 @@ func TestHashLookupMultiInsertIgnore(t *testing.T) { exec(t, conn, "commit") // Verify - qr = exec(t, conn, "select id3, id4 from t2 order by id3") - if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(10) INT64(20)] [INT64(30) INT64(40)] [INT64(50) INT64(60)]]"; got != want { - t.Errorf("select:\n%v want\n%v", got, want) - } - qr = exec(t, conn, "select id3, id4 from t2_id4_idx order by id3") - if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(10) INT64(20)] [INT64(30) INT64(40)] [INT64(50) INT64(60)]]"; got != want { - t.Errorf("select:\n%v want\n%v", got, want) - } + assertMatches(t, conn, "select id3, id4 from t2 order by id3", "[[INT64(10) INT64(20)] [INT64(30) INT64(40)] [INT64(50) INT64(60)]]") + assertMatches(t, conn, "select id3, id4 from t2_id4_idx order by id3", "[[INT64(10) INT64(20)] [INT64(30) INT64(40)] [INT64(50) INT64(60)]]") } func TestConsistentLookupUpdate(t *testing.T) { From 723fef02ef8a874f54f4e5139b9141704d626576 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 10 Sep 2020 17:28:17 +0530 Subject: [PATCH 13/16] removed vindex check Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/found_rows_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index 47b45720a5f..dcf82f90be7 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -35,7 +35,6 @@ func TestFoundRows(t *testing.T) { defer conn.Close() exec(t, conn, "insert into t2(id3,id4) values(1,2), (2,2), (3,3), (4,3), (5,3)") - assertMatches(t, conn, "select * from t2_id4_idx", "") assertFoundRowsValue(t, conn, "select * from t2", 5) assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) From 9f2a0b6fb73547f7b7902017de3fece1eaccb530 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 11 Sep 2020 14:48:58 +0200 Subject: [PATCH 14/16] added end to end test for StreamExecute Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 26 ++++++++++++------- .../vtgate/planbuilder/sql_calc_found_rows.go | 24 ++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index dcf82f90be7..464177dbf4a 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -21,6 +21,8 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "vitess.io/vitess/go/mysql" @@ -36,22 +38,28 @@ func TestFoundRows(t *testing.T) { exec(t, conn, "insert into t2(id3,id4) values(1,2), (2,2), (3,3), (4,3), (5,3)") - assertFoundRowsValue(t, conn, "select * from t2", 5) - assertFoundRowsValue(t, conn, "select * from t2 limit 2", 2) - assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 limit 2", 5) - assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id3 = 4 limit 2", 1) - assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 3 limit 2", 3) - assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS id4, count(id3) from t2 where id3 = 3 group by id4 limit 1", 1) + runTests := func(workload string) { + assertFoundRowsValue(t, conn, "select * from t2", workload, 5) + assertFoundRowsValue(t, conn, "select * from t2 limit 2", workload, 2) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 limit 2", workload, 5) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id3 = 4 limit 2", workload, 1) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS * from t2 where id4 = 3 limit 2", workload, 3) + assertFoundRowsValue(t, conn, "select SQL_CALC_FOUND_ROWS id4, count(id3) from t2 where id3 = 3 group by id4 limit 1", workload, 1) + } + + runTests("oltp") + exec(t, conn, "set workload = olap") + runTests("olap") // cleanup test data exec(t, conn, "delete from t2") - exec(t, conn, "delete from t2_id4_idx") // TODO systay do we really need to do this manually? + exec(t, conn, "delete from t2_id4_idx") } -func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query string, count int) { +func assertFoundRowsValue(t *testing.T, conn *mysql.Conn, query, workload string, count int) { exec(t, conn, query) qr := exec(t, conn, "select found_rows()") got := fmt.Sprintf("%v", qr.Rows) want := fmt.Sprintf(`[[UINT64(%d)]]`, count) - require.Equal(t, want, got) + assert.Equalf(t, want, got, "Workload: %s\nQuery:%s\n", workload, query) } diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go index af566272379..07dff25cf15 100644 --- a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go +++ b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go @@ -51,7 +51,7 @@ func (s *sqlCalcFoundRows) Primitive() engine.Primitive { //Order implements the builder interface func (s *sqlCalcFoundRows) Order() int { - panic("unreachable") + return s.LimitQuery.Order() } //ResultColumns implements the builder interface @@ -60,13 +60,13 @@ func (s *sqlCalcFoundRows) ResultColumns() []*resultColumn { } //Reorder implements the builder interface -func (s *sqlCalcFoundRows) Reorder(int) { - panic("unreachable") +func (s *sqlCalcFoundRows) Reorder(order int) { + s.LimitQuery.Reorder(order) } //First implements the builder interface func (s *sqlCalcFoundRows) First() builder { - panic("unreachable") + return s.LimitQuery.First() } //PushFilter implements the builder interface @@ -95,23 +95,23 @@ func (s *sqlCalcFoundRows) PushOrderBy(sqlparser.OrderBy) (builder, error) { } //SetUpperLimit implements the builder interface -func (s *sqlCalcFoundRows) SetUpperLimit(sqlparser.Expr) { - panic("unreachable") +func (s *sqlCalcFoundRows) SetUpperLimit(count sqlparser.Expr) { + s.LimitQuery.SetUpperLimit(count) } //PushMisc implements the builder interface -func (s *sqlCalcFoundRows) PushMisc(*sqlparser.Select) { - panic("unreachable") +func (s *sqlCalcFoundRows) PushMisc(sel *sqlparser.Select) { + s.LimitQuery.PushMisc(sel) } //SupplyVar implements the builder interface -func (s *sqlCalcFoundRows) SupplyVar(int, int, *sqlparser.ColName, string) { - panic("unreachable") +func (s *sqlCalcFoundRows) SupplyVar(from, to int, col *sqlparser.ColName, varname string) { + s.LimitQuery.SupplyVar(from, to, col, varname) } //SupplyCol implements the builder interface -func (s *sqlCalcFoundRows) SupplyCol(*sqlparser.ColName) (rc *resultColumn, colNumber int) { - panic("unreachable") +func (s *sqlCalcFoundRows) SupplyCol(col *sqlparser.ColName) (*resultColumn, int) { + return s.LimitQuery.SupplyCol(col) } //SupplyWeightString implements the builder interface From 766324882442f37bc9f383af39824f111b86f8fe Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 14 Sep 2020 08:44:44 +0200 Subject: [PATCH 15/16] fix stream-execute bug in engine Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/found_rows_test.go | 1 + go/vt/vtgate/engine/sql_calc_found_rows.go | 26 +++++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/go/test/endtoend/vtgate/found_rows_test.go b/go/test/endtoend/vtgate/found_rows_test.go index 464177dbf4a..0f7143a3b28 100644 --- a/go/test/endtoend/vtgate/found_rows_test.go +++ b/go/test/endtoend/vtgate/found_rows_test.go @@ -52,6 +52,7 @@ func TestFoundRows(t *testing.T) { runTests("olap") // cleanup test data + exec(t, conn, "set workload = oltp") exec(t, conn, "delete from t2") exec(t, conn, "delete from t2_id4_idx") } diff --git a/go/vt/vtgate/engine/sql_calc_found_rows.go b/go/vt/vtgate/engine/sql_calc_found_rows.go index dc3136d4946..65c3f7574dc 100644 --- a/go/vt/vtgate/engine/sql_calc_found_rows.go +++ b/go/vt/vtgate/engine/sql_calc_found_rows.go @@ -19,7 +19,7 @@ package engine import ( "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" - "vitess.io/vitess/go/vt/proto/vtrpc" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/evalengine" ) @@ -58,7 +58,7 @@ func (s SQLCalcFoundRows) Execute(vcursor VCursor, bindVars map[string]*querypb. return nil, err } if len(countQr.Rows) != 1 || len(countQr.Rows[0]) != 1 { - return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "count query is not a scalar") + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "count query is not a scalar") } fr, err := evalengine.ToUint64(countQr.Rows[0][0]) if err != nil { @@ -75,17 +75,31 @@ func (s SQLCalcFoundRows) StreamExecute(vcursor VCursor, bindVars map[string]*qu return err } - return s.CountPrimitive.StreamExecute(vcursor, bindVars, wantfields, func(countQr *sqltypes.Result) error { + var fr *uint64 + + err = s.CountPrimitive.StreamExecute(vcursor, bindVars, wantfields, func(countQr *sqltypes.Result) error { + if len(countQr.Rows) == 0 && countQr.Fields != nil { + // this is the fields, which we can ignore + return nil + } if len(countQr.Rows) != 1 || len(countQr.Rows[0]) != 1 { - return vterrors.Errorf(vtrpc.Code_INTERNAL, "count query is not a scalar") + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "count query is not a scalar") } - fr, err := evalengine.ToUint64(countQr.Rows[0][0]) + toUint64, err := evalengine.ToUint64(countQr.Rows[0][0]) if err != nil { return err } - vcursor.Session().SetFoundRows(fr) + fr = &toUint64 return nil }) + if err != nil { + return err + } + if fr == nil { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "count query for SQL_CALC_FOUND_ROWS never returned a value") + } + vcursor.Session().SetFoundRows(*fr) + return nil } //GetFields implements the Primitive interface From 7fc97623ac2fdca8a20cbff45d2d200234ea2082 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 14 Sep 2020 09:24:32 +0200 Subject: [PATCH 16/16] make sure to set session state when running in OLAP mode Signed-off-by: Andres Taylor --- go/vt/vtgate/executor.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 7f1dae2b025..b5e81ec4b93 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -999,6 +999,7 @@ func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession result := &sqltypes.Result{} byteCount := 0 seenResults := false + var foundRows uint64 err = plan.Instructions.StreamExecute(vcursor, bindVars, true, func(qr *sqltypes.Result) error { // If the row has field info, send it separately. // TODO(sougou): this behavior is for handling tests because @@ -1011,8 +1012,10 @@ func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession seenResults = true } + foundRows += uint64(len(qr.Rows)) for _, row := range qr.Rows { result.Rows = append(result.Rows, row) + for _, col := range row { byteCount += col.Len() } @@ -1040,6 +1043,12 @@ func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession logStats.ExecuteTime = time.Since(execStart) e.updateQueryCounts(plan.Instructions.RouteType(), plan.Instructions.GetKeyspaceName(), plan.Instructions.GetTableName(), int64(logStats.ShardQueries)) + // save session stats for future queries + if !safeSession.foundRowsHandled { + safeSession.FoundRows = foundRows + } + safeSession.RowCount = -1 + return err }