Re: Fwd: Proposal: variant of regclass

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