Brendonovich / prisma-client-rust

Type-safe database access for Rust
https://prisma.brendonovich.dev
Apache License 2.0
1.75k stars 106 forks source link

Question about design decision #356

Closed LazerJesus closed 1 year ago

LazerJesus commented 1 year ago

Hey, I recently started using this package and I've been wondering why a particular design decision was made the way it was. Namely, why do we pass in the objects' properties as ordered parameters on creation? This feels weird to me. This way would feel authentic in Lisp, but a struct would feel more rusty.

let post: post::Data = client.post().create(
        // Required fields are individual arguments.
        // Wrapping their values is not necessary.
        true,
        "what up".to_string(),
        // All other fields can be passed in the last argument
        vec![
            post::content::set(Some("content".to_string()))
        ]
    )

It causes readability issues for me. I tried to get around this by wrapping all fields explicitly in model::field::set(x),, but on required properties, this throws a trait bound error for me. E0277 the trait bound std::string::String: From<front::Set> is not satisfied

I'd love to know why you chose this way. Best, Finn

Brendonovich commented 1 year ago

This design was just a copy of Prisma Client Go's syntax, which was the basis for this whole project.

I've currently got a lot of unreleased changes on main, with one being the addition of a Create struct for each model that allows doing what you describe. basically just taking the inputs to create and putting them in a struct: image The struct will still have required fields + _params instead of all fields as 1) It's familiar 2) Including all non-required fields as Option<...> would require specifying each field or using Default, which I don't want to implement as I think that sort of implicit behaviour isn't great for creating DB records. 3) I think being able to create DB params from iterators allows for some cool DX

LazerJesus commented 1 year ago

can you explain the +_params bit to me? is your differentiation between required fields == fields an optional fields == _params?

personally, i dont think i would mind having Option<...> on the struct definition if i can just omit it on the instance. and then passing the instance to the create() function feels like the right syntax from the outside.

Brendonovich commented 1 year ago

can you explain the +_params bit to me? is your differentiation between required fields == fields an optional fields == _params?

Yeah, each required field gets a dedicated field, all the optional ones get passed in to _params

personally, i dont think i would mind having Option<...> on the struct definition if i can just omit it on the instance.

Not sure I get this, you'd have to manually set each optional value you're not using to None - no omitting fields you don't want

and then passing the instance to the create() function feels like the right syntax from the outside.

You wouldn't pass it to create(), you'd call Create::to_query which would call create under the hood for you, from which point you'd have the same builder API as using create

Here's an example from Spacedrive

image
LazerJesus commented 1 year ago

are you still working on the API? if so, i'd love to debate it.

Brendonovich commented 1 year ago

Yeah for sure, probably easier if you hop in our Discord and we can chat about it

LazerJesus commented 11 months ago

for posterity:

the api i'd think is most intuitive would be looking something like this. lets say i the schema is defined as:

model Post {
  id           String @id
  content      String
  otherContent String?
}

and i want to create a Post.

in my rust code i have a struct Post that mirrors the schema model.

pub struct Post {
    pub id: String,
    pub content: String,
    pub other_content: Option<String>,
}

probably that would be autogenerated and i just import with the rest of the client and other definitions. when i want to create one in the db, id call the create function like this:

use crate::{prisma::Post, prisma::PrismaClient};
let prisma_client: prisma::PrismaClient = prisma::new_client()

let post = Post {
    id: String::from("123"),
    content: String::from("This is the post content"),
    other_content: Some(String::from("This is the optional content")),
};

let post = client.post().create(post).exec().await.unwrap();

with other_content being present or not, which might look like this

let post = client
        .post()
        .create(
            Post {
               id: String::from("1234"),
               content: String::from("This is the post content"),
            }
        )
        .exec()
        .await
        .unwrap();

and the same basic mechanic for all CRU behaviour. i get an instance back on Read, and i pass one on Update. what do you think?

Brendan - PCR:

in my rust code i have a struct Post that mirrors the schema model I'm not willing to change {model}::Data, soz. The module-per-model and module-per-field approaches are IMO better than anything else out there.

when i want to create one in the db, id call the create function like this: This is similar to the upcoming {model}::Create api, but I see you've taken a different approach for optional fields. As I said before, I'm not a huge fan of optional fields into the struct, as I don't want {model}::Create to implement Default, which would pretty much be a requirement, otherwise users would always have to specify every optional field. Keep in mind that Rust doesn't allow half-filling the fields of a struct, you have to either specify all of them or specify some and spread in another whole instance like Default::default()

with other_content being present or not, which might look like this As mentioned above, you just can't do this. Rust doesn't allow partial struct declaration.

i get an instance back on Read, and i pass one on Update. This won't work. Between receiving an instance on read and passing it to update, how would I know what fields have changed? This is asking for an API more like SeaORM and is more of an ActiveRecord style, which is exactly what I designed PCR to not be. In my experience treating DB values as raw data and nothing more has been the most helpful.

Mind you, none of what I just said is actually stopping your vision of the API from existing. The PCR generator is actually split into 2 crates, crates/cli which does the code generation, and crates/sdk which provides the base for building a generator in the first place. It'd be 100% possible to write your own generator + client that consumes types from PCR, but provides your style of API.

knvpk commented 6 months ago

Hi @Brendonovich did you find any way to do that,

Brendonovich commented 6 months ago

@knvpk If you're referring to the ActiveRecord-style API I haven't looked into it, and won't be the one to do so. If someone else wants to give it a shot then they can but I'll be sticking with the raw data approach.