On Thu, 2010-12-16 at 10:10 -0800, tonnzor wrote: > My idea was to fix this in a very straight way - change ForeignKey > behavior (the piece of code that generates many_set method) - so it > doesn't take into account null. > > Let me explain a bit more. If foreign key is null - then there's no > link between Many and One object. So it many_set should return empty > set disregarding DB state.
This isn't true if the field pointed to (i.e. primary key by default) allows NULL values - in that case a ForeignKey field with a NULL value can and should return a non-empty set of values when the related objects lookup is done. > Using object state (saved/not saved) makes this thing much more > complicated and have unpredicted not covered areas. If user changed > object PK and didn't save that into DB - that's his decision, we > should not keep the state. Your point about changing the PK is something I hadn't thought of. It is much more broadly relevant, actually, since a ForeignKey can point to *any* field, not just a primary key. (Although in practice I doubt that happens much). This means that any time you use a ForeignRelatedObjectsDescriptor on an object, and the relevant fields that might be pointed to by other data in the database have not been saved to the database, you can get incorrect results. So, the 'adding' flag is not enough, since that only covers the PK. So, my patch covers probably 99% or more of cases, in that it refuses to do a query using a PK value of an object that has not yet been added to the DB. But it doesn't catch the other cases - other fields that have changed, and the primary key being changed. The general solution requires tracking all the fields to see whether they've been saved to the DB or not, which I don't think is worth it for this edge case We can either say: - with the provided patch we catch most of the errors, so let's go with that. - leave the code as it is, and just be clear in documentation that retrieving related objects from the DB when the related information isn't actually saved to the DB is obviously not going to work. Luke -- "Outside of a dog, a book is a man's best friend... inside of a dog, it's too dark to read." Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
