On Tue, Apr 14, 2020 at 7:02 PM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > On 2020-Apr-13, Robert Haas wrote: > > + ereport(ERROR, > > + (errcode(ERRCODE_WRONG_OBJECT_TYPE), > > + errmsg("action cannot be performed on relation \"%s\"", > > + RelationGetRelationName(rel)), > > > > Super-vague. > > Maybe, but note that the patch proposed to replace this current error > message: > ERROR: foo is not an index or foreign table > with > ERROR: action cannot be performed on "foo" > DETAIL: "foo" is a materialized view.
Sure, but the point is that this case is not improved nearly as much as most of the others. In a whole bunch of cases, he made the error message describe the attempted operation, but here he didn't. I'm not saying that makes it worse than what we had before, just that it would be better if we could make this look like the other cases the patch also changes. > This could be improved if we had stringification of ALTER TABLE > subcommand types: > > ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on "foo" > DETAIL: "foo" is a gummy bear. > or > ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on foo > DETAIL: This action cannot be performed on gummy bears. > > but that seems material for a different patch. Even without that, you could at least say "this form of ALTER TABLE is not supported for foo" or something like that. I'm not trying to block the patch. I think it's a good patch. I was just making an observation about some parts of it where it seems like we could try slightly harder to do better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company