Re: Fwd: Proposal: variant of regclass

Hi Amit,

Thanks for your reviewing. I updated the patch.
I fixed the oids and removed the witespace.

On Fri, 31 Jan 2014 16:09:35 +0530
Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com> wrote:

> 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 <nagata(at)sraoss(dot)co(dot)jp> 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 <nagata(at)sraoss(dot)co(dot)jp> wrote:
> >
> > > On Thu, 23 Jan 2014 13:19:37 +0200
> > > Marti Raudsepp <marti(at)juffo(dot)org> 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.
> > > >
> > > > <yugo-n(at)sranhm(dot)sra(dot)co(dot)jp>: mail for srasce.sra.co.jp loops back to
> > myself
> > > > <t-ishii(at)sra(dot)co(dot)jp>: mail for srasce.sra.co.jp loops back to myself
> > > >
> > > > ---------- Forwarded message ----------
> > > > From: Marti Raudsepp <marti(at)juffo(dot)org>
> > > > Date: Thu, Jan 23, 2014 at 3:39 AM
> > > > Subject: Re: [HACKERS] Proposal: variant of regclass
> > > > To: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
> > > > Cc: Tatsuo Ishii <ishii(at)postgresql(dot)org>, pgsql-hackers
> > > > <pgsql-hackers(at)postgresql(dot)org>, Vik Fearing <vik(dot)fearing(at)dalibo(dot)com>,
> > > > Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>,
> > > > Pavel Golub <pavel(at)gf(dot)microolap(dot)com>, Pavel Golub
> > > > <pavel(at)microolap(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Pavel
> > > > Stěhule <pavel(dot)stehule(at)gmail(dot)com>
> > > >
> > > >
> > > > On Wed, Jan 22, 2014 at 1:44 PM, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
> > wrote:
> > > > > On Wed, 22 Jan 2014 20:04:12 +0900 (JST)
> > > > > Tatsuo Ishii <ishii(at)postgresql(dot)org> 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 <nagata(at)sraoss(dot)co(dot)jp>
> > >
> > >
> > > --
> > > Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> > > To make changes to your subscription:
> > > http://www.postgresql.org/mailpref/pgsql-hackers
> >
> >
> > --
> > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
> >
> >
> > --
> > Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
> >
> >

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>