Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@

v1.3.3, 2025-08-12
-------------------
* [BUGFIX] Evaluator fix for Not regression

v1.3.2, 2025-08-06
-------------------
* [BUGFIX] More Evaluator fixes
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.3.2
1.3.3
26 changes: 17 additions & 9 deletions lib/sparkql/evaluator.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@

# Using an instance of ExpressionResolver to resolve the individual expressions,
# this class will evaluate the rest of a parsed sparkql string to true or false.
# Namely, this class will handle all the nesting, boolean algebra, and dropped
# fields. Plus, it has some optimizations built in to skip the processing for
# any expressions that don't contribute to the net result of the filter.
class Sparkql::Evaluator
include Sparkql::Token

attr_reader :processed_count

def initialize(expression_resolver)
Expand Down Expand Up @@ -40,6 +43,11 @@ def build_structures(levels, block_groups, expressions)
block = expression[:block_group]
block_group = block_groups[block]

if expression[:conjunction] == NOT && expression[:conjunction_level] == level
expression[:conjunction] = AND
expression[:unary] = NOT
end

unless block_group
block_groups[block] ||= block_builder(expression, level)
block_group = block_groups[block]
Expand All @@ -48,9 +56,9 @@ def build_structures(levels, block_groups, expressions)

# When dealing with Not expression conjunctions at the block level,
# it's far simpler to convert it into the equivalent "And Not"
if block_group[:conjunction] == "Not"
block_group[:unary] = "Not"
block_group[:conjunction] = "And"
if block_group[:conjunction] == NOT
block_group[:unary] = NOT
block_group[:conjunction] = AND
end

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

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

if expression[:unary] == "Not"
if expression[:unary] == NOT
expression_result = !expression_result
end

Expand All @@ -124,19 +132,19 @@ def process_structures(levels, block_groups)
end

case expression[:conjunction]
when 'Not'
when NOT
block_result &= !expression_result
when 'And'
when AND
block_result &= expression_result
when 'Or'
when OR
block_result |= expression_result
else
# Not a supported conjunction. We skip over this for backwards
# compatibility.
end
end

# block_group.delete(:expressions)
block_group.delete(:expressions)
block_group[:result] = block_result
final_result = block_result
end
Expand Down
13 changes: 10 additions & 3 deletions lib/sparkql/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ module Sparkql::Token
NULL = /NULL|null|Null/.freeze
# Reserved words
RANGE_OPERATOR = 'Bt'.freeze
EQUALITY_OPERATORS = %w[Eq Ne].freeze
EQUAL = 'Eq'.freeze
NOT_EQUAL = 'Ne'.freeze
EQUALITY_OPERATORS = [EQUAL, NOT_EQUAL].freeze

OPERATORS = %w[Gt Ge Lt Le] + EQUALITY_OPERATORS
UNARY_CONJUNCTIONS = ['Not'].freeze
CONJUNCTIONS = %w[And Or].freeze

NOT = 'Not'.freeze
AND = 'And'.freeze
OR = 'Or'.freeze
UNARY_CONJUNCTIONS = [NOT].freeze
CONJUNCTIONS = [AND, OR].freeze
end
75 changes: 63 additions & 12 deletions test/unit/evaluator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,39 +64,93 @@ def test_nots_stay_bad
def test_dropped_field_handling
assert sample("Test Eq 'Drop' And Test Eq true")
assert !sample("Test Eq 'Drop' And Test Eq false")
assert !sample("Test Eq 'Drop' Or Test Eq false")

assert sample("Test Eq 'Drop' Or Test Eq true")
assert !sample("Test Eq 'Drop' Or Test Eq false")

assert sample("Test Eq false And Test Eq 'Drop' Or Test Eq true")
assert !sample("Test Eq false And Test Eq 'Drop' Or Test Eq false")

assert sample("Test Eq false Or (Test Eq 'Drop' And Test Eq true)")
assert !sample("Test Eq false Or (Test Eq 'Drop' And Test Eq false)")

assert sample("Test Eq false Or (Not Test Eq 'Drop' And Test Eq true)")
assert !sample("Test Eq false Or (Not Test Eq 'Drop' And Test Eq false)")

assert sample("Test Eq true Not Test Eq 'Drop' And Test Eq true")
assert !sample("Test Eq true Not Test Eq 'Drop' And Test Eq false")
assert !sample("Test Eq false Not Test Eq 'Drop' And Test Eq false")

assert sample("Test Eq true And Test Eq 'Drop' Not Test Eq false")
assert !sample("Test Eq true And Test Eq 'Drop' Not Test Eq true")
assert !sample("Test Eq true And Test Eq 'Drop' Not Test Eq true")

assert sample("Test Eq true Not (Test Eq 'Drop' And Test Eq false)")
assert !sample("Test Eq true Not (Test Eq 'Drop' And Test Eq true)")
assert !sample("Test Eq true Not (Test Eq 'Drop' And Test Eq true)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

end

def test_nesting
assert sample("Test Eq true Or (Test Eq true) And Test Eq false And (Test Eq true)")
assert sample("Test Eq true Or (Test Eq false) And Test Eq false And (Test Eq false)")
assert sample("Test Eq false Or (Test Eq true) And Test Eq true And (Test Eq true)")
assert !sample("Test Eq false Or (Test Eq false) And Test Eq false And (Test Eq false)")
assert !sample("Test Eq false Or (Test Eq true) And Test Eq false And (Test Eq false)")
assert !sample("Test Eq false Or (Test Eq false) And Test Eq true And (Test Eq false)")
assert !sample("Test Eq false Or (Test Eq false) And Test Eq false And (Test Eq true)")

assert sample("Test Eq true Or ((Test Eq false) And Test Eq false) And (Test Eq false)")
assert sample("(Test Eq false Or Test Eq true) Or (Test Eq false Or Test Eq false)")
assert sample("(Test Eq true And Test Eq true) Or (Test Eq false)")
assert sample("(Test Eq true And Test Eq true) Or (Test Eq false And Test Eq true)")
assert !sample("(Test Eq false And Test Eq true) Or (Test Eq false)")

assert sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq true)")
assert !sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) And Test Eq true")
assert !sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) Or Test Eq false")
assert sample("Test Eq true And ((Test Eq true And Test Eq false) Or Test Eq false) Or Test Eq true")
assert !sample("(Test Eq true Or Test Eq true) And Test Eq false")
assert !sample("(Test Eq true Or Test Eq true) And (Test Eq false)")

assert sample("(Test Eq true Or Test Eq true) And (Test Eq false Or Test Eq true)")
assert !sample("(Test Eq true Or Test Eq true) And (Test Eq false Or Test Eq false)")

assert sample("(Test Eq true) Not Test Eq false And (Test Eq true)")
assert !sample("(Test Eq true) Not Test Eq true And (Test Eq true)")
assert !sample("(Test Eq false) Not Test Eq false And (Test Eq true)")
assert !sample("(Test1 Eq true) Not Test2 Eq false And (Test3 Eq false)")
end

def test_nots
assert sample("Test Eq true Not Test Eq false")
assert !sample("Test Eq true Not Test Eq true")
assert !sample("Test Eq false Not Test Eq true")
assert !sample("Test Eq false Not Test Eq false")

assert sample("Test Eq true And Test Eq true Not Test Eq false")
assert !sample("Test Eq false And Test Eq true Not Test Eq false")
assert !sample("Test Eq true And Test Eq true Not Test Eq true")
assert !sample("Test Eq true And Test Eq false Not Test Eq false")

assert sample("Test Eq true Not (Test Eq false Or Test Eq false)")
assert sample("Test Eq true Not (Test Eq false And Test Eq false)")
assert !sample("Test Eq true Not (Test Eq false Or Test Eq true)")
assert !sample("Test Eq true Not (Test Eq true Or Test Eq false)")
assert !sample("Test Eq true Not (Not Test Eq false)")
assert !sample("Test Eq false And Test Eq true Not Test Eq false")
assert !sample("Test Eq true Not (Test Eq true Or Test Eq true)")
assert !sample("Test Eq false Not (Test Eq false Or Test Eq false)")

assert sample("Test Eq true Not (Test Eq false And Test Eq false)")
assert sample("Test Eq true Not (Test Eq true And Test Eq false)")
assert sample("Test Eq true Not (Test Eq false And Test Eq true)")
assert !sample("Test Eq true Not (Test Eq true And Test Eq true)")
assert !sample("Test Eq false Not (Test Eq false And Test Eq false)")

assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq false)")
assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq false Or Test Eq true)")
assert sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq true)")
assert !sample("Test Eq true Not (Test Eq false Or Test Eq false) And (Test Eq false Or Test Eq false)")
assert !sample("Test Eq true Not (Test Eq false Or Test Eq true) And (Test Eq true Or Test Eq false)")
assert !sample("Test Eq true Not (Test Eq true Or Test Eq false) And (Test Eq true Or Test Eq false)")
assert !sample("Test Eq false Not (Test Eq false Or Test Eq false) And (Test Eq true Or Test Eq false)")
end

def test_unary_nots
Expand All @@ -109,17 +163,14 @@ def test_unary_nots

def test_unary_double_nots
assert sample("Not (Not(Not Test Eq true))")
assert !sample("Not (Not(Not Test Eq false))")

assert sample("Test Eq true Not (Not Test Eq true)")
assert !sample("Test Eq true Not (Not Test Eq false)")
assert !sample("Test Eq false Not (Not Test Eq true)")
end

def test_examples
# This one is based on a real life example that had problems.
#
# CurrentPrice Bt 130000.00,180000.00 And PropertySubType Eq 'Single Family Residence' And
# SchoolDistrict Eq 'Byron Center','Grandville','Jenison' And MlsStatus Eq 'Active' And
# BathsTotal Bt 1.50,9999.00 And BedsTotal Bt 3,99 And PropertyType Eq 'A'
# Not "Garage"."Garage2" Eq 'No' And "Pool"."OutdoorAbove" Eq true
# And "Pool"."OutdoorInground" Eq true Not "Substructure"."Michigan Basement" Eq true

assert !sample("Test Eq false And Test Eq true And " \
"Test Eq false And Test Eq true And " \
"Test Eq true And Test Eq true And Test Eq true " \
Expand Down