From: | Noah Misch <noah(at)2ndQuadrant(dot)com> |
---|---|
To: | Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kohei Kaigai <Kohei(dot)Kaigai(at)emea(dot)nec(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [v9.2] Fix leaky-view problem, part 1 |
Date: | 2011-07-09 09:36:40 |
Message-ID: | 20110709093639.GB2142@tornado.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jul 09, 2011 at 10:52:33AM +0200, Kohei KaiGai wrote:
> 2011/7/9 Noah Misch <noah(at)2ndquadrant(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | hubert depesz lubaczewski | 2011-07-09 10:29:25 | Enhanced psql in core? |
Previous Message | Kohei KaiGai | 2011-07-09 08:52:33 | Re: [v9.2] Fix leaky-view problem, part 1 |