On Fri, Apr 4, 2014 at 8:48 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> I see. Here's an updated patch with a bit of minor refactoring to
> clean that up, and some improvements to the documentation.
>
> I was all ready to commit this when I got cold feet. What's bothering
> me is that the patch, as written, mimics the exact behavior of the
> text->regproc cast, including the fact that the supplying an OID,
> written as a number, will always return that OID, whether it exists or
> not:
>
> rhaas=# select to_regclass('1259'), to_regclass('pg_class');
> to_regclass | to_regclass
> -------------+-------------
> pg_class | pg_class
> (1 row)
>
> rhaas=# select to_regclass('12590'), to_regclass('does_not_exist');
> to_regclass | to_regclass
> -------------+-------------
> 12590 |
> (1 row)
>
> I think that's unacceptably weird behavior.
Agreed. Actually I had also noticed this behaviour, but ignored it thinking
that we should just consider behavior change for to_ function incase name
is passed. However after you pointed, it looks pretty wrong for OID cases.
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.
> 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?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com