On Sat, Jul 09, 2011 at 10:52:33AM +0200, Kohei KaiGai wrote:
> 2011/7/9 Noah Misch <noah@2ndquadrant.com>:
> > On Sat, Jul 09, 2011 at 09:00:30AM +0200, Kohei KaiGai wrote:
> >> The attached patch is a revised version according to the approach that updates
> >> pg_class system catalog before AlterTableInternal().
> >> It invokes the new ResetViewOptions when rel->rd_options is not null, and it set
> >> null on the pg_class.reloptions of the view and increments command counter.
> >
> >> + /*
> >> + * ResetViewOptions
> >> + *
> >> + * It clears all the reloptions prior to replacing
> >> + */
> >> + static void
> >> + ResetViewOptions(Oid viewOid)
> >> + {
> >> + Relation pg_class;
> >> + HeapTuple oldtup;
> >> + HeapTuple newtup;
> >> + Datum values[Natts_pg_class];
> >> + bool nulls[Natts_pg_class];
> >> + bool replaces[Natts_pg_class];
> >> +
> >> + pg_class = heap_open(RelationRelationId, RowExclusiveLock);
> >> +
> >> + oldtup = SearchSysCache1(RELOID, DatumGetObjectId(viewOid));
> >
> > Use SearchSysCacheCopy1, since you're modifying the tuple.
> >
> The heap_modify_tuple() allocates a new tuple as a copy of old tuple.
> No need to worry about.
Ah, yes. Sorry for the noise.
> >> + if (!HeapTupleIsValid(oldtup))
> >> + elog(ERROR, "cache lookup failed for relation %u", viewOid);
> >> +
> >> + memset(values, 0, sizeof(values));
> >> + memset(nulls, false, sizeof(nulls));
> >> + memset(replaces, false, sizeof(replaces));
> >> +
> >> + replaces[Anum_pg_class_reloptions - 1] = true;
> >> + nulls[Anum_pg_class_reloptions - 1] = true;
> >> +
> >> + newtup = heap_modify_tuple(oldtup, RelationGetDescr(pg_class),
> >> + values, nulls, replaces);
> >> + simple_heap_update(pg_class, &newtup->t_self, newtup);
> >> +
> >> + CatalogUpdateIndexes(pg_class, newtup);
> >> +
> >> + ReleaseSysCache(oldtup);
> >> +
> >> + heap_close(pg_class, RowExclusiveLock);
> >> +
> >> + CommandCounterIncrement();
> >
> > Why is a CCI necessary?
> >
> ATExecSetRelOptions() reference the view to be updated using syscache,
> however, this update will not become visible without CCI.
> In the result, it will reference old tuple, then get an error because
> it tries to
> update already updated tuple.
Okay, thanks.
