Re: [Sender Address Forgery]Re: error message when subscriptiontarget is a partitioned table

On 2019/01/07 16:35, Michael Paquier wrote:
> On Mon, Jan 07, 2019 at 01:49:49PM +0900, Amit Langote wrote:
>>  {
>>      /*
>> -     * We currently only support writing to regular tables.
>> +     * We currently only support writing to regular tables.  However, give
>> +     * a more specific error for partitioned and foreign tables.
>>       */
>> +    if (relkind == RELKIND_PARTITIONED_TABLE)
>> +        ereport(ERROR,
>> +                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>> +                 errmsg("\"%s.%s\" is a partitioned table",
>> +                        nspname, relname),
>> +                 errdetail("Partitioned tables are not
>> supported as logical replication targets.")));
> 
> Could it be possible to avoid a full sentence in the primary error
> message?  Usually these are avoided:
> https://www.postgresql.org/docs/devel/error-style-guide.html
> 
> It seems to me that we may want something more like:
> Primary: "could not use \"%s.%s\" as logical replication target".
> Detail: "Relation %s.%s is a foreign table", "not a table", etc.

I've thought about that before and I tend to agree with you.  Maybe:

ERROR: cannot use "%s.%s" as logical replication target
DETAIL: Using partitioned tables as logical replication target is not
supported.

Sounds a bit repetitive, but perhaps it's better to use the words "not
supported" in the DETAIL message.

Thanks,
Amit