From: | Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com> |
---|---|
To: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tatsuo Ishii <ishii(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(at)microolap(dot)com, Andres Freund <andres(at)2ndquadrant(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Subject: | Re: Proposal: variant of regclass |
Date: | 2014-01-20 07:11:07 |
Message-ID: | CACoZds1kFKgQv+qoyYbJf=231+f1NRh3u3r5pVSLktNrMM-tKg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
I have begun the review as part of the commitfest. Below are my comments
....
------------
*_guts() functions are defined as returning Datum, while they are actually
returning Oid. They should be defined as returning Oid.
Also the PG_RETURN_OID() has been still used in some of the *_guts()
functions. They should use 'return';
------------
In pg_proc.h, to_regproc() has been defined as function returning
*regclass* type.
------------
I, as a user would be happier if we also have to_regprocedure() and
to_regoperator(). The following query looks a valid use-case where one
needs to find if a particular function exists. Using to_regproc('sum') does
not make sense here because it will return InvalidOid, which will not tell
us whether that is because there is no such function or whether there are
duplicate function names.
select * from pg_proc where oid = to_regprocedure('sum(int)');
------------
The changes in parse_type.c look all good, except some cosmetic changes:
The parameters are not aligned one below the other for these two lines :
typenameTypeIdAndModMissingOk(ParseState *pstate, const TypeName *typeName,
Oid *typeid_p, int32 *typmod_p)
Similar thing for typenameTypeIdAndMod_guts().
--------------
Please also add some testcases in the regression tests.
On 14 January 2014 12:58, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> Here is the patch to implement to_regclass, to_regproc, to_regoper,
> and to_regtype. They are new functions similar to regclass, regproc,
> regoper, and regtype except that if requested object is not found,
> returns InvalidOid, rather than raises an error.
>
> On Tue, 31 Dec 2013 12:10:56 +0100
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
> > 2013/12/31 Tatsuo Ishii <ishii(at)postgresql(dot)org>
> >
> > > > On 12/31/2013 02:38 AM, Tatsuo Ishii wrote:
> > > >> Before proceeding the work, I would like to make sure that
> followings
> > > >> are complete list of new functions. Inside parentheses are
> > > >> corresponding original functions.
> > > >>
> > > >> toregproc (regproc)
> > > >> toregoper (regoper)
> > > >> toregclass (regclass)
> > > >> toregtype (regtype)
> > > >
> > > > Pardon the bikeshedding, but those are hard to read for me. I would
> > > > prefer to go with the to_timestamp() model and add an underscore to
> > > > those names.
> > >
> > > I have no problem with adding "to_". Objection?
> > >
> >
> > I like to_reg* too
> >
> > Regards
> >
> > Pavel
> >
> >
> > >
> > > Best regards,
> > > --
> > > Tatsuo Ishii
> > > SRA OSS, Inc. Japan
> > > English: http://www.sraoss.co.jp/index_en.php
> > > Japanese: http://www.sraoss.co.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
>
>
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2014-01-20 07:42:54 | Re: [PATCH] Negative Transition Aggregate Functions (WIP) |
Previous Message | Jeevan Chalke | 2014-01-20 06:23:58 | Re: [BUGS] surprising to_timestamp behavior |