json-logic-js icon indicating copy to clipboard operation
json-logic-js copied to clipboard

Consistent, language independent logic

Open YaraslauZhylko opened this issue 6 years ago • 6 comments

I was trying to find a way to store simple automation rules in the database and pass them between UI and backend. And json-logic seems to be a perfect fit for that purpose.

So I took the Python port of it and found out it was not working as expected in some cases. Trying to mend the port by creating a pull request I referred to this repo to get the etalon behaviour.

But I soon found out that some operators (only numeric operations so far) behave inconsistently or are dependent on JavaScript logic. While they should probably be following human (or Vulcan :) ) logic to make json-logic language-independent.

Here are some examples that I've tested using the "Play with It" page:

Arithmetic:

  • "+": {"+": []} = 0 // Should be null, cannot calculate the sum of nothing {"+": [2]} = 2 // OK {"+": [2,3]} = 5 // OK {"+": [2,3,4]} = 9 // OK, chainable {"+": [2,3,4,5]} = 14 // OK {"+": [null]} = null // OK, cannot calculate the sum of nothing {"+": [2,null,4]} = null // OK, cannot calculate the sum of numbers and nothing

  • "-": {"-": []} = null // OK, cannot calculate the difference of nothing {"-": [10]} = -10 // OK, described in the docs as negation {"-": [10,2]} = 8 // OK {"-": [10,2,3]} = 8 // Should be 5, as subtraction should also be "chainable" like addition {"-": [10,2,3,4]} = 8 // Should be 1, see above {"-": [null]} = 0 // Should be null, cannot calculate the difference of nothing; compare to results for {"+": [null]} {"-": [2,null,4]} = 2 // Should be null, cannot calculate the difference of numbers and nothing

  • "*": {"*": []} = no output // Should be null, cannot calculate the product of nothing {"*": [2]} = 2 // OK {"*": [2,3]} = 6 // OK {"*": [2,3,4]} = 24 // OK, chainable {"*": [2,3,4,5]} = 120 // OK {"*": [null]} = null // OK, cannot calculate the product of nothing; compare to results for {"-": [null]} {"*": [2,null,4]} = null // OK, cannot calculate the product of numbers and nothing, compare to results for {"-": [2,null,4]}

  • "/": {"/": []} = null // OK, cannot calculate the quotient of nothing {"/": [10]} = null // Should be 10, compare to results for {"*": [2]} {"/": [10,2]} = 5 // OK {"/": [10,2,4]} = 5 // Should be 1.25, as division should also be "chainable" like addition or multiplication {"/": [10,2,4,5]} = 5 // Should be 0.25, see above {"/": [null]} = null // OK, cannot calculate the quotient of nothing; compare to results for {"-": [null]} {"/": [2,null,4]} = null // OK, cannot calculate the quotient of numbers and nothing, compare to results for {"-": [2,null,4]}

  • "%": {"%": []} = null // OK, cannot divide nothing {"%": [10]} = null // Should be 10, compare to results for {"*": [2]} {"%": [10,4]} = 2 // OK {"%": [10,4,2]} = 2 // Should be 0, as module should also be "chainable" {"%": [null]} = null // OK, cannot calculate divide nothing {"%": [2,null]} = null // OK, cannot divide numbers by nothing {"%": [null,2]} = 0 // Should be null, cannot divide nothing by numbers

Comparison:

  • ">": {">": []} = false // OK, cannot compare nothing {">": [5]} = false // OK {">": [5,2]} = true // OK {">": [5,2,3]} = true // Should be false, should be chainable, see results of {"<": [2,6,5]} {">": [5,2,4,3]} = true // Should be false, see above; not sure whether 3+ element chaining should be allowed {">": [null]} = false // OK, one cannot compare nothing {">": [5,null]} = true // Should be false, cannot compare numbers to nothing

  • ">=": {">=": []} = false // OK, cannot compare nothing {">=": [5]} = false // OK {">=": [5,2]} = true // OK {">=": [5,2,3]} = true // Should be false, should be chainable, see results of {"<": [2,6,5]} {">=": [5,2,4,4]} = true // Should be false, see above; not sure whether 3+ element chaining should be allowed {">=": [null]} = false // OK, cannot compare nothing {">=": [5,null]} = true // Should be false, cannot compare numbers to nothing

  • "<": {"<": []} = false // OK, cannot compare nothing {"<": [2]} = false // OK {"<": [2,5]} = true // OK {"<": [2,6,5]} = false // OK, chainable, see results of {">": [5,2,3]} {"<": [2,5,6,1]} = true // Should be false, last element is less than the previous; not sure whether 3+ element chaining should be allowed {"<": [null]} = false // OK, cannot compare nothing {"<": [null,2]} = true // Should be false, cannot compare numbers to nothing

  • "<=": {"<=": []} = false // OK, cannot compare nothing {"<=": [2]} = false // OK {"<=": [2,5]} = true // OK {"<=": [2,6,5]} = false // OK, chainable, see results of {">": [5,2,3]} {"<=": [2,5,6,1]} = true // Should be false, last element is less than the previous; not sure whether 3+ element chaining should be allowed {"<=": [null]} = false // OK, one cannot compare nothing {"<=": [null,2]} = true // Should be false, cannot compare numbers to nothing

