Skip to content

Commit 47d764f

Browse files
committed
Fixes #84: Implement validity check when comparing, and ensure that invalid 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.
1 parent 0e8f997 commit 47d764f

File tree

6 files changed

+401
-161
lines changed

6 files changed

+401
-161
lines changed

bintree.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,9 @@ func (state *binTreeState) checkNode(index int) {
347347
return
348348
} else if defNode.NodeType == nodeTypeNot {
349349
if state.data[defNode.Left] == binTreeStateTrue {
350-
state.MarkNode(index, !true)
350+
state.MarkNode(index, false)
351351
} else if state.data[defNode.Left] == binTreeStateFalse {
352-
state.MarkNode(index, !false)
352+
state.MarkNode(index, true)
353353
}
354354
return
355355
} else if defNode.NodeType == nodeTypeLoop {
@@ -374,6 +374,7 @@ func (state *binTreeState) MarkNode(index int, value bool) {
374374
} else {
375375
state.data[index] = binTreeStateFalse
376376
}
377+
377378
state.resolveRecursive(index)
378379

379380
// We are done if we are the root node

fastMatcher.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,27 +265,39 @@ func (m *FastMatcher) matchOp(op *OpNode, litVal *FastVal) error {
265265
}
266266

267267
var opRes bool
268+
var validOp bool
269+
var compareOut int
268270
switch op.Op {
269271
case OpTypeEquals:
270-
opRes = lhsVal.Equals(rhsVal)
272+
opRes, validOp = lhsVal.Equals(rhsVal)
271273
case OpTypeLessThan:
272-
opRes = lhsVal.Compare(rhsVal) < 0
274+
compareOut, validOp = lhsVal.Compare(rhsVal)
275+
opRes = compareOut < 0
273276
case OpTypeLessEquals:
274-
opRes = lhsVal.Compare(rhsVal) <= 0
277+
compareOut, validOp = lhsVal.Compare(rhsVal)
278+
opRes = compareOut <= 0
275279
case OpTypeGreaterThan:
276-
opRes = lhsVal.Compare(rhsVal) > 0
280+
compareOut, validOp = lhsVal.Compare(rhsVal)
281+
opRes = compareOut > 0
277282
case OpTypeGreaterEquals:
278-
opRes = lhsVal.Compare(rhsVal) >= 0
283+
compareOut, validOp = lhsVal.Compare(rhsVal)
284+
opRes = compareOut >= 0
279285
case OpTypeMatches:
280-
opRes = lhsVal.Matches(rhsVal)
286+
opRes, validOp = lhsVal.Matches(rhsVal)
281287
case OpTypeExists:
282288
opRes = true
289+
validOp = true
283290
default:
284291
panic("invalid op type")
285292
}
286293

287294
// Mark the result of this operation
288-
m.buckets.MarkNode(bucketIdx, opRes)
295+
if !validOp {
296+
// TODO FIX
297+
m.buckets.MarkNode(bucketIdx, false)
298+
} else {
299+
m.buckets.MarkNode(bucketIdx, opRes)
300+
}
289301

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

500512
// Apply the overall loop result to the binary tree
501-
m.buckets.MarkNode(loopBucketIdx, loopState)
513+
if loopState {
514+
m.buckets.MarkNode(loopBucketIdx, true)
515+
} else {
516+
m.buckets.MarkNode(loopBucketIdx, false)
517+
}
502518

503519
return nil
504520
}

fastMatcher_test.go

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,9 @@ func TestMatcherNotTrueEquals(t *testing.T) {
158158
}
159159

160160
func TestMatcherMissingNotEquals(t *testing.T) {
161-
// This tests a specific case where a missing value should actually
162-
// result in a truthy result in the expression. Due to the nature
163-
// of our bintree implementation, this needs special handling.
161+
// This tests a specific case where both missing value and false booleans
162+
// will return truthy results, since the "true" value here translates to
163+
// a true boolean
164164
runJSONExprMatchTest(t, `
165165
["not",
166166
["equals",
@@ -169,14 +169,15 @@ func TestMatcherMissingNotEquals(t *testing.T) {
169169
]
170170
]
171171
`, []string{
172-
"5b47eb0950e9076fc0aecd52",
173172
"5b47eb093771f06ced629663",
174173
"5b47eb09ffac5a6ce37042e7",
175174
"5b47eb095c3ad73b9925f7f8",
176175
"5b47eb0962222a37d066e231",
177176
"5b47eb09996a4154c35b2f98",
178177
"5b47eb091f57571d3c3b1aa1",
179178
"5b47eb098eee4b4c4330ec64",
179+
"5b47eb0936ff92a567a0307e",
180+
"5b47eb096b1d911c0b9492fb",
180181
})
181182
}
182183

@@ -214,22 +215,6 @@ func TestMatcherNotExists(t *testing.T) {
214215
})
215216
}
216217

217-
func TestMatcherDisparateTypeEquals(t *testing.T) {
218-
// TODO(brett19): Should probably discuss whether type-cast equals
219-
// actually makes sense... This validates that these something like:
220-
// (true == "thisShouldBeABoolean") === true
221-
// which may not actually make a whole lot of sense...
222-
runJSONExprMatchTest(t, `
223-
["equals",
224-
["field", "sometimesValue"],
225-
["value", "thisShouldBeABoolean"]
226-
]
227-
`, []string{
228-
"5b47eb0936ff92a567a0307e",
229-
"5b47eb096b1d911c0b9492fb",
230-
})
231-
}
232-
233218
func TestMatcherSometimesMissingBoolEquals(t *testing.T) {
234219
runJSONExprMatchTest(t, `
235220
["equals",

0 commit comments

Comments
 (0)