Looking for discussion on #5535
James Bennett
allow a single underscore for a foreign key lookup; in other words,
the following two examples would become equivalent:
UserProfile.objects.get(user__id=3) #Currently works
UserProfile.objects.get(user_id=3) #Currently does not work
Relevant ticket is #5535:
http://code.djangoproject.com/ticket/5535
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
Marty Alchin
> David Cramer has asked that Django's ORM lookup syntax be changed to
> allow a single underscore for a foreign key lookup; in other words,
> the following two examples would become equivalent:
>
> UserProfile.objects.get(user__id=3) #Currently works
> UserProfile.objects.get(user_id=3) #Currently does not work
I second the notion of continuing to obscure the magical fields in
general usage. Wherever reasonable, models should look and act exactly
as they're defined in their classes. Drawing further attention to the
fact that there are in fact "invisible" attributes will just cause
more confusion, I would think.
Anyway, I'm no expert on the matter, but that's my take on it.
-Gul
Tom Tobin
> allow a single underscore for a foreign key lookup; in other words,
> the following two examples would become equivalent:
>
> UserProfile.objects.get(user__id=3) #Currently works
> UserProfile.objects.get(user_id=3) #Currently does not work
>
> Relevant ticket is #5535:
>
> http://code.djangoproject.com/ticket/5535
*Way* -1. I don't want to be looking at code and wondering whether a
particular query is dealing with a local field named "foo_bar" or a
related model "foo" with a field "bar".
Michael Radziej
>
> David Cramer has asked that Django's ORM lookup syntax be changed to
> allow a single underscore for a foreign key lookup; in other words,
> the following two examples would become equivalent:
>
> UserProfile.objects.get(user__id=3) #Currently works
> UserProfile.objects.get(user_id=3) #Currently does not work
>
> Relevant ticket is #5535:
>
> http://code.djangoproject.com/ticket/5535
It's not about the number of underscores. It becomes more clear if you have
a custom primary key, such as in:
Model Fitze(models.Base):
eek=...(primary_key = True, ...)
Model Fatze(models.Base):
fitze = ForeignKey(Fitze)
Here, it's to allow Fatze.objects.get(fitze_id=3) vs.
Fatze.objects.get(fitze__eek=3)
The latter looks like it did a join (though this is optimized away, isn't
it?)
For me, the point is that the following code is bad:
fatze = models.Fatze.objects.get(id=1) # whatever
x = fatze.fitze.eek
It requires another database query, so it's better to write:
x = fatze.fitze_id
Thus, I often also try to use this in get(), which doesn't work, as David
noticed.
Another point is, I don't really understand what ubernostrum proposes in the
ticket to replace
Fatze.objects.get_or_create(fitze_id=3)
Do you really want to resort to this?
Fatze.objects.get_or_create(fitze__id=3, defaults = {'fitze': 3})
(And I rather think this does not even work.)
Michael
--
noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
Tel +49-911-9352-0 - Fax +49-911-9352-100
http://www.noris.de - The IT-Outsourcing Company
Vorstand: Ingo Kraupa (Vorsitzender), Joachim Astel, Hansjochen Klenk -
Vorsitzender des Aufsichtsrats: Stefan Schnabel - AG Nürnberg HRB 17689
Waylan Limberg
> UserProfile.objects.get(user__id=3) #Currently works
This is obviously a foreign key and clearly refers to the id field of
the User model. Nuf' said.
> UserProfile.objects.get(user_id=3) #Currently does not work
There is no indication here as to whether this refers to a foreign key
or not. Change the model to something other than User (which has a
know convention) and I would assume this is referring to a user_id
field of the UserProfile model. True, the foreign key is most likely
stored in a user_id column in the db, but a quick glance at the above
query provides no indication of that. It could just be a poorly named
column with no relation to User. If it does, in fact, point to a
foreign key, my assumption would be that a lighter/faster SQL
statement would be generated avoiding the need to access the User
table. More likely, the same SQL would be generated for either, so
this is just misleading.
So among the two above examples, leaving things alone is less
confusing, as Marty Alchin points out.
However, the ticket also seems to indicate that this works:
UserProfile.objects.create(user_id=3)
Which is not so consistent. I get the impression that this is the
motivation for dcramer's argument. If is works for create, why not for
get? Perhaps it shouldn't work for create either, although we probably
don't want to go there for backwards compatibility issues.
Just my $0.02
--
----
Waylan Limberg
way...@gmail.com
David Cramer
have an instance of the row (the id) I'm trying to insert. I'm *not*
going to query for this instance (I'll write SQL before I add extra
queries just to do an update/insert), and I don't like creating a fake
temporary object, which will probably not work w/ the shared memory
patch. (e.g. mykey=Model(id=3))
James Bennett
> Yes, my biggest argument is I want to use .get_or_create() and I don't
> have an instance of the row (the id) I'm trying to insert. I'm *not*
> going to query for this instance (I'll write SQL before I add extra
> queries just to do an update/insert), and I don't like creating a fake
> temporary object, which will probably not work w/ the shared memory
> patch. (e.g. mykey=Model(id=3))
And it's been pointed out to you *twice* now that you don't have to
have an instance to do this.
Collin Grady
> *Way* -1. I don't want to be looking at code and wondering whether a
> particular query is dealing with a local field named "foo_bar" or a
> related model "foo" with a field "bar".
Except when you do user__id it /is/ referring to the local column, it
just magically drops the join on you, even though it looks like it will
do one :)
--
Collin Grady
Save the whales. Collect the whole set.
David Cramer
if I'm wrong, but I think it throws an error about it needing to be a
<ForeigNkeyClass> instance.
On Sep 19, 11:30 am, "James Bennett" <ubernost...@gmail.com> wrote:
Collin Grady
> I don't believe .create() allows you to say myforeignkey=1, correct me
> if I'm wrong, but I think it throws an error about it needing to be a
> <ForeigNkeyClass> instance.
But .create() already works with fkeyname_id, it's just get() and
get_or_create() that don't :)
--
Collin Grady
BOFH excuse #150:
Arcserve crashed the server again.
James Bennett
> if I'm wrong, but I think it throws an error about it needing to be a
> <ForeigNkeyClass> instance.
OK, so let's do this exhaustively.
>>> my_id = 3
>>> UserProfile.objects.create(user_id=my_id) #works
>>> UserProfile.objects.get(user__id=my_id) # works
>>> UserProfile.objects.get_or_create(user__id=my_id, defaults={
'user_id': my_id }) #works
That covers get(), create() and get_or_create(). In none of these
cases did you need to have an instance of the related model available
in memory.
So: since your argument about "needing an instance" is unfounded,
what's to gain by breaking the consistency of the lookup syntax?
One more time to recap:
1. The lookup syntax *always* uses double underscores for relations.
2. The lookup syntax *only* lets you look up by things that actually are fields.
Your proposal would violate both of these points of consistency, by
breaking (1) to allow a single underscore and breaking (2) to allow
lookup by something that, rather than being a field, is an
implementation detail of the field.
What's the gain?
Jacob Kaplan-Moss
> But .create() already works with fkeyname_id, it's just get() and
> get_or_create() that don't :)
So seems to me the best way to solve this is to make get_or_create()
understand how to swizzle a lookup of the form "fk__id" or "fk__pk"
into a create() arg of "fk_id".
That is, make::
Foo.objects.get_or_create(bar__id=1)
Work the same way as::
Foo.objects.get_or_create(bar__id=1, defaults={"bar_id": 1})
Have I got it right?
Jacob
Collin Grady
> 1. The lookup syntax *always* uses double underscores for relations.
But checking fkey_id isn't trying to cross the relation - it wants the
local value :)
> 2. The lookup syntax *only* lets you look up by things that actually are fields.
*cough* .get(pk=1)
:)
--
Collin Grady
"MacDonald has the gift on compressing the largest amount of words into
the smallest amount of thoughts."
-- Winston Churchill
Collin Grady
> That is, make::
>
> Foo.objects.get_or_create(bar__id=1)
>
> Work the same way as::
>
> Foo.objects.get_or_create(bar__id=1, defaults={"bar_id": 1})
That would probably be better than the current, but I'm still on David's
side in thinking that allowing bar_id in get, create, and get_or_create
is far more consistent, since you can also use bar_id as a normal
attribute lookup and as a kwarg in making a model instance.
It just seems abnormal that it works everywhere /except/ .get() and
.get_or_create()
--
Collin Grady
Live within your income, even if you have to borrow to do so.
-- Josh Billings
Gary Wilson
> David Cramer said the following:
>> I don't believe .create() allows you to say myforeignkey=1, correct me
>> if I'm wrong, but I think it throws an error about it needing to be a
>> <ForeigNkeyClass> instance.
Yes, you are correct.
> But .create() already works with fkeyname_id, it's just get() and
> get_or_create() that don't :)
I say you are looking at it backwards here. It's not that get() and
get_or_create() don't work with fkeyname_id, it's that Model() and create()
don't work with fkeyname.
To take the examples in the ticket description:
==============
MyModel.objects.create(author_id=1) # Works
but it shouldn't. Instead, we should make the following allowed:
MyModel.objects.create(author=1)
MyModel.objects.create(author='1')
--------------
MyModel.objects.create(author=Author(id=1)) # Works
This is a tricky way around django.db.models.base.Model.__init__'s current
enforcement of the foreign key field (without the _id) wanting an instance,
but shouldn't be needed if we implement the above (meaning create() would act
like get() in that you could pass either an int, a str, or a model instance to
the foreign key field).
==============
MyModel.objects.get(author_id=1) # Doesn't Work
as it shouldn't.
--------------
MyModel.objects.get(author=Author(id=1)) # Works
as it should, but sort of pointless since foreign key field in get() can
already take an int or str too.
--------------
MyModel.objects.get_or_create(author_id=1) # Doesn't Work
as it shouldn't. But, the following should be made to work:
MyModel.objects.get_or_create(author=1)
MyModel.objects.get_or_create(author='1')
MyModel.objects.get_or_create(..., defaults={'author': 1})
MyModel.objects.get_or_create(..., defaults={'author': '1'})
And they would work if django.db.models.base.Model.__init__ is fixed as
mentioned above with create().
--------------
Gary
Collin Grady
> I say you are looking at it backwards here. It's not that get() and
> get_or_create() don't work with fkeyname_id, it's that Model() and create()
> don't work with fkeyname.
I disagree strongly with that. It's already established that
obj.fkeyname is the object, and obj.fkeyname_id is the ID, so why break
that?
It's far more consistent to just make fkeyname_id work in .get() and
.get_or_create(), in my opinion.
--
Collin Grady
Don't let your status become too quo!
Simon Litchfield
> >>> my_id = 3
> >>> UserProfile.objects.create(user_id=my_id) #works
> >>> UserProfile.objects.get(user__id=my_id) # works
> >>> UserProfile.objects.get_or_create(user__id=my_id, defaults={'user_id': my_id }) #works
That's it, right there in a nutshell. Empty your head of Django for a
second, have another look; do you not see the inconsistency there?
Why confuse the issue? Foreign keys are always named fieldname_id by
default, regardless of the foreign table's primary key field name. So,
the argument about readability, and confusing foo_bar with foo__bar is
unfounded. It's always gonna be foo__bar for foreign key, and foo_bar
for the local field. Moreover, foo__id and foo_id are the same value
anyway.
And perhaps more importantly, both of these .get* examples here
generate an unnecessary join anyway.
Bottom line is, surely any ORM should allow you to perform a simple
single table lookup based on *any* field in the given table?
Simon Litchfield
> This is a tricky way around django.db.models.base.Model.__init__'s current
> enforcement of the foreign key field (without the _id) wanting an instance,
> but shouldn't be needed if we implement the above (meaning create() would act
> like get() in that you could pass either an int, a str, or a model instance to
> the foreign key field).
Hmmm, I'd almost say this was the best solution; ie, losing the magic
fkname_id notation altogether in favour of making it consistent (in a
gradual phased manner of course!).
The only problem here though, we'd need to continue the trend, by
changing model.__set__ so that by setting instance.fkname = 47 it'd
lazy load instance.fkname as a proper model instance, rather than an
int (or str/date/time etc).
Great, but then how do I get the fkname_id int without having to hit
the database? Looks like we're straight back to the drawing board with
the magic fkname_id notation again!
James Bennett
> That's it, right there in a nutshell. Empty your head of Django for a
> second, have another look; do you not see the inconsistency there?
No :)
As I see it, the existence of the "somefieldname_id" property is an
implementation detail of ForeignKey -- the data comes back as part of
the row we select, and we need to stick it somewhere.
Now, consider the problems with the proposal to magically make this
work in the DB API:
1. It breaks consistency with how we currently do things: except for
the "pk" shortcut (which is well justified on its own), you can only
look up on things that are actual fields on the model, not by
arbitrary properties which happen to correspond to things in the
database, and if you want to do something based on a relation you use
the double-underscore syntax in all cases (regardless of whether we
can optimize joins out of the eventual query). This makes a consistent
API, which would break if we suddenly allow the "foreignkey_id" syntax
to work.
2. What happens if somebody actually has a non-foreign-key field whose
name ends with the sequence "_id"? There are plenty of use cases for
this, and then suddenly we've got a fine kettle of fish because we
have to start guessing about whether this is trying to reference a
local field or use magical foreign-key querying. And the possibilities
for name collisions would mean lots of nasty workarounds in model
class validation and more places where we might have to force people
to use explicit related_name.
> And perhaps more importantly, both of these .get* examples here
> generate an unnecessary join anyway.
No, they don't. The join is optimized away.
> Bottom line is, surely any ORM should allow you to perform a simple
> single table lookup based on *any* field in the given table?
Bottom line is *you can already do this*. You and David and Collin
aren't really saying "it's not possible to do this in Django", you're
saying "it's possible, we just don't like the syntax for it". Jacob's
proposal above -- making get_or_create() smart enough to figure out
that a join in the query should become a default value in creating the
instance -- seems like the right way to make get_or_create() easier to
use in this case, and once that happens I don't honestly see a reason
to break the DB API's consistency for sake of exposing an
implementation detail. In fact, at this point I'm leaning toward
submitting a patch to turn the damned thing into a private
double-underscore instance variable just so people can't get at it ;)
Michael Radziej
> As I see it, the existence of the "somefieldname_id" property is an
> implementation detail of ForeignKey -- the data comes back as part of
> the row we select, and we need to stick it somewhere.
Ehm ... does this mean that this might change, or is this part of the stable
API?
Collin Grady
I still disagree with you there. It would only break consistency if it
was actually crossing the relation and checking the ID field of the
other table.
Since it is *not* a cross-relation lookup, not using __ syntax is *not*
inconsistent. On the contrary, it now becomes *more* consistent with
normal data access, such as obj.fkey_id
> 2. What happens if somebody actually has a non-foreign-key field whose
> name ends with the sequence "_id"?
Then it finds that field as normal, and everything continues on. You'd
only have to fallback to attname if you don't find a matching field name.
--
Collin Grady
"Life is too important to take seriously."
-- Corky Siegel
Simon Litchfield
all relative I guess, you think we're upside down, I think you're
upside down :-) Just a shame to burn thousands of man hours on
something which is so plainly inconsistent to fresh eyes.
David Cramer
"MYmodel(id=1)" which is a hack. This technically shouldn't be done as
you're instantiating an object without a dataset. What happens when
the shared memory select related patch were to go in? You'd now have
an instance of a model thats completely empty, and would then be
referenced in memory.
On Sep 24, 6:54 am, Michael Radziej <m...@noris.de> wrote:
> On Mon, Sep 24, James Bennett wrote:
> > As I see it, the existence of the "somefieldname_id" property is an
> > implementation detail of ForeignKey -- the data comes back as part of
> > the row we select, and we need to stick it somewhere.
>
> Ehm ... does this mean that this might change, or is this part of the stable
> API?
>
> Michael
>
> --
> noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
> Tel +49-911-9352-0 - Fax +49-911-9352-100http://www.noris.de- The IT-Outsourcing Company
James Bennett
> The only way this can be done as of current is by saying
> "MYmodel(id=1)" which is a hack. This technically shouldn't be done as
> you're instantiating an object without a dataset. What happens when
> the shared memory select related patch were to go in? You'd now have
> an instance of a model thats completely empty, and would then be
> referenced in memory.
No, no, a thousand times no. Read any of my posts in this thread
before you start going off again about having to instantiate an
object.