itchyny / gojq

Pure Go implementation of jq
MIT License
3.26k stars 118 forks source link

Support for context.Context in func(any, []any) any #218

Open AuroraV opened 1 year ago

AuroraV commented 1 year ago

  I am very grateful for the excellent open-source code you have provided, and I have used it in my project. However, I noticed that your func(any, []any) any function does not support the context.Context parameter. I saw the env.ctx parameter in your source code, so I was wondering if it would be possible to update the function signature to support the context.Context parameter. This would greatly enhance the usability of your code and make it more compatible with other libraries. Here is an example of what I saw in your code:

x, args := env.pop(), env.args[:argcnt]
for i := 0; i < argcnt; i++ {
    args[i] = env.pop()
}
w := v[0].(func(any, []any) any)(x, args)

  I noticed that the env.ctx variable is consistent with the context.Context type. Therefore, I think adding the context.Context parameter would be a great improvement.

  If you have any suggestions or recommended supported methods, please let me know. Thank you very much for your time and help.

Best regards.

itchyny commented 1 year ago

I'm just for curious, what kind of custom function you want to integrate with gojq?

AuroraV commented 1 year ago

I'm just for curious, what kind of custom function you want to integrate with gojq?

@itchyny Sorry for such late response.I need to follow up on the jq function debug results obtained by inputting different http bodies for the same jq code, and share the same session information under the same request. eg:

    optFn := func(key string) gojq.CompilerOption {
        return gojq.WithFunction(key, 1, 1,
            func(ctx context.Context, a any, _ []any) any {
                claim := getClaim(ctx)
                id, _ := a.(int64)
                name, _ := callHttp(claim.sessinId, id)
                claim.DebugMap[key] = name
                return name
            })
    }

    userInputCode := `{"user_name":getUserName(.user_id), "company_name":getCompanyName(.company_id)}`
    query, _ := gojq.Parse(userInputCode)
    code, _ := gojq.Compile(query, optFn("getUserName"), optFn("getCompanyName"))

    Handler := func(w http.ResponseWriter, r *http.Request) {
        // ctx has some claims from middleware
        ctx := r.Context()
        body, _ := ioutil.ReadAll(r.Body)
        var data map[string]interface{}
        _ = json.Unmarshal(body, &data)
        res := code.RunWithContext(ctx, body)
        val, _ := res.Next()
        if err, ok := val.(error); ok {
            panic(err)
        }
        result := val.(map[string]interface{})
        if err := doSth(result); err != nil {
            claim := getClaim(ctx)
            w.Write(claim.DebugMap)
            w.WriteHeader(http.StatusInternalServerError)
            return
        }
        w.Write([]byte("success"))
        w.WriteHeader(http.StatusOK)
        return
    }

    http.ListenAndServe(":8080", http.HandlerFunc(Handler))

In my case, I implemented a configuration similar to low-code through gojq code (userInputCode). HTTP requests may take message events, and I used WithFunction to provide plugin capabilities to users, allowing them to freely assemble data and do certain things

xakepp35 commented 11 months ago

We also need such functionality, because it turns out there is no way to pass different parameters after code was compiled. Variables and functions can only be set at compile time and nothing passes request context at runtime, and for intense use it looks like we are spending noticeable cpu time inside compile, with lots of memory allocations. It would be supereme if compiled code may be reused, ctx will just do the job.

Not to break compatibility you may, for example, just add opcall_ctx opcode alongside with WithCtxFunction - that way it won't affect neither compatibility, neither execution speed of old code..

Very demanding feature!

itchyny commented 11 months ago

For your usecase you can use WithVariables.

xakepp35 commented 11 months ago

Unfortunately, no, we cant. Its just not enough for all things we're performing with it

By adding the ability to work with context, we easily solved all of these problems, so we forked and started using context extensively. We achieved code simplifications and measured 10-30% performance gains with it, which is important for us.

We hope that one day we can drop our temporary fork (its just merging main+json_numbers+AuroraV's work+fixes because his merge request broke something and there were panicking builtins), and switch back to this repository.

All that is required for that is the ability to register a function signature with context, which would accept context from RunWithContext. I even suggested how to do so without breaking changes.