Re: Proposal: variant of regclass

From: Tatsuo Ishii <ishii(at)postgresql(dot)org>
To: marti(at)juffo(dot)org
Cc: nagata(at)sraoss(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org, ishii(at)postgresql(dot)org, vik(dot)fearing(at)dalibo(dot)com, robertmhaas(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, pavel(at)gf(dot)microolap(dot)com, pavel(at)microolap(dot)com, andres(at)2ndquadrant(dot)com, pavel(dot)stehule(at)gmail(dot)com
Subject: Re: Proposal: variant of regclass
Date: 2014-01-22 11:04:12
Message-ID: 20140122.200412.487876788831364412.t-ishii@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Tue, Jan 14, 2014 at 9:28 AM, 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.
>
> + static Datum regclass_guts(char *class_name_or_oid, bool raiseError);
>
> Minor bikeshedding, a lot of code currently uses an argument named
> "missing_ok" for this purpose (with inverse meaning of course). Any
> reasons why you chose "raiseError" instead?

Originally the proposal checks errors like syntactical one in addition
to missing objects. So I think "raiseError" was more appropriate at
that time. Now they only check missing objects. So renaming to
"missing_ok" could be more appropriate.

> I only had a brief look at the patch, so maybe I'm missing something.
> But I don't think you should create 3 variants of these functions:
> * parseTypeString calls parseTypeString_guts with false
> * parseTypeStringMissingOk calls parseTypeString_guts with true
> * parseTypeString_guts
>
> And this is just silly:
>
> if (raiseError)
> parseTypeString(typ_name_or_oid, &result, &typmod);
> else
> parseTypeStringMissingOk(typ_name_or_oid, &result, &typmod);
>
> Just add an argument to parseTypeString and patch all the callers.

Leave the disccusion to Yugo..

>> if requested object is not found,
>> returns InvalidOid, rather than raises an error.
>
> I thought the consensus was that returning NULL is better than
> InvalidOid? From an earlier message:
>
> On Thu, Dec 5, 2013 at 5:25 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> Another advantage of this approach is that, IIUC, type input functions
>> can't return a NULL value. So 'pg_klass'::regclass could return 0,
>> but not NULL. On the other hand, toregclass('pg_klass') *could*
>> return NULL, which seems conceptually cleaner.

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

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2014-01-22 11:40:35 Re: Patch: Show process IDs of processes holding a lock; show relation and tuple infos of a lock to acquire
Previous Message Peter Geoghegan 2014-01-22 10:39:40 Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core