Обсуждение: [PATCH] Docs: Make notes on sequences and rollback more obvious
Hi all I'm seeing enough questions on pgsql-general and stack overflow to suggest that the docs for how sequences interact with transaction rollback. Take the most recent post on -general, where the person read at least the tutorial, but had no idea about the exemption. The attached patch: - Moves the note about nextval() from the footer to be inside the nextval description - Adds an xref from the advanced-transactions tutorial where the poster noted their point of confusion, noting the exemption and pointing to the docs on nextval. - A pointer from the docs on SERIAL types to the nextval notes on tx rollback. Comments would be appreciated. -- Craig Ringer
Вложения
Craig Ringer <ringerc@ringerc.id.au> wrote: > I'm seeing enough questions on pgsql-general and stack overflow > to suggest that the docs for how sequences interact with > transaction rollback. Yeah, I've noticed a surprising number of people who are being surprised by the non-transactional nature of sequences (and serial columns) in spite of current caveats in the docs; so I agree that we should punch that up in the docs a bit. > The attached patch: > > - Moves the note about nextval() from the footer to be inside the > nextval description > > - Adds an xref from the advanced-transactions tutorial where the > poster noted their point of confusion, noting the exemption and > pointing to the docs on nextval. > > - A pointer from the docs on SERIAL types to the nextval notes on > tx rollback. > > Comments would be appreciated. I haven't reviewed it in detail but noticed an apparent editing error: "which are used the counters" should probably have an "as" thrown in there. Or something. -Kevin
On 08/04/2012 04:12 AM, Kevin Grittner wrote: > I haven't reviewed it in detail but noticed an apparent editing error: > "which are used the counters" should probably have an "as" thrown in > there. Or something. Thanks. Editing fail. I revised that spot repeatedly to try to keep it short and simple without in any way implying that SEQUENCEs are *only* used for SERIAL columns. Fixed attached.
Вложения
On Sat, Aug 4, 2012 at 12:56 AM, Craig Ringer <ringerc@ringerc.id.au> wrote: > On 08/04/2012 04:12 AM, Kevin Grittner wrote: >> I haven't reviewed it in detail but noticed an apparent editing error: >> "which are used the counters" should probably have an "as" thrown in there. >> Or something. > > Thanks. Editing fail. I revised that spot repeatedly to try to keep it short > and simple without in any way implying that SEQUENCEs are *only* used for > SERIAL columns. > > Fixed attached. In datatype.sgml, I think that adding that <important> block in the middle of the existing paragraph is too choppy. I moved it down a bit, changed it to a note, expanded it a little, and fixed some typos and markup. In func.sgml, I chose to keep the <important> at the end, instead of switching the order of the paragraphs as you did, but I moved it up under nextval instead of having it at the end, as you had it. I kept your note in setval() but cleaned it up a bit. I did not commit the advanced.sgml changes. I am not sure I believe the assertion that any function or type with special transactional behavior will include a documentation mention. It doesn't seem like a terribly future-proof assertion at any rate. With respect to the mention of autocommit, I think it would be good to add something there, but maybe it should cross-reference our existing documentation mentions of autocommit. Also, it's a bit ambiguous the way it's worded whether you get the automatic BEGIN/COMMIT with autocommit=on or with autocommit=off; somehow we should try to clarify what we mean a little more there. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 08/07/2012 02:27 AM, Robert Haas wrote: > I did not commit the advanced.sgml changes. That's arguably the most important point to raise this. The most recent question came from someone who actually bothered to RTFM and believed based on the advanced-transactions page that rollback rolls *everything* back. Some kind of hint that there are execptions is IMO very important. I'm not sure what the best form for it to take is. > I am not sure I believe > the assertion that any function or type with special transactional > behavior will include a documentation mention. It absolutely should, but I guess that doesn't mean it's guaranteed to. > It doesn't seem like a > terribly future-proof assertion at any rate. With respect to the > mention of autocommit, I think it would be good to add something > there, but maybe it should cross-reference our existing documentation > mentions of autocommit. Also, it's a bit ambiguous the way it's > worded whether you get the automatic BEGIN/COMMIT with autocommit=on > or with autocommit=off; somehow we should try to clarify what we mean > a little more there. Yeah. I should've kept that separate, as it was something I noticed in passing, rather than central to the changes. -- Craig Ringer
On Tue, Aug 7, 2012 at 3:59 AM, Craig Ringer <ringerc@ringerc.id.au> wrote: > On 08/07/2012 02:27 AM, Robert Haas wrote: >> >> I did not commit the advanced.sgml changes. > > That's arguably the most important point to raise this. The most recent > question came from someone who actually bothered to RTFM and believed based > on the advanced-transactions page that rollback rolls *everything* back. > > Some kind of hint that there are execptions is IMO very important. I'm not > sure what the best form for it to take is. I'm not sure, either. Maybe we should avoid blanket statements and just say something like: Note: Some operations on sequences are non-transactional and will not be rolled back on transaction abort. See <xref>. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> wrote: > Craig Ringer <ringerc@ringerc.id.au> wrote: >> On 08/07/2012 02:27 AM, Robert Haas wrote: >>> >>> I did not commit the advanced.sgml changes. >> >> That's arguably the most important point to raise this. The most >> recent question came from someone who actually bothered to RTFM >> and believed based on the advanced-transactions page that >> rollback rolls *everything* back. >> >> Some kind of hint that there are execptions is IMO very >> important. I'm not sure what the best form for it to take is. > > I'm not sure, either. Maybe we should avoid blanket statements > and just say something like: > > Note: Some operations on sequences are non-transactional and will > not be rolled back on transaction abort. See <xref>. I also think it's a problem that one can get through the entire "Concurrency Control" chapter (mvcc.sgml) without a clue that sequences aren't transactional. I think maybe a mention in the Introduction section of that chapter with a <ref> would be appropriate. -Kevin
On Tuesday, August 07, 2012 09:45:35 AM Kevin Grittner wrote: [...snipped...] > I also think it's a problem that one can get through the entire > "Concurrency Control" chapter (mvcc.sgml) without a clue that > sequences aren't transactional. I think maybe a mention in the > Introduction section of that chapter with a <ref> would be > appropriate. > +1 > -Kevin
On Tue, Aug 07, 2012 at 03:59:42PM +0800, Craig Ringer wrote: > On 08/07/2012 02:27 AM, Robert Haas wrote: > >I did not commit the advanced.sgml changes. > > That's arguably the most important point to raise this. The most > recent question came from someone who actually bothered to RTFM and > believed based on the advanced-transactions page that rollback rolls > *everything* back. Perhaps we should see about correcting that misapprehension. When PostgreSQL does any irreversible process <http://en.wikipedia.org/wiki/Irreversible_process> such as incrementing a sequence, writing a file, sending an email, etc., it can't be rolled back. Might it be useful to find those irreversible operations we document and mark same? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 08/07/2012 09:45 PM, Kevin Grittner wrote: > I also think it's a problem that one can get through the entire > "Concurrency Control" chapter (mvcc.sgml) without a clue that > sequences aren't transactional. I think maybe a mention in the > Introduction section of that chapter with a <ref> would be > appropriate. How about this? Is it accurate to suggest that sequences behave as if they were always in "dirty read" isolation? Or would you instead say that "changes made to a sequence are immediately visible to all other transactions" ? [PATCH] Make sure you can't read through mvcc.sgml without realising that SEQUENCEs and SERIAL don't follow the rules. --- doc/src/sgml/mvcc.sgml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index 8f88582..eed1f85 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *************** *** 260,265 **** --- 260,276 ---- command <xref linkend="sql-set-transaction">. </para> + <important> + <para> + Some PostgreSQL data types and functions have special transactional + behaviour. Notably, SEQUENCEs + behave as if the isolation level is Dirty Read, irrespective + of the current isolation level, and are exempt from transaction + rollback. See <xref linkend="functions-sequence">. + SEQUENCEs are used by the SERIAL data types. See <xref linkend="datatype-serial">. + </para> + </important> + <sect2 id="xact-read-committed"> <title>Read Committed Isolation Level</title> -- 1.7.11.2 -- Craig Ringer
On 08/07/2012 09:45 PM, Kevin Grittner wrote: > I also think it's a problem that one can get through the entire > "Concurrency Control" chapter (mvcc.sgml) without a clue that > sequences aren't transactional. I'm also wondering about adding something like the following summary of features with odd transactional behaviour. I'm sure there's more than I've listed, but nothing is jumping out at me. <sect1 id="mvcc-exceptions"> <title>Exceptions to normal transactional rules</title> <para> Some PostgreSQL features, functions and data types differ from the usual transactional behaviour describedin this chapter. Differences are generally mentioned in the documentation sections for the features they affect.Such exceptions are collected here for easy reference. </para> <itemizedlist> <listitem> <para> Serial pseudo-types <xref linkend="datatype-serial"> </para> </listitem> <listitem> <para> <literal>SEQUENCE</literal>s - <xref linkend="functions-sequence"> </para> </listitem> <listitem> <para> Advisory locks - <xref linkend="advisory-locks"> </para> </listitem> <listitem> <para> Disk I/O to files outside the database, as performed by <literal>COPY ... TO</literal>,adminpack functions, and other add-ons. See <xref linkend="sql-copy">, <xref linkend="adminpack">. </para> </listitem> </itemizedlist> </sect1>
2012/8/7 Kevin Grittner <Kevin.Grittner@wicourts.gov>: > I also think it's a problem that one can get through the entire > "Concurrency Control" chapter (mvcc.sgml) without a clue that > sequences aren't transactional. It is possible to say that they *are* transactional when considering the following definition: nextval() doesn’t always give you “the” next value, but “some” next value that is higher than the one gotten by any preceding transactions. I personally like it better to introduce this minor complexity in the definition of sequences, rather than messing with the definition of transactionality. Nicolas -- A. Because it breaks the logical sequence of discussion. Q. Why is top posting bad?
On 08/18/2012 05:19 PM, Nicolas Barbier wrote: > 2012/8/7 Kevin Grittner <Kevin.Grittner@wicourts.gov>: > >> I also think it's a problem that one can get through the entire >> "Concurrency Control" chapter (mvcc.sgml) without a clue that >> sequences aren't transactional. > > It is possible to say that they *are* transactional when considering > the following definition: nextval() doesn’t always give you “the” next > value, but “some” next value that is higher than the one gotten by any > preceding transactions. > > I personally like it better to introduce this minor complexity in the > definition of sequences, rather than messing with the definition of > transactionality. I guess they're semi-transactional. You don't get dirty reads unless you actually `SELECT ... FROM some_transaction` which isn't really official API. OTOH, one transaction affects another, and they aren't subject to rollbacks. -- Craig Ringer
On Sat, Aug 18, 2012 at 1:34 AM, Craig Ringer <ringerc@ringerc.id.au> wrote: > On 08/07/2012 09:45 PM, Kevin Grittner wrote: >> >> I also think it's a problem that one can get through the entire >> "Concurrency Control" chapter (mvcc.sgml) without a clue that >> sequences aren't transactional. I think maybe a mention in the >> Introduction section of that chapter with a <ref> would be >> appropriate. > > > How about this? Is it accurate to suggest that sequences behave as if they > were always in "dirty read" isolation? I don't think so. I would think that a dirty read would allow unresolved data to be visible, but upon rollback of the other transaction would stop seeing the "dirty" data. That doesn't describe sequences. A better explanation is that sequence advancement is autonomously committed. > Or would you instead say that > "changes made to a sequence are immediately visible to all other > transactions" ? Yes, that sounds better. Cheers, Jeff
On 08/19/2012 03:01 AM, Jeff Janes wrote: >> >Or would you instead say that >> >"changes made to a sequence are immediately visible to all other >> >transactions" ? > Yes, that sounds better. OK, how about the attached series, then? The 2nd probably needs improvement - and I expect I've missed some other areas that aren't strictly transactional. Comments? Working branch: https://github.com/ringerc/postgres/tree/sequence_documentation_fixes -- Craig Ringer
Вложения
Trying again with the attachments; the archiver only seemed to see the first patch despite all three being attached. Including patches inline; if you want 'em prettier, see: https://github.com/ringerc/postgres/tree/sequence_documentation_fixes Subject: [PATCH 1/3] Make sure you can't read through mvcc.sgml without realising that not everything is MVCC. --- doc/src/sgml/mvcc.sgml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index 8f88582..9dc65f5 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *************** *** 260,265 **** --- 260,277 ---- command <xref linkend="sql-set-transaction">. </para> + <important> + <para> + Some <productname>PostgreSQL</productname> data types and functions have + special rules regarding transactional behaviour. In particular, changes + made to a <literal>SEQUENCE</literal> (and therefore the counter of a + <literal>SERIAL</literal>) are immediately visible to all other + transactions and are not rolled back if the transaction that made the + changes aborts. See <xref linkend="functions-sequence"> and + <xref linkend="datatype-serial">. + </para> + </important> + <sect2 id="xact-read-committed"> <title>Read Committed Isolation Level</title> -- 1.7.11.2 Subject: [PATCH 2/3] Collect a list of features with abberant transactional behaviour --- doc/src/sgml/mvcc.sgml | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index 9dc65f5..e2930c9 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *************** SELECT pg_advisory_lock(q.id) FROM *** 1540,1543 **** --- 1540,1610 ---- indexes should be used instead. </para> </sect1> + + <sect1 id="mvcc-exceptions"> + <title>Exceptions to normal transactional rules</title> + + <para> + Some PostgreSQL features, functions and data types differ from the + usual transactional behaviour described in this chapter. Differences + are generally mentioned in the documentation sections for the + features they affect. Such exceptions are collected here for + easy reference. + </para> + + <para> + The following actions and features don't follow the typical + transactional rules: + </para> + + <itemizedlist> + <listitem> + <para> + Serial pseudo-types <xref linkend="datatype-serial"> + </para> + </listitem> + <listitem> + <para> + <literal>SEQUENCE</literal>s - <xref linkend="functions-sequence"> + </para> + </listitem> + <listitem> + <para> + Advisory locks - <xref linkend="advisory-locks"> + </para> + </listitem> + <listitem> + <para> + Disk writes to files outside the database, as performed by + <literal>COPY ... TO</literal>, adminpack functions, and other add-ons. + See <xref linkend="sql-copy">, <xref linkend="adminpack">. + </para> + </listitem> + <listitem> + <para> + Any network I/O or inter-process communication not explicitly + described as transactional in its documentation. For example, + sending an email from PL/PerlU would not be transactional; + the email would be sent before the transaction commits and + could not be un-sent if the transaction were to roll back. + </listitem> + </itemizedlist> + + <note> + <para> + When working with external non-transactional resources like files + on disk or network sockets the two-phase commit feature can be + useful. See: <xref linkend="sql-prepare-transaction"> + </para> + <para> + LISTEN/NOTIFY provides a lighter weight but still transaction-friendly method of + triggering changes outside the database in response to changes inside the + database. A LISTENing helper program running outside the database can + perform actions when it gets a NOTIFY after a transaction commits. See: + <xref linkend="sql-notify">. + </para> + </note> + + </sect1> + </chapter> -- 1.7.11.2 Subject: [PATCH 3/3] Change xref of <important/> note re SERIAL to point to mvcc-exceptions --- doc/src/sgml/mvcc.sgml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index e2930c9..0de4b75 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *************** *** 267,274 **** made to a <literal>SEQUENCE</literal> (and therefore the counter of a <literal>SERIAL</literal>) are immediately visible to all other transactions and are not rolledback if the transaction that made the ! changes aborts. See <xref linkend="functions-sequence"> and ! <xref linkend="datatype-serial">. </para> </important> --- 267,273 ---- made to a <literal>SEQUENCE</literal> (and therefore the counter of a <literal>SERIAL</literal>) are immediately visible to all other transactions and are not rolledback if the transaction that made the ! changes aborts. See <xref linkend="mvcc-exceptions">. </para> </important> -- 1.7.11.2
On Mon, Aug 20, 2012 at 4:45 AM, Craig Ringer <ringerc@ringerc.id.au> wrote: > Trying again with the attachments; the archiver only seemed to see the first > patch despite all three being attached. Including patches inline; if you > want 'em prettier, see: > > https://github.com/ringerc/postgres/tree/sequence_documentation_fixes > > > Subject: [PATCH 1/3] Make sure you can't read through mvcc.sgml without > realising that not everything is MVCC. > The first of these three patches looks good to me, so I committed it. I am not convinced that the others are ready to go in. AFAICS, there hasn't been any discussion of whether a list of non-transactional features would be a useful thing to have, or if so where it should be located in the docs and what should go into it. I'm not necessarily opposed to adding something, but I think it needs some actual discussion before we commit anything. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 08/21/2012 11:18 PM, Robert Haas wrote: > On Mon, Aug 20, 2012 at 4:45 AM, Craig Ringer <ringerc@ringerc.id.au> wrote: >> Trying again with the attachments; the archiver only seemed to see the first >> patch despite all three being attached. Including patches inline; if you >> want 'em prettier, see: >> >> https://github.com/ringerc/postgres/tree/sequence_documentation_fixes >> >> >> Subject: [PATCH 1/3] Make sure you can't read through mvcc.sgml without >> realising that not everything is MVCC. >> > > The first of these three patches looks good to me, so I committed it. > I am not convinced that the others are ready to go in. AFAICS, there > hasn't been any discussion of whether a list of non-transactional > features would be a useful thing to have, or if so where it should be > located in the docs and what should go into it. I'm not necessarily > opposed to adding something, but I think it needs some actual > discussion before we commit anything. Fine by me; I just thought a concrete proposed change might get people talking about it better than my doing some more broad hand-waving on the topic. Anyone? Should we add a section that lists exceptions to normal transactional behaviour in one place, so instead of having to say "SEQUENCEs and some other features" or "various types, functions and features" there's something *concrete* to point to when discussing transactional oddities? + + <sect1 id="mvcc-exceptions"> + <title>Exceptions to normal transactional rules</title> + + <para> + Some PostgreSQL features, functions and data types differ from the + usual transactional behaviour described in this chapter. Differences + are generally mentioned in the documentation sections for the + features they affect. Such exceptions are collected here for + easy reference. + </para> + + <para> + The following actions and features don't follow the typical + transactional rules: + </para> + + <itemizedlist> + <listitem> + <para> + Serial pseudo-types <xref linkend="datatype-serial"> + </para> + </listitem> + <listitem> + <para> + <literal>SEQUENCE</literal>s - <xref linkend="functions-sequence"> + </para> + </listitem> + <listitem> + <para> + Advisory locks - <xref linkend="advisory-locks"> + </para> + </listitem> + <listitem> + <para> + Disk writes to files outside the database, as performed by + <literal>COPY ... TO</literal>, adminpack functions, and other add-ons. + See <xref linkend="sql-copy">, <xref linkend="adminpack">. + </para> + </listitem> + <listitem> + <para> + Any network I/O or inter-process communication not explicitly + described as transactional in its documentation. For example, + sending an email from PL/PerlU would not be transactional; + the email would be sent before the transaction commits and + could not be un-sent if the transaction were to roll back. + </listitem> + </itemizedlist> + + <note> + <para> + When working with external non-transactional resources like files + on disk or network sockets the two-phase commit feature can be + useful. See: <xref linkend="sql-prepare-transaction"> + </para> + <para> + LISTEN/NOTIFY provides a lighter weight but still transaction-friendly method of + triggering changes outside the database in response to changes inside the + database. A LISTENing helper program running outside the database can + perform actions when it gets a NOTIFY after a transaction commits. See: + <xref linkend="sql-notify">. + </para> + </note> + + </sect1> + </chapter>