On Fri, Jan 31, 2014 at 6:31 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> Hi Amit,
>
> Thanks for your reviewing. I updated the patch.
> I fixed the oids and removed the witespace.
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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
