Re: ALTER command reworks

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER command reworks
Date: 2013-01-17 00:13:22
Message-ID: CADyhKSWKuzEO3DOmNHQs00wqDGF2YzPbRq7NOStaYobuLhxg_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2013/1/15 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Kohei KaiGai escribió:
>> 2013/1/15 Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(at)kaigai(dot)gr(dot)jp>

Attachment Content-Type Size
pgsql-v9.3-alter-reworks.3-rename.v9.patch application/octet-stream 54.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2013-01-17 00:48:46 Re: review: pgbench - aggregation of info written into log
Previous Message Tatsuo Ishii 2013-01-16 23:48:01 Re: review: pgbench - aggregation of info written into log