Min/max:

  • "min": {"min": []} = null // OK, cannot choose from nothing {"min": [2]} = 2 // OK {"min": [3,2]} = 2 // OK {"min": [4,3,2]} = 2 // OK, chainable {"min": [5,4,3,2]} = 2 // OK, chainable {"min": [null]} = 0 // Should be null, null is not 0 {"min": [5,null]} = 0 // Should be null, cannot choose lowest from numbers and nothing

  • "max": {"max": []} = null // OK, cannot choose from nothing {"max": [2]} = 2 // OK {"max": [2,3]} = 3 // OK {"max": [2,3,4]} = 4 // OK, chainable {"max": [2,3,4,5]} = 5 // OK, chainable {"max": [null]} = 0 // Should be null, null is not 0 {"max": [5,null]} = 5 // Should be null, cannot choose highest from numbers and nothing

P.S.: Comments above assume that all null values must be treated as bad values for arithmetic. But if users want to treat null as 0 or 1 they may always use the default value argument for "var".

I think that code for those operators should be reviewed to ensure etalon, language/platform-independent behaviour of json-logic.

What's your opinion, @jwadhams?

YaraslauZhylko avatar Nov 26 '17 23:11 YaraslauZhylko

I also suppose that the following operators should be made chainable too (with minor modifications regarding number of arguments and null values):

  • "==": {"==": []} = false // Cannot compare nothing {"==": [5]} = false // Cannot compare a single value {"==": [5,3]} = false {"==": [5,5]} = true {"==": [5,"5"]} = true {"==": [5,5,1]} = false {"==": [5,5.0,"5",{"+":[2,3]}]} = true {"==": [null]} = false // Cannot compare nothing {"==": [null,null]} = false // Cannot compare nothing to nothing, there's a "missing" operator for that {"==": [null,0]} = false // Cannot compare nothing to real value

  • "===": {"===": []} = false // Cannot compare nothing {"===": [5]} = false // Cannot compare a single value {"===": [5,3]} = false {"===": [5,5]} = true {"===": [5,"5"]} = false {"===": [5,5,1]} = false {"===": [5,5.0,"5",{"+":[2,3]}]} = false {"===": [null]} = false // Cannot compare nothing {"===": [null,null]} = false // Cannot compare nothing to nothing, there's a "missing" operator for that {"===": [null,0]} = false // Cannot compare nothing to a real value

  • "!=": {"!=": []} = false // Cannot compare nothing {"!=": [5]} = false // Cannot compare a single value {"!=": [5,3]} = true {"!=": [5,5]} = false {"!=": [5,"5"]} = false {"!=": [5,5,1]} = false {"!=": [5,5.0,"5",{"+":[2,3]}]} = false {"!=": [null]} = false // Cannot compare nothing {"!=": [null,null]} = false // Cannot compare nothing to nothing, there's a {"!": {"missing": "something"}} construct for that {"!=": [null,0]} = false // Cannot compare nothing to a real value

  • "!==": {"!==": []} = false // Cannot compare nothing {"!==": [5]} = false // Cannot compare a single value {"!==": [5,3]} = true {"!==": [5,5]} = false {"!==": [5,"5"]} = true {"!==": [5,5,1]} = false {"!==": [5,5.0,"5",{"+":[2,3]}]} = false {"!==": [null]} = false // Cannot compare nothing {"!==": [null,null]} = false // Cannot compare nothing to nothing, there's a {"!": {"missing": "something"}} construct for that {"!==": [null,0]} = false // Cannot compare nothing to a real value

  • "!!": {"!!": []} = false // Cannot evaluate nothing {"!!": [true]} = true {"!!": [false]} = false {"!!": [true,1]} = true {"!!": [5,2]} = true {"!!": [5,false]} = false {"!!": [5,5,false]} = false {"!!": [5,5.0,"5",{"+":[2,3]}]} = true {"!!": [null]} = false // Cannot evaluate nothing, there's a {"!": {"missing": "something"}} construct for that {"!!": [null,null]} = false // Cannot evaluate nothing multiple times, there's a {"!": {"missing": ["something", "something_else"]}} construct for that {"!!": [null,0]} = false // Cannot evaluate nothing together with other items

  • "!": {"!": []} = false // Cannot evaluate nothing {"!": [true]} = false {"!": [false]} = true {"!": [true,1]} = false {"!": [5,2]} = false {"!": [0,false]} = true {"!": [0,0,true]} = false {"!": [0,0.0,"",{"-":[2,2]}]} = true {"!": [null]} = false // Cannot evaluate nothing, there's a {"missing": "something"} construct for that {"!": [null,null]} = false // Cannot evaluate nothing multiple times, there's a {"missing": ["something", "something_else"]} construct for that {"!": [null,0]} = false // Cannot evaluate nothing together with other items

