Re: ATT_FOREIGN_TABLE and ATWrongRelkindError()

On 2015/10/23 6:06, Robert Haas wrote:
> On Wed, Oct 21, 2015 at 1:51 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> This may just be nitpicking but I noticed that ATWrongRelkindError() could
>> emit a better message in case of such errors during ALTER COLUMN DEFAULT
>> and  ALTER COLUMN SET STORAGE than "%s is of the wrong type" which is what
>> it would emit now. Just need to add a couple of cases to the switch there:
>>
>> + case ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE:
>> +     msg = _("\"%s\" is not a table, view or foreign table");
>> +     break;
>>
>> + case ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE:
>> +     msg = _("\"%s\" is not a table, materialized view, or foreign table");
>> +     break;
>>
>> Attached adds those.

> Good catch.  Committed and back-patched to 9.5.

Thanks, Amit and Robert!

This is really really nitpicking, but I noticed that there is an
implicit rule concerning the message format in ATWrongRelkindError; if
more than two objects are present, the message is "\"%s\" is not a foo,
bar, or baz". ("or" is preceded by a comma!)  So, would it be better
that the former is "\"%s\" is not a table, view, or foreign table"?

BTW, I found an incorrect error message in ATWrongRelkindError.
Attached is a patch for fixing the message.

Best regards,
Etsuro Fujita