Skip to content

Commit 075619c

Browse files
authored
API-9331 fix regressions on 'Not' operations (#80)
* API-9331 fix regressions on 'Not' operations * versioning * token cleanup
1 parent 637a832 commit 075619c

5 files changed

Lines changed: 96 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
2+
v1.3.3, 2025-08-12
3+
-------------------
4+
* [BUGFIX] Evaluator fix for Not regression
5+
16
v1.3.2, 2025-08-06
27
-------------------
38
* [BUGFIX] More Evaluator fixes

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.3.2
1+
1.3.3

lib/sparkql/evaluator.rb

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
12
# Using an instance of ExpressionResolver to resolve the individual expressions,
23
# this class will evaluate the rest of a parsed sparkql string to true or false.
34
# Namely, this class will handle all the nesting, boolean algebra, and dropped
45
# fields. Plus, it has some optimizations built in to skip the processing for
56
# any expressions that don't contribute to the net result of the filter.
67
class Sparkql::Evaluator
8+
include Sparkql::Token
9+
710
attr_reader :processed_count
811

912
def initialize(expression_resolver)
@@ -40,6 +43,11 @@ def build_structures(levels, block_groups, expressions)
4043
block = expression[:block_group]
4144
block_group = block_groups[block]
4245

46+
if expression[:conjunction] == NOT && expression[:conjunction_level] == level
47+
expression[:conjunction] = AND
48+
expression[:unary] = NOT
49+
end
50+
4351
unless block_group
4452
block_groups[block] ||= block_builder(expression, level)
4553
block_group = block_groups[block]
@@ -48,9 +56,9 @@ def build_structures(levels, block_groups, expressions)
4856

4957
# When dealing with Not expression conjunctions at the block level,
5058
# it's far simpler to convert it into the equivalent "And Not"
51-
if block_group[:conjunction] == "Not"
52-
block_group[:unary] = "Not"
53-
block_group[:conjunction] = "And"
59+
if block_group[:conjunction] == NOT
60+
block_group[:unary] = NOT
61+
block_group[:conjunction] = AND
5462
end
5563

5664
# Every block group _must_ be seen as an expression in another block
@@ -100,7 +108,7 @@ def process_structures(levels, block_groups)
100108
block_group[:expressions].each do |expression|
101109
# If we encounter any or's in the same block group, we can cheat at
102110
# resolving the rest, if we are at a true
103-
if block_result && expression[:conjunction] == 'Or'
111+
if block_result && expression[:conjunction] == OR
104112
break
105113
end
106114

@@ -114,7 +122,7 @@ def process_structures(levels, block_groups)
114122
end
115123
next if expression_result == :drop
116124

117-
if expression[:unary] == "Not"
125+
if expression[:unary] == NOT
118126
expression_result = !expression_result
119127
end
120128

@@ -124,19 +132,19 @@ def process_structures(levels, block_groups)
124132
end
125133

126134
case expression[:conjunction]
127-
when 'Not'
135+
when NOT
128136
block_result &= !expression_result
129-
when 'And'
137+
when AND
130138
block_result &= expression_result
131-
when 'Or'
139+
when OR
132140
block_result |= expression_result
133141
else
134142
# Not a supported conjunction. We skip over this for backwards
135143
# compatibility.
136144
end
137145
end
138146

139-
# block_group.delete(:expressions)
147+
block_group.delete(:expressions)
140148
block_group[:result] = block_result
141149
final_result = block_result
142150
end

lib/sparkql/token.rb

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,15 @@ module Sparkql::Token
2424
NULL = /NULL|null|Null/.freeze
2525
# Reserved words
2626
RANGE_OPERATOR = 'Bt'.freeze
27-
EQUALITY_OPERATORS = %w[Eq Ne].freeze
27+
EQUAL = 'Eq'.freeze
28+
NOT_EQUAL = 'Ne'.freeze
29+
EQUALITY_OPERATORS = [EQUAL, NOT_EQUAL].freeze
30+
2831
OPERATORS = %w[Gt Ge Lt Le] + EQUALITY_OPERATORS
29-
UNARY_CONJUNCTIONS = ['Not'].freeze
30-
CONJUNCTIONS = %w[And Or].freeze
32+
33+
NOT = 'Not'.freeze
34+
AND = 'And'.freeze
35+
OR = 'Or'.freeze
36+
UNARY_CONJUNCTIONS = [NOT].freeze
37+
CONJUNCTIONS = [AND, OR].freeze
3138
end

test/unit/evaluator_test.rb

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,39 +64,93 @@ def test_nots_stay_bad
6464
def test_dropped_field_handling
6565
assert sample("Test Eq 'Drop' And Test Eq true")
6666
assert !sample("Test Eq 'Drop' And Test Eq false")
67-
assert !sample("Test Eq 'Drop' Or Test Eq false")
67+
6868
assert sample("Test Eq 'Drop' Or Test Eq true")
69+
assert !sample("Test Eq 'Drop' Or Test Eq false")
70+
6971
assert sample("Test Eq false And Test Eq 'Drop' Or Test Eq true")
72+
assert !sample("Test Eq false And Test Eq 'Drop' Or Test Eq false")
73+
7074
assert sample("Test Eq false Or (Test Eq 'Drop' And Test Eq true)")
75+
assert !sample("Test Eq false Or (Test Eq 'Drop' And Test Eq false)")
76+
77+
assert sample("Test Eq false Or (Not Test Eq 'Drop' And Test Eq true)")
78+
assert !sample("Test Eq false Or (Not Test Eq 'Drop' And Test Eq false)")
79+
80+
assert sample("Test Eq true Not Test Eq 'Drop' And Test Eq true")
81+
assert !sample("Test Eq true Not Test Eq 'Drop' And Test Eq false")
82+
assert !sample("Test Eq false Not Test Eq 'Drop' And Test Eq false")
83+
84+
assert sample("Test Eq true And Test Eq 'Drop' Not Test Eq false")
85+
assert !sample("Test Eq true And Test Eq 'Drop' Not Test Eq true")
86+
assert !sample("Test Eq true And Test Eq 'Drop' Not Test Eq true")
87+
88+
assert sample("Test Eq true Not (Test Eq 'Drop' And Test Eq false)")
89+
assert !sample("Test Eq true Not (Test Eq 'Drop' And Test Eq true)")
90+
assert !sample("Test Eq true Not (Test Eq 'Drop' And Test Eq true)")
7191
end
7292

7393
def test_nesting
7494
assert sample("Test Eq true Or (Test Eq true) And Test Eq false And (Test Eq true)")
95+
assert sample("Test Eq true Or (Test Eq false) And Test Eq false And (Test Eq false)")
96+
assert sample("Test Eq false Or (Test Eq true) And Test Eq true And (Test Eq true)")
97+
assert !sample("Test Eq false Or (Test Eq false) And Test Eq false And (Test Eq false)")
98+
assert !sample("Test Eq false Or (Test Eq true) And Test Eq false And (Test Eq false)")
99+
assert !sample("Test Eq false Or (Test Eq false) And Test Eq true And (Test Eq false)")
100+
assert !sample("Test Eq false Or (Test Eq false) And Test Eq false And (Test Eq true)")
101+
75102
assert sample("Test Eq true Or ((Test Eq false) And Test Eq false) And (Test Eq false)")
76103
assert sample("(Test Eq false Or Test Eq true) Or (Test Eq false Or Test Eq false)")
77104
assert sample("(Test Eq true And Test Eq true) Or (Test Eq false)")
78105
assert sample("(Test Eq true And Test Eq true) Or (Test Eq false And Test Eq true)")
79106
assert !sample("(Test Eq false And Test Eq true) Or (Test Eq false)")
107+
80108
assert sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq true)")
81109
assert !sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) And Test Eq true")
82110
assert !sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) Or Test Eq false")
83111
assert sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) Or Test Eq true")
84112
assert !sample("(Test Eq true Or Test Eq true) And Test Eq false")
85113
assert !sample("(Test Eq true Or Test Eq true) And (Test Eq false)")
114+
86115
assert sample("(Test Eq true Or Test Eq true) And (Test Eq false Or Test Eq true)")
87116
assert !sample("(Test Eq true Or Test Eq true) And (Test Eq false Or Test Eq false)")
117+
118+
assert sample("(Test Eq true) Not Test Eq false And (Test Eq true)")
119+
assert !sample("(Test Eq true) Not Test Eq true And (Test Eq true)")
120+
assert !sample("(Test Eq false) Not Test Eq false And (Test Eq true)")
121+
assert !sample("(Test1 Eq true) Not Test2 Eq false And (Test3 Eq false)")
88122
end
89123

