On Thu, Apr 3, 2014 at 5:43 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Apr 2, 2014 at 1:41 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Mon, Mar 31, 2014 at 7:08 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>>> Hi Amit Kapila,
>>>
>>> Thank you for your reviewing. I updated the patch to v5.
>>
>> I have checked the latest version and found few minor improvements that
>> are required:
>>
>> 1.
>> ! if (!missing_ok)
>> ! ereport(ERROR,
>> ! (errcode(ERRCODE_UNDEFINED_OBJECT),
>> ! errmsg("type \"%s\" does not exist",
>> ! TypeNameToString(typeName)),
>> ! parser_errposition(NULL, typeName->location)));
>>
>> pfree(buf.data); seems to be missing in error cases.
>
> Eh, surely this is being done in some memory context that an error
> will reset anyway?
Right, it will get reset in error. However still we need to free for missing_ok
case and when it is successful in getting typeid. So don't you think it is
better to just free once before calling LookupTypeName()?
The code is right in it's current form as well, it's just a minor suggestion
for improvement, so if you think current way the code written is okay, then
ignore this suggestion.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com