YaraslauZhylko avatar Dec 01 '17 12:12 YaraslauZhylko

Alternatively, all comparison operators could be forced to take exactly 2 arguments and evaluation operators - exactly 1 argument. Throwing exceptions if there number is incorrect.

  • "==": {"==": []} = exception {"==": [5]} = exception {"==": [5,3]} = false {"==": [5,5]} = true {"==": [5,"5"]} = true {"==": [5,5,1]} = exception {"==": [5,5.0,"5",{"+":[2,3]}]} = exception {"==": [null,null]} = false // Cannot compare nothing to nothing, there's a "missing" operator for that {"==": [null,0]} = false // Cannot compare nothing to real value

  • "===": {"===": []} = exception {"===": [5]} = exception {"===": [5,3]} = false {"===": [5,5]} = true {"===": [5,"5"]} = false {"===": [5,5,1]} = exception {"===": [5,5.0,"5",{"+":[2,3]}]} = exception {"===": [null,null]} = false // Cannot compare nothing to nothing, there's a "missing" operator for that {"===": [null,0]} = false // Cannot compare nothing to a real value

  • "!=": {"!=": []} = exception {"!=": [5]} = exception {"!=": [5,3]} = true {"!=": [5,5]} = false {"!=": [5,"5"]} = false {"!=": [5,5,1]} = exception {"!=": [5,5.0,"5",{"+":[2,3]}]} = exception {"!=": [null,null]} = false // Cannot compare nothing to nothing, there's a {"!": {"missing": "something"}} construct for that {"!=": [null,0]} = false // Cannot compare nothing to a real value

  • "!==": {"!==": []} = exception {"!==": [5]} = exception {"!==": [5,3]} = true {"!==": [5,5]} = false {"!==": [5,"5"]} = true {"!==": [5,5,1]} = exception {"!==": [5,5.0,"5",{"+":[2,3]}]} = exception {"!==": [null,null]} = false // Cannot compare nothing to nothing, there's a {"!": {"missing": "something"}} construct for that {"!==": [null,0]} = false // Cannot compare nothing to a real value

  • ">": {">": []} = exception {">": [5]} = exception {">": [5,2]} = true {">": [5,2,3]} = exception {">": [5,2,4,3]} = exception {">": [5,null]} = false // Cannot compare numbers to nothing

  • ">=": {">=": []} = exception {">=": [5]} = exception {">=": [5,2]} = true {">=": [5,2,3]} = exception {">=": [5,2,4,4]} = exception {">=": [5,null]} = false // Cannot compare numbers to nothing

  • "<": {"<": []} = exception {"<": [2]} = exception {"<": [2,5]} = true {"<": [2,6,5]} = exception {"<": [2,5,6,1]} = exception {"<": [null,2]} = false // Cannot compare numbers to nothing

  • "<=": {"<=": []} = exception {"<=": [2]} = exception {"<=": [2,5]} = true {"<=": [2,6,5]} = exception {"<=": [2,5,6,1]} = exception {"<=": [null,2]} = false // Cannot compare numbers to nothing

  • "!!": {"!!": []} = exception {"!!": [true]} = true {"!!": [false]} = false {"!!": [true,1]} = exception {"!!": [5,2]} = exception {"!!": [5,false]} = exception {"!!": [5,5,false]} = exception {"!!": [5,5.0,"5",{"+":[2,3]}]} = exception {"!!": [null]} = false // Cannot evaluate nothing, there's a {"!": {"missing": "something"}} construct for that

  • "!": {"!": []} = exception {"!": [true]} = false {"!": [false]} = true {"!": [true,1]} = exception {"!": [5,2]} = exception {"!": [0,false]} = exception {"!": [0,0,true]} = exception {"!": [0,0.0,"",{"-":[2,2]}]} = exception {"!": [null]} = false // Cannot evaluate nothing, there's a {"missing": "something"} construct for that

But in that case we'll probably have to dump the "between" functionality and replace it with "and":

  • {"<": [1,{"var": "something"},5]} => {"and": [{"<": [1,{"var": "something"}]}, {"<": [{"var": "something"},5]}]}
  • {"<=": [1,{"var": "something"},5]} => {"and": [{"<=": [1,{"var": "something"}]}, {"<=": [{"var": "something"},5]}]}

YaraslauZhylko avatar Dec 01 '17 12:12 YaraslauZhylko