90124
def test_nots
91125
assert sample("Test Eq true Not Test Eq false")
92126
assert !sample("Test Eq true Not Test Eq true")
127+
assert !sample("Test Eq false Not Test Eq true")
128+
assert !sample("Test Eq false Not Test Eq false")
129+
130+
assert sample("Test Eq true And Test Eq true Not Test Eq false")
131+
assert !sample("Test Eq false And Test Eq true Not Test Eq false")
132+
assert !sample("Test Eq true And Test Eq true Not Test Eq true")
133+
assert !sample("Test Eq true And Test Eq false Not Test Eq false")
134+
93135
assert sample("Test Eq true Not (Test Eq false Or Test Eq false)")
94-
assert sample("Test Eq true Not (Test Eq false And Test Eq false)")
95136
assert !sample("Test Eq true Not (Test Eq false Or Test Eq true)")
96137
assert !sample("Test Eq true Not (Test Eq true Or Test Eq false)")
97-
assert !sample("Test Eq true Not (Not Test Eq false)")
98-
assert !sample("Test Eq false And Test Eq true Not Test Eq false")
138+
assert !sample("Test Eq true Not (Test Eq true Or Test Eq true)")
139+
assert !sample("Test Eq false Not (Test Eq false Or Test Eq false)")
140+
141+
assert sample("Test Eq true Not (Test Eq false And Test Eq false)")
142+
assert sample("Test Eq true Not (Test Eq true And Test Eq false)")
143+
assert sample("Test Eq true Not (Test Eq false And Test Eq true)")
144+
assert !sample("Test Eq true Not (Test Eq true And Test Eq true)")
145+
assert !sample("Test Eq false Not (Test Eq false And Test Eq false)")
146+
147+
assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq false)")
148+
assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq false Or Test Eq true)")
149+
assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq true)")
99150
assert !sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq false Or Test Eq false)")
151+
assert !sample("Test Eq true Not (Test Eq false Or Test Eq true) And (Test Eq true Or Test Eq false)")
152+
assert !sample("Test Eq true Not (Test Eq true Or Test Eq false) And (Test Eq true Or Test Eq false)")
153+
assert !sample("Test Eq false Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq false)")
100154
end
101155

