Re: [HACKERS] Vacuum analyze bug CAUGHT - Mailing list pgsql-hackers

Is this patch still valid?


[Charset iso-8859-1 unsupported, filtering to ASCII...]
> >
> > Hiroshi Inoue wrote:
> > >
> > > > crashtest=> vacuum analyze;
> > > > NOTICE:  Rel pg_type: TID 4/3: InsertTransactionInProgress 129915
> > > > - can't shrink relation
> > ...
> > >
> > > CREATE TABLE doesn't lock system tables till end of transaction.
> > > It's a cause of these NOTICE messages.
> > >
> > > Should we lock system tables till end of transaction ?
> >
> > No, if we allow DDL statements inside BEGIN/END
> > (in long transaction).
> >
> > > Moreover CREATE TABLE doesn't acquire any lock for pg_attribute
> > >                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > while tuples are inserted into pg_attribute.
> > > Concurrent vacuum may corrupt pg_attribute.
> >
> > Should be fixed!
> >
>
> Seems CREATE TABLE don't acquire any lock for pg_relcheck and
> pg_attrdef as well as pg_attribute. There may be other pg_.......
>
> Here is a patch.
> This patch also removes UnlockRelation() in heap_destroy_with_catalog().
>
> Regards.
>
> Hiroshi Inoue
> Inoue@tpf.co.jp
>
> *** catalog/heap.c.orig    Tue Sep  7 08:52:04 1999
> --- catalog/heap.c    Fri Sep 10 16:43:18 1999
> ***************
> *** 547,552 ****
> --- 547,553 ----
>        */
>       Assert(rel);
>       Assert(rel->rd_rel);
> +     LockRelation(rel, AccessExclusiveLock);
>       hasindex = RelationGetForm(rel)->relhasindex;
>       if (hasindex)
>           CatalogOpenIndices(Num_pg_attr_indices, Name_pg_attr_indices, idescs);
> ***************
> *** 607,612 ****
> --- 608,614 ----
>           dpp++;
>       }
>
> +     UnlockRelation(rel, AccessExclusiveLock);
>       heap_close(rel);
>
>       /*
> ***************
> *** 1330,1336 ****
>
>       rel->rd_nonameunlinked = TRUE;
>
> -     UnlockRelation(rel, AccessExclusiveLock);
>
>       heap_close(rel);
>
> --- 1332,1337 ----
> ***************
> *** 1543,1553 ****
> --- 1544,1556 ----
>       values[Anum_pg_attrdef_adbin - 1] =
> PointerGetDatum(textin(attrdef->adbin));
>       values[Anum_pg_attrdef_adsrc - 1] =
> PointerGetDatum(textin(attrdef->adsrc));
>       adrel = heap_openr(AttrDefaultRelationName);
> +     LockRelation(adrel, AccessExclusiveLock);
>       tuple = heap_formtuple(adrel->rd_att, values, nulls);
>       CatalogOpenIndices(Num_pg_attrdef_indices, Name_pg_attrdef_indices,
> idescs);
>       heap_insert(adrel, tuple);
>       CatalogIndexInsert(idescs, Num_pg_attrdef_indices, adrel, tuple);
>       CatalogCloseIndices(Num_pg_attrdef_indices, idescs);
> +     UnlockRelation(adrel, AccessExclusiveLock);
>       heap_close(adrel);
>
>       pfree(DatumGetPointer(values[Anum_pg_attrdef_adbin - 1]));
> ***************
> *** 1606,1616 ****
> --- 1609,1621 ----
>       values[Anum_pg_relcheck_rcbin - 1] =
> PointerGetDatum(textin(check->ccbin));
>       values[Anum_pg_relcheck_rcsrc - 1] =
> PointerGetDatum(textin(check->ccsrc));
>       rcrel = heap_openr(RelCheckRelationName);
> +     LockRelation(rcrel, AccessExclusiveLock);
>       tuple = heap_formtuple(rcrel->rd_att, values, nulls);
>       CatalogOpenIndices(Num_pg_relcheck_indices, Name_pg_relcheck_indices,
> idescs);
>       heap_insert(rcrel, tuple);
>       CatalogIndexInsert(idescs, Num_pg_relcheck_indices, rcrel, tuple);
>       CatalogCloseIndices(Num_pg_relcheck_indices, idescs);
> +     UnlockRelation(rcrel, AccessExclusiveLock);
>       heap_close(rcrel);
>
>       pfree(DatumGetPointer(values[Anum_pg_relcheck_rcname - 1]));
>
>
>
>
> ************
>
>


--
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026