Skip site navigation (1) Skip section navigation (2)

Re: and it's not a bunny rabbit, either

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: and it's not a bunny rabbit, either
Date: 2010-12-29 04:54:46
Message-ID: AANLkTi=UXXcr3Fu4ch=3KW9HfFkvWM4vnZwixH45xUhJ@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Mon, Dec 27, 2010 at 2:06 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> The problem is that alter table actions AT_AddIndex and
> AT_AddConstraint don't tie neatly back to a particular piece of
> syntax.  The message as written isn't incomprehensible (especially if
> you're reading it in English) but it definitely leaves something to be
> desired.
>
> Ideas?

Here's a somewhat more complete patch implementing this concept, plus
adding additional messages for non-support of constraints, rules, and
triggers.  More could be done in this vein, but this picks a decent
fraction of the low-hanging fruit.

I've had to change some of the heap_open(rv) calls to
relation_open(rv) to avoid having the former throw the wrong error
message before the latter kicks in.  I think there might be stylistic
objections to that, but I'm not sure what else to propose.  I'm
actually pretty suspicious that many of the heap_open(rv) calls I
*didn't* change are either already a little iffy or likely to become
so once the SQL/MED stuff for foreign tables goes in.  They make it
easy to forget that we've got a whole pile of relkinds and you
actually need to really think about which ones you can handle.

For example, on unpatched head:

rhaas=# create view v as select 1 as a;
CREATE VIEW
rhaas=# cluster v;
ERROR:  there is no previously clustered index for table "v"

The error message is demonstrably correct in the sense that, first,
there isn't any table v, only a view v, so surely table v has no
clustered index - or anything else; and second, even if we construe
table "v" to mean view "v", it is certainly right to say it has no
clustered index because it does not - and can not - have any indexes
at all.  But as undeniably true as that error message is, it's a bad
error message.  With the patch:

rhaas=# cluster v;
ERROR:  views do not support CLUSTER

That's more like it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment: not-a-whatever-v2.patch
Description: text/x-patch (31.4 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Joel JacobsonDate: 2010-12-29 07:27:34
Subject: Re: pg_dump --split patch
Previous:From: Karl LehenbauerDate: 2010-12-29 02:23:58
Subject: Re: Revised patches to add table function support to PL/Tcl (TODO item)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group