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

JSON data type #44

Open
sgrif opened this issue Dec 3, 2015 · 30 comments
Open

JSON data type #44

sgrif opened this issue Dec 3, 2015 · 30 comments

Comments

@sgrif
Copy link
Member

@sgrif sgrif commented Dec 3, 2015

So let's talk borderline absurd features. I'm trying to figure out how we can support this type, while still being useful. I feel like this is harder to do than in dynamic languages where we're just an arbitrary map or array. However, I think we can do something moderately useful with serde integration (I am unsure if this should be in core, or a separate trait)

However, I am fairly convinced that we can do impl<T> ToSql<Json> for T where T: serde::ser::Serialize and impl<T> FromSql<Json> for T where T: serde::de::Deserialize (note: As written, those two almost certainly require specialization. We can work around this with boilerplate macros to implement for individual types in the short term).

The following operators exist for the JSON data types:

  • ->: Query DSL should just implement Index for Json expressions. Output should be Expression<SqlType=Json>. We might need to do some hackery since Index requires returning a reference. A Copy constraint will likely be involved as well. Might need to be a method if we really can't work with Index, but boy it would be amazing to write filter(json_column["foo"].eq("bar"))
  • ->>: Unsure if we should support. If so, will need to be a method. Unclear on the right name.
  • #>: Unsupported
  • #>>: Unsupported
  • @>: Implemented as method contains. Argument should be AsExpression<Json>, return type is Bool`
  • <@: Unsupported
  • ?: Implemented as method has_key. Argument should be AsExpression<VarChar>, return type is Bool
  • ?| and ?&: Same as above. Don't have an opinion on the method names.

We should also support as many functions as possible from http://www.postgresql.org/docs/9.4/static/functions-json.html. As I write this, I'm fairly convinced this should be a separate crate. If anyone wants to tackle this, let me know and I'll add a new repo for it. I do want this to be supported under the general Diesel umbrella though, even if it's not in the core crate.

@sgrif
Copy link
Member Author

@sgrif sgrif commented Dec 3, 2015

And just so it's clear, if we do this right, we should be able to have anything which implements serde::ser::Serialize implement AsExpression<Json> via ToSql<Json>

Loading

@archer884
Copy link

@archer884 archer884 commented May 10, 2016

So, I was just trying to use Jsonb in a table and I had kind of assumed--until I ran into this error--that diesel would give the json to me as a string and I could deserialize it myself. No dice. Instead, I get a compilation error. Rather than wait for the absurd feature (querying jsonb with diesel, I guess?), what about just letting diesel give me the data instead of exploding? :)

Loading

@sgrif
Copy link
Member Author

@sgrif sgrif commented May 10, 2016

Yes, you can fairly easily add support for any types that you want within
your application. I'm on my phone but will give examples later

Rather than wait for the absurd feature

Unsure what that is supposed to mean

On Tue, May 10, 2016, 5:02 PM archer884 notifications@github.com wrote:

So, I was just trying to use Jsonb in a table and I had kind of
assumed--until I ran into this error--that diesel would give the json to me
as a string and I could deserialize it myself. No dice. Instead, I get a
compilation error. Rather than wait for the absurd feature (querying jsonb
with diesel, I guess?), what about just letting diesel give me the data
instead of exploding? :)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#44 (comment)

Loading

@archer884
Copy link

@archer884 archer884 commented May 10, 2016

That part was quoting you from above:

So let's talk borderline absurd features.

:)

Loading

@sgrif
Copy link
Member Author

@sgrif sgrif commented May 10, 2016

Oh. I didn't realize this was a comment on an existing issue. XD

Loading

@sgrif
Copy link
Member Author

@sgrif sgrif commented May 10, 2016

So basically what you'd need to do is:

struct JsonB;

impl HasSqlType<JsonB> for Pg {
    // ...
}

impl FromSql<JsonB, Pg> for String {
    // ...
}

expression_impls!(JsonB -> String);

Unsure what the binary rep of jsonb is so you might need to do some digging there

Loading

@jimmycuadra
Copy link
Contributor

@jimmycuadra jimmycuadra commented Jun 20, 2016

@archer884 Did you end up implementing this as Sean described? I have a table with a jsonb column and was hoping someone had already done the heavy lifting of creating a Diesel type for it that I could copy/paste. :}

Loading

@lancecarlson
Copy link
Contributor

@lancecarlson lancecarlson commented Nov 21, 2016

I need this too. This and Tsvector get hung up. How difficult is this to implement? It looks like I may have to drop down to rust-postgres...

Loading

@theduke
Copy link
Contributor

@theduke theduke commented Dec 22, 2016

Any movement on this?

It's really crucial for me.

Most important would be just supporting fields with a "Value" type from serde_json.

Loading

@norcalli
Copy link

@norcalli norcalli commented Dec 23, 2016

I would like to use diesel for a project at my job, and we use jsonb extensively. I would like to take on the project of implementing this, but I would need some help getting started.

Loading

@emk
Copy link
Contributor

@emk emk commented Dec 31, 2016

I have a rather pressing need for this on a work project that's experimenting heavily with diesel, and I can almost certainly talk my boss into letting me work on this on company time extremely soon. (We really like diesel so far! But we want to use it for more things.)

Probably the nicest and richest JSON representation in Rust right now is serde_json::Value. It's supported by lots of other libraries, and you can serialize and deserialize it in either direction:

raw JSON String or Vec<u8> serde_json::Value Rust data structure implementing Serialize or Deserialize

It's super handy. The rust-postgres crate has some code showing how to set this up.

Of course, this could be put under a serde_json feature, like the Uuid and chrono types are.

Should I just go ahead and bang out a PR showing what I mean?

Loading

@killercup
Copy link
Member

@killercup killercup commented Dec 31, 2016

Wow, I hadn't realized how many people want to have this! :) @emk, it would be amazing if you got company time to work on this! Maybe you, @archer884 and @norcalli can all work together on this!

Should I just go ahead and bang out a PR showing what I mean?

Maybe. Or, you could try and do this as a plugin crate. (If you think adding this to diesel is easier and more efficient, you can of course still open a PR!)

Diesel has a bunch of facilities to make implementing custom types easier, and as @sgrif mentions in the issue description, it may be possible to add this in a separate crate. This may be a worthwhile effort as it could also show that it is possible to write plugins for diesel and how to do it. Currently, there is just diesel_full_text_search that I know of (and it doesn't even have a Readme… I just field diesel-rs/diesel_full_text_search#1).

(Initially I was worried how quickly you'll run into the limits of the current orphan rules, but I think you'll be fine if you define a custom Jsonb type and impl all traits on this, or as extension traits.)

Loading

@emk
Copy link
Contributor

@emk emk commented Dec 31, 2016

@killercup Thank you for the encouragement!

I've opened a "work in progress" PR at #561, just so we can visualize what I'm thinking about. I agree that it would be an interesting possibility to do this as a separate plugin, but I'd prefer to try that after we see it working inside diesel itself!

Also, for the first version, I'm just targeting serialization and deserialization, and not the specialized JSON query operators. If we can begin by getting JSON into and out of the database, that will already help with a lot of use cases.

Anyway, anybody who wants to help out is encouraged to check out #561, and submit comments, bug reports, and further PRs. :-) I'll continue the implementation discussion there.

Loading

@emk
Copy link
Contributor

@emk emk commented Dec 31, 2016

I now have a preliminary implementation of the json and jsonb data types and conversion to and from serde_json::Value at #561! I haven't attempted to implement any of the JSON-specific query operators.

See #561 for instructions on how to try this out, and please let me know if it works!

Loading

@sgrif sgrif added this to the 0.10 milestone Jan 3, 2017
@sgrif
Copy link
Member Author

@sgrif sgrif commented Jan 3, 2017

Hey folks -- today's my first day back from holiday vacation. I'm getting caught up on issues now. I've gone ahead and added this feature to the 0.10 timeline, as there's clearly a lot of demand for it.

https://github.com/diesel-rs/diesel_full_text_search was mostly just a proof of concept to demonstrate some of the basics of how to add support for additional extensions outside of Diesel. It "conveniently" tackled some types which didn't require ToSql/FromSql implementations. I'd like to see a plugin crate done to ensure we have appropriate APIs in place, but I'm fine with adding this to Diesel proper for now and exploring that space at a later date.

I'll leave implementation specific comments on #561.

Loading

@lholden
Copy link

@lholden lholden commented Jan 4, 2017

@sgrif have you seen the rust-postgres-derive crate? "Syntax extensions to automatically derive FromSql and ToSql implementations for Postgres enum, domain, and composite types." https://github.com/sfackler/rust-postgres-derive

Something like it might make sense for Diesel.

Loading

@sgrif sgrif removed this from the 0.11 milestone Feb 15, 2017
@dbrgn
Copy link
Contributor

@dbrgn dbrgn commented Jun 27, 2017

PR #561 was merged, but this is still open. (For the record, Jsonb support can be enabled with the serde_json feature.)

It looks like the basic support is here, but without the operators, right?

Loading

@killercup
Copy link
Member

@killercup killercup commented Jun 27, 2017

@dbrgn exactly. We should probably make this a meta issue with a check list of stuff we want to have.

Loading

@tyre
Copy link
Contributor

@tyre tyre commented Aug 23, 2018

What's the work involved in supporting any serde_json::Serializable vs. just serde_json::Value? Happy to help out if I can.

The use case is an API client that returns a struct. The struct is serializable, but isn't a serde_json::Value, so to store the raw value I need to re-serialize, then re-de-serialize into a generic serde_json::Value.

Loading

@jonnybrooks
Copy link

@jonnybrooks jonnybrooks commented Nov 30, 2018

Was wondering if any progress has been made implementing json operators e.g. -> and such yet? If not, I'd love to try to implement this as I'm very interested in seeing this functionality in Diesel

Loading

@weiznich
Copy link
Member

@weiznich weiznich commented Nov 30, 2018

@jonnybrooks As far as I know there is no process in those operators made. Feel free to try to implement it. If you hit any problem just ask in out gitter room.

Loading

@Elrendio
Copy link

@Elrendio Elrendio commented Oct 30, 2019

Hello,

First thanks for diesel, it's amazing !

I can't manage to use the @> operator on a Jsonb column with the method contains. I thought is was implemented based on @sgrif original comment but I can't found any trace of it in the docs (even when generated with feature serde_json).

Here's my diesel dependency in my Cargo.toml:

diesel = { version = "1.4.2", features = ["chrono", "postgres", "r2d2", "serde_json"] }

An extract of the code:

use crate::schema;

use diesel::{dsl, prelude::*};

// Some code and in a function I end up having this where `integration_data` is of type
// `serde_json::Value`:
let filter = schema::item_characteristics::item_id
    .eq(schema::items::id)
    .and(schema::item_characteristics::integration_data.contains(integration_data));

The associated schema.rs:

table! {
	items (id) {
		id -> Int4,
		retailer_id -> Int4,
		quantity -> Int4,
		try_fill -> Bool,
		distributed_price_et -> Nullable<Float8>,
		public_price_et -> Float8,
		non_promo_public_price_et -> Float8,
		sr_block_distribution -> Bool,
		sr_block_fill -> Bool,
		refreshed_at -> Timestamptz,
		updated_at -> Timestamptz,
		created_at -> Timestamptz,
		locked_at -> Nullable<Timestamptz>,
	}
}

table! {
	item_characteristics (item_id) {
		item_id -> Int4,
		owned_by_retailer -> Bool,
		seen_in_retailer_inventory -> Bool,
		collection_started_at -> Nullable<Timestamptz>,
		integration_data -> Jsonb,
		updated_at -> Timestamptz,
	}
}

And finally the compilation error:

error[E0599]: no method named `contains` found for type `schema::item_characteristics::columns::integration_data` in the current scope
  --> /Users/elrendio/Workspace/Stockly/Main/Stocks/src/items/lock.rs:86:58
   |
86 |                       .and(schema::item_characteristics::integration_data.contains(integration_data)),
   |                                                                           ^^^^^^^^
   |
  ::: /Users/elrendio/Workspace/Stockly/Main/Stocks/src/schema.rs:87:1
   |
87 | / table! {
88 | |     item_characteristics (item_id) {
89 | |         item_id -> Int4,
90 | |         owned_by_retailer -> Bool,
...  |
95 | |     }
96 | | }
   | |_- method `contains` not found for this
   |
   = note: the method `contains` exists but the following trait bounds were not satisfied:
           `&mut schema::item_characteristics::columns::integration_data : diesel::PgArrayExpressionMethods`
           `&schema::item_characteristics::columns::integration_data : diesel::PgArrayExpressionMethods`
           `schema::item_characteristics::columns::integration_data : diesel::PgArrayExpressionMethods`
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following traits define an item `contains`, perhaps you need to implement one of them:
           candidate #1: `std::ops::RangeBounds`
           candidate #2: `diesel::PgArrayExpressionMethods`
           candidate #3: `percent_encoding::EncodeSet`
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

If it's not implemented my company would gladly invest some time to contribute provided some basic guidelines on how to do it.

Thanks a lot !

Loading

@weiznich
Copy link
Member

@weiznich weiznich commented Oct 30, 2019

@Elrendio The list in Seans original comment is more or less a wish list. As far as I'm aware none of those operators are implemented yet.
That said: Diesel provides the diesel_infix_operator! which allows to define that operator easily on yourself.
Also if someone finds some time to implement them in a proper way (like PgArrayExpressionMethods here) feel free to submit a PR.

Loading

@Elrendio
Copy link

@Elrendio Elrendio commented Oct 30, 2019

Thank you @weiznich, we'll look at the PgArrayExpressionMethods and try to open a PR in the next few weeks :)

Loading

@gh67uyyghj
Copy link

@gh67uyyghj gh67uyyghj commented Nov 15, 2019

@weiznich I am sorry if I sound stupid, but would you just add a simple example of how this macro diesel_infix_operator!() works to filter json columns by fields in postgres?

Loading

@weiznich
Copy link
Member

@weiznich weiznich commented Nov 15, 2019

@gh67uyyghj The api documentation already has an example.

Loading

@cameron-martin
Copy link

@cameron-martin cameron-martin commented Mar 8, 2021

It seems that part of what is being proposed here (#44 (comment)) contradicts with #1950 (comment).

Loading

@weiznich
Copy link
Member

@weiznich weiznich commented Mar 8, 2021

@cameron-martin Feel free to provide a PR adding the corresponding impl's for allowing that without relying on any macro additionally to the normal diesel/serde derives to be called there.

Loading

@cameron-martin
Copy link

@cameron-martin cameron-martin commented Mar 8, 2021

Does this mean your position of not wanting this in diesel core (#1950 (comment)) has changed?

Loading

@weiznich
Copy link
Member

@weiznich weiznich commented Mar 8, 2021

I'm fine with having it there as long as this can be done by just adding a bunch of impl to diesel itself (behind the serde_json feature flag) and as long as someone contributes that code + some tests.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet