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

working on json_path_exists #4286

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

valkrypton
Copy link
Contributor

@weiznich I was trying to implement the jsonb_path_exists function. One of its argument is of type Jsonpath. Currently there is no such type in diesel, so I tried to implement it using the jsonpath_rust crate.

I implemented the FromSql trait for it, but I could not figure out how to implement the ToSql trait. I'd appreciate if you could take a look and help me with this or maybe i am going completely in the wrong direction with this?

@weiznich
Copy link
Member

weiznich commented Oct 2, 2024

Thanks for working on this. The FromSql implementation needs to read the binary representation of a json path as defined in the PostgreSQL protocol. Unfortunately the documentation of that format is mostly not existing, at least for anything that's not a trivial type. That means you likely need to perform some experimentation. One possibility would be to just setup a test case that loads a jsonpath value from the database and inspect the output. That should tell us how postgres serializes these data. Hopefully that's just the path as string or something simple like that. The other way is to dig through the postgres source code and find something there. This file might be a starting point.

@valkrypton
Copy link
Contributor Author

Hey @weiznich, I tried working on this again, and I somewhat got to implement the Jsonpath type. But now the doctest for the function are failing due to the following error:

failures:

---- diesel/src/pg/expression/functions.rs - pg::expression::functions::jsonb_path_exists (line 2290) stdout ----
error[E0599]: the method `get_result` exists for struct `jsonb_path_exists<Jsonb, Jsonpath, Bound<Jsonb, Value>, Bound<Jsonpath, JsonPath>>`, but its trait bounds were not satisfied
    --> diesel/src/pg/expression/functions.rs:2306:71
     |
18   |   let exists = jsonb_path_exists::<Jsonb,Jsonpath,_,_>(jsonb,json_path).get_result::<bool>(connection)?;
     |                                                                         ^^^^^^^^^^ method cannot be called due to unsatisfied trait bounds
     |
    ::: /Users/ali.tariq/diesel/diesel/src/pg/expression/functions.rs:2284:1
     |
2284 | / define_sql_function! {
2285 | |     /// This form of jsonb_object takes keys and values pairwise from two separate arrays.
2286 | |     /// In all other respects it is identical to the one-argument form.
2287 | |     ///
...    |
2310 | |     fn jsonb_path_exists<J: JsonbOrNullableJsonb + SingleValue, P: MaybeNullableValue<Jsonpath>>(jsonb: J, path: P) -> Bool;
2311 | | }
     | |_- doesn't satisfy `_: RunQueryDsl<_>` or `_: Table`
     |
     = note: the full type name has been written to '/var/folders/gn/jjjy0cpx2qvcq_ncmnq8bf3m0000gp/T/rustdoctestDCO66I/rust_out.long-type-7427101102093426123.txt'
     = note: consider using `--verbose` to print the full type name to the console
     = note: the following trait bounds were not satisfied:
             `diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: Table`
             which is required by `diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: diesel::RunQueryDsl<_>`
             `&diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: Table`
             which is required by `&diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: diesel::RunQueryDsl<_>`
             `&mut diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: Table`
             which is required by `&mut diesel::pg::expression::functions::jsonb_path_exists_utils::jsonb_path_exists<Jsonb, Jsonpath, diesel::expression::bound::Bound<Jsonb, Value>, diesel::expression::bound::Bound<Jsonpath, JsonPath>>: diesel::RunQueryDsl<_>`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0599`.
Couldn't compile the test.

failures:
    diesel/src/pg/expression/functions.rs - pg::expression::functions::jsonb_path_exists (line 2290)

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 310 filtered out; finished in 0.67s

error: doctest failed, to rerun pass `--doc`

Some pointers would be appreciated on how to implement the missing traits.

/// # let connection = &mut establish_connection();
/// let jsonb:Value = serde_json::json!({"a":[1,2,3,4,5]});
/// let json_path = jsonpath_rust::path!("$.a[ ? (@ >= 2 && @ <= 4)]");
/// let exists = jsonb_path_exists::<Jsonb,Jsonpath,_,_>(jsonb,json_path).get_result::<bool>(connection)?;
Copy link
Member

@weiznich weiznich Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to write something like that here:

Suggested change
/// let exists = jsonb_path_exists::<Jsonb,Jsonpath,_,_>(jsonb,json_path).get_result::<bool>(connection)?;
/// let exists = diesel::select(jsonb_path_exists::<Jsonb,Jsonpath,_,_>(jsonb,json_path)).get_result::<bool>(connection)?;

The underlying issue is that just calling the sql function only creates a query fragment that only represents the function, not a full SELECT statement. By using diesel::select you explicitly construct a SELECT statement without from cause, while using whatever you provided as select clause.

That should resolve the compiler error

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

Successfully merging this pull request may close these issues.

2 participants