Re: Fwd: Proposal: variant of regclass

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: CABRT9RDwj3NPYkECNdL_F-a3z_aOzE6O6oA-aqr_E9O6gpeqcg(at)mail(dot)gmail(dot)com
Cc: Marti Raudsepp <marti(at)juffo(dot)org>, 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>
Subject: Re: Fwd: Proposal: variant of regclass
Date: 2014-01-28 08:38:11
Message-ID: 20140128173811.e5360aa5.nagata@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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>

Attachment Content-Type Size
to_regclass.patch.v2 application/octet-stream 50.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2014-01-28 08:39:50 Re: Infinite recursion in row-security based on updatable s.b. views
Previous Message Christian Kruse 2014-01-28 08:08:35 Re: Patch: Show process IDs of processes holding a lock; show relation and tuple infos of a lock to acquire