Hello. Commit c6b92041d3 changed the definition of RelationNeedsWAL().
-#define RelationNeedsWAL(relation) \ - ((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT) +#define RelationNeedsWAL(relation) \ + ((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT && \ + (XLogIsNeeded() || \ + (relation->rd_createSubid == InvalidSubTransactionId && \ + relation->rd_firstRelfilenodeSubid == InvalidSubTransactionId))) On the other hand I found this usage. plancat.c:128 get_relation_info() > /* Temporary and unlogged relations are inaccessible during recovery. */ > if (!RelationNeedsWAL(relation) && RecoveryInProgress()) > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > errmsg("cannot access temporary or unlogged > relations during recovery"))); It works as expected accidentally, but the meaning is off. WAL-skipping optmization is irrelevant to the condition for the error. I found five misues in the tree. Please find the attached. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
>From 2912e1db38ff034e27e4010eff5b2e5afcce3f85 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyoga....@gmail.com> Date: Wed, 13 Jan 2021 15:52:03 +0900 Subject: [PATCH] Fix misuses of RelationNeedsWAL The definition of the macro RelationNeedsWAL has been changed by c6b92041d3 to include conditions related to the WAL-skip optimization but some uses of the macro are not relevant to the optimization. That misuses are harmless for now as they are only executed while wal_level >= replica or WAL-skipping is inactive. However, this should be corrected to prevent future hazard. --- src/backend/catalog/pg_publication.c | 2 +- src/backend/optimizer/util/plancat.c | 2 +- src/include/utils/rel.h | 15 +++++++++++---- src/include/utils/snapmgr.h | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 5f8e1c64e1..f3060a4cf1 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -67,7 +67,7 @@ check_publication_add_relation(Relation targetrel) errdetail("System tables cannot be added to publications."))); /* UNLOGGED and TEMP relations cannot be part of publication. */ - if (!RelationNeedsWAL(targetrel)) + if (!RelationIsWalLogged(targetrel)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("table \"%s\" cannot be replicated", diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index da322b453e..0500efcdb9 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -126,7 +126,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, relation = table_open(relationObjectId, NoLock); /* Temporary and unlogged relations are inaccessible during recovery. */ - if (!RelationNeedsWAL(relation) && RecoveryInProgress()) + if (!RelationIsWalLogged(relation) && RecoveryInProgress()) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary or unlogged relations during recovery"))); diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 10b63982c0..810806a542 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -552,16 +552,23 @@ typedef struct ViewOptions (relation)->rd_smgr->smgr_targblock = (targblock); \ } while (0) +/* + * RelationIsPermanent + * True if relation is WAL-logged. + */ +#define RelationIsWalLogged(relation) \ + ((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT) + /* * RelationNeedsWAL - * True if relation needs WAL. + * True if relation needs WAL at the time. * * Returns false if wal_level = minimal and this relation is created or * truncated in the current transaction. See "Skipping WAL for New * RelFileNode" in src/backend/access/transam/README. */ #define RelationNeedsWAL(relation) \ - ((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT && \ + (RelationIsWalLogged(relation) && \ (XLogIsNeeded() || \ (relation->rd_createSubid == InvalidSubTransactionId && \ relation->rd_firstRelfilenodeSubid == InvalidSubTransactionId))) @@ -619,7 +626,7 @@ typedef struct ViewOptions */ #define RelationIsAccessibleInLogicalDecoding(relation) \ (XLogLogicalInfoActive() && \ - RelationNeedsWAL(relation) && \ + RelationIsWalLogged(relation) && \ (IsCatalogRelation(relation) || RelationIsUsedAsCatalogTable(relation))) /* @@ -635,7 +642,7 @@ typedef struct ViewOptions */ #define RelationIsLogicallyLogged(relation) \ (XLogLogicalInfoActive() && \ - RelationNeedsWAL(relation) && \ + RelationIsWalLogged(relation) && \ !IsCatalogRelation(relation)) /* routines in utils/cache/relcache.c */ diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 579be352c5..7be922a9f1 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -37,7 +37,7 @@ */ #define RelationAllowsEarlyPruning(rel) \ ( \ - RelationNeedsWAL(rel) \ + RelationIsWalLogged(rel) \ && !IsCatalogRelation(rel) \ && !RelationIsAccessibleInLogicalDecoding(rel) \ ) -- 2.27.0