I think there is a case of talking past each other going on here.

On Thursday, September 19, 2013 11:33:00 PM UTC+3, Aymeric Augustin wrote:

> Nonetheless, in my opinion, the root cause of the perceived misbehavior is 
> that user code (not Django code) catches IntegrityError inside an atomic 
> block, preventing __exit__ from detecting that a database exception has 
> happened.
>

This is *not* what is causing the problem in this case. The problem here is 
that if you do this inside atomic block:
try:
    obj.save()
except IntegrityError:
    pass

you can continue to use the connection, but after the outer atomic block is 
exited the transaction is rolled back. The combination of allowing usage of 
connection but rolling it back is the foot gun I have been talking about..

Note that this isn't about DatabaseErrors. *Any* exception bubbling from 
atomic(savepoint=False) block will cause this issue. 

DatabaseErrors must be caught outside the atomic block, as documented. 
> Otherwise transaction.atomic will fail in various interesting and database 
> dependent ways. It's harder to diagnose on databases that don't fully 
> enforce transactional behavior ie. all but PostgreSQL. 
>

Again, this is not about DatabaseErrors at all. DatabaseErrors will mark 
"errors_occured". Errors bubbling from atomic(savepoint=False) will mark 
needs_rollback.

> If I recall correctly there is transaction.set_rollback(False) which can 
> be used to remove forced rollback. 
>
> This is a private API for a reason. Someone deeply familiar with the 
> implementation details of transaction management in Django 1.6 and with an 
> esoteric transaction management system might find this private API useful 
> but it's very obviously not the answer to the original question. Please, 
> everyone, don't do this. 
>

This is documented API... 

>
> > In general, there are three ways to respond to errors inside 
> transactions: 
> >   1. allow usage of the TX (MySQL, SQLite etc), allow user to decide 
> commit/rollback 
> >   2. forbid usage of the TX (PostgreSQL), force it to be rolled back. 
> >   3. allow usage of the TX, but force rollback (Django) 
>
> You're comparing apples to oranges here. If Django is backed by 
> PostgreSQL, how can Django allow using the transaction if PostgreSQL 
> forbids it?
>

Yes, you get #1 on PostgreSQL. But if you are using something other than 
PostgreSQL + Django you will get #3 I don't want that, and it seems you 
don't either. Yet this is what is provided. We can change this to #1 on all 
databases.

>
> > To me it seems explicit error when using connection in "forced to 
> rollback" state is better than allowing saving more data, then silently 
> rolling back the transaction. As you said this is useless. 
>
> Django doesn't allow saving more data in a broken transaction. If a 
> DatabaseError occurs, execution jumps straight to __exit__ which sees an 
> exception and triggers a rollback.


No, this doesn't happen. When using savepoint=False, you get needs_rollback 
flagged for the connection. The connection is still usable. Anything you do 
will be silently rolled back.
 

> Of course, you can disregard the documentation and break this expectation 
> by catching the exception. But there's no way around this in Python. A 
> context manager cannot prevent catching exceptions. 
>

> > To be clear, this is about exceptions (database or other) in 
> atomic(savepoint=False) blocks, not about caught IntegrityErrors - Django 
> will allow you to continue and commit the TX in the caught IntgerityError 
> case assuming the DB allows that. 
>
> I designed this part of Django to forbid continuing and committing the 
> transaction. The reason is cross-database compatibility. I had to enforce 
> the behavior of the most restrictive database, which is also the most 
> correct, namely PostgreSQL. 
>

>
> Sure, you can do virtually anything by disregarding the docs and abusing 
> private APIs, but I wouldn't describe that as "Django will allow you to…" 
>
> If I still haven't convinced you that I know what I'm talking about, 
> here's an example with the exact same problem but without 
> atomic(savepoint=False). 
>
> with transaction.atomic: 
>     try: 
>         do_stuff_in_db()                # raises DatabaseError 
>     except DatabaseError: 
>         pass 
>     do_more_stuff_in_db()        # works only on lax databases eg. MySQL 
>   
>
In this example, it's totally possible that the transaction.atomic block 
> will end up with a rollback even if do_more_stuff_in_db() succeed. Why? 
> Because the commit is likely to fail if a statement failed during the 
> transaction, and that will result in an implicit rollback!


So, why allow using the connection then?

But this case isn't as bad as:
with transaction.atomic():
    try:
        obj.save()
    except IntegrityError:
        pass
    another_obj.save()

Here, another_obj.save() will succeed. But the transaction will be *always* 
rolled back, silently. I don't see any good reason not to error out when 
using a connection which is going to be rolled back in always.

What would you think of a database that does:
> begin;
> insert into foobar values(1);
OUT: IntegrityError!
> insert into foobar values(2);
> commit;

you don't get error on commit - you just get a silent rollback. This is 
exactly what is happening in Django currently.

At the highest level, my design is based on avoiding implicit rollbacks 
> because explicit is better than implicit. But you can still force one if 
> you try hard enough. 
>

Isn't there an implicit rollback going on here?

 - 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to