I'm assuming there's some more work to do here, but I was hoping to get opinions from core devs on what's been discussed so far. Some of the feedback above was that because save() can only return a rows-modified count of 1 right now, that it's silly to even change. So the tentative decision is simply to have no changes to the Model and QuerySet APIs at all right now (a generally safe decision, all other things aside).
So, what are core-dev thoughts on going on ahead and returning row-modified counts from Model.save() and QuerySet.update/delete? If not, are there any comments on the code changes to date (which are fairly minimal), or is this something that everybody would be generally okay with me wrapping up very soon? Is this generally something that there is low interest in because my ultimate use-case (avoiding optimistic concurrency problems) is a non-issue for some majority of Django developers? Thanks! -- Steven On Mon, Oct 17, 2011 at 10:02 PM, Steven Cummings <estebis...@gmail.com>wrote: > Well, per discussion so far I've rolled the changes back [1] to simply > ensuring that the low-level query objects return usable counts. There was > discussion of whether delete should return counts for immediate objects > only or related as well, but I further decided to simply impose no API > changes on base Model for now. > > [1] > https://github.com/estebistec/django/commit/f178b72af132dd1ba588b89fe6915f5e9ba841d0 > -- > Steven > > > > On Sun, Oct 16, 2011 at 4:48 PM, Steven Cummings <estebis...@gmail.com>wrote: > >> Any opinions from core devs before I go back and make adjustments, or are >> some waiting to see the patch before weighing in? >> >> -- >> Steven >> >> >> >> On Thu, Oct 13, 2011 at 11:48 PM, Steven Cummings >> <estebis...@gmail.com>wrote: >> >>> On Thu, Oct 13, 2011 at 1:06 AM, Anssi Kääriäinen < >>> anssi.kaariai...@thl.fi> wrote: >>>> >>>> Now I have the feeling that I have gone through this exact same >>>> discussion before, and have had the exact same misunderstanding, too, >>>> before. So, sorry for that... >>>> >>> >>> It's cool. Better to make sure we're all clear here on the different >>> opinions and options. >>> >>> >>>> > I think a reasonable option to discuss might be leaving the save() >>>> API as it >>>> > is and rolling this enhancement back to the internal code (i.e., >>>> > UpdateQuery, DeleteQuery) returning counts to support the prospective >>>> > enhancements I've alluded to, and/or overrides of save(). Until there >>>> are >>>> > any changes to save(), I agree it is not going to be useful info. >>>> However >>>> > for delete it seems immediately usable (and then we're left with the >>>> debate >>>> > of counting immediate-only or including related objects). >>>> >>>> I would go with immediate only, with the ability to get the counts for >>>> cascaded deletes per object type as a kwarg option. The kwarg option >>>> could be left for later implementation. One reason for immediate only >>>> is that at least PostgreSQL and MySQL does it that way for ON DELETE >>>> CASCADE foreign keys. So, if you are getting the value from the cursor >>>> and using ON DELETE CASCADE instead of doing the cascade in Django, >>>> you will not get the cascade counts anyways. And even if you do the >>>> cascade in Django, then it would be consistent with what a SQL >>>> database would report. >>> >>> >>> Well, by those conventions and all of the feedback I've gotten so far, >>> counting only immediate objects in a delete seems like the best way. >>> >>> Are there other opinions on this issue or the return value of save? Do >>> any core devs have insight into any potential changes in the ORM that would >>> be affected by this decision? >>> >>> Thanks. >>> >> >> > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
