On Mon, Apr 5, 2021, at 12:27 AM, Bharath Rupireddy wrote: > On Fri, Mar 26, 2021 at 9:25 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com > <mailto:bharath.rupireddyforpostgres%40gmail.com>> wrote: > > Here's the v3 patch rebased on the latest master. > > Here's the v4 patch reabsed on the latest master, please review it further. /* UNLOGGED and TEMP relations cannot be part of publication. */ if (!RelationIsPermanent(targetrel)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("table \"%s\" cannot be replicated", - RelationGetRelationName(targetrel)), - errdetail("Temporary and unlogged relations cannot be replicated."))); + { + if (RelationUsesLocalBuffers(targetrel)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is a temporary table", + RelationGetRelationName(targetrel)), + errdetail("Temporary tables cannot be added to publications."))); + else if (targetrel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is an unlogged table", + RelationGetRelationName(targetrel)), + errdetail("Unlogged tables cannot be added to publications."))); + }
RelationIsPermanent(), RelationUsesLocalBuffers(), and targetrel->rd_rel->relpersistence all refers to relpersistence. Hence, it is not necessary to test !RelationIsPermanent(). I would slightly rewrite the commit message to something like: Improve publication error messages Adding a foreign table into a publication prints an error saying "foo is not a table". Although, a foreign table is not a regular table, this message could possibly confuse users. Provide a suitable error message according to the object class (table vs foreign table). While at it, separate unlogged/temp table error message into 2 messages. -- Euler Taveira EDB https://www.enterprisedb.com/
