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-27 19:06:52
Message-ID: AANLkTimm-5Le7-JP7FEQJ1=oi0K7ORL7DvDoRcAE_Ugj@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 27, 2010 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> or if we go with the some-assembly required version, perhaps:
>
>> "tables do not support %s"
>> "views do not support %s"
>> "indexes do not support %s"
>
> +1 for that way.  Although personally I'd reverse the phrasing:
>
>        /* translator: %s is the name of a SQL command */
>        %s does not support tables

To me, it seems as though it's the object which doesn't support the
operation, rather than the other way around. Reversing it makes most
sense to me in cases where it's an implementation restriction, such as
"DROP COLUMN does not support views". But at least to me, the
phrasing "SET WITHOUT CLUSTER does not support views" suggests that
SET WITHOUT CLUSTER is somehow defective, which is not really the
message I think we want to convey there. But maybe we need some other
votes.

I took a crack at implementing this and the results were mixed - see
attached patch. It works pretty well for the most part, but there are
a couple of warts. For ALTER TABLE commands like DROP COLUMN and SET
WITHOUT CLUSTER the results look pretty good, and I find the new error
messages a definite improvement over the old ones. It's not quite so
good for setting reloptions or attoptions. You get something like:

ERROR: views do not support SET (...)
ERROR: views do not support ALTER COLUMN SET (...)

...which might actually be OK, if not fantastically wonderful. One
might think of mitigating this problem by writing "ALTER TABLE SET
(...)" rather than just "SET (...)", but that's not easily done
because there are several equivalent forms (for example, a view can be
modified with either ALTER VIEW or ALTER TABLE, for historical - or
perhaps hysterical - reasons). An even bigger problem is this:

rhaas=# alter view v add primary key (a);
ERROR: views do not support ADD INDEX

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?

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

Attachment Content-Type Size
not-a-whatever.patch text/x-patch 21.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2010-12-27 19:13:33 Re: C++ keywords in headers (was Re: [GENERAL] #include <funcapi.h>)
Previous Message Tom Lane 2010-12-27 18:33:02 Re: Reduce lock levels for ADD and DROP COLUMN