On Saturday, September 21, 2013 8:07:19 PM UTC+3, Aymeric Augustin wrote: > > On 21 sept. 2013, at 15:53, Richard Ward <[email protected]<javascript:>> > wrote: > > I'm OK with trying to to raise exceptions when making a query and >> get_rollback returns True. I'm not sure it's doable; patch welcome ;-) > > > How about this: > https://github.com/RichardOfWard/django/commit/cb46c75db275db59b54511c090286255bd9cc46d > > It raises the same error that PostgreSQL would raise if you try and do any > SQL when in a transaction with 'needs_rollback' is True. I've tested this > with PostgreSQL, MySQL and SQLite3, and they all now behave in the same way > that PostgreSQL behave currently (I'm running the test suite at the moment, > its taking a while). > > > I wrote a patch based on this idea and unfortunately it doesn't work: > > https://code.djangoproject.com/attachment/ticket/21134/21134-failed-attempt.patch > > This patch triggers a large number of failures in the tests, because it > prevents some read queries which are currently allowed. You didn't notice > because your patch only works when DEBUG = True and the test suite runs > with DEBUG = False. > > If we step back from the implementation, the issue here is that MySQL and > Oracle implicitly add a savepoint before each query, and under some > circumstances implicitly rollback to that savepoint. This behavior allows > transaction management schemes that don't map well to a context manager / > decorator API like Django's. See also > https://code.djangoproject.com/ticket/11156#comment:14 which has the same > root cause. > > I made a significant effort trying to implement the suggestions in this > thread and didn't obtain any results. If someone still wants to change > something in this area, please write a patch with tests; I will review it. > >
I would go for an approach where all queries in marked-for-rollback block are prevented. See https://github.com/akaariai/django/compare/ticket_21134 Note that this is the test case I am complaining about: r1 = Reporter.objects.create(first_name='foo', last_name='bar') with transaction.atomic(): r2 = Reporter(first_name='foo', last_name='bar2', id=r1.id) try: r2.save(force_insert=True) except IntegrityError: r2.save(force_update=True) self.assertEqual(Reporter.objects.get(pk=r1.pk).last_name, 'bar2') The last assert currently fails. There is no explicit use of savepoint=False. It is reasonable to expect that the code succeeds on databases that allow this coding pattern. So, as is, this API is bad. If running read-only queries in marked-for-rollback blocks is absolutely wanted, then lets just switch model.save() and other similar ORM operations to use real savepoints. This is expensive for some use cases, but adding a flag to avoid that cost can be done later on. The tx API is harder to change after 1.6 release. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