Or we could spare the "<" and "<=" operators allowing them to have a third optional argument.

  • "<": {"<": []} = exception {"<": [2]} = exception {"<": [2,5]} = true {"<": [2,6,5]} = false {"<": [2,5,6,1]} = exception {"<": [null,2]} = false // Cannot compare numbers to nothing

  • "<=": {"<=": []} = exception {"<=": [2]} = exception {"<=": [2,5]} = true {"<=": [2,6,5]} = false {"<=": [2,5,6,1]} = exception {"<=": [null,2]} = false // Cannot compare numbers to nothing

But that would sacrifice consistency...

YaraslauZhylko avatar Dec 01 '17 13:12 YaraslauZhylko

@jwadhams,

I've already forked the repository and made changes described in the first two posts on my local machine. I'm ready to commit them and initiate a pull request as soon as you decide whether those changes are viable for the project.

I'm also ready to update the Python repository to match the latest JS version if needed.

YaraslauZhylko avatar Dec 01 '17 13:12 YaraslauZhylko

A small example of why I consider nulls should be ruled out:

Let's imagine a simple thermostat that reads room temperature and turns the heating on when it's cold.

It has the following rule:

{"if": [
    {"<": [{"var": "room_temperature"}, 15]}, "We are freezing! Turn all heating on!",
    "Keep calm and carry on."
]}

If it gets normal temperature readings it does nothing:

{"room_temperature": 23.5} => "Keep calm and carry on."

If the temperature gets a little colder the thermostat turns the heating on:

{"room_temperature": 9.5} => "We are freezing! Turn all heating on!"

But what is the data is missing? The sensor gets damaged or disconnected? Connection drops? Or there's just no data yet?

{"room_temperature": null} => "We are freezing! Turn all heating on!"
{} => "We are freezing! Turn all heating on!"

That's what we'll get!

Json Logic decides that it's time to panic and switch everything on just because we do not have a value to analyse. The rule is all about analysing a valid value. It cannot make decisions without it. But it actually does. Because it assumes null to be 0. And 0 < 15.

How can we tell if the apple is tasty when we've got none? Let's just assume it is tasty without actually tasting it? :) The rule should fail stating that the apple IS NOT tasty. Want to check if it is sour? Add another rule. But it should also fail stating that the apple IS NOT sour either. Every assumption should fail if we got no valid data.

We can write a correct rule to check for value existence. But in current Json Logic implementation we'll have to overcomplicate it:

{"if": [
    {"and": [
        {"!": [{"missing": "room_temperature"}]},
        {"<": [{"var": "room_temperature"}, 15]}
    ]}, "We are freezing! Turn all heating on!",
    "Keep calm and carry on."
]}

Looks ugly to me...

And even more ugly if actual values are retrieved on demand or require some other form of extensive calculation. Because under the hood the "missing" operator actually retrieves the value to make sure it is not null or empty string.

But if we rule nulls out of the evaluation the original "pretty" rule will return the expected result according to the human logic.

{"room_temperature": 23.5} => "Keep calm and carry on."
{"room_temperature": 9.5} => "We are freezing! Turn all heating on!"
{"room_temperature": null} => "Keep calm and carry on."
{} => "Keep calm and carry on."

Even if we add another "else if" statement...

{"if": [
    {"<": [{"var": "room_temperature"}, 15]}, "We are freezing! Turn all heating on!",
    {">=": [{"var": "room_temperature"}, 15]}, "I'm comfortable with that...",
    "Keep calm and carry on."
]}

...the results will still be acceptable for human logic as none of the conditions will apply:

{"room_temperature": 23.5} => "I'm comfortable with that..."
{"room_temperature": 9.5} => "We are freezing! Turn all heating on!"
{"room_temperature": null} => "Keep calm and carry on."
{} => "Keep calm and carry on."

But what about the guys and gals who still want nulls to be treated as 0's? There desires will be much easer to implement compared to using the "missing" operator:

{"var": ["room_temperature", 0]}

That's all folks! :)

You just explicitly state: "If there's no value use 0 instead." And from now on there's no more assumptions as to how the empty value is treated.

P.S.: Implicit assumptions get even worse when it comes to arithmetic where "+", "-", "*" and "/" operators treat nulls differently...

P.P.S.: Nothing to say about different implicit assumptions in different programming languages. :) And Json Logic is supposed to act precisely the same in all of them to be efficient.

YaraslauZhylko avatar Dec 02 '17 21:12 YaraslauZhylko

This is excellent. Has anyone considered making a semi-formal (or formal) specification for JSON logic? I don't see something resembling a "spec", I looked at https://jessemitchell.me/json-logic-engine/ too.

Edit: I just saw that there is https://jsonlogic.com/tests.json, maybe that is the "spec"?

josephdpurcell avatar Jan 22 '23 02:01 josephdpurcell