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
|
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: