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.

> +     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?

> + }

In any event, we seem to be converging on a version of parts 0 and 1 that are
ready for committer.  However, Robert contends that this will not be committed
separately from part 2.  Unless someone wishes to contest that, I suggest we
mark this Returned with Feedback and let the CF entry for part 2 subsume its
future development.  Does that sound reasonable?

Thanks,
nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to