On Wed, 22 Jan 2014 20:04:12 +0900 (JST) Tatsuo Ishii <is...@postgresql.org> wrote:
> > 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? > > 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 <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. > > 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 <nag...@sraoss.co.jp> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers