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.

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

Spec change - merge() function #4

Open
glenveegee opened this issue May 18, 2020 · 2 comments
Open

Spec change - merge() function #4

glenveegee opened this issue May 18, 2020 · 2 comments

Comments

@glenveegee
Copy link

Background

In all the example given in the spec page for the merge() functionLITERAL types are provided as arguments to the function. While this demonstrates the behaviour it is not at all useful in practical terms. Literals can only be explicitly defined by the editor of an expression. Much more useful would be to merge on results of expressions or on projections.

Examples:

search([{a: "AY", x: "EX"}, {a: "EY", b: "BEE"}], 'merge(@)')
// OUTPUTS: {"a": "EY", "b": "BEE", "x": "EX"}

search([{a: "AY", x: "EX"}, {a: "EY", b: "BEE"}], 'merge([{FED: @[0].x}, {BAR: @[1].b}])')
// OUTPUTS: {"BAR": "BEE", "FED": "EX"}

In order to do this merge would have to accept either TYPE_OBJECT or TYPE_ARRAY to work and the function would need to be able to switch on either type to handle this.

Discussion

Array can contain any value and ideally we may want to restrict it to a new TYPE_ARRAY_OBJECT type so that we can guarantee only objects can be used in the merge. One interesting side effect of this in javascript is that merging arrays results in array members being keyed in their index. This means we can do the following with very little code change:

search([{"A": "ONE"}, "TWO", {"C": "THREE"}], 'merge(@)')
// OUTPUTS: {"0": "ZERO", "A": "ONE", "C": "THREE"}

// With all JSON array member types:

search([{"A": "ONE"}, true, null, "BAR", ["ZERO"], 666], 'merge(@)')
// OUTPUTS: {"0": "ZERO", "1": "A", "2": "R", "A": "ONE"}

Implementation in jmespath.ts

// Runtime
  private functionMerge: RuntimeFunction<JSONObject[], JSONObject> = resolvedArgs => {
    let merged = {};
    for (let i = 0; i < resolvedArgs.length; i += 1) {
      const current = resolvedArgs[i];
      if (Array.isArray(current)) {
        merged = Object.assign(merged, ...current);
      } else {
        merged = Object.assign(merged, current);
      }
    }
    return merged;
  };

// and in the function table

    merge: {
      _func: this.functionMerge,
      _signature: [
        {
          types: [InputArgument.TYPE_OBJECT, InputArgument.TYPE_ARRAY],
          variadic: true,
        },
      ],
    },
@pelepelin
Copy link

I totally 👍 to motivation section and examples in "examples" section. It seems for now [{"a": "b"}, {"c": "d"}] input can't be flatten with either merge(@) or [].

However, the first example in "discussion" section looks buggy, and the second is quite counter-intuitive.

@jkupferer
Copy link

I still like this proposal. I certainly expected that I could pass a list of dictionaries to merge and was surprised that it did not work as I expected.

In gitter chat hell-racer suggested use of from_items(zip([].keys(@)[], [].values(@)[])) as equivalent to merge(@) in this case when dealing with a list of dictionaries.
https://app.gitter.im/#/room/#jmespath_chat:gitter.im/$_o5kjoa8v_annVi8CkeY8zWKXcpK5X6Ck6iVxyOKdh8

It does what I need, but looks a bit mysterious. It sure would be nice if merge could just handle this use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants