Re: refactoring comment.c

On Fri, Aug 6, 2010 at 11:15 PM, KaiGai Kohei <kaigai@kaigai.gr.jp> wrote:
> [brief review]

OK, here's an updated patch:

1. I fixed the typo Alvaro spotted.

2. I haven't done anything about moving the definition of
ObjectAddress elsewhere, as Alvaro suggested, because I'm not sure
quite where it ought to go.  I still think it's a good idea, though
I'm not dead set on it, either.  Suggestions?

3. I fixed the issue Kaigai Kohei spotted, regarding
LargeObjectRelationId vs. LargeObjectMetadataRelationId, by adding a
grotty hack.  However, I feel that I'm not so much adding a new grotty
hack as working around an existing grotty hack which was added for
reasons I'm unclear on.  Is there a pg_upgrade-related reason not to
revert the original hack instead?

4. In response to Kaigai Kohei's complaint about lockmode possibly
being NoLock, I've just added an Assert() that it isn't, in lieu of
trying to do something sensible in that case.  I can't at present
think of a situation in which being able to call it that way would be
useful, and the Assert() seems like it ought to be enough warning to
anyone coming along later that they'd better think twice before
thinking that will work.

5. Since I'm hoping Tom will read this, I ran it through filterdiff.  :-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company