react-querybuilder/react-querybuilder

Disable Simplification when Formatting jsonLogic Query

Closed this issue · 12 comments

Description of the Feature

I would like to request the addition of a feature that allows the disabling of automatic simplification when mapping DefaultRuleGroupType to JsonLogic using the React Query Builder. Currently, when a rule group contains a single rule and uses an and or or combinator, it simplifies the output by removing the combinator and returning only the single rule. There are other examples of this simplification but this is the simplest to demonstrate. This feature could introduce an option, perhaps disableSimplification, that maintains the original group structure in the JsonLogic output, regardless of the number of rules it contains. Ideally a bijection would be exist between the DefaultRuleGroupType and JsonLogic

Use Case

In certain scenarios, especially when integrating with systems that expect specific JsonLogic structures, the simplification of rule groups can lead to unexpected behavior or errors. For instance, consider a system that interprets the presence of an and combinator in a specific way, even if it contains only one rule. The current behavior, where {"and": [{"var": "temperature", ">": 22}]} is simplified to {"var": "temperature", ">": 22}, might not trigger the desired logic in such systems.

Example:

  • Input with React Query Builder:

{ "combinator": "and", "rules": [ { "field": "temperature", "value": 22, "operator": ">" } ] }

  • Current jsonLogicFormat output:

{"var": "temperature", ">": 22}

  • Desired output with disableSimplification enabled:

{"and": [{"var": "temperature", ">": 22}]}

Thanks for taking a look!

I can definitely see why retaining the group hierarchy—irrespective of the number of nested rules—would be preferable in some cases, so this seems like a good idea. Also, I generally try to make the output of parseX(formatQuery(query, 'X')) equivalent to query (X being "JsonLogic" in this case). An option to retain grouping would make that more likely.

I'll see if I can come up with something in the next few days and then run it by you. I need to check if it would be applicable to any other export formats.

I should have remembered this since it was only a couple of months ago, but we discussed this issue in #609 and #664. As of v7, the only collapsing done is when the outermost group (the query object itself) has a single child object. When that happens the lone child rule/group determines the JsonLogic structure and the root group is essentially ignored.

Honestly I'm not sure why I did it that way, either originally or as updated in v7. #609 (comment) sounds like I may have been trying to avoid breaking changes from the original implementation.

I'm thinking about removing all "collapse" conditions, but I need to do some research first (and self-reflection apparently!) to figure out if there was ever a good reason for collapsing anything. Wouldn't want to introduce a regression.

Totally understandable! I had found those issues as well and thought about just commenting there.

While you are introspective, figured I should bring up that I think collapse also occurs when child groups have a single rule. The child combinator is ignored and the rule is absorbed into the parent it appears.

Thanks for reconsidering this feature!

Also observing some weird behavior when mapping a Field that has ValueEditorType of multiselect, to JsonLogic with formatQuery. The result is undefined when using the in operator and and empty array as the value, [].

For example

{ "combinator": "and", "rules": [ { "field": "color", "value": [], "operator": "in" } ] }

is not mapping.

I've tried similarly using

{ "combinator": "and", "rules": [ { "field": "color", "value": "", "operator": "in" } ] }

which just collapses to the same jsonLogic as if the DefaultRuleGroupType were

{ "combinator": "and", "rules": [ { "field": "color", "value": "", "operator": "==" } ] }

Im assuming removing collapse would solve the issue but wanted to bring it to your attention.

Also observing some weird behavior when mapping a Field that has valueEditorType of multiselect, to JsonLogic with formatQuery. The result is undefined when using the in operator and and empty array as the value, [].

This is intentional, since "x in an empty array" would be necessarily false for any value of x. This is similar to how WHERE fieldName IN () is invalid SQL. Relevant code here (the "else" condition of line 53):

case 'in':
case 'notIn': {
// TODO: extract this map function
const valueAsArray = toArray(value).map(fieldOrNumberRenderer);
if (valueAsArray.length > 0) {
const jsonRule: RQBJsonLogic = { in: [fieldObject, valueAsArray] };
return negateIfNotOp(operator, jsonRule);
}
return false;
}

I created #692 to resolve the original issue from this thread but I'll probably wait to merge it until I get some more feedback.

Yes, while you are correct that applying the query would not match any results, this should be out of scope for a compiler and is subsequently causing us issues. Imagine if, for example, SQL returned a syntax error when it compiled verifiably empty queries instead of returning empty results.

Just to confirm, I checked that on the jsonLogic Supported Operations page that the query in question returns without error if you want to check for yourself.

Thanks for creating the PR!

That makes sense. FWIW I didn't mean to appear to justify the existing logic, just explain why it was implemented that way. Avoiding invalid SQL shouldn't be a reason to cause unnecessary problems with some other format LOL.

I'll update the PR to process all arrays regardless of size. I should probably check the other formats (MongoDB comes to mind first) and make sure they don't need the same update.

Edit: updated.

This is fixed in v7.2.1.

Thanks for fixing this!

Wondering if parseJsonLogic was removed in the update? Is there an alternative if so since I was using it to map jsonLogic to the DefaultRuleGroupType

It wasn't removed, but you have to import it like this in v7:

import { parseJsonLogic } from 'react-querybuilder/parseJsonLogic';

See v7 migration guide.

Ahh maybe I should read the docs.... Thank you!

PS: Just a heads up that this is now working for our project! Thanks for being so responsive!

That's great to hear! Happy to help.