Re: ALTER command reworks

Kohei KaiGai <kaigai@kaigai.gr.jp>

From: Kohei KaiGai <kaigai@kaigai.gr.jp>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Dimitri Fontaine <dimitri@2ndquadrant.fr>, PgHacker <pgsql-hackers@postgresql.org>
Date: 2013-01-17T00:13:22Z
Lists: pgsql-hackers

Attachments

2013/1/15 Alvaro Herrera <alvherre@2ndquadrant.com>:
> Kohei KaiGai escribió:
>> 2013/1/15 Alvaro Herrera <alvherre@2ndquadrant.com>:
>> > Kohei KaiGai escribió:
>> >
>> >> The attached patch is a rebased version towards the latest master
>> >> branch, and fix up the issue around error messages on name conflicts.
>> >
>> > I assume the lock.c changes are just a bollixed merge, right?
>> >
>> Yes, I'll check it and rebase it.
>
> Wait for a bit before publishing a new version -- I'm going to push the
> other patch so that you can merge on top.
>
> Note that I'm going to commit a new function like this:
>
> /*
>  * Raise an error to the effect that an object of the given name is already
>  * present in the given namespace.
>  */
> static void
> report_namespace_conflict(Oid classId, const char *name, Oid nspOid)
> {
>         char   *msgfmt;
>
>         Assert(OidIsValid(nspOid));
>
>
> For this patch you will need to create a separate function which does
> the conflicting-name report for objects that are not in a namespace.
> Mixing both in-schema and schemaless objects in the same report function
> seems messy to me.
>
This attached patch is the rebased one towards the latest master branch.

I noticed that your proposed design also allows to unify ALTER code of
OPERATOR CLASS / FAMILY; that takes index access method for its
namespace in addition to name and namespace.
So, AlterObjectRename_internal and AlterObjectNamespace_internal have
four of special case handling to check name / namespace conflict.

The latest master lookups syscache within special case handing block
as follows:

    if (classId == ProcedureRelationId)
    {
        HeapTuple   tup;
        Form_pg_proc proc;

        tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(objid));
        if (!HeapTupleIsValid(tup))
            elog(ERROR, "cache lookup failed for function %u", objid);
        proc = (Form_pg_proc) GETSTRUCT(tup);

        IsThereFunctionInNamespace(NameStr(proc->proname), proc->pronargs,
                                   proc->proargtypes, nspOid);
        heap_freetuple(tup);
    }

But, we already pulls a relevant tuple from syscache on top of
AlterObjectNamespace_internal. So, I removed cache lookup code here.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>