Re: ALTER TABLE lock strength reduction patch is unsafe

On Mon, Jan 02, 2012 at 04:33:28PM -0300, Alvaro Herrera wrote:
> 
> Excerpts from Noah Misch's message of lun ene 02 16:25:25 -0300 2012:
> > On Mon, Jan 02, 2012 at 06:41:31PM +0000, Simon Riggs wrote:
> > > On Mon, Jan 2, 2012 at 6:06 PM, Noah Misch <noah@leadboat.com> wrote:
> > > > On Mon, Jan 02, 2012 at 05:09:16PM +0000, Simon Riggs wrote:
> > > >> Attached patch makes SnapshotNow into an MVCC snapshot, initialised at
> > > >> the start of each scan iff SnapshotNow is passed as the scan's
> > > >> snapshot. It's fairly brief but seems to do the trick.
> > > >
> > > > That's a neat trick. ?However, if you start a new SnapshotNow scan while one is
> > > > ongoing, the primordial scan's snapshot will change mid-stream.
> > > 
> > > Do we ever do that? (and if so, Why?!? or perhaps just Where?)
> > 
> > I hacked up your patch a bit, as attached, to emit a WARNING for any nested
> > use of SnapshotNow.  This made 97/127 test files fail.  As one example,
> > RelationBuildRuleLock() does TextDatumGetCString() for every tuple of its
> > SnapshotNow scan.  That may need a detoast, which itself runs a scan.
> 
> Uh, I thought detoasting had its own visibility test function .. I mean,
> otherwise, what is HeapTupleSatisfiesToast for?

The SnapshotNow scan was actually to build the relcache entry for the toast
table before scanning the toast table itself.  Stack trace:

#0  InitSnapshotNowIfNeeded (snap=0xb7e8c0) at snapmgr.c:216
#1  0x000000000048f773 in index_beginscan_internal (indexRelation=0x7f9a091e5cc8, nkeys=2, norderbys=0, snapshot=0xb7e8c0) at indexam.c:295
#2  0x000000000048f7e9 in index_beginscan (heapRelation=0x7f9a091a6c60, indexRelation=0xb7e8c0, snapshot=0xb7e8c0, nkeys=152984776, norderbys=0) at indexam.c:238
#3  0x000000000048e1de in systable_beginscan (heapRelation=0x7f9a091a6c60, indexId=<value optimized out>, indexOK=<value optimized out>, snapshot=0xb7e8c0, nkeys=2, key=0x7fffe211b610) at genam.c:289
#4  0x00000000007287db in RelationBuildTupleDesc (targetRelId=<value optimized out>, insertIt=1 '\001') at relcache.c:455
#5  RelationBuildDesc (targetRelId=<value optimized out>, insertIt=1 '\001') at relcache.c:880
#6  0x000000000072a050 in RelationIdGetRelation (relationId=2838) at relcache.c:1567
#7  0x00000000004872c0 in relation_open (relationId=2838, lockmode=1) at heapam.c:910
#8  0x0000000000487328 in heap_open (relationId=12052672, lockmode=152984776) at heapam.c:1052
#9  0x000000000048a737 in toast_fetch_datum (attr=<value optimized out>) at tuptoaster.c:1586
#10 0x000000000048bf74 in heap_tuple_untoast_attr (attr=0xb7e8c0) at tuptoaster.c:135
#11 0x0000000000737d77 in pg_detoast_datum_packed (datum=0xb7e8c0) at fmgr.c:2266
#12 0x00000000006f0fb0 in text_to_cstring (t=0xb7e8c0) at varlena.c:135
#13 0x0000000000727083 in RelationBuildRuleLock (relation=0x7f9a091b3818) at relcache.c:673
#14 0x000000000072909e in RelationBuildDesc (targetRelId=<value optimized out>, insertIt=1 '\001') at relcache.c:886
#15 0x000000000072a050 in RelationIdGetRelation (relationId=11119) at relcache.c:1567
#16 0x00000000004872c0 in relation_open (relationId=11119, lockmode=0) at heapam.c:910
#17 0x0000000000487483 in relation_openrv_extended (relation=<value optimized out>, lockmode=<value optimized out>, missing_ok=<value optimized out>) at heapam.c:1011
#18 0x000000000048749c in heap_openrv_extended (relation=0xb7e8c0, lockmode=152984776, missing_ok=5 '\005') at heapam.c:1110
#19 0x000000000053570c in parserOpenTable (pstate=0xc885f8, relation=0xc88258, lockmode=1) at parse_relation.c:835
#20 0x000000000053594d in addRangeTableEntry (pstate=0xc885f8, relation=0xc88258, alias=0x0, inh=1 '\001', inFromCl=1 '\001') at parse_relation.c:901
#21 0x0000000000524dbf in transformTableEntry (pstate=0xc885f8, n=0xc88258, top_rte=0x7fffe211bd78, top_rti=0x7fffe211bd84, relnamespace=0x7fffe211bd70, containedRels=0x7fffe211bd68) at parse_clause.c:445
#22 transformFromClauseItem (pstate=0xc885f8, n=0xc88258, top_rte=0x7fffe211bd78, top_rti=0x7fffe211bd84, relnamespace=0x7fffe211bd70, containedRels=0x7fffe211bd68) at parse_clause.c:683
#23 0x0000000000526124 in transformFromClause (pstate=0xc885f8, frmList=<value optimized out>) at parse_clause.c:129
#24 0x000000000050c321 in transformSelectStmt (pstate=0xb7e8c0, parseTree=0xc88340) at analyze.c:896
#25 transformStmt (pstate=0xb7e8c0, parseTree=0xc88340) at analyze.c:186
#26 0x000000000050d583 in parse_analyze (parseTree=0xc88340, sourceText=0xc87828 "table pg_stat_user_tables;", paramTypes=0x0, numParams=0) at analyze.c:94
#27 0x00000000006789c5 in pg_analyze_and_rewrite (parsetree=0xc88340, query_string=0xc87828 "table pg_stat_user_tables;", paramTypes=0x0, numParams=0) at postgres.c:583
#28 0x0000000000678c77 in exec_simple_query (query_string=0xc87828 "table pg_stat_user_tables;") at postgres.c:941
#29 0x0000000000679997 in PostgresMain (argc=<value optimized out>, argv=<value optimized out>, username=0xbe03c0 "nm") at postgres.c:3881
#30 0x0000000000633d41 in BackendRun () at postmaster.c:3587
#31 BackendStartup () at postmaster.c:3272
#32 ServerLoop () at postmaster.c:1350
#33 0x0000000000635330 in PostmasterMain (argc=1, argv=0xbdc170) at postmaster.c:1110
#34 0x00000000005d206b in main (argc=1, argv=0xbdc170) at main.c:199