From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: refactoring comment.c |
Date: | 2010-08-19 19:34:23 |
Message-ID: | 26245.1282246463@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Any other kibitzing before I commit this?
Sure ...
+ * If the object is a relation or a child object of a relation (e.g. an
+ * attribute or contraint, *relp will set to point to that relation). This
Parenthesis in the wrong place here, grammar and spelling not much better.
Also, I still feel that this comment could do better about explaining the
behavior, particularly with respect to locking. Perhaps say
+ * If the target object is a relation or a child object of a relation
+ * (e.g. an attribute or constraint), the relation is also opened, and *relp
+ * receives the open relcache entry pointer; otherwise *relp is set to NULL.
+ * This is a bit grotty but it makes life simpler, since the caller will
+ * typically need the relcache entry too. Caller must close the relcache
+ * entry when done with it. The relation is locked with the specified
+ * lockmode if the target object is the relation itself or an attribute,
+ * but for other child objects, only AccessShareLock is acquired on the
+ * relation.
+ ScanKeyInit(&skey[0], ObjectIdAttributeNumber, BTEqualStrategyNumber,
+ F_OIDEQ, ObjectIdGetDatum(address.objectId));
There's a standard convention for the layout of ScanKeyInit calls, and
this isn't it. Trivial, I know, but it's better to make similar code
look similar.
There's no longer any need for a diff in src/backend/parser/Makefile.
+ #define OBJECTADDRESS_H
+
+ #include "nodes/parsenodes.h"
+ #include "nodes/pg_list.h"
+ #include "storage/lock.h"
+ #include "utils/rel.h"
You shouldn't need pg_list.h here, as parsenodes.h surely includes it.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2010-08-19 19:47:43 | Re: Fw: patch for pg_ctl.c to add windows service start-type |
Previous Message | Tom Lane | 2010-08-19 18:17:42 | Re: wip: functions median and percentile |