feat(sqlite): add json_array_length fn #4479
Conversation
| /// assert_eq!(major, 3); | ||
| /// assert!(minor >= 46); |
There was a problem hiding this comment.
I would rather skip the test in this case instead of panicing, as we do for the other tests as well: https://github.com/diesel-rs/diesel/blob/master/diesel/src/sqlite/expression/functions.rs#L82-L87
| /// assert_eq!(major, 3); | ||
| /// assert!(minor >= 46); |
There was a problem hiding this comment.
I would rather skip the test in this case instead of panicing, as we do for the other tests as well: https://github.com/diesel-rs/diesel/blob/master/diesel/src/sqlite/expression/functions.rs#L82-L87
| /// | ||
| /// assert_eq!(json!(0), result); | ||
| /// | ||
| /// # Ok(()) |
There was a problem hiding this comment.
We also want a test here that checks what happens if you pass a SQL NULL value to the function. So something like
let result = diesel::select(json_array_length::<Nullable<Json, _>>(None::<serde_json::Value>())
Possibly we also want variants that use Jsonb instead of Json as SQL type.
| /// # } | ||
| /// ``` | ||
| #[sql_name = "json_array_length"] | ||
| fn json_array_length_with_path<J: JsonOrNullableJsonOrJsonbOrNullableJsonb + MaybeNullableValue<Json>>(j: J, path: Text) -> J::Out; |
There was a problem hiding this comment.
This must unconditionally return a Nullable<Json> or Nullable<Jsonb> type as the nullablity of the return value depends on runtime constraints (whether path exists or not).
| fn json_array_length_with_path<J: JsonOrNullableJsonOrJsonbOrNullableJsonb + MaybeNullableValue<Json>>(j: J, path: Text) -> J::Out; | |
| fn json_array_length_with_path<J: JsonOrNullableJsonOrJsonbOrNullableJsonb + IntoNullable>(j: J, path: Text) -> J::Nullable; |
There was a problem hiding this comment.
The J::Nullable requires me to add + SingleValue to J which gets me this error:
the trait `sql_types::SingleValue` is not implemented for `<J as sql_types::IntoNullable>::Nullable`
Haven't been able to find a solution yet. Any insights?
There was a problem hiding this comment.
I had another look and it turns out that the suggestion was not correct. This function needs to return Nullable<Integer> instead. I just push a fix in 4e181aa for that + rebased the PR to incorporate recent changes from master branch.
This should be good to merge now. Thanks again for contributing.
There was a problem hiding this comment.
Thank you!
As I am still fairly unfamiliar to the project, I hope to be even more helpful next time.
weiznich
force-pushed
the
feat/add-json-array-length-fn-to-sqlite
branch
from
8352d8a to
4e181aa
Compare
Add json_array_length to sqlite json functions
Notes
use diesel::dsl::json_array_lengthwas the postgres version ?!use diesel::dsl::json_array_length_with_pathwas not foundjson_prettyin the same file below a certain versionInstall sqlite3 lib
sqlite-autoconf-<VERSION>.tar.gzfrom https://www.sqlite.org/download.htmltar xvfz ../sqlite-autoconf-<VERSION>.tar.gz./configuremakesudo make installlibsqlite3.so(find / -name libsqlite3.so ...=> in my case/usr/lib/x86_64-linux-gnu) and copy the file (in my case had to make a symbolic link too)