Re: Fwd: Proposal: variant of regclass

On Sat, Apr 5, 2014 at 1:10 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> The reason of this behavior is that in out functions (regclassout), we return
> the OID as it is incase it doesn't exist.  One way to fix this is incase of
> OID input parameters, we check if the passed OID exists in to_* functions
> and return NULL if it doesn't exist. I think by this way we can retain
> the similarity of input parameters between types and to_* functions and
> making to_* functions return NULL incase OID doesn't exist makes it
> similar to case when user passed name.

We could do that, but that's not my preferred fix.

>> My suggestion is to
>> restructure the code so that to_regclass() only accepts a name, not an
>> OID, and make its charter precisely to perform a name lookup and
>> return an OID (as regclass) or NULL if there's no match.
>
> How will we restrict user from passing some number in string form?
> Do you mean that incase user has passed OID, to_* functions should
> return NULL or if found that it is OID then return error incase of to_*
> functions?

Each of the _guts functions first handles the case where the input is
exactly "-"; then it checks for an all-numeric value, which is
interpreted an OID; then it has a lengthy chunk of logic to handle the
case where we're in bootstrap mode; and if none of those cases handle
the situation, then it ends by doing the lookup in the "normal" way,
in each case marked with a comment that says "Normal case".  I think
that we should do only the last part - what in each case follows the
"normal case" comment - for the to_reg* functions.

In other words, let's revert the whole refactoring of this file to
create reg*_guts functions, and instead just copy the relevant logic
for the name lookups into the new functions.  For to_regproc(), for
example, it would look like this (untested):
     names = stringToQualifiedNameList(pro_name_or_oid);     clist = FuncnameGetCandidates(names, -1, NIL, false,
false,false);     if (clist == NULL || clist->next != NULL)          result = InvalidOid;     else          result =
clist->oid;

With that change, this patch will actually get a whole lot smaller,
change less already-existing code, and deliver cleaner behavior.

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