Skip to content

Commit

Permalink
Mismatched tests on map values produce incorrect logic. (#2754)
Browse files Browse the repository at this point in the history
* fixed multiple mismatched tests for value maps.
* more fixes to map value checks. linter and test fixes.
* fixed to idiomatic Go.
* seenArgs wasn't being updated although the logic used it, fixed.
* added some math tests.
* readded some list optimizations because len checks exist.
* Review by Manish
* removed failure on undefined vars to return empty list, and reverted test to previous state.
* fixed bug in getReversePredicates.
  • Loading branch information
srfrog authored Nov 28, 2018
1 parent a2c002e commit 18d8d25
Show file tree
Hide file tree
Showing 6 changed files with 402 additions and 175 deletions.
31 changes: 15 additions & 16 deletions query/math.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package query
import (
"github.com/dgraph-io/dgraph/types"
"github.com/dgraph-io/dgraph/x"
"github.com/golang/glog"
)

type mathTree struct {
Expand All @@ -31,7 +32,7 @@ type mathTree struct {

// processBinary handles the binary operands like
// +, -, *, /, %, max, min, logbase
func processBinary(mNode *mathTree) (err error) {
func processBinary(mNode *mathTree) error {
destMap := make(map[uint64]types.Val)
aggName := mNode.Fn

Expand All @@ -54,7 +55,7 @@ func processBinary(mNode *mathTree) (err error) {
// Use the constant value that was supplied.
rVal = cr
}
err = ag.ApplyVal(lVal)
err := ag.ApplyVal(lVal)
if err != nil {
return err
}
Expand All @@ -69,13 +70,12 @@ func processBinary(mNode *mathTree) (err error) {
return nil
}

if mpl != nil || mpr != nil {
if len(mpl) != 0 || len(mpr) != 0 {
for k := range mpr {
if err := f(k); err != nil {
return err
}
}

for k := range mpl {
if _, ok := mpr[k]; ok {
continue
Expand All @@ -85,15 +85,15 @@ func processBinary(mNode *mathTree) (err error) {
}
}
mNode.Val = destMap
return
return nil
}

if cl.Value != nil && cr.Value != nil {
// Both maps are nil, so 2 constatns.
ag := aggregator{
name: aggName,
}
err = ag.ApplyVal(cl)
err := ag.ApplyVal(cl)
if err != nil {
return err
}
Expand All @@ -104,13 +104,12 @@ func processBinary(mNode *mathTree) (err error) {
mNode.Const, err = ag.Value()
return err
}
x.Fatalf("Empty maps and constant")
return nil
}

// processUnary handles the unary operands like
// u-, log, exp, since, floor, ceil
func processUnary(mNode *mathTree) (err error) {
func processUnary(mNode *mathTree) error {
destMap := make(map[uint64]types.Val)
srcMap := mNode.Child[0].Val
aggName := mNode.Fn
Expand All @@ -120,7 +119,7 @@ func processUnary(mNode *mathTree) (err error) {
}
if ch.Const.Value != nil {
// Use the constant value that was supplied.
err = ag.ApplyVal(ch.Const)
err := ag.ApplyVal(ch.Const)
if err != nil {
return err
}
Expand All @@ -129,7 +128,7 @@ func processUnary(mNode *mathTree) (err error) {
}

for k, val := range srcMap {
err = ag.ApplyVal(val)
err := ag.ApplyVal(val)
if err != nil {
return err
}
Expand All @@ -146,7 +145,7 @@ func processUnary(mNode *mathTree) (err error) {
// processBinaryBoolean handles the binary operands which
// return a boolean value.
// All the inequality operators (<, >, <=, >=, !=, ==)
func processBinaryBoolean(mNode *mathTree) (err error) {
func processBinaryBoolean(mNode *mathTree) error {
destMap := make(map[uint64]types.Val)
srcMap := mNode.Child[0].Val
aggName := mNode.Fn
Expand All @@ -173,11 +172,11 @@ func processBinaryBoolean(mNode *mathTree) (err error) {
}

// processTernary handles the ternary operand cond()
func processTernary(mNode *mathTree) (err error) {
func processTernary(mNode *mathTree) error {
destMap := make(map[uint64]types.Val)
aggName := mNode.Fn
condMap := mNode.Child[0].Val
if condMap == nil {
if len(condMap) == 0 {
return x.Errorf("Expected a value variable in %v but missing.", aggName)
}
varOne := mNode.Child[1].Val
Expand Down Expand Up @@ -211,13 +210,13 @@ func processTernary(mNode *mathTree) (err error) {
return nil
}

func evalMathTree(mNode *mathTree) (err error) {
func evalMathTree(mNode *mathTree) error {
if mNode.Const.Value != nil {
return nil
}
if mNode.Var != "" {
if mNode.Val == nil {
return x.Errorf("Variable %v not yet populated or missing.", mNode.Var)
if len(mNode.Val) == 0 {
glog.V(2).Infof("Variable %v not yet populated or missing.", mNode.Var)
}
// This is a leaf node whose value is already populated. So return.
return nil
Expand Down
Loading

0 comments on commit 18d8d25

Please sign in to comment.