On Tue, Jan 14, 2014 at 9:28 AM, Yugo Nagata <nag...@sraoss.co.jp> wrote:
> Here is the patch to implement to_regclass, to_regproc, to_regoper,
> and to_regtype.

+ static Datum regclass_guts(char *class_name_or_oid, bool raiseError);

Minor bikeshedding, a lot of code currently uses an argument named
"missing_ok" for this purpose (with inverse meaning of course). Any
reasons why you chose "raiseError" instead?

I only had a brief look at the patch, so maybe I'm missing something.
But I don't think you should create 3 variants of these functions:
* parseTypeString calls parseTypeString_guts with false
* parseTypeStringMissingOk calls parseTypeString_guts with true
* parseTypeString_guts

And this is just silly:

if (raiseError)
    parseTypeString(typ_name_or_oid, &result, &typmod);
else
    parseTypeStringMissingOk(typ_name_or_oid, &result, &typmod);

Just add an argument to parseTypeString and patch all the callers.

> if requested object is not found,
> returns InvalidOid, rather than raises an error.

I thought the consensus was that returning NULL is better than
InvalidOid? From an earlier message:

On Thu, Dec 5, 2013 at 5:25 PM, Robert Haas <robertmh...@gmail.com> wrote:
> Another advantage of this approach is that, IIUC, type input functions
> can't return a NULL value.  So 'pg_klass'::regclass could return 0,
> but not NULL.  On the other hand, toregclass('pg_klass') *could*
> return NULL, which seems conceptually cleaner.

Regards,
Marti


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to