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

Async and sync in the same evaluation #77

Closed
Allam76 opened this issue Sep 13, 2017 · 11 comments
Closed

Async and sync in the same evaluation #77

Allam76 opened this issue Sep 13, 2017 · 11 comments

Comments

@Allam76
Copy link

Allam76 commented Sep 13, 2017

Hello again. I have another issue I would need some input on.

I work with knex, a sql builder that uses chained methods to build the sql and then at the end uses a promise.
Converted jsonata to work with async/await simply by copy paste whenever it was generator functions. This works reasonably well.

Now I need to use both in the same evaluation, first chain a few methods in sync, run await and then await on all the following expressions.

Could we do a ~([expression]) addition to the syntax or something of the sort to tell jsonata that we should await?

Many thanks
Martin

@andrew-coleman
Copy link
Member

Hi Martin. Thanks for raising this. At the moment I am struggling to understand exactly the issue that are describing, so if you could expand with code examples, that would be really useful.

I’m wondering if this is an issue with the jsonata language itself, or with the implementation. JSONata is designed as a declarative language and not an imperative language. In other words, the syntax is used to describe the desired result and not the steps a computer has to take to achieve that result. So there is no syntax to control the flow of execution. I only say that because that new syntax you have proposed looks as if its purpose is to control the flow of execution. I also need to mindful of not tying jsonata into only being implementable in javascript. I know of a group who are creating a native implementation in Java.

Having said all that, I’m keen to understand the issue further so that we can enable your use-case.

@Allam76
Copy link
Author

Allam76 commented Sep 14, 2017

Hello Andrew.
Jsonata becomes a very versatile general purpose tool with these small additions. So moving a little bit outside of the philosophy could reap huge benefits. I'm using this on a custom platform almost identical to node-red you have at IBM.

I see jsonata as a json object manipulation tool. With 3 lines of code i could extend to also accept data from a database via an orm. So in fact, I see my database as one gigantic json blob.... Perfect for jsonata. I believe we don't need much imagination to see the power in that. However in some cases then the performance is not that good and being able to use the power of the orm is critical.

if you want to build a sql string with an orm, you do something like: meta.build().select("*").from("table") and get an output: "SELECT * FROM "table"". Then at the end of these chained methods you wait for the promise. Now to get that to work, I need to tell jsonata when to wait and when not to. In this case after all functions. In another case however, say: "$meta($first().second)" I would like to retrieve the result after each method, thereby not waiting until the end.

Jsonata does the first by standard and the second in my extension, I need both.....

Hope this makes sense.
Cheers Martin

@Allam76
Copy link
Author

Allam76 commented Sep 14, 2017

Sorry one more thing. The first case works perfectly well with vanilla jsonata, so the "imperative" part works very well. It is when I want to mix, I have problems.

@andrew-coleman
Copy link
Member

andrew-coleman commented Oct 18, 2017

The problem here, I think, is that the knex Builder object which chains all the select/from/etc methods is also a thenable object. When jsonata sees that, it always tries to invoke the then function as part of its async processing. However, for knex, we only want this to happen at the end of the function chain. I have created a fix for this in branch thenable together with a set of unit tests that invoke a trivial example of this 'chainable/thenable' pattern. I have also installed and tested with with knex to verify that expression such as $knex.select().from("authors").where("isbn", "=", $knex.select().from("books").isbn) work as expected.

Let me know if this works for you.

@Allam76
Copy link
Author

Allam76 commented Oct 20, 2017

First of all, tremendous thanks for building this addition, it is of great value here!:-)

I made my own tests and confirm that it works as defined in the unit tests.

So now we are almost there, there is only one small piece missing and that is to wait for the promise inside the expression. I see that using the standard $count function it does wait for the promise to fulfill before the count is called but this does not seems to work for custom functions...

Example:
( $vari := $count($knex("business_document").select().where({"id":"1"})); {"return":$vari} )
works as the result becomes {return:1}. However with a custom dummy function I get:
( $vari := $dummy($knex("business_document").select().where({"id":"1"})); {"return":$vari} )
{return:Builder}. That is, the knex builder and not the result.

Is there any way to add this functionality also to custom functions? Or add a dummy function to the standard ones?

Again many, many thanks for building this. It makes jsonata even more awsome!

@andrew-coleman
Copy link
Member

andrew-coleman commented Oct 20, 2017

Hmm, that's odd. The following code works for me, could you try running this, please:

var jsonata = require('jsonata');
var knex = require('knex')({
    client: 'sqlite3',
    connection: {
        filename: "./mydb.sqlite"
    },
    useNullAsDefault: true
});

expression = '( $vari := $dummy($knex("business_document").select().where({"id":"1"})); {"return":$vari} )';

var dummy = function(arg) {
    console.log('DUMMY', arg);
    return arg;
};

var bindings = {
    'knex': knex,
    'dummy': dummy
};

var expr = jsonata(expression);

var callback = function(err, result) {
    if(err) {
        console.log('ERROR:', err);
    } else {
        console.log('Result:', result);
    }
};

expr.evaluate({}, bindings, callback);

I get the following output:

DUMMY { id: '1', surname: 'coleman', isbn: '0123456789' }
Result: { return: { id: '1', surname: 'coleman', isbn: '0123456789' } }

so the value of arg passed to custom function dummy is the result from the DB call. I'd be interested in seeing your code.

@Allam76
Copy link
Author

Allam76 commented Oct 20, 2017

The difference seems to be that I use async/await.

With a callback as in your example above, I get the same result as you. With the following:
var result = await expression.evaluate({}, bindings); I get the builder in the result.

Can I conclude that I should avoid using async/await and wrap my callback instead? Could this be made to work with async/await?

Many, many thanks, Martin

@andrew-coleman
Copy link
Member

Hi Martin,
Yes, you do need to supply the callback argument to the evaluate method. This switches the engine into a mode that supports asynchronous functions (custom functions that return promises). Without this callback, the engine reverts to a pure synchronous mode (for backwards compatibility reasons prior to v1.2). As you say, you can wrap the callback instead, as I've done in the unit tests:

var jsonataPromise = function(expr, data, bindings) {
    return new Promise(function(resolve, reject) {
        expr.evaluate(data, bindings, function(error, response) {
            if(error) reject(error);
            resolve(response);
        });
    });
};

Given you now have this working, I'll merge thenable into master, and publish a minor release to NPM.

@Allam76
Copy link
Author

Allam76 commented Oct 21, 2017

Hello Andrew,
Thank you so much, this is a wonderful feature!

Speaking about merging, when will the 1.4 be merged as well? That is another extremely useful feature what is tremendously useful for us.

Many thanks, Martin

@Allam76
Copy link
Author

Allam76 commented Oct 24, 2017

Hello Andrew,

Today I tested with chained method with binding and then it did not go so great:

var bindings = {
    'knex': knex,
    'dummy': dummy
};
var expression = jsonata(`$knex($.table).select().where({"last_name":$.name})`);
expression.evaluate({table:"contract_party", name:"Dumont"},bindings,(err,res)=>{
     console.log(err);
     console.log(res);
});

In this case table input (contract_party) is picked up correctly but name (Dumont) is evaluated to undefined in the where method.

UPDATE:
This can be solved by using $$ instead of $ as context:

var expression = jsonata(`$knex($.table).select().where({"last_name":$$.name})`);

Note that some times when I run the above, the jsonata inside the where method does not get executed, strange error I cannot reproduce anymore.

Cheers
Martin

@andrew-coleman
Copy link
Member

Closing this issue as resolved. Feel free to re-open if you have any more problems with this.

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

2 participants