expr-lang / expr

Expression language and expression evaluation for Go
https://expr-lang.org
MIT License
5.85k stars 378 forks source link

Enable comparison feature like python #664

Closed ckganesan closed 3 weeks ago

ckganesan commented 1 month ago

This pull request adds support for a comparison feature similar to Python.

ok not in [false] && 10 == 10 && 0.5 < 2 == ok not in [false]
1 > 2 < 3
30 > 19 in 18..31 != true
5 == x > 4
antonmedv commented 1 month ago

Expr already has chained comp operation (https://github.com/expr-lang/expr/pull/581)

1 < x < 9

One issue we encountered with implementing Python style ComparisonNode is backward comp.

In Expr next expressions parsed as left:

1 < 2 == true
(1 < 2) == true
(true) == true

So implementing == was not feasible. And we implemented only < > <= >=.

I think those comparison should cover much of the requirements.

Also I think allowing to use == leads to whore DX. For example, 30 > 19 in 18..31 != true how to read this?

30 > 19 in 18..31 != true
(30 > 19) in 18..31 != true
30 > (19 in 18..31) != true

@bizywizy may remember more.

bizywizy commented 1 month ago

Yes, I think it's too late to change this behaviour

42 == 42 == true // expr: true; python: false
13 < 42 == true // expr: true; python: false
13 > 2 == true // expr: true; python: false
ckganesan commented 1 month ago

Yes, I think it's too late to change this behaviour

42 == 42 == true // expr: true; python: false
13 < 42 == true // expr: true; python: false
13 > 2 == true // expr: true; python: false

This change won`t affect expr functionality but it will use the functionality like python

42 == 42 == true
CompareNode{
        Left: IntegerNode{
                Value: 42,
        },
        Operators: []string{
                "==",
                "==",
        },
        Comparators: []ast.Node{
                IntegerNode{
                        Value: 42,
                },
                BoolNode{
                        Value: true,
                },
        },
}
0  OpPush  <0>  42
1  OpPush  <0>  42
2  OpEqualInt
3  OpTrue
4  OpEqual

true
13 < 42 == true
CompareNode{
        Left: IntegerNode{
                Value: 13,
        },
        Operators: []string{
                "<",
                "==",
        },
        Comparators: []ast.Node{
                IntegerNode{
                        Value: 42,
                },
                BoolNode{
                        Value: true,
                },
        },
}
0  OpPush  <0>  13
1  OpPush  <1>  42
2  OpLess
3  OpTrue
4  OpEqual

true
13 > 2 == true
CompareNode{
        Left: IntegerNode{
                Value: 13,
        },
        Operators: []string{
                ">",
                "==",
        },
        Comparators: []ast.Node{
                IntegerNode{
                        Value: 2,
                },
                BoolNode{
                        Value: true,
                },
        },
}
0  OpPush  <0>  13
1  OpPush  <1>  2
2  OpMore
3  OpTrue
4  OpEqual

true
ckganesan commented 1 month ago

Expr already has chained comp operation (#581)

1 < x < 9

One issue we encountered with implementing Python style ComparisonNode is backward comp.

In Expr next expressions parsed as left:

1 < 2 == true
(1 < 2) == true
(true) == true

So implementing == was not feasible. And we implemented only < > <= >=.

I think those comparison should cover much of the requirements.

Also I think allowing to use == leads to whore DX. For example, 30 > 19 in 18..31 != true how to read this?

30 > 19 in 18..31 != true
(30 > 19) in 18..31 != true
30 > (19 in 18..31) != true

@bizywizy may remember more.

The == and != operators function based on the right-hand expression. If the right-hand expression is a boolean, the left-hand expression will be evaluated as a boolean. Ex : 30 > 19 in 18..31 != true

30 > 19 && (18 <= 19 <= 31) != true
CompareNode{
        Left: IntegerNode{
                Value: 30,
        },
        Operators: []string{
                ">",
                "&&",
                "!=",
        },
        Comparators: []ast.Node{
                IntegerNode{
                        Value: 19,
                },
                CompareNode{
                        Left: IntegerNode{
                                Value: 18,
                        },
                        Operators: []string{
                                "<=",
                                "<=",
                        },
                        Comparators: []ast.Node{
                                IntegerNode{
                                        Value: 19,
                                },
                                IntegerNode{
                                        Value: 31,
                                },
                        },
                },
                BoolNode{
                        Value: true,
                },
        },
}
0   OpPush  <0>  30
1   OpPush  <1>  19
2   OpMore
3   OpJumpIfFalse  <9>  (13)
4   OpPop
5   OpPush  <2>  18
6   OpPush  <1>  19
7   OpLessOrEqual
8   OpJumpIfFalse  <4>  (13)
9   OpPop
10  OpPush  <1>  19
11  OpPush  <3>  31
12  OpLessOrEqual
13  OpTrue
14  OpEqual
15  OpNot

false
antonmedv commented 1 month ago

Then I don't understand what this PR tries to implement. What's different?

ckganesan commented 1 month ago

Then I don't understand what this PR tries to implement. What's different?

This PR supports all comparison operators(>, >=, <, <=, in, !=, ==, in, matches, contains, startsWith, endsWith) in chained comparison operations.

antonmedv commented 1 month ago

But why?