diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.75k stars 1.07k forks source link

JSON data type #44

Open sgrif opened 8 years ago

sgrif commented 8 years ago

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:

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 commented 8 years ago

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>

archer884 commented 8 years ago

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? :)

sgrif commented 8 years ago

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 https://github.com/diesel-rs/diesel/issues/44#issuecomment-218290337

archer884 commented 8 years ago

That part was quoting you from above:

So let's talk borderline absurd features.

:)

sgrif commented 8 years ago

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

sgrif commented 8 years ago

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

jimmycuadra commented 8 years ago

@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. :}

lancecarlson commented 7 years ago

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...

theduke commented 7 years ago

Any movement on this?

It's really crucial for me.

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

norcalli commented 7 years ago

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.

emk commented 7 years ago

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?

killercup commented 7 years ago

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.)

emk commented 7 years ago

@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.

emk commented 7 years ago

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!

sgrif commented 7 years ago

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.

lholden commented 7 years ago

@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.

dbrgn commented 7 years ago

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?

killercup commented 7 years ago

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

tyre commented 6 years ago

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.

jonnybrooks commented 5 years ago

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

weiznich commented 5 years ago

@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.

Elrendio commented 5 years ago

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 !

weiznich commented 5 years ago

@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.

Elrendio commented 5 years ago

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

gh67uyyghj commented 4 years ago

@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?

weiznich commented 4 years ago

@gh67uyyghj The api documentation already has an example.

cameron-martin commented 3 years ago

It seems that part of what is being proposed here (https://github.com/diesel-rs/diesel/issues/44#issuecomment-161479190) contradicts with https://github.com/diesel-rs/diesel/issues/1950#issuecomment-452898798.

weiznich commented 3 years ago

@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.

cameron-martin commented 3 years ago

Does this mean your position of not wanting this in diesel core (https://github.com/diesel-rs/diesel/issues/1950#issuecomment-452898798) has changed?

weiznich commented 3 years ago

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.