Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add string comparison support #99

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add string comparison support #99

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52975c1
feat: add string comparison support
eddycharly Jun 27, 2024
867d64a
Merge branch 'main' into cmp-strings
eddycharly Jun 27, 2024
9f8a53f
Merge branch 'main' into cmp-strings
eddycharly Jun 28, 2024
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
44 changes: 28 additions & 16 deletions pkg/interpreter/interpreter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,24 +121,36 @@ func (intr *treeInterpreter) execute(node parsing.ASTNode, value interface{}, fu
case parsing.TOKNE:
return !util.ObjsEqual(left, right), nil
}
leftNum, ok := util.ToNumber(left)
if !ok {
return nil, nil
}
rightNum, ok := util.ToNumber(right)
if !ok {
return nil, nil
if leftStr, ok := left.(string); ok {
if rightStr, ok := right.(string); ok {
switch node.Value {
case parsing.TOKGT:
return leftStr > rightStr, nil
case parsing.TOKGTE:
return leftStr >= rightStr, nil
case parsing.TOKLT:
return leftStr < rightStr, nil
case parsing.TOKLTE:
return leftStr <= rightStr, nil
}
}
}
switch node.Value {
case parsing.TOKGT:
return leftNum > rightNum, nil
case parsing.TOKGTE:
return leftNum >= rightNum, nil
case parsing.TOKLT:
return leftNum < rightNum, nil
case parsing.TOKLTE:
return leftNum <= rightNum, nil
if leftNum, ok := util.ToNumber(left); ok {
if rightNum, ok := util.ToNumber(right); ok {
switch node.Value {
case parsing.TOKGT:
return leftNum > rightNum, nil
case parsing.TOKGTE:
return leftNum >= rightNum, nil
case parsing.TOKLT:
return leftNum < rightNum, nil
case parsing.TOKLTE:
return leftNum <= rightNum, nil
}
}
}
// TODO: don't we want to return an error here ?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@springcomp WDYT about returning an error here ?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that this is already covered in the original code by returning nil, nil.
As this is a non-standard extension, I think we should return that as well if types do not match.

I like the attempt to cast as numbers and compare that.
This means that comparing number-like strings will produce the correct output.

Please, be aware that comparing strings may still exhibit confusing output.
For instance the following two strings will not be considered equal:

  • élément (é LATIN SMALL LETTER E WITH ACUTE ACCENT (U+00E9), …
  • élément (e LATIN SMALL LETTER E (U+0065), ◌́ COMBINING ACUTE ACCENT (U+0301), …

return nil, nil
case parsing.ASTExpRef:
return func(data interface{}) (interface{}, error) {
return intr.execute(node.Children[0], data, functionCaller)
Expand Down
25 changes: 25 additions & 0 deletions pkg/interpreter/interpreter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,31 @@ func TestCanSupportProjectionsWithStructs(t *testing.T) {
assert.Equal([]interface{}{"first", "second", "third"}, result)
}

func TestCompareStrings(t *testing.T) {
assert := assert.New(t)
data := []string{"a", "b", "c"}
{
result, err := search(t, "@[?@ > 'a']", data)
assert.Nil(err)
assert.Equal([]interface{}{"b", "c"}, result)
}
{
result, err := search(t, "@[?@ >= 'b']", data)
assert.Nil(err)
assert.Equal([]interface{}{"b", "c"}, result)
}
{
result, err := search(t, "@[?@ < 'b']", data)
assert.Nil(err)
assert.Equal([]interface{}{"a"}, result)
}
{
result, err := search(t, "@[?@ <= 'b']", data)
assert.Nil(err)
assert.Equal([]interface{}{"a", "b"}, result)
}
}

func TestCanSupportSliceOfStructsWithFunctions(t *testing.T) {
assert := assert.New(t)
data := []scalars{{"a1", "b1"}, {"a2", "b2"}}
Expand Down