Re: RangeVarGetRelid()

On Thu, Nov 17, 2011 at 11:49:06PM -0500, Robert Haas wrote:
> On Thu, Nov 17, 2011 at 10:48 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Thu, Nov 17, 2011 at 08:59:58PM -0500, Robert Haas wrote:
> >> --- a/src/include/catalog/namespace.h
> >> +++ b/src/include/catalog/namespace.h
> >> @@ -47,9 +47,15 @@ typedef struct OverrideSearchPath
> >> ? ? ? bool ? ? ? ? ? ?addTemp; ? ? ? ? ? ? ? ?/* implicitly prepend temp schema? */
> >> ?} OverrideSearchPath;
> >>
> >> +typedef void (*RangeVarGetRelidCallback)(const RangeVar *relation, Oid relId,
> >> + ? ? Oid oldRelId);
> >>
> >> -extern Oid ? RangeVarGetRelid(const RangeVar *relation, LOCKMODE lockmode,
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bool missing_ok, bool nowait);
> >> +#define RangeVarGetRelid(relation, lockmode, missing_ok, nowait) \
> >> + ? ? ? ? ? ? RangeVarGetRelidExtended(relation, lockmode, missing_ok, nowait, NULL)
> >> +
> >> +extern Oid ? RangeVarGetRelidExtended(const RangeVar *relation,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?LOCKMODE lockmode, bool missing_ok, bool nowait,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?RangeVarGetRelidCallback callback);
> >
> > I like the split of RangeVarGetRelidExtended from RangeVarGetRelid. ?Shall we
> > also default missing_ok=false and nowait=false for RangeVarGetRelid?
>
> I thought about that, but just did it this way for now to keep the
> patch small for review purposes.

Fair enough.

> nowait = true is only used in one
> place, so it probably makes sense to default it to false in the
> non-extended version. But there are a number of callers who want
> missing_ok = true behavior, so I'm inclined not to think that one
> should be available in the non-extended version.

[assuming the "not" in your last sentence was unintended]

I count 1/25 callers overriding nowait and 3/25 overriding missing_ok. So, it's
looking like a less-common override than the callback function will come to be.