QueryModel design - All in one derive?

[weiznich]

The new #[derive(QueryModel] attribute (as proposed in #4600) currently internally manages the equivalent of #[derive(Queryable)] and #[derive(Selectable)] as well. The main argument for this behavior is to have one derive that can be applied for loading data, instead of requiring the user to write down 3 or 4 derives. We could extend this behavior to also include #[derive(Identifiable)] if we detect a id column or a #[diesel(primary_key(_))] attribute is present. This would add a trait that is conditionally present on top of that behavior which could be surprising.
On the other hand this make using this derive a bit more restrictive as users cannot anymore mix and match manual and automatic impls as required anymore.

[Mingun]

I think "Don't derive any of the other traits" is more aligned with Rust design for, for example, Eq and PartialEq traits.

[weiznich]

So I had some time to think about this again. I can see your point why you might believe that one derive == one trait is more aligned with rust's general design. I'm nevertheless the opinion that this is not the behavior we want to have here. There are several reasons why I believe just having the derive implementing several traits is much more meaningful in this case.

• You cannot use #[derive(QueryModel)] without #[derive(Selectable)] (or a manual impl) otherwise. This literally leads to a compiler error. I would guess that > 99% of our users will never have a manual impl there, but always use the derives, so it's just additional work for them to type both derives + it's a huge new user blocker.
• Without automatically implementing QueryModel we cannot have QueryModel design - Default for `#[diesel(check_for_backend()]` #4680, which in turn is a drastically improvement to how compilation errors look. We cannot just change #[derive(Selectable)] as that would be a breaking change.
• You cannot use the actual query without #[derive(Queryable)]. It goes even that far that #[derive(Selectable)] assumes that you also derive Queryable (or have a otherwise matching impl). Not doing that will again lead to hard to understand compilation errors, which are in turn a huge problem for new users. I consider that a mistake that we did not have a single derive for both before.

Now I argued quite a bit with "It's harder for new users", so lets also look how it would be for existing "expert" users:

• They are free to just not use the derive, but continue to use the existing Selectable + Queryable derives. Those are not planned to go away anytime soon
• If they want to use the new derive but want to customize one of the traits, they still can do that. For Selectable and Queryable existing derives exist, but they can also write manual impls. For QueryModel for now only the all-in one derive might exist, so if they don't derive that they need to at least manually implement QueryModel (but not Selectable or Queryable as there are existing derives). So for manually implementing QueryModel the work that would need to be done is exactly the same as if that was a "single trait" derive. If you would like to customize the Selectable or Queryable impl you would need to also implement QueryModel manually. That's a tiny bit more work, although I'm really curios how common that use case would be. If it turns out to be quite common, we still can add another derive to only derive QueryModel for this specific user group.

This leads for me to the conclusion that #[derive(QueryModel)] being equivalent to #[derive(Selectable, Queryable)] + implementing the new trait is the best solution.

As for also deriving Identifiable: It seems reasonable to not doing that, as it's not always required, nor can it always exist. So that's a case where it seems meaningful to only have it when it's explicitly requested by the user.