Skip to content

Support json as parameter #35

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
jobot0 opened this issue Apr 28, 2022 · 3 comments · May be fixed by #65
Open

Support json as parameter #35

jobot0 opened this issue Apr 28, 2022 · 3 comments · May be fixed by #65

Comments

@jobot0
Copy link

jobot0 commented Apr 28, 2022

Hello!

Thanks for the amazing work :) Would it be possible to add Serde json support to perform queries based on json input?

@soedirgo
Copy link
Collaborator

Hmm, maybe we should instead be doing insert_tostring where T: ToString? That makes it more general and doesn't lock us down to serde.

But in that case it's not difficult to simply run to_string() on the value and pass it to insert, e.g.

let x = json!(r#"{"hello":"world"}"#);
let resp = client
    .from("table")
    .insert(x.to_string())
// ...

@jobot0
Copy link
Author

jobot0 commented Apr 29, 2022

Yep you're right it would lock us down to serde. However, should we prefer making the manipulation on user side either? Sorry, I'm quite new to Rust idioms and rules. Thus I was a bit surprised as the underlying code seems to be based on reqwest which accepts json with Serialize trait. Once again sorry if I misunderstood the codebase intention or implementation 😅

@soedirgo
Copy link
Collaborator

soedirgo commented May 4, 2022

No probs!

There might be more upside to enabling it in reqwest - e.g. you otherwise need to add a Content-Type: application/json header, which doesn't apply here because we implicitly set that header.

reqwest also converts the json to Vec<u8> instead of converting it to String first (we might want to do this as well). Since we're doing to_string() in insert_json anyway, I find it cleaner to just use to_string() when passing the argument (just a few extra keystrokes), which also works with e.g. miniserde.

@strykejern strykejern linked a pull request Oct 11, 2024 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants