Issue: Add support for querying models by primary-key value
Author: tmoertel
State: closed
Created: 2012-07-31 23:07:03

This (tiny!) patch allows you to query for a model instance by simply
providing its primary-key value as a query argument.  You can use this
new convention anywhere Q objects are allowed, but the most-common
expected use case is this:

```
blog = Blog.get(1)  # get blog w/ primary key of 1
```

Rationale.

Why this patch?  In short, because it simplifies one common use-case
and eliminates the need to break encapsulation for another.

Currently, to fetch a model instance when you know its primary-key
value, you must also know its primary-key name:

```
blog = Blog.get(id=1)  # relies on knowledge that Blog's pkey is "id"
```

Since the model already has this knowledge, why not just let the model
take care of it?

```
blog = Blog.get(1)
```

Moreover, when writing generic code that takes a model as a parameter,
you must pry into peewee's internals to learn what the model's primary
key is called.  For example:

```
def get_required_web_model_instance(model, id):
    try:
        return model.get(**{model._meta.pk_name: id})  # naughty!
    except model.DoesNotExist:
        raise flask.abort(404)
```

If we let the model take care of the details, however, we need not
breach peewee's encapsulation barriers to write model-generic code:

```
def get_required_web_model_instance(model, id):
    try:
        return model.get(id)  # nice!
    except model.DoesNotExist:
        raise flask.abort(404)
```


There is a small problem in the implementation, which is that it does not take into account non-integer primary keys.  That could be worked around though.  I agree with this patch from the point-of-view that it simplifies the API but am not sure if it is worth changing the behavior of the entire `where` method (which calls `parseq`) to allow "naked" values to be passed in and implicitly taken to express `== <pk>`.  I would suggest, if you want this, to implement a `get_pk` method on `Model`.


Thanks for your feedback.

Leaving aside for the moment non-integer primary keys, which I'll be happy to fix, let's assume that we add `get_pk`. Even if we have this method, won't we still need the equivalent for `where` filtering and the like?

For example, consider the following code, in which we want to get a blog entry given by an entry identifier, provided that the entry is owned by the blog that matches a given blog identifier and that the blog is owned by the currently logged in user:

``` python
class User(Model):
    """A person who can own blogs."""
    # more fields here

class Blog(Model):
    """A blog, which can own entries."""
    owner = ForeignKeyField(User)
    # more fields here

class Entry(Model):
    """A post to a blog."""
    blog = ForeignKeyField(Blog)
    # more fields here


def read_entry(user, blog_id, entry_id):
    """Handle request for /blog/<blog_id>/entry/<entry_id>.

    Args:
      user:  Authenticated User-model instance for the logged-in user.
      blog_id:  Blog identifier from URL.
      entry_id:  Entry identifier from URL.

    Returns:
      Rendered HTML.

    Raises:
      Abort(404):  if no such blog entry is visible to the user.

    """
    query = (Entry.select().where(id=entry_id)
             .join(Blog).where(id=blog_id, owner=user))
    try:
        entry = query.get()
    except Entry.DoesNotExist:
        raise Abort(404)
    return render("entry", entry=entry)
```

It's easy to write this kind of code for specific model instances, where you know the names of all the primary keys in advance. But for generic code that gets passed Entry-like models and Blog-like models and their respective instance identifiers at run time, you can't write the code (reliably) without having it pry into peewee's internals to find what the models' primary keys are called:

``` python
def read_general_entry(user, blog_model, blog_id, entry_model, entry_id):
    blog_pkname = blog_model._meta.pk_name
    entry_pkname = entry_model._meta.pk_name
    query = (Entry.select().where(**{entry_pkname: entry_id})
             .join(Blog).where(owner=user, **{blog_pkname: blog_id}))
    # the rest as before
```

Without something like the change I'm proposing, is there any clean way to solve problems like the above from user code?

As an alternative to "naked" identifier values, would you be more agreeable to adding a PK class that could be used wherever Q could? So you could write

``` python
entry = entry_model.get(PK(entry_id))
entry = entry_model.select().where(PK(entry_id).join(blog_model).where(PK(blog_id), user=user)
```

Thoughts?


While all of your suggestions have merit, I think I'm going to pass for now on adding something like this.  Adding yet another method of querying to the (already large) available methods seems like a bad plan.  _"Syntactic sugar causes cancer of the semicolon"_.


Thanks for your response.

Can I ask a favor? Given the current API, how would you recommend that I write model-generic code? (I can't figure out how to do it without prying into peewee's implementation internals (or requiring that all models use the same primary-key name). Am I missing something?)


If you take a look here, https://github.com/coleifer/flask-peewee/blob/master/flask_peewee/admin.py#L142

Basically, you're right:

``` python

def get_pk(klass, pk):
    return klass.get(**{klass._meta.pk_name: pk})
```


What do you think about adding a public `get_pk_name()` method to models? It won't be super-convenient to use (`**{klass.get_pk_name(): pk}` doesn't exactly flow off the keyboard), but at least people won't have to make their code dependent upon peewee's internal implementation when they need knowledge of the primary key. That's the kind of thing that gets flagged at code reviews.

But maybe such a change would go against the grain of peewee's design? (My hunch about why there's not already something like `get_pk_name()` is that you wanted peewee to be smart enough that people wouldn't _have_ to know the names of keys: during joins and other common operations, peewee would just take care of that stuff. I figured that leaving the method off was your way of ensuring that users would come to you with those cases where peewee's current design wasn't able to be fully transparent about keys, so you could consider ways of making it transparent for those cases. All of this is to say: I'll understand if you don't want to add such a method. I wouldn't.)


i've gone ahead and added `get_pk_dict` and `get_pk_name` methods to `Model` -- added in 04dace3bc9f71d93bbbfd7ab27e74bb81b59875b and 023d481199d6cea7d1ca223e3e46535496a81368

Hope this helps!


Works for me :-)

Thanks.