From 1481e85129a1732fefd93e383f08257fabbe345b Mon Sep 17 00:00:00 2001 From: hanfei Date: Fri, 28 Oct 2016 18:25:36 +0800 Subject: [PATCH 1/3] *: add a flag to restrict cartesian product. --- executor/executor_test.go | 10 ++++++++++ plan/optimizer.go | 28 ++++++++++++++++++++++------ tidb-server/main.go | 4 ++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 8c6d832a9a..94a17a2853 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -31,6 +31,7 @@ import ( "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/model" "github.com/pingcap/tidb/parser" + "github.com/pingcap/tidb/plan" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/util/testkit" @@ -599,6 +600,15 @@ func (s *testSuite) TestJoin(c *C) { result = tk.MustQuery("select a.c1 from t a , t1 b where a.c1 = b.c1 order by a.c1;") result.Check(testkit.Rows("1", "2", "3", "4", "5", "6", "7")) + plan.AllowCartesianProduct = false + _, err := tk.Exec("select * from t, t1") + c.Check(err, NotNil) + _, err = tk.Exec("select * from t left join t1") + c.Check(err, NotNil) + _, err = tk.Exec("select * from t right join t1") + c.Check(err, NotNil) + plan.AllowCartesianProduct = true + } func (s *testSuite) TestMultiJoin(c *C) { diff --git a/plan/optimizer.go b/plan/optimizer.go index 9576d36284..fa81801117 100644 --- a/plan/optimizer.go +++ b/plan/optimizer.go @@ -23,6 +23,8 @@ import ( "github.com/pingcap/tidb/terror" ) +var AllowCartesianProduct = true + // Optimize does optimization and creates a Plan. // The node must be prepared first. func Optimize(ctx context.Context, node ast.Node, is infoschema.InfoSchema) (Plan, error) { @@ -49,6 +51,9 @@ func Optimize(ctx context.Context, node ast.Node, is infoschema.InfoSchema) (Pla if err != nil { return nil, errors.Trace(err) } + if !AllowCartesianProduct && existsCartesianProduct(logic) { + return nil, ErrCartesianProductUnsupported + } logic = EliminateProjection(logic) info, err := logic.convert2PhysicalPlan(&requiredProperty{}) if err != nil { @@ -60,6 +65,17 @@ func Optimize(ctx context.Context, node ast.Node, is infoschema.InfoSchema) (Pla } return p, nil } +func existsCartesianProduct(p LogicalPlan) bool { + if join, ok := p.(*Join); ok && len(join.EqualConditions) == 0 { + return join.JoinType == InnerJoin || join.JoinType == LeftOuterJoin || join.JoinType == RightOuterJoin + } + for _, child := range p.GetChildren() { + if existsCartesianProduct(child.(LogicalPlan)) { + return true + } + } + return false +} // PrepareStmt prepares a raw statement parsed from parser. // The statement must be prepared before it can be passed to optimize function. @@ -86,12 +102,12 @@ const ( // Optimizer base errors. var ( - ErrOneColumn = terror.ClassOptimizer.New(CodeOneColumn, "Operand should contain 1 column(s)") - ErrSameColumns = terror.ClassOptimizer.New(CodeSameColumns, "Operands should contain same columns") - ErrMultiWildCard = terror.ClassOptimizer.New(CodeMultiWildCard, "wildcard field exist more than once") - ErrUnSupported = terror.ClassOptimizer.New(CodeUnsupported, "unsupported") - ErrInvalidGroupFuncUse = terror.ClassOptimizer.New(CodeInvalidGroupFuncUse, "Invalid use of group function") - ErrIllegalReference = terror.ClassOptimizer.New(CodeIllegalReference, "Illegal reference") + ErrOneColumn = terror.ClassOptimizer.New(CodeOneColumn, "Operand should contain 1 column(s)") + ErrSameColumns = terror.ClassOptimizer.New(CodeSameColumns, "Operands should contain same columns") + ErrMultiWildCard = terror.ClassOptimizer.New(CodeMultiWildCard, "wildcard field exist more than once") + ErrCartesianProductUnsupported = terror.ClassOptimizer.New(CodeUnsupported, "Cartesian product is unsupported") + ErrInvalidGroupFuncUse = terror.ClassOptimizer.New(CodeInvalidGroupFuncUse, "Invalid use of group function") + ErrIllegalReference = terror.ClassOptimizer.New(CodeIllegalReference, "Illegal reference") ) func init() { diff --git a/tidb-server/main.go b/tidb-server/main.go index c85eff664e..4a8bd3d230 100644 --- a/tidb-server/main.go +++ b/tidb-server/main.go @@ -56,6 +56,7 @@ var ( reportStatus = flag.Bool("report-status", true, "If enable status report HTTP service.") logFile = flag.String("log-file", "", "log file path") joinCon = flag.Int("join-concurrency", 5, "the number of goroutines that participate joining.") + crossJoin = flag.Bool("cross-join", true, "whether support cartesian product or not.") metricsAddr = flag.String("metrics-addr", "", "prometheus pushgateway address, leaves it empty will disable prometheus push.") metricsInterval = flag.Int("metrics-interval", 15, "prometheus client push interval in second, set \"0\" to disable prometheus push.") binlogSocket = flag.String("binlog-socket", "", "socket file to write binlog") @@ -92,6 +93,9 @@ func main() { if joinCon != nil && *joinCon > 0 { plan.JoinConcurrency = *joinCon } + if crossJoin != nil { + plan.AllowCartesianProduct = *crossJoin + } // Call this before setting log level to make sure that TiDB info could be printed. printer.PrintTiDBInfo() log.SetLevelByString(cfg.LogLevel) From 1fad7632f5c2604d5812724a238521e0f06e676b Mon Sep 17 00:00:00 2001 From: hanfei Date: Fri, 28 Oct 2016 18:49:46 +0800 Subject: [PATCH 2/3] fix ci --- plan/optimizer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/plan/optimizer.go b/plan/optimizer.go index fa81801117..7ad14d5455 100644 --- a/plan/optimizer.go +++ b/plan/optimizer.go @@ -23,6 +23,7 @@ import ( "github.com/pingcap/tidb/terror" ) +// AllowCartesianProduct means whether tidb allows cartesian join without equal conditions. var AllowCartesianProduct = true // Optimize does optimization and creates a Plan. From 3fc9ae6bf5d632a5a94db6ef984d4e820eb0b227 Mon Sep 17 00:00:00 2001 From: hanfei Date: Fri, 28 Oct 2016 18:55:30 +0800 Subject: [PATCH 3/3] remove useless check --- executor/executor_test.go | 10 +++++----- tidb-server/main.go | 4 +--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 94a17a2853..613fece0d6 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -602,11 +602,11 @@ func (s *testSuite) TestJoin(c *C) { plan.AllowCartesianProduct = false _, err := tk.Exec("select * from t, t1") - c.Check(err, NotNil) - _, err = tk.Exec("select * from t left join t1") - c.Check(err, NotNil) - _, err = tk.Exec("select * from t right join t1") - c.Check(err, NotNil) + c.Check(plan.ErrCartesianProductUnsupported.Equal(err), IsTrue) + _, err = tk.Exec("select * from t left join t1 on 1") + c.Check(plan.ErrCartesianProductUnsupported.Equal(err), IsTrue) + _, err = tk.Exec("select * from t right join t1 on 1") + c.Check(plan.ErrCartesianProductUnsupported.Equal(err), IsTrue) plan.AllowCartesianProduct = true } diff --git a/tidb-server/main.go b/tidb-server/main.go index 4a8bd3d230..04e8379090 100644 --- a/tidb-server/main.go +++ b/tidb-server/main.go @@ -93,9 +93,7 @@ func main() { if joinCon != nil && *joinCon > 0 { plan.JoinConcurrency = *joinCon } - if crossJoin != nil { - plan.AllowCartesianProduct = *crossJoin - } + plan.AllowCartesianProduct = *crossJoin // Call this before setting log level to make sure that TiDB info could be printed. printer.PrintTiDBInfo() log.SetLevelByString(cfg.LogLevel)