Merge pull request #211 from pingcap/coocood/clone-cleanup

expression: remove return error in Clone interface
pingcap · Sep 21, 2015 · 6b7c3bf · 6b7c3bf
2 parents 5c1320f + ded2295
commit 6b7c3bf
Show file tree

Hide file tree

Showing 54 changed files with 140 additions and 298 deletions.
diff --git a/expression/expression.go b/expression/expression.go
@@ -25,7 +25,7 @@ import (
 // See https://dev.mysql.com/doc/refman/5.7/en/expressions.html
 type Expression interface {
 	// Clone clones another Expression.
-	Clone() (Expression, error)
+	Clone() Expression
 	// Eval evaluates expression.
 	Eval(ctx context.Context, args map[interface{}]interface{}) (v interface{}, err error)
 	// IsStatic returns whether this expression can be evaluated statically or not.

diff --git a/expression/expressions/between.go b/expression/expressions/between.go
@@ -41,23 +41,11 @@ type Between struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (b *Between) Clone() (expression.Expression, error) {
-	expr, err := b.Expr.Clone()
-	if err != nil {
-		return nil, err
-	}
-
-	left, err := b.Left.Clone()
-	if err != nil {
-		return nil, err
-	}
-
-	right, err := b.Right.Clone()
-	if err != nil {
-		return nil, err
-	}
-
-	return &Between{Expr: expr, Left: left, Right: right, Not: b.Not}, nil
+func (b *Between) Clone() expression.Expression {
+	expr := b.Expr.Clone()
+	left := b.Left.Clone()
+	right := b.Right.Clone()
+	return &Between{Expr: expr, Left: left, Right: right, Not: b.Not}
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/between_test.go b/expression/expressions/between_test.go
@@ -54,8 +54,7 @@ func (s *testBetweenSuite) TestBetween(c *C) {
 		b, err := NewBetween(t.Expr, Value{t.Left}, Value{t.Right}, t.Not)
 		c.Assert(err, IsNil)
 		c.Assert(len(b.String()), Greater, 0)
-		_, err = b.Clone()
-		c.Assert(err, IsNil)
+		c.Assert(b.Clone(), NotNil)
 		c.Assert(b.IsStatic(), Equals, !ContainAggregateFunc(t.Expr))
 
 		v, err := b.Eval(nil, m)
@@ -87,9 +86,7 @@ func (s *testBetweenSuite) TestBetween(c *C) {
 		c.Assert(err, NotNil)
 
 		b := Between{Expr: t.Expr, Left: t.Left, Right: t.Right, Not: false}
-		_, err = b.Clone()
-		c.Assert(err, NotNil)
-
+		c.Assert(b.Clone(), NotNil)
 		_, err = b.Eval(nil, nil)
 		c.Assert(err, NotNil)
 	}

diff --git a/expression/expressions/binop.go b/expression/expressions/binop.go
@@ -107,18 +107,10 @@ func (o *BinaryOperation) IsIdentRelOpVal() (bool, string, interface{}, error) {
 }
 
 // Clone implements the Expression Clone interface.
-func (o *BinaryOperation) Clone() (expression.Expression, error) {
-	l, err := o.L.Clone()
-	if err != nil {
-		return nil, err
-	}
-
-	r, err := o.R.Clone()
-	if err != nil {
-		return nil, err
-	}
-
-	return NewBinaryOperation(o.Op, l, r), nil
+func (o *BinaryOperation) Clone() expression.Expression {
+	l := o.L.Clone()
+	r := o.R.Clone()
+	return NewBinaryOperation(o.Op, l, r)
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/binop_test.go b/expression/expressions/binop_test.go
@@ -128,16 +128,13 @@ func (s *testBinOpSuite) TestComparisonOp(c *C) {
 		expr := NewBinaryOperation(t.op, Value{t.arg}, mock)
 		_, err := expr.Eval(nil, nil)
 		c.Assert(err, NotNil)
-
-		_, err = expr.Clone()
-		c.Assert(err, NotNil)
+		c.Assert(expr.Clone(), NotNil)
 
 		expr = NewBinaryOperation(t.op, mock, Value{t.arg})
 		_, err = expr.Eval(nil, nil)
 		c.Assert(err, NotNil)
 
-		_, err = expr.Clone()
-		c.Assert(err, NotNil)
+		c.Assert(expr.Clone(), NotNil)
 	}
 
 	mock.err = nil
@@ -255,8 +252,7 @@ func (s *testBinOpSuite) TestLogicOp(c *C) {
 			c.Assert(v, DeepEquals, int64(x))
 		}
 
-		_, err = expr.Clone()
-		c.Assert(err, IsNil)
+		c.Assert(expr.Clone(), NotNil)
 	}
 
 	// test error

diff --git a/expression/expressions/builtin_control_test.go b/expression/expressions/builtin_control_test.go
@@ -107,8 +107,8 @@ func (s *testBuiltinSuite) TestCaseWhen(c *C) {
 	}
 
 	f := FunctionCase{mockExpr{val: 4}, whens0, mockExpr{val: "else"}}
-	cf, err := f.Clone()
-	c.Assert(err, IsNil)
+	cf := f.Clone()
+	c.Assert(cf, NotNil)
 	v, err := f.Eval(nil, nil)
 	c.Assert(err, IsNil)
 	cv, err := cf.Eval(nil, nil)

diff --git a/expression/expressions/call.go b/expression/expressions/call.go
@@ -76,13 +76,9 @@ func NewCall(f string, args []expression.Expression, distinct bool) (v expressio
 }
 
 // Clone implements the Expression Clone interface.
-func (c *Call) Clone() (expression.Expression, error) {
-	list, err := cloneExpressionList(c.Args)
-	if err != nil {
-		return nil, err
-	}
-
-	return &Call{F: c.F, Args: list, Distinct: c.Distinct}, nil
+func (c *Call) Clone() expression.Expression {
+	list := cloneExpressionList(c.Args)
+	return &Call{F: c.F, Args: list, Distinct: c.Distinct}
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/call_test.go b/expression/expressions/call_test.go
@@ -30,8 +30,7 @@ func (s *testCallSuite) TestCall(c *C) {
 	c.Assert(err, IsNil)
 
 	c.Assert(f.IsStatic(), IsTrue)
-	_, err = f.Clone()
-	c.Assert(err, IsNil)
+	c.Assert(f.Clone(), NotNil)
 
 	c.Assert(len(f.String()), Greater, 0)
 	m := map[interface{}]interface{}{}
@@ -46,8 +45,7 @@ func (s *testCallSuite) TestCall(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(len(f.String()), Greater, 0)
 	c.Assert(f.IsStatic(), IsFalse)
-	_, err = f.Clone()
-	c.Assert(err, IsNil)
+	c.Assert(f.Clone(), NotNil)
 
 	// test error
 	f, err = NewCall("abs", []expression.Expression{Value{1}, Value{2}}, true)
@@ -73,9 +71,7 @@ func (s *testCallSuite) TestCall(c *C) {
 	f, err = NewCall("abs", []expression.Expression{mock}, true)
 	c.Assert(err, IsNil)
 
-	_, err = f.Clone()
-	c.Assert(err, NotNil)
+	c.Assert(f.Clone(), NotNil)
 	_, err = f.Eval(nil, nil)
 	c.Assert(err, NotNil)
-
 }
diff --git a/expression/expressions/case.go b/expression/expressions/case.go
@@ -71,16 +71,10 @@ func (w *WhenClause) String() string {
 }
 
 // Clone implements the Expression Clone interface.
-func (w *WhenClause) Clone() (expression.Expression, error) {
-	ne, err := w.Expr.Clone()
-	if err != nil {
-		return nil, err
-	}
-	nr, err := w.Result.Clone()
-	if err != nil {
-		return nil, err
-	}
-	return &WhenClause{Expr: ne, Result: nr}, nil
+func (w *WhenClause) Clone() expression.Expression {
+	ne := w.Expr.Clone()
+	nr := w.Result.Clone()
+	return &WhenClause{Expr: ne, Result: nr}
 }
 
 // IsStatic implements the Expression IsStatic interface.
@@ -99,39 +93,29 @@ type FunctionCase struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (f *FunctionCase) Clone() (expression.Expression, error) {
+func (f *FunctionCase) Clone() expression.Expression {
 	var (
-		nv  expression.Expression
-		ne  expression.Expression
-		nw  expression.Expression
-		err error
+		nv expression.Expression
+		ne expression.Expression
+		nw expression.Expression
 	)
 	if f.Value != nil {
-		nv, err = f.Value.Clone()
-		if err != nil {
-			return nil, err
-		}
+		nv = f.Value.Clone()
 	}
 	ws := make([]*WhenClause, 0, len(f.WhenClauses))
 	for _, w := range f.WhenClauses {
-		nw, err = w.Clone()
-		if err != nil {
-			return nil, err
-		}
+		nw = w.Clone()
 		ws = append(ws, nw.(*WhenClause))
 	}
 
 	if f.ElseClause != nil {
-		ne, err = f.ElseClause.Clone()
-		if err != nil {
-			return nil, err
-		}
+		ne = f.ElseClause.Clone()
 	}
 	return &FunctionCase{
 		Value:       nv,
 		WhenClauses: ws,
 		ElseClause:  ne,
-	}, nil
+	}
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/cast.go b/expression/expressions/cast.go
@@ -46,17 +46,14 @@ type FunctionCast struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (f *FunctionCast) Clone() (expression.Expression, error) {
-	expr, err := f.Expr.Clone()
-	if err != nil {
-		return nil, err
-	}
+func (f *FunctionCast) Clone() expression.Expression {
+	expr := f.Expr.Clone()
 	nf := &FunctionCast{
 		Expr:         expr,
 		Tp:           f.Tp,
 		FunctionType: f.FunctionType,
 	}
-	return nf, nil
+	return nf
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/cast_test.go b/expression/expressions/cast_test.go
@@ -43,8 +43,7 @@ func (s *testCastSuite) TestCast(c *C) {
 	c.Assert(len(expr.String()), Greater, 0)
 
 	f.Tp = mysql.TypeLonglong
-	_, err := expr.Clone()
-	c.Assert(err, IsNil)
+	c.Assert(expr.Clone(), NotNil)
 
 	c.Assert(expr.IsStatic(), IsTrue)
 
@@ -75,8 +74,7 @@ func (s *testCastSuite) TestCast(c *C) {
 	c.Assert(v, Equals, nil)
 
 	expr.Expr = mockExpr{err: errors.New("must error")}
-	_, err = expr.Clone()
-	c.Assert(err, NotNil)
+	c.Assert(expr.Clone(), NotNil)
 
 	_, err = expr.Eval(nil, nil)
 	c.Assert(err, NotNil)

diff --git a/expression/expressions/cmp_subquery.go b/expression/expressions/cmp_subquery.go
@@ -39,18 +39,10 @@ type CompareSubQuery struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (cs *CompareSubQuery) Clone() (expression.Expression, error) {
-	l, err := cs.L.Clone()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	r, err := cs.R.Clone()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	return &CompareSubQuery{L: l, Op: cs.Op, R: r.(*SubQuery), All: cs.All}, nil
+func (cs *CompareSubQuery) Clone() expression.Expression {
+	l := cs.L.Clone()
+	r := cs.R.Clone()
+	return &CompareSubQuery{L: l, Op: cs.Op, R: r.(*SubQuery), All: cs.All}
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/convert.go b/expression/expressions/convert.go
@@ -32,16 +32,13 @@ type FunctionConvert struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (f *FunctionConvert) Clone() (expression.Expression, error) {
-	expr, err := f.Expr.Clone()
-	if err != nil {
-		return nil, err
-	}
+func (f *FunctionConvert) Clone() expression.Expression {
+	expr := f.Expr.Clone()
 	nf := &FunctionConvert{
 		Expr:    expr,
 		Charset: f.Charset,
 	}
-	return nf, nil
+	return nf
 }
 
 // IsStatic implements the Expression IsStatic interface.

diff --git a/expression/expressions/convert_test.go b/expression/expressions/convert_test.go
@@ -42,8 +42,8 @@ func (*testConvertSuite) TestConvert(c *C) {
 		fs := f.String()
 		c.Assert(len(fs), Greater, 0)
 
-		f1, err := f.Clone()
-		c.Assert(err, IsNil)
+		f1 := f.Clone()
+		c.Assert(f1, NotNil)
 
 		r, err := f.Eval(nil, nil)
 		c.Assert(err, IsNil)

diff --git a/expression/expressions/default.go b/expression/expressions/default.go
@@ -33,9 +33,9 @@ type Default struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (v *Default) Clone() (expression.Expression, error) {
+func (v *Default) Clone() expression.Expression {
 	newV := *v
-	return &newV, nil
+	return &newV
 }
 
 // IsStatic implements the Expression IsStatic interface, always returns false.

diff --git a/expression/expressions/default_test.go b/expression/expressions/default_test.go
@@ -26,14 +26,13 @@ func (s *testDefaultSuite) TestDefault(c *C) {
 	e := Default{}
 
 	c.Assert(e.String(), Equals, "default")
-	_, err := e.Clone()
-	c.Assert(err, IsNil)
+	c.Assert(e.Clone(), NotNil)
 
 	c.Assert(e.IsStatic(), IsFalse)
 
 	m := map[interface{}]interface{}{}
 
-	_, err = e.Eval(nil, m)
+	_, err := e.Eval(nil, m)
 	c.Assert(err, NotNil)
 
 	m[ExprEvalDefaultName] = "id"

diff --git a/expression/expressions/exists_subquery.go b/expression/expressions/exists_subquery.go
@@ -29,13 +29,9 @@ type ExistsSubQuery struct {
 }
 
 // Clone implements the Expression Clone interface.
-func (es *ExistsSubQuery) Clone() (expression.Expression, error) {
-	sel, err := es.Sel.Clone()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	return &ExistsSubQuery{Sel: sel.(*SubQuery)}, nil
+func (es *ExistsSubQuery) Clone() expression.Expression {
+	sel := es.Sel.Clone()
+	return &ExistsSubQuery{Sel: sel.(*SubQuery)}
 }
 
 // IsStatic implements the Expression IsStatic interface.