Skip to content

Commit

Permalink
Fixes #84: Implement validity check when comparing, and ensure that i…
Browse files Browse the repository at this point in the history
…nvalid type comparisons

           will result in a resolve state that is neither true nor false. This will help
           ensure a more well-defined matcher behavior and easier to be documented and understood.
           Based on the validity component, null comparisons (Issue #84) can now be addressed
           correctly without the use of workarounds such as math.MaxUint or the like.
  • Loading branch information
nelio2k committed Nov 4, 2019
1 parent 0e8f997 commit 47d764f
Show file tree
Hide file tree
Showing 6 changed files with 401 additions and 161 deletions.
5 changes: 3 additions & 2 deletions bintree.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,9 @@ func (state *binTreeState) checkNode(index int) {
return
} else if defNode.NodeType == nodeTypeNot {
if state.data[defNode.Left] == binTreeStateTrue {
state.MarkNode(index, !true)
state.MarkNode(index, false)
} else if state.data[defNode.Left] == binTreeStateFalse {
state.MarkNode(index, !false)
state.MarkNode(index, true)
}
return
} else if defNode.NodeType == nodeTypeLoop {
Expand All @@ -374,6 +374,7 @@ func (state *binTreeState) MarkNode(index int, value bool) {
} else {
state.data[index] = binTreeStateFalse
}

state.resolveRecursive(index)

// We are done if we are the root node
Expand Down
32 changes: 24 additions & 8 deletions fastMatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,27 +265,39 @@ func (m *FastMatcher) matchOp(op *OpNode, litVal *FastVal) error {
}

var opRes bool
var validOp bool
var compareOut int
switch op.Op {
case OpTypeEquals:
opRes = lhsVal.Equals(rhsVal)
opRes, validOp = lhsVal.Equals(rhsVal)
case OpTypeLessThan:
opRes = lhsVal.Compare(rhsVal) < 0
compareOut, validOp = lhsVal.Compare(rhsVal)
opRes = compareOut < 0
case OpTypeLessEquals:
opRes = lhsVal.Compare(rhsVal) <= 0
compareOut, validOp = lhsVal.Compare(rhsVal)
opRes = compareOut <= 0
case OpTypeGreaterThan:
opRes = lhsVal.Compare(rhsVal) > 0
compareOut, validOp = lhsVal.Compare(rhsVal)
opRes = compareOut > 0
case OpTypeGreaterEquals:
opRes = lhsVal.Compare(rhsVal) >= 0
compareOut, validOp = lhsVal.Compare(rhsVal)
opRes = compareOut >= 0
case OpTypeMatches:
opRes = lhsVal.Matches(rhsVal)
opRes, validOp = lhsVal.Matches(rhsVal)
case OpTypeExists:
opRes = true
validOp = true
default:
panic("invalid op type")
}

// Mark the result of this operation
m.buckets.MarkNode(bucketIdx, opRes)
if !validOp {
// TODO FIX
m.buckets.MarkNode(bucketIdx, false)
} else {
m.buckets.MarkNode(bucketIdx, opRes)
}

// Check if running this values ops has resolved the entirety
// of the expression, if so we can leave immediately.
Expand Down Expand Up @@ -498,7 +510,11 @@ func (m *FastMatcher) matchLoop(token tokenType, tokenData []byte, loop *LoopNod
m.buckets.SetStallIndex(previousStallIndex)

// Apply the overall loop result to the binary tree
m.buckets.MarkNode(loopBucketIdx, loopState)
if loopState {
m.buckets.MarkNode(loopBucketIdx, true)
} else {
m.buckets.MarkNode(loopBucketIdx, false)
}

return nil
}
Expand Down
25 changes: 5 additions & 20 deletions fastMatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ func TestMatcherNotTrueEquals(t *testing.T) {
}

func TestMatcherMissingNotEquals(t *testing.T) {
// This tests a specific case where a missing value should actually
// result in a truthy result in the expression. Due to the nature
// of our bintree implementation, this needs special handling.
// This tests a specific case where both missing value and false booleans
// will return truthy results, since the "true" value here translates to
// a true boolean
runJSONExprMatchTest(t, `
["not",
["equals",
Expand All @@ -169,14 +169,15 @@ func TestMatcherMissingNotEquals(t *testing.T) {
]
]
`, []string{
"5b47eb0950e9076fc0aecd52",
"5b47eb093771f06ced629663",
"5b47eb09ffac5a6ce37042e7",
"5b47eb095c3ad73b9925f7f8",
"5b47eb0962222a37d066e231",
"5b47eb09996a4154c35b2f98",
"5b47eb091f57571d3c3b1aa1",
"5b47eb098eee4b4c4330ec64",
"5b47eb0936ff92a567a0307e",
"5b47eb096b1d911c0b9492fb",
})
}

Expand Down Expand Up @@ -214,22 +215,6 @@ func TestMatcherNotExists(t *testing.T) {
})
}

func TestMatcherDisparateTypeEquals(t *testing.T) {
// TODO(brett19): Should probably discuss whether type-cast equals
// actually makes sense... This validates that these something like:
// (true == "thisShouldBeABoolean") === true
// which may not actually make a whole lot of sense...
runJSONExprMatchTest(t, `
["equals",
["field", "sometimesValue"],
["value", "thisShouldBeABoolean"]
]
`, []string{
"5b47eb0936ff92a567a0307e",
"5b47eb096b1d911c0b9492fb",
})
}

func TestMatcherSometimesMissingBoolEquals(t *testing.T) {
runJSONExprMatchTest(t, `
["equals",
Expand Down
Loading

0 comments on commit 47d764f

Please sign in to comment.