102156
def test_unary_nots
@@ -109,17 +163,14 @@ def test_unary_nots
109163

110164
def test_unary_double_nots
111165
assert sample("Not (Not(Not Test Eq true))")
166+
assert !sample("Not (Not(Not Test Eq false))")
167+
168+
assert sample("Test Eq true Not (Not Test Eq true)")
169+
assert !sample("Test Eq true Not (Not Test Eq false)")
170+
assert !sample("Test Eq false Not (Not Test Eq true)")
112171
end
113172

114173
def test_examples
115-
# This one is based on a real life example that had problems.
116-
#
117-
# CurrentPrice Bt 130000.00,180000.00 And PropertySubType Eq 'Single Family Residence' And
118-
# SchoolDistrict Eq 'Byron Center','Grandville','Jenison' And MlsStatus Eq 'Active' And
119-
# BathsTotal Bt 1.50,9999.00 And BedsTotal Bt 3,99 And PropertyType Eq 'A'
120-
# Not "Garage"."Garage2" Eq 'No' And "Pool"."OutdoorAbove" Eq true
121-
# And "Pool"."OutdoorInground" Eq true Not "Substructure"."Michigan Basement" Eq true
122-
123174
assert !sample("Test Eq false And Test Eq true And " \
124175
"Test Eq false And Test Eq true And " \
125176
"Test Eq true And Test Eq true And Test Eq true " \

0 commit comments

Comments
 (0)