On Mon, Dec 16, 2013 at 8:22 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>> static Datum regclass_gut(char *class_name_or_oid, bool raiseError);
>>> static List *stringToQualifiedNameList_gut(const char *string, bool raiseError);
>>
>> Please spell that as "guts" not "gut".
>
> Thanks. I see.
>
>>> regclass_gut is called from regclassin and toregclass and do the most
>>> job before regclassin did. "raiseError" flag controls whether an error
>>> is raised or not when an invalid argument (for example non existent
>>> relation) is given. For this purpose, regclass_gut wraps the call to
>>> oidin using a PG_TRY block.
>>
>> I do not think that use of PG_TRY is either necessary or acceptable
>> --- for example, it will happily trap and discard query-cancel errors,
>> as well as other errors that are not necessarily safe to ignore.
>> If you don't want to risk an error on an invalid numeric string,
>> how about parsing the integer yourself with sscanf?
>
> Fair enough. I will remove the part.
>
>> More generally, though, I don't see a great need to try to promise
>> that this function *never* throws any errors, and so I'm also suspicious
>> of the hacking you've done on stringToQualifiedNameList. I'm even
>> less happy about the idea that this patch might start reaching into
>> things like makeRangeVarFromNameList. I think it's sufficient if it
>> doesn't throw an error on name-not-found; you don't have to try to
>> prevent weird syntax problems from throwing errors.
>
> For the pgpool-II use case, I'm happy to follow you because pgpool-II
> always does a grammatical check (using raw parser) on a query first
> and such syntax problem will be pointed out, thus never reaches to
> the state where calling toregclass.
>
> One concern is, other users expect toregclass to detect such syntax
> problems. Anybody?
It seems fine to me if the new function ignores the specific error of
"relation does not exist" while continuing to throw other errors.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company