Re: Fwd: Proposal: variant of regclass - Mailing list pgsql-hackers

From Yugo Nagata
Subject Re: Fwd: Proposal: variant of regclass
Date
Msg-id 20140226152612.c5ceed85.nagata@sraoss.co.jp
Whole thread Raw
In response to Re: Fwd: Proposal: variant of regclass  (Marti Raudsepp <marti@juffo.org>)
Responses Re: Fwd: Proposal: variant of regclass  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
Thanks for your a lot of comments. I revised the patch according to
comments from Robert Haas and Marti Raudsepp.

I changed to_reg* functions to return NULL if the object isn't found.
They return InvalidOid(0) when '0' is passed as parameter, of course.

I also tested this in bootstrap mode by editting postgres.bki as:

create pg_test 10000 without_oids
 (
 oper = regoper,
 proc = regproc,
 class = regclass,
 type = regtype
 )
open pg_test
insert (0,0,0,0)
insert ("||/", "now", "pg_class", "int4")
close pg_test

Regards,

Yugo Nagata

Robert Haas <robertmhaas@gmail.com> wrote:

> This patch contains several whitespace-only hunks.  Please revert them.
>
> I don't like the changes to typenameTypeIdAndMod().  The code for the
> missing_ok case shouldn't look completely different from the code for
> the !missing_ok case.  It would be cleaner to start by duplicating
> typenameType into typenameTypeIdAndMod and then adjusting it as needed
> to support the new argument.  It might be better still to just change
> parseTypeString() to call LookupTypeName() directly, and add the
> necessary logic to handle missing_ok there.  The advantage of that is
> that you wouldn't need to modify everybody who is calling
> typenameTypeIdAndMod(); there are a decent number of such callers
> here, and there might be third-party code calling that as well.
>
> I think the changes this patch makes to OpernameGetCandidates() need a
> bit of further consideration.  The new argument is called missing_ok,
> but OpernameGetCandidates() can already return an empty list.  What
> that new argument does is tolerate a missing schema name.  So we could
> call the argument missing_schema_ok, I guess, but I'm still not sure I
> like that.  I don't have a better proposal right at the moment,
> though.

On Thu, 6 Feb 2014 21:25:01 +0200
Marti Raudsepp <marti@juffo.org> wrote:
> * You added some unnecessary spaces at the beginning of the linein
> OpernameGetCandidates.
> * regclass_guts and regtype_guts can be simplified further by moving
> the ereport() code into regclassin, thereby saving the "if
> (missing_ok)"
> * I would rephrase the documentation paragraph as:
>
> to_regclass, to_regproc, to_regoper and to_regtype are functions
> similar to the regclass, regproc, regoper and regtype casts, except
> that they return InvalidOid (0) when the object is not found, instead
> of raising an error.
>
> On Wed, Jan 22, 2014 at 1:04 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
> >> I thought the consensus was that returning NULL is better than
> >> InvalidOid? From an earlier message:
>
> > 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
>
> And there are pg_relation_filenode, pg_stat_get_backend_dbid,
> pg_stat_get_backend_userid which return NULL::oid. In general I don't
> like magic values like 0 in SQL. For example, this query would give
> unexpected results because InvalidOid has some other meaning:
>
> select * from pg_aggregate where aggfinalfn=to_regproc('typo');
>
> Regards,
> Marti
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


--
Yugo Nagata <nagata@sraoss.co.jp>

Attachment

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: jsonb and nested hstore
Next
From: Kouhei Kaigai
Date:
Subject: Re: Custom Scan APIs (Re: Custom Plan node)