Excerpts from Dimitri Fontaine's message of jue nov 04 11:37:37 -0300 2010:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> >> /* check for duplicate name (more friendly than unique-index failure) */
> >> if (SearchSysCacheExists2(TYPENAMENSP,
> >> CStringGetDatum(name),
> >> ObjectIdGetDatum(nspOid)))
> >
> > Hmm, this check is wrong anyway, because you're looking in the pg_type
> > syscache for objects from an arbitrary catalog. That needs to be fixed
> > somehow, but perhaps it needs to be handled by the callers, not in this
> > routine. Otherwise you're going to need to pass the syscache ID, as
> > well as Datums identifying the object, and the number of Datums.
>
> How embarrassing. I wonder why this works, too:
>
> dim=# alter operator utils.@>(utils.ltree, utils.ltree) set schema public;
> ALTER OPERATOR
Well, I guess the operator doesn't exist in the pg_type syscache, so it
doesn't ereport(ERROR).
> Well, I'll go fix as you say, putting the check back into the
> callers. That won't help a bit with the code duplication feeling we have
> when reading the patch, though. Any idea on this front?
I'm not really sure about the code duplication bits. There are plenty
of things you cannot factor into common routines because of the need to
handle different GETSTRUCT args, different number of syscache arguments,
etc. The ALTER OWNER implementation is already "duplicated" for each
database object. Your patch is already reducing duplication by moving
some common checks into namespace.c.
If there are more things that you could do by specifying a syscache ID
and datums to be passed to it, perhaps it would make sense to use them
as parameters to a function that does the whole bunch together. This
probably doesn't belong into namespace.c though.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
