aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2020-07-21 12:38:08 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2020-07-21 12:38:08 -0400
commit43ef3c4c360a10226732b62ab3b2097cdf089dce (patch)
tree60e8d537ef296185cee551f54c5210b09a5e82fd /src
parentb7103bbe34aa3d66f4618d0abdee5d3107ea8f91 (diff)
downloadpostgresql-43ef3c4c360a10226732b62ab3b2097cdf089dce.tar.gz
postgresql-43ef3c4c360a10226732b62ab3b2097cdf089dce.zip
Assert that we don't insert nulls into attnotnull catalog columns.
The executor checks for this error, and so does the bootstrap catalog loader, but we never checked for it in retail catalog manipulations. The folly of that has now been exposed, so let's add assertions checking it. Checking in CatalogTupleInsert[WithInfo] and CatalogTupleUpdate[WithInfo] should be enough to cover this. Back-patch to v10; the aforesaid functions didn't exist before that, and it didn't seem worth adapting the patch to the oldest branches. But given the risk of JIT crashes, I think we certainly need this as far back as v11. Pre-v13, we have to explicitly exclude pg_subscription.subslotname and pg_subscription_rel.srsublsn from the checks, since they are mismarked. (Even if we change our mind about applying BKI_FORCE_NULL in the branch tips, it doesn't seem wise to have assertions that would fire in existing databases.) Discussion: https://postgr.es/m/298837.1595196283@sss.pgh.pa.us
Diffstat (limited to 'src')
-rw-r--r--src/backend/catalog/indexing.c57
1 files changed, 57 insertions, 0 deletions
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index f237e62bc90..56070aff54d 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -20,6 +20,8 @@
#include "access/htup_details.h"
#include "catalog/index.h"
#include "catalog/indexing.h"
+#include "catalog/pg_subscription.h"
+#include "catalog/pg_subscription_rel.h"
#include "executor/executor.h"
#include "utils/rel.h"
@@ -168,6 +170,53 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
}
/*
+ * Subroutine to verify that catalog constraints are honored.
+ *
+ * Tuples inserted via CatalogTupleInsert/CatalogTupleUpdate are generally
+ * "hand made", so that it's possible that they fail to satisfy constraints
+ * that would be checked if they were being inserted by the executor. That's
+ * a coding error, so we only bother to check for it in assert-enabled builds.
+ */
+#ifdef USE_ASSERT_CHECKING
+
+static void
+CatalogTupleCheckConstraints(Relation heapRel, HeapTuple tup)
+{
+ /*
+ * Currently, the only constraints implemented for system catalogs are
+ * attnotnull constraints.
+ */
+ if (HeapTupleHasNulls(tup))
+ {
+ TupleDesc tupdesc = RelationGetDescr(heapRel);
+ bits8 *bp = tup->t_data->t_bits;
+
+ for (int attnum = 0; attnum < tupdesc->natts; attnum++)
+ {
+ Form_pg_attribute thisatt = TupleDescAttr(tupdesc, attnum);
+
+ /*
+ * Through an embarrassing oversight, pre-v13 installations have
+ * pg_subscription.subslotname and pg_subscription_rel.srsublsn
+ * marked as attnotnull, which they should not be. Ignore those
+ * flags.
+ */
+ Assert(!(thisatt->attnotnull && att_isnull(attnum, bp) &&
+ !((thisatt->attrelid == SubscriptionRelationId &&
+ thisatt->attnum == Anum_pg_subscription_subslotname) ||
+ (thisatt->attrelid == SubscriptionRelRelationId &&
+ thisatt->attnum == Anum_pg_subscription_rel_srsublsn))));
+ }
+ }
+}
+
+#else /* !USE_ASSERT_CHECKING */
+
+#define CatalogTupleCheckConstraints(heapRel, tup) ((void) 0)
+
+#endif /* USE_ASSERT_CHECKING */
+
+/*
* CatalogTupleInsert - do heap and indexing work for a new catalog tuple
*
* Insert the tuple data in "tup" into the specified catalog relation.
@@ -184,6 +233,8 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
{
CatalogIndexState indstate;
+ CatalogTupleCheckConstraints(heapRel, tup);
+
indstate = CatalogOpenIndexes(heapRel);
simple_heap_insert(heapRel, tup);
@@ -204,6 +255,8 @@ void
CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
CatalogIndexState indstate)
{
+ CatalogTupleCheckConstraints(heapRel, tup);
+
simple_heap_insert(heapRel, tup);
CatalogIndexInsert(indstate, tup);
@@ -225,6 +278,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
{
CatalogIndexState indstate;
+ CatalogTupleCheckConstraints(heapRel, tup);
+
indstate = CatalogOpenIndexes(heapRel);
simple_heap_update(heapRel, otid, tup);
@@ -245,6 +300,8 @@ void
CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
CatalogIndexState indstate)
{
+ CatalogTupleCheckConstraints(heapRel, tup);
+
simple_heap_update(heapRel, otid, tup);
CatalogIndexInsert(indstate, tup);