Improve documentation on PostgreSQL transactions, exceptions and savepoints

[Richard Davies <richard.davies@…>]

The documentation on raw SQL queries does not mention the need to call transaction.commit_unless_managed() and transaction.savepoint*() in certain cases. This patch adds to the main documentation on raw SQL, and also replaces a second copy which exists today with a link to the main documentation.

See ​http://groups.google.com/group/django-developers/browse_thread/thread/620ccff354c859c2 for a heated discussion on Django developers whether the current behavior is good - this patch only seeks to better document where we are today.

[Richard Davies <richard.davies@…>]

Improve documentation on raw SQL queries

[Malcolm Tredinnick]

Looks good. A few small changes I would suggest to clear up some problems:

1. Django does not have a mode called "auto-commit". The document says it's similar to a database's auto-commit behaviour, but that means it's not identical and so we shouldn't use that term to describe Django's mode. It will lead to even more confusion than when people don't read the current text. Now, if they do read the text, they'll end up with the wrong term. So let's find a better way of describing that mode (e.g "when Django isn't configured for manual transaction management (via the XYZ setting)" or something similar).
2. I think this change should also be clear that things like commit_unless_managed() are only needed for data-changing operations. Select queries for example (one of the more common uses of custom SQL) don't require a call to that method and people don't have to go around adding it everywhere just for that case. It currently sounds all very intrusive, but it's really just for one particular style of query, so make that clear.
3. The documentation of savepoints should go in the transaction document, not in the raw SQL section. It's transaction related, after all. That documentation should be written something along the lines of "if you want your application to work transparently with PostgreSQL 8.x ..." and probably also include a note that savepoints don't work on PostgreSQL 7.x, so if your data changing transaction fails there, you're screwed and have to just roll back.

[Richard Davies <richard.davies@…>]

Updated for Malcolm's comments

[Richard Davies <richard.davies@…>]

Here's an update incorporating Malcolm's comments. Note that I de-duplicate descriptions of raw SQL in doc/topics/db/models.txt and doc/topics/db/sql.txt. These are currently marginally different. I'm assuming that sql.txt is the current master version and that the other one is outdated, but the committer should double-check this.

[Russell Keith-Magee]

(In [10657]) [1.0.X] Fixed #9206 -- Clarified documentation of transaction handling in raw SQL, and error recovery for Postgres. Thanks to Richard Davies for the suggestion and draft text.

Merge of r10655 from trunk.

[Richard Davies <richard.davies@…>]

1.1 specific piece missed in r10655

[Richard Davies <richard.davies@…>]

Russell - thanks for the check in. One small piece which you missed, so I've attached it as "extra.diff" and reopened the ticket. This last bit is Django 1.1 specific.

[Russell Keith-Magee]

Richard: Thanks, but I left that bit out deliberately - I might be missing the point, but I didn't really see what that section was adding to the discussion. The fact that django uses a pseudo-autocommit mode is covered earlier, and isn't Postgres specific. The operation of the database-native autocommit mode is discussed in the docs for the Postgres binding (which is linked to). To me, the 'missing' paragraph only served to confuse the issue of what autocommit meant. This comes back to Malcolm's comment about auto-commit confusion.

I'm fully willing to admit that more documentation may be required - It just isn't obvious to me what is missing (or how the paragraph I omitted could be turned into the missing piece). If you feel there is some vital information that needs to be included, feel free to work up another draft for that paragraph and we'll take a look at it.

[Richard Davies <richard.davies@…>]

New draft of extra piece

[Richard Davies <richard.davies@…>]

OK - I've worked up another draft, which I think explains it better - my point is that savepoints are not necessary for people using the database-native autocommit mode.

[Russell Keith-Magee]

This is better, but two comments:

1. The versionadded flag doesn't render - you can't have a versionadded in between bullet points like you have used

2. When discussing savepoints, you present an example. You have tried to keep the example simple, which is good - but as a result, it isn't obvious to me how savepoints are any different to normal rollbacks. This possibly points to a bigger problem - the fact that savepoints aren't documented anywhere (as far as I can make out). If there was a good block of savepoint documentation, you could just use a link to that documentation. If savepoint docs existed, you wouldn't need an example at all - you could just link to the relevant section and move on.

[Richard Davies <richard.davies@…>]

New draft again of extra piece

[Richard Davies <richard.davies@…>]

OK - here's another version which is rather longer and spells out the different behaviours in full, hence answering your comment 2. On reflection, I think all of this detail is worth including, as you say.

On your comment 1, if that use of versionadded does not work then I have no idea how to solve my problem! So I have omitted the flag entirely. The last bullet point is still Django 1.1 specific, so I guess you should either only commit to the Django 1.1 branch, or alternatively also commit to the Django 1.0.x branch with that bullet point removed.

On the issue of separate savepoint documentation, I think you are right that there is none. However, I believe that they were specifically introduced for exactly this situation [8314], so there's no shame in putting the primary documentation here.

[Richard Davies <richard.davies@…>]

(aside - I believe the behaviour which my last version fully documents is a serious bug in the combination of Django and PostgreSQL. However various tickets to resolve it have gone nowhere (#5043, #9205) so we should at least clearly document it!)

[Russell Keith-Magee]

This draft may be longer, but it isn't any clearer. Look at this from the point of view of someone who doesn't know what a savepoint is - now, what is the difference between a simple transaction rollback, and using a savepoint? In both cases, a.save() and c.save() succeeds, and b.save() fails. The only difference is that if you use savepoints, you need 2 more lines of code. Your example is not sufficiently complex to demonstrate any significant difference between the two use cases.

Again, what is needed here is proper documentation of savepoints, not a bandaid over the problem. It doesn't particularly matter if this is done as part of the transaction docs, or if it is done as a standalone savepoint document. What is important is that the feature - and Django's interface to it - is explained in full.

Also - simply omitting the tag because it doesn't compile isn't an acceptable solution. If something is v1.1 specific, it needs to be marked as such. I will backport these docs with the v1.1 point removed, but that doesn't remove the need to tag the new feature in the v1.1 docs.

[Richard Davies <richard.davies@…>]

Another new draft

[Richard Davies <richard.davies@…>]

Fixed typo

[Richard Davies <richard.davies@…>]

I've split out separate documentation of savepoints, have added some more explanation of the different between savepoint and transaction rollback (the example is sufficiently complex, just wasn't sufficiently well explained) and have reformatted so that I can use versionadded again.

Hope this is now OK!

[Russell Keith-Magee]

(In [10791]) Fixed #9206 -- Added documentation on savepoints, and how to use them to recover from errors in PostgreSQL. Thanks to Richard Davies for the draft text.

[Russell Keith-Magee]

(In [10792]) [1.0.X] Fixed #9206 -- Added documentation on savepoints, and how to use them to recover from errors in PostgreSQL. Thanks to Richard Davies for the draft text.

Merge of r10791 from trunk.

[Jacob]

Milestone 1.1 deleted