There are duplicate oids in pg_proc.h : make[3]: Entering directory `/tmp/git-pg/src/backend/catalog' cd ../../../src/include/catalog && '/usr/bin/X11/perl' ./duplicate_oids 3180 3195 3196 3197
------------- There is a whitespace diff in regoperatorin and regprocedurein() definition. ------------ Other than this, there are no more issues from my side. I have only checked the part of the patch that was modified as per *my* review comments. I will leave the rest of the review for the other reviewer. On 28 January 2014 14:08, Yugo Nagata <[email protected]> wrote: > Hi Amit and Marti, > > I revised the patch. Could you please review this? > > The fixes include: > > - Fix *_guts() function definition to return Oid instead of Datum > > - Fix to_regproc() definition in pg_proc.h > > - Fix some indentation > > - Add regression test > > - Fix to use missing_ok instead of raiseError > > - Merge parseTypeString > > - Remove *_guts() and *MissingOk() and merge to one function about > parseTypeString and typenameTypeIdAndMod > > - Fix to not raise error even when schema name doesn't exist > > This is a new from the previous patch. In previous, specifying wrong > schema > name raises an error like: > > =# SELECT to_regproc('ng_catalog.now'); > ERROR : schema "ng_catalog" doew not exist > > > On Fri, 24 Jan 2014 12:35:27 +0900 > Yugo Nagata <[email protected]> wrote: > > > On Thu, 23 Jan 2014 13:19:37 +0200 > > Marti Raudsepp <[email protected]> wrote: > > > > > Resending to Tatsuo Ishii and Yugo Nagata, your email server was > > > having problems yesterday: > > > > Thanks for resending! > > > > > > > > This is the mail system at host sraigw2.sra.co.jp. > > > > > > <[email protected]>: mail for srasce.sra.co.jp loops back to > myself > > > <[email protected]>: mail for srasce.sra.co.jp loops back to myself > > > > > > ---------- Forwarded message ---------- > > > From: Marti Raudsepp <[email protected]> > > > Date: Thu, Jan 23, 2014 at 3:39 AM > > > Subject: Re: [HACKERS] Proposal: variant of regclass > > > To: Yugo Nagata <[email protected]> > > > Cc: Tatsuo Ishii <[email protected]>, pgsql-hackers > > > <[email protected]>, Vik Fearing <[email protected]>, > > > Robert Haas <[email protected]>, Tom Lane <[email protected]>, > > > Pavel Golub <[email protected]>, Pavel Golub > > > <[email protected]>, Andres Freund <[email protected]>, Pavel > > > Stěhule <[email protected]> > > > > > > > > > On Wed, Jan 22, 2014 at 1:44 PM, Yugo Nagata <[email protected]> > wrote: > > > > On Wed, 22 Jan 2014 20:04:12 +0900 (JST) > > > > Tatsuo Ishii <[email protected]> wrote: > > > > 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? > > > > > > Yes, for parseTypeString certainly. There have been many refactorings > > > like that in the past and all of them use this pattern. > > > > Ok. I'll rewrite the definition and there callers. > > > > > > > > typenameTypeIdAndMod is less clear since the code paths differ so > > > much, maybe keep 2 versions (merging back to 1 function is OK too, but > > > in any case you don't need 3). > > > > I'll also fix this in either way to not use typenameTypeIdAndMod_guts. > > > > > > > > typenameTypeIdAndModMissingOk(...) > > > { > > > Type tup = LookupTypeName(pstate, typeName, typmod_p); > > > if (tup == NULL || !((Form_pg_type) GETSTRUCT(tup))->typisdefined) > > > *typeid_p = InvalidOid; > > > else > > > *typeid_p = HeapTupleGetOid(tup); > > > > > > if (tup) > > > ReleaseSysCache(tup); > > > } > > > typenameTypeIdAndMod(...) > > > { > > > Type tup = typenameType(pstate, typeName, typmod_p); > > > *typeid_p = HeapTupleGetOid(tup); > > > ReleaseSysCache(tup); > > > } > > > > > > ---- > > > > > > Also, there's no need for "else" here: > > > if (raiseError) > > > ereport(ERROR, ...); > > > else > > > return InvalidOid; > > > > > > Regards, > > > Marti > > > > > > -- > > Yugo Nagata <[email protected]> > > > > > > -- > > Sent via pgsql-hackers mailing list ([email protected]) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-hackers > > > -- > Yugo Nagata <[email protected]> > > > -- > Sent via pgsql-hackers mailing list ([email protected]) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > >
