From f84556fe2227469692e4c2d2c35ade44b980a423 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Thu, 18 Feb 2016 16:37:03 -0500 Subject: [PATCH] Update binary expressions to handle mixed math between integers and floats This also changes it so all NumericLiterals are treated as a float rather than sometimes being cast to an integer. Fixes #5740. --- influxql/select.go | 230 ++++++++++++++++++---------------------- influxql/select_test.go | 110 ++++++++++++++----- 2 files changed, 190 insertions(+), 150 deletions(-) diff --git a/influxql/select.go b/influxql/select.go index 302d652eb9..97ffe75b87 100644 --- a/influxql/select.go +++ b/influxql/select.go @@ -372,16 +372,22 @@ func buildExprIterator(expr Expr, ic IteratorCreator, opt IteratorOptions) (Iter } func buildRHSTransformIterator(lhs Iterator, rhs Literal, op Token, ic IteratorCreator, opt IteratorOptions) (Iterator, error) { - fn := binaryExprFunc(iteratorDataType(lhs), op) + fn := binaryExprFunc(iteratorDataType(lhs), literalDataType(rhs), op) switch fn := fn.(type) { case func(float64, float64) float64: - input, ok := lhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected rhs to be FloatIterator, got %T", rhs) + var input FloatIterator + switch lhs := lhs.(type) { + case FloatIterator: + input = lhs + case IntegerIterator: + input = &integerFloatCastIterator{input: lhs} + default: + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a FloatIterator", lhs) } + lit, ok := rhs.(*NumberLiteral) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a NumberLiteral", lhs) } return &floatTransformIterator{ input: input, @@ -393,33 +399,20 @@ func buildRHSTransformIterator(lhs Iterator, rhs Literal, op Token, ic IteratorC return p }, }, nil - case func(int64, int64) int64: - input, ok := lhs.(IntegerIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected rhs to be IntegerIterator, got %T", rhs) - } - lit, ok := rhs.(*NumberLiteral) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) - } - return &integerTransformIterator{ - input: input, - fn: func(p *IntegerPoint) *IntegerPoint { - if p == nil { - return nil - } - p.Value = fn(p.Value, int64(lit.Val)) - return p - }, - }, nil case func(float64, float64) bool: - input, ok := lhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", lhs) + var input FloatIterator + switch lhs := lhs.(type) { + case FloatIterator: + input = lhs + case IntegerIterator: + input = &integerFloatCastIterator{input: lhs} + default: + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a FloatIterator", lhs) } + lit, ok := rhs.(*NumberLiteral) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", rhs) + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a NumberLiteral", lhs) } return &floatBoolTransformIterator{ input: input, @@ -436,45 +429,27 @@ func buildRHSTransformIterator(lhs Iterator, rhs Literal, op Token, ic IteratorC } }, }, nil - case func(int64, int64) bool: - input, ok := lhs.(IntegerIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", lhs) - } - lit, ok := rhs.(*NumberLiteral) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", rhs) - } - return &integerBoolTransformIterator{ - input: input, - fn: func(p *IntegerPoint) *BooleanPoint { - if p == nil { - return nil - } - return &BooleanPoint{ - Name: p.Name, - Tags: p.Tags, - Time: p.Time, - Value: fn(p.Value, int64(lit.Val)), - Aux: p.Aux, - } - }, - }, nil } return nil, fmt.Errorf("unable to construct rhs transform iterator from %T and %T", lhs, rhs) } func buildLHSTransformIterator(lhs Literal, rhs Iterator, op Token, ic IteratorCreator, opt IteratorOptions) (Iterator, error) { - fn := binaryExprFunc(iteratorDataType(rhs), op) + fn := binaryExprFunc(literalDataType(lhs), iteratorDataType(rhs), op) switch fn := fn.(type) { case func(float64, float64) float64: + var input FloatIterator + switch rhs := rhs.(type) { + case FloatIterator: + input = rhs + case IntegerIterator: + input = &integerFloatCastIterator{input: rhs} + default: + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a FloatIterator", rhs) + } + lit, ok := lhs.(*NumberLiteral) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) - } - input, ok := rhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected rhs to be FloatIterator, got %T", rhs) + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a NumberLiteral", lhs) } return &floatTransformIterator{ input: input, @@ -486,33 +461,20 @@ func buildLHSTransformIterator(lhs Literal, rhs Iterator, op Token, ic IteratorC return p }, }, nil - case func(int64, int64) int64: - lit, ok := lhs.(*NumberLiteral) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) - } - input, ok := rhs.(IntegerIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected rhs to be IntegerIterator, got %T", rhs) - } - return &integerTransformIterator{ - input: input, - fn: func(p *IntegerPoint) *IntegerPoint { - if p == nil { - return nil - } - p.Value = fn(int64(lit.Val), p.Value) - return p - }, - }, nil case func(float64, float64) bool: + var input FloatIterator + switch rhs := rhs.(type) { + case FloatIterator: + input = rhs + case IntegerIterator: + input = &integerFloatCastIterator{input: rhs} + default: + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a FloatIterator", rhs) + } + lit, ok := lhs.(*NumberLiteral) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) - } - input, ok := rhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", rhs) + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a NumberLiteral", lhs) } return &floatBoolTransformIterator{ input: input, @@ -529,45 +491,32 @@ func buildLHSTransformIterator(lhs Literal, rhs Iterator, op Token, ic IteratorC } }, }, nil - case func(int64, int64) bool: - lit, ok := lhs.(*NumberLiteral) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be NumberLiteral, got %T", lhs) - } - input, ok := rhs.(IntegerIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", rhs) - } - return &integerBoolTransformIterator{ - input: input, - fn: func(p *IntegerPoint) *BooleanPoint { - if p == nil { - return nil - } - return &BooleanPoint{ - Name: p.Name, - Tags: p.Tags, - Time: p.Time, - Value: fn(int64(lit.Val), p.Value), - Aux: p.Aux, - } - }, - }, nil } return nil, fmt.Errorf("unable to construct lhs transform iterator from %T and %T", lhs, rhs) } func buildTransformIterator(lhs Iterator, rhs Iterator, op Token, ic IteratorCreator, opt IteratorOptions) (Iterator, error) { - fn := binaryExprFunc(iteratorDataType(lhs), op) + fn := binaryExprFunc(iteratorDataType(lhs), iteratorDataType(rhs), op) switch fn := fn.(type) { case func(float64, float64) float64: - left, ok := lhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", lhs) + var left FloatIterator + switch lhs := lhs.(type) { + case FloatIterator: + left = lhs + case IntegerIterator: + left = &integerFloatCastIterator{input: lhs} + default: + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a FloatIterator", lhs) } - right, ok := rhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", rhs) + + var right FloatIterator + switch rhs := rhs.(type) { + case FloatIterator: + right = rhs + case IntegerIterator: + right = &integerFloatCastIterator{input: rhs} + default: + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a FloatIterator", rhs) } return &floatTransformIterator{ input: left, @@ -586,11 +535,11 @@ func buildTransformIterator(lhs Iterator, rhs Iterator, op Token, ic IteratorCre case func(int64, int64) int64: left, ok := lhs.(IntegerIterator) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", lhs) + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a IntegerIterator", lhs) } right, ok := rhs.(IntegerIterator) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", rhs) + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a IntegerIterator", rhs) } return &integerTransformIterator{ input: left, @@ -607,13 +556,24 @@ func buildTransformIterator(lhs Iterator, rhs Iterator, op Token, ic IteratorCre }, }, nil case func(float64, float64) bool: - left, ok := lhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", lhs) + var left FloatIterator + switch lhs := lhs.(type) { + case FloatIterator: + left = lhs + case IntegerIterator: + left = &integerFloatCastIterator{input: lhs} + default: + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a FloatIterator", lhs) } - right, ok := rhs.(FloatIterator) - if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be FloatIterator, got %T", rhs) + + var right FloatIterator + switch rhs := rhs.(type) { + case FloatIterator: + right = rhs + case IntegerIterator: + right = &integerFloatCastIterator{input: rhs} + default: + return nil, fmt.Errorf("type mismatch on RHS, unable to use %T as a FloatIterator", rhs) } return &floatBoolTransformIterator{ input: left, @@ -637,11 +597,11 @@ func buildTransformIterator(lhs Iterator, rhs Iterator, op Token, ic IteratorCre case func(int64, int64) bool: left, ok := lhs.(IntegerIterator) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", lhs) + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a IntegerIterator", lhs) } right, ok := rhs.(IntegerIterator) if !ok { - return nil, fmt.Errorf("type mismatch, expected lhs to be IntegerIterator, got %T", rhs) + return nil, fmt.Errorf("type mismatch on LHS, unable to use %T as a IntegerIterator", rhs) } return &integerBoolTransformIterator{ input: left, @@ -681,13 +641,31 @@ func iteratorDataType(itr Iterator) DataType { } } -func binaryExprFunc(typ DataType, op Token) interface{} { +func literalDataType(lit Literal) DataType { + switch lit.(type) { + case *NumberLiteral: + return Float + case *StringLiteral: + return String + case *BooleanLiteral: + return Boolean + default: + return Unknown + } +} + +func binaryExprFunc(typ1 DataType, typ2 DataType, op Token) interface{} { var fn interface{} - switch typ { + switch typ1 { case Float: fn = floatBinaryExprFunc(op) case Integer: - fn = integerBinaryExprFunc(op) + switch typ2 { + case Float: + fn = floatBinaryExprFunc(op) + default: + fn = integerBinaryExprFunc(op) + } } return fn } diff --git a/influxql/select_test.go b/influxql/select_test.go index 9be919302e..c29cb10ec1 100644 --- a/influxql/select_test.go +++ b/influxql/select_test.go @@ -1304,18 +1304,18 @@ func TestSelect_BinaryExpr_Integer(t *testing.T) { Name: "rhs binary add", Statement: `SELECT value + 2 FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 22}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 12}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 21}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 22}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 12}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 21}}, }, }, { Name: "lhs binary add", Statement: `SELECT 2 + value FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 22}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 12}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 21}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 22}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 12}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 21}}, }, }, { @@ -1331,18 +1331,18 @@ func TestSelect_BinaryExpr_Integer(t *testing.T) { Name: "rhs binary multiply", Statement: `SELECT value * 2 FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 40}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 20}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 38}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 40}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 20}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 38}}, }, }, { Name: "lhs binary multiply", Statement: `SELECT 2 * value FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 40}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 20}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 38}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 40}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 20}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 38}}, }, }, { @@ -1358,18 +1358,18 @@ func TestSelect_BinaryExpr_Integer(t *testing.T) { Name: "rhs binary subtract", Statement: `SELECT value - 2 FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 18}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 8}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 17}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 18}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 8}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 17}}, }, }, { Name: "lhs binary subtract", Statement: `SELECT 2 - value FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: -18}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: -8}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: -17}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: -18}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: -8}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: -17}}, }, }, { @@ -1385,18 +1385,18 @@ func TestSelect_BinaryExpr_Integer(t *testing.T) { Name: "rhs binary division", Statement: `SELECT value / 2 FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 10}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 5}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 9}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 10}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 5}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 9.5}}, }, }, { Name: "lhs binary division", Statement: `SELECT 38 / value FROM cpu`, Points: [][]influxql.Point{ - {&influxql.IntegerPoint{Name: "cpu", Time: 0 * Second, Value: 1}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 5 * Second, Value: 3}}, - {&influxql.IntegerPoint{Name: "cpu", Time: 9 * Second, Value: 2}}, + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 1.9}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 3.8}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 2.0}}, }, }, { @@ -1418,6 +1418,68 @@ func TestSelect_BinaryExpr_Integer(t *testing.T) { } } +// Ensure a SELECT binary expr queries can be executed on mixed iterators. +func TestSelect_BinaryExpr_Mixed(t *testing.T) { + var ic IteratorCreator + ic.CreateIteratorFn = func(opt influxql.IteratorOptions) (influxql.Iterator, error) { + return &IntegerIterator{Points: []influxql.IntegerPoint{ + {Name: "cpu", Time: 0 * Second, Value: 20, Aux: []interface{}{float64(20), int64(10)}}, + {Name: "cpu", Time: 5 * Second, Value: 10, Aux: []interface{}{float64(10), int64(15)}}, + {Name: "cpu", Time: 9 * Second, Value: 19, Aux: []interface{}{float64(19), int64(5)}}, + }}, nil + } + + for _, test := range []struct { + Name string + Statement string + Points [][]influxql.Point + }{ + { + Name: "mixed binary add", + Statement: `SELECT total + value FROM cpu`, + Points: [][]influxql.Point{ + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 30}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 25}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 24}}, + }, + }, + { + Name: "mixed binary subtract", + Statement: `SELECT total - value FROM cpu`, + Points: [][]influxql.Point{ + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 10}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: -5}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 14}}, + }, + }, + { + Name: "mixed binary multiply", + Statement: `SELECT total * value FROM cpu`, + Points: [][]influxql.Point{ + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 200}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: 150}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: 95}}, + }, + }, + { + Name: "mixed binary division", + Statement: `SELECT total / value FROM cpu`, + Points: [][]influxql.Point{ + {&influxql.FloatPoint{Name: "cpu", Time: 0 * Second, Value: 2}}, + {&influxql.FloatPoint{Name: "cpu", Time: 5 * Second, Value: float64(10) / float64(15)}}, + {&influxql.FloatPoint{Name: "cpu", Time: 9 * Second, Value: float64(19) / float64(5)}}, + }, + }, + } { + itrs, err := influxql.Select(MustParseSelectStatement(test.Statement), &ic, nil) + if err != nil { + t.Errorf("%s: parse error: %s", test.Name, err) + } else if a := Iterators(itrs).ReadAll(); !deep.Equal(a, test.Points) { + t.Errorf("%s: unexpected points: %s", test.Name, spew.Sdump(a)) + } + } +} + // Ensure a SELECT (...) query can be executed. func TestSelect_ParenExpr(t *testing.T) { var ic IteratorCreator