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

Commit43ef3c4

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 parentb7103bb commit43ef3c4

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
@@ -20,6 +20,8 @@
2020
#include"access/htup_details.h"
2121
#include"catalog/index.h"
2222
#include"catalog/indexing.h"
23+
#include"catalog/pg_subscription.h"
24+
#include"catalog/pg_subscription_rel.h"
2325
#include"executor/executor.h"
2426
#include"utils/rel.h"
2527

@@ -167,6 +169,53 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
167169
ExecDropSingleTupleTableSlot(slot);
168170
}
169171

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

236+
CatalogTupleCheckConstraints(heapRel,tup);
237+
187238
indstate=CatalogOpenIndexes(heapRel);
188239

189240
simple_heap_insert(heapRel,tup);
@@ -204,6 +255,8 @@ void
204255
CatalogTupleInsertWithInfo(RelationheapRel,HeapTupletup,
205256
CatalogIndexStateindstate)
206257
{
258+
CatalogTupleCheckConstraints(heapRel,tup);
259+
207260
simple_heap_insert(heapRel,tup);
208261

209262
CatalogIndexInsert(indstate,tup);
@@ -225,6 +278,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
225278
{
226279
CatalogIndexStateindstate;
227280

281+
CatalogTupleCheckConstraints(heapRel,tup);
282+
228283
indstate=CatalogOpenIndexes(heapRel);
229284

230285
simple_heap_update(heapRel,otid,tup);
@@ -245,6 +300,8 @@ void
245300
CatalogTupleUpdateWithInfo(RelationheapRel,ItemPointerotid,HeapTupletup,
246301
CatalogIndexStateindstate)
247302
{
303+
CatalogTupleCheckConstraints(heapRel,tup);
304+
248305
simple_heap_update(heapRel,otid,tup);
249306

250307
CatalogIndexInsert(indstate,tup);

0 commit comments

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