On Wed, Apr 2, 2014 at 11:27 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> Right, it will get reset in error. However still we need to free for missing_ok
> case and when it is successful in getting typeid. So don't you think it is
> better to just free once before calling LookupTypeName()?
>
> The code is right in it's current form as well, it's just a minor suggestion
> for improvement, so if you think current way the code written is okay, then
> ignore this suggestion.
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. 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.
Another thing I noticed is that you'll still get an error from
to_regtype() if the target type is a shell type:
rhaas=# create type i_am_a_shell;
CREATE TYPE
rhaas=# select 'i_am_a_shell'::regtype;
ERROR: type "i_am_a_shell" is only a shell
LINE 1: select 'i_am_a_shell'::regtype;
^
rhaas=# select to_regtype('i_am_a_shell');
ERROR: type "i_am_a_shell" is only a shell
Since the point here is to ignore errors that would otherwise be
caused by types not existing, ignoring errors from a type being a
shell (i.e. barely existing) seems like a possibly good idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company