On Wed, 22 Jan 2014 20:04:12 +0900 (JST)
Tatsuo Ishii <ishii@postgresql.org> wrote:
> > On Tue, Jan 14, 2014 at 9:28 AM, Yugo Nagata <nagata@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?
>
> Originally the proposal checks errors like syntactical one in addition
> to missing objects. So I think "raiseError" was more appropriate at
> that time. Now they only check missing objects. So renaming to
> "missing_ok" could be more appropriate.
>
> > 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.
>
> Leave the disccusion to Yugo..
parseTypeString() is called by some other functions and I avoided
influences of modifying the definition on them, since this should
raise errors in most cases. This is same reason for other *MissingOk
functions in parse_type.c.
Is it better to write definitions of these function and all there 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 <robertmhaas@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.
>
> Not sure. There's already at least one counter example:
>
> pg_my_temp_schema() oid OID of session's temporary schema, or 0 if none
>
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese: http://www.sraoss.co.jp
--
Yugo Nagata <nagata@sraoss.co.jp>
