Re: Fwd: Proposal: variant of regclass

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Marti Raudsepp <marti(at)juffo(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
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>
Subject: Re: Fwd: Proposal: variant of regclass
Date: 2014-02-26 06:26:12
Message-ID: 20140226152612.c5ceed85.nagata@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(at)gmail(dot)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(at)juffo(dot)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(at)postgresql(dot)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(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.v4 application/octet-stream 42.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2014-02-26 06:50:32 Re: Custom Scan APIs (Re: Custom Plan node)
Previous Message Stephen Frost 2014-02-26 04:50:42 Re: jsonb and nested hstore