Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf451029

Browse files
committed
Assert that we don't insert nulls into attnotnull catalog columns.
The executor checks for this error, and so does the bootstrap catalogloader, but we never checked for it in retail catalog manipulations.The folly of that has now been exposed, so let's add assertionschecking it. Checking in CatalogTupleInsert[WithInfo] andCatalogTupleUpdate[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 thisas far back as v11.Pre-v13, we have to explicitly exclude pg_subscription.subslotnameand pg_subscription_rel.srsublsn from the checks, since they aremismarked. (Even if we change our mind about applying BKI_FORCE_NULLin the branch tips, it doesn't seem wise to have assertions thatwould fire in existing databases.)Discussion:https://postgr.es/m/298837.1595196283@sss.pgh.pa.us
1 parent99b0c5d commitf451029

File tree

2 files changed

+58
-4
lines changed

2 files changed

+58
-4
lines changed

‎doc/src/sgml/bki.sgml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,7 @@
122122
if they are fixed-width and are not preceded by any nullable column.
123123
Where this rule is inadequate, you can force correct marking by using
124124
<literal>BKI_FORCE_NOT_NULL</literal>
125-
and <literal>BKI_FORCE_NULL</literal> annotations as needed. But note
126-
that <literal>NOT NULL</literal> constraints are only enforced in the
127-
executor, not against tuples that are generated by random C code,
128-
so care is still needed when manually creating or updating catalog rows.
125+
and <literal>BKI_FORCE_NULL</literal> annotations as needed.
129126
</para>
130127

131128
<para>

‎src/backend/catalog/indexing.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
#include"access/htup_details.h"
1919
#include"catalog/index.h"
2020
#include"catalog/indexing.h"
21+
#include"catalog/pg_subscription.h"
22+
#include"catalog/pg_subscription_rel.h"
2123
#include"executor/executor.h"
2224
#include"utils/rel.h"
2325

@@ -164,6 +166,53 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
164166
ExecDropSingleTupleTableSlot(slot);
165167
}
166168

169+
/*
170+
* Subroutine to verify that catalog constraints are honored.
171+
*
172+
* Tuples inserted via CatalogTupleInsert/CatalogTupleUpdate are generally
173+
* "hand made", so that it's possible that they fail to satisfy constraints
174+
* that would be checked if they were being inserted by the executor. That's
175+
* a coding error, so we only bother to check for it in assert-enabled builds.
176+
*/
177+
#ifdefUSE_ASSERT_CHECKING
178+
179+
staticvoid
180+
CatalogTupleCheckConstraints(RelationheapRel,HeapTupletup)
181+
{
182+
/*
183+
* Currently, the only constraints implemented for system catalogs are
184+
* attnotnull constraints.
185+
*/
186+
if (HeapTupleHasNulls(tup))
187+
{
188+
TupleDesctupdesc=RelationGetDescr(heapRel);
189+
bits8*bp=tup->t_data->t_bits;
190+
191+
for (intattnum=0;attnum<tupdesc->natts;attnum++)
192+
{
193+
Form_pg_attributethisatt=TupleDescAttr(tupdesc,attnum);
194+
195+
/*
196+
* Through an embarrassing oversight, pre-v13 installations have
197+
* pg_subscription.subslotname and pg_subscription_rel.srsublsn
198+
* marked as attnotnull, which they should not be. Ignore those
199+
* flags.
200+
*/
201+
Assert(!(thisatt->attnotnull&&att_isnull(attnum,bp)&&
202+
!((thisatt->attrelid==SubscriptionRelationId&&
203+
thisatt->attnum==Anum_pg_subscription_subslotname)||
204+
(thisatt->attrelid==SubscriptionRelRelationId&&
205+
thisatt->attnum==Anum_pg_subscription_rel_srsublsn))));
206+
}
207+
}
208+
}
209+
210+
#else/* !USE_ASSERT_CHECKING */
211+
212+
#defineCatalogTupleCheckConstraints(heapRel,tup) ((void) 0)
213+
214+
#endif/* USE_ASSERT_CHECKING */
215+
167216
/*
168217
* CatalogTupleInsert - do heap and indexing work for a new catalog tuple
169218
*
@@ -182,6 +231,8 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
182231
CatalogIndexStateindstate;
183232
Oidoid;
184233

234+
CatalogTupleCheckConstraints(heapRel,tup);
235+
185236
indstate=CatalogOpenIndexes(heapRel);
186237

187238
oid=simple_heap_insert(heapRel,tup);
@@ -206,6 +257,8 @@ CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
206257
{
207258
Oidoid;
208259

260+
CatalogTupleCheckConstraints(heapRel,tup);
261+
209262
oid=simple_heap_insert(heapRel,tup);
210263

211264
CatalogIndexInsert(indstate,tup);
@@ -229,6 +282,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
229282
{
230283
CatalogIndexStateindstate;
231284

285+
CatalogTupleCheckConstraints(heapRel,tup);
286+
232287
indstate=CatalogOpenIndexes(heapRel);
233288

234289
simple_heap_update(heapRel,otid,tup);
@@ -249,6 +304,8 @@ void
249304
CatalogTupleUpdateWithInfo(RelationheapRel,ItemPointerotid,HeapTupletup,
250305
CatalogIndexStateindstate)
251306
{
307+
CatalogTupleCheckConstraints(heapRel,tup);
308+
252309
simple_heap_update(heapRel,otid,tup);
253310

254311
CatalogIndexInsert(indstate,tup);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp