public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Fix misuses of RelationNeedsWAL
3+ messages / 3 participants
[nested] [flat]
* [PATCH] Fix misuses of RelationNeedsWAL
@ 2021-01-13 06:52 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Kyotaro Horiguchi @ 2021-01-13 06:52 UTC (permalink / raw)
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
----Next_Part(Wed_Jan_13_16_07_05_2021_621)----
^ permalink raw reply [nested|flat] 3+ messages in thread
* Missing cases from SPI_result_code_string()
@ 2023-02-20 18:52 Dean Rasheed <[email protected]>
2023-02-20 19:39 ` Re: Missing cases from SPI_result_code_string() Tom Lane <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Dean Rasheed @ 2023-02-20 18:52 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Another one noticed from the MERGE RETURNING patch -- the switch
statement in SPI_result_code_string() is missing cases for
SPI_OK_TD_REGISTER and SPI_OK_MERGE.
The SPI_OK_TD_REGISTER case goes back all the way, so I suppose it
should be back-patched to all supported branches, though evidently
this is not something anyone is likely to care about.
The SPI_OK_MERGE case is perhaps a little more visible (e.g., execute
MERGE from PL/Perl using $rv = spi_exec_query() and then examine
$rv->{status}). It's also missing from the docs for SPI_Execute().
Having tested that it now works as expected, I don't think there's
much point in adding a regression test case for it though.
Regards,
Dean
Attachments:
[text/x-patch] SPI_result_code_string.patch (1.1K, ../../CAEZATCUg8V+K+GcafOPqymxk84Y_prXgfe64PDoopjLFH6Z0Aw@mail.gmail.com/2-SPI_result_code_string.patch)
download | inline diff:
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
new file mode 100644
index 7581661..651930a
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -438,6 +438,15 @@ typedef struct SPITupleTable
</varlistentry>
<varlistentry>
+ <term><symbol>SPI_OK_MERGE</symbol></term>
+ <listitem>
+ <para>
+ if a <command>MERGE</command> was executed
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><symbol>SPI_OK_INSERT_RETURNING</symbol></term>
<listitem>
<para>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
new file mode 100644
index 61f03e3..e3a170c
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2029,6 +2029,10 @@ SPI_result_code_string(int code)
return "SPI_OK_REL_REGISTER";
case SPI_OK_REL_UNREGISTER:
return "SPI_OK_REL_UNREGISTER";
+ case SPI_OK_TD_REGISTER:
+ return "SPI_OK_TD_REGISTER";
+ case SPI_OK_MERGE:
+ return "SPI_OK_MERGE";
}
/* Unrecognized code ... return something useful ... */
sprintf(buf, "Unrecognized SPI code %d", code);
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Missing cases from SPI_result_code_string()
2023-02-20 18:52 Missing cases from SPI_result_code_string() Dean Rasheed <[email protected]>
@ 2023-02-20 19:39 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2023-02-20 19:39 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Dean Rasheed <[email protected]> writes:
> Another one noticed from the MERGE RETURNING patch -- the switch
> statement in SPI_result_code_string() is missing cases for
> SPI_OK_TD_REGISTER and SPI_OK_MERGE.
Ugh. Grepping around, it looks like pltcl_process_SPI_result
is missing a case for SPI_OK_MERGE as well.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-02-20 19:39 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 06:52 [PATCH] Fix misuses of RelationNeedsWAL Kyotaro Horiguchi <[email protected]>
2023-02-20 18:52 Missing cases from SPI_result_code_string() Dean Rasheed <[email protected]>
2023-02-20 19:39 ` Re: Missing cases from SPI_result_code_string() Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox