Re: and it's not a bunny rabbit, either - Mailing list pgsql-hackers

From Robert Haas
Subject Re: and it's not a bunny rabbit, either
Date
Msg-id AANLkTimm-5Le7-JP7FEQJ1=oi0K7ORL7DvDoRcAE_Ugj@mail.gmail.com
Whole thread Raw
In response to Re: and it's not a bunny rabbit, either  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: and it's not a bunny rabbit, either  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Mon, Dec 27, 2010 at 10:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Reduce lock levels for ADD and DROP COLUMN
Next
From: Peter Geoghegan
Date:
Subject: Re: C++ keywords in headers (was Re: [GENERAL] #include )