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

Commit091e22b

Browse files
committed
Clean up treatment of missing default and CHECK-constraint records.
Andrew Gierth reported that it's possible to crash the backend if nopg_attrdef record is found to match an attribute that has atthasdef set.AttrDefaultFetch warns about this situation, but then leaves behinda relation tupdesc that has null "adbin" pointer(s), which most placesdon't guard against.We considered promoting the warning to an error, but throwing errorsduring relcache load is pretty drastic: it effectively locks one outof using the relation at all. What seems better is to leave theload-time behavior as a warning, but then throw an error in any codepath that wants to use a default and can't find it. This confinesthe error to a subset of INSERT/UPDATE operations on the table, andin particular will at least allow a pg_dump to succeed.Also, we should fix AttrDefaultFetch to not leave any null pointersin the tupdesc, because that just creates an untested bug hazard.While at it, apply the same philosophy of "warn at load, throw erroronly upon use of the known-missing info" to CHECK constraints.CheckConstraintFetch is very nearly the same logic as AttrDefaultFetch,but for reasons lost in the mists of time, it was throwing ERROR forthe same cases that AttrDefaultFetch treats as WARNING. Make the twofunctions more nearly alike.In passing, get rid of potentially-O(N^2) loops in equalTupleDescby making AttrDefaultFetch sort the entries after fetching them,so that equalTupleDesc can assume that entries in two equal tupdescsmust be in matching order. (CheckConstraintFetch already was sortingCHECK constraints, but equalTupleDesc hadn't been told about it.)There's some argument for back-patching this, but with such a smallnumber of field reports, I'm content to fix it in HEAD.Discussion:https://postgr.es/m/87pmzaq4gx.fsf@news-spur.riddles.org.uk
1 parent9de9294 commit091e22b

File tree

6 files changed

+173
-159
lines changed

6 files changed

+173
-159
lines changed

‎src/backend/access/common/tupdesc.c

Lines changed: 22 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,7 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
174174
cpy->defval= (AttrDefault*)palloc(cpy->num_defval*sizeof(AttrDefault));
175175
memcpy(cpy->defval,constr->defval,cpy->num_defval*sizeof(AttrDefault));
176176
for (i=cpy->num_defval-1;i >=0;i--)
177-
{
178-
if (constr->defval[i].adbin)
179-
cpy->defval[i].adbin=pstrdup(constr->defval[i].adbin);
180-
}
177+
cpy->defval[i].adbin=pstrdup(constr->defval[i].adbin);
181178
}
182179

183180
if (constr->missing)
@@ -203,10 +200,8 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
203200
memcpy(cpy->check,constr->check,cpy->num_check*sizeof(ConstrCheck));
204201
for (i=cpy->num_check-1;i >=0;i--)
205202
{
206-
if (constr->check[i].ccname)
207-
cpy->check[i].ccname=pstrdup(constr->check[i].ccname);
208-
if (constr->check[i].ccbin)
209-
cpy->check[i].ccbin=pstrdup(constr->check[i].ccbin);
203+
cpy->check[i].ccname=pstrdup(constr->check[i].ccname);
204+
cpy->check[i].ccbin=pstrdup(constr->check[i].ccbin);
210205
cpy->check[i].ccvalid=constr->check[i].ccvalid;
211206
cpy->check[i].ccnoinherit=constr->check[i].ccnoinherit;
212207
}
@@ -328,10 +323,7 @@ FreeTupleDesc(TupleDesc tupdesc)
328323
AttrDefault*attrdef=tupdesc->constr->defval;
329324

330325
for (i=tupdesc->constr->num_defval-1;i >=0;i--)
331-
{
332-
if (attrdef[i].adbin)
333-
pfree(attrdef[i].adbin);
334-
}
326+
pfree(attrdef[i].adbin);
335327
pfree(attrdef);
336328
}
337329
if (tupdesc->constr->missing)
@@ -352,10 +344,8 @@ FreeTupleDesc(TupleDesc tupdesc)
352344

353345
for (i=tupdesc->constr->num_check-1;i >=0;i--)
354346
{
355-
if (check[i].ccname)
356-
pfree(check[i].ccname);
357-
if (check[i].ccbin)
358-
pfree(check[i].ccbin);
347+
pfree(check[i].ccname);
348+
pfree(check[i].ccbin);
359349
}
360350
pfree(check);
361351
}
@@ -412,7 +402,6 @@ bool
412402
equalTupleDescs(TupleDesctupdesc1,TupleDesctupdesc2)
413403
{
414404
inti,
415-
j,
416405
n;
417406

418407
if (tupdesc1->natts!=tupdesc2->natts)
@@ -488,22 +477,13 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
488477
n=constr1->num_defval;
489478
if (n!= (int)constr2->num_defval)
490479
return false;
480+
/* We assume here that both AttrDefault arrays are in adnum order */
491481
for (i=0;i<n;i++)
492482
{
493483
AttrDefault*defval1=constr1->defval+i;
494-
AttrDefault*defval2=constr2->defval;
495-
496-
/*
497-
* We can't assume that the items are always read from the system
498-
* catalogs in the same order; so use the adnum field to identify
499-
* the matching item to compare.
500-
*/
501-
for (j=0;j<n;defval2++,j++)
502-
{
503-
if (defval1->adnum==defval2->adnum)
504-
break;
505-
}
506-
if (j >=n)
484+
AttrDefault*defval2=constr2->defval+i;
485+
486+
if (defval1->adnum!=defval2->adnum)
507487
return false;
508488
if (strcmp(defval1->adbin,defval2->adbin)!=0)
509489
return false;
@@ -534,25 +514,21 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
534514
n=constr1->num_check;
535515
if (n!= (int)constr2->num_check)
536516
return false;
517+
518+
/*
519+
* Similarly, we rely here on the ConstrCheck entries being sorted by
520+
* name. If there are duplicate names, the outcome of the comparison
521+
* is uncertain, but that should not happen.
522+
*/
537523
for (i=0;i<n;i++)
538524
{
539525
ConstrCheck*check1=constr1->check+i;
540-
ConstrCheck*check2=constr2->check;
541-
542-
/*
543-
* Similarly, don't assume that the checks are always read in the
544-
* same order; match them up by name and contents. (The name
545-
* *should* be unique, but...)
546-
*/
547-
for (j=0;j<n;check2++,j++)
548-
{
549-
if (strcmp(check1->ccname,check2->ccname)==0&&
550-
strcmp(check1->ccbin,check2->ccbin)==0&&
551-
check1->ccvalid==check2->ccvalid&&
552-
check1->ccnoinherit==check2->ccnoinherit)
553-
break;
554-
}
555-
if (j >=n)
526+
ConstrCheck*check2=constr2->check+i;
527+
528+
if (!(strcmp(check1->ccname,check2->ccname)==0&&
529+
strcmp(check1->ccbin,check2->ccbin)==0&&
530+
check1->ccvalid==check2->ccvalid&&
531+
check1->ccnoinherit==check2->ccnoinherit))
556532
return false;
557533
}
558534
}

‎src/backend/commands/tablecmds.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,21 +2501,24 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
25012501
if (attribute->atthasdef)
25022502
{
25032503
Node *this_default = NULL;
2504-
AttrDefault *attrdef;
2505-
inti;
25062504

25072505
/* Find default in constraint structure */
2508-
Assert(constr != NULL);
2509-
attrdef = constr->defval;
2510-
for (i = 0; i < constr->num_defval; i++)
2506+
if (constr != NULL)
25112507
{
2512-
if (attrdef[i].adnum == parent_attno)
2508+
AttrDefault *attrdef = constr->defval;
2509+
2510+
for (int i = 0; i < constr->num_defval; i++)
25132511
{
2514-
this_default = stringToNode(attrdef[i].adbin);
2515-
break;
2512+
if (attrdef[i].adnum == parent_attno)
2513+
{
2514+
this_default = stringToNode(attrdef[i].adbin);
2515+
break;
2516+
}
25162517
}
25172518
}
2518-
Assert(this_default != NULL);
2519+
if (this_default == NULL)
2520+
elog(ERROR, "default expression not found for attribute %d of relation \"%s\"",
2521+
parent_attno, RelationGetRelationName(relation));
25192522

25202523
/*
25212524
* If it's a GENERATED default, it might contain Vars that

‎src/backend/executor/execMain.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,15 @@ ExecRelCheck(ResultRelInfo *resultRelInfo,
16091609
MemoryContextoldContext;
16101610
inti;
16111611

1612+
/*
1613+
* CheckConstraintFetch let this pass with only a warning, but now we
1614+
* should fail rather than possibly failing to enforce an important
1615+
* constraint.
1616+
*/
1617+
if (ncheck!=rel->rd_rel->relchecks)
1618+
elog(ERROR,"%d pg_constraint record(s) missing for relation \"%s\"",
1619+
rel->rd_rel->relchecks-ncheck,RelationGetRelationName(rel));
1620+
16121621
/*
16131622
* If first time through for this result relation, build expression
16141623
* nodetrees for rel's constraint expressions. Keep them in the per-query
@@ -1862,7 +1871,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
18621871
}
18631872
}
18641873

1865-
if (constr->num_check>0)
1874+
if (rel->rd_rel->relchecks>0)
18661875
{
18671876
constchar*failed;
18681877

‎src/backend/parser/parse_utilcmd.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,8 +1239,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
12391239
(CREATE_TABLE_LIKE_DEFAULTS |CREATE_TABLE_LIKE_GENERATED))&&
12401240
constr!=NULL)
12411241
{
1242-
AttrDefault*attrdef=constr->defval;
1243-
12441242
for (parent_attno=1;parent_attno <=tupleDesc->natts;
12451243
parent_attno++)
12461244
{
@@ -1264,6 +1262,7 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
12641262
(table_like_clause->options&CREATE_TABLE_LIKE_DEFAULTS)))
12651263
{
12661264
Node*this_default=NULL;
1265+
AttrDefault*attrdef=constr->defval;
12671266
AlterTableCmd*atsubcmd;
12681267
boolfound_whole_row;
12691268

@@ -1276,7 +1275,9 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
12761275
break;
12771276
}
12781277
}
1279-
Assert(this_default!=NULL);
1278+
if (this_default==NULL)
1279+
elog(ERROR,"default expression not found for attribute %d of relation \"%s\"",
1280+
parent_attno,RelationGetRelationName(relation));
12801281

12811282
atsubcmd=makeNode(AlterTableCmd);
12821283
atsubcmd->subtype=AT_CookedColumnDefault;

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,25 +1228,28 @@ build_column_default(Relation rel, int attrno)
12281228
}
12291229

12301230
/*
1231-
*Scan to see ifrelation has a default for this column.
1231+
*Ifrelation has a default for this column, fetch that expression.
12321232
*/
1233-
if (att_tup->atthasdef&&rd_att->constr&&
1234-
rd_att->constr->num_defval>0)
1233+
if (att_tup->atthasdef)
12351234
{
1236-
AttrDefault*defval=rd_att->constr->defval;
1237-
intndef=rd_att->constr->num_defval;
1238-
1239-
while (--ndef >=0)
1235+
if (rd_att->constr&&rd_att->constr->num_defval>0)
12401236
{
1241-
if (attrno==defval[ndef].adnum)
1237+
AttrDefault*defval=rd_att->constr->defval;
1238+
intndef=rd_att->constr->num_defval;
1239+
1240+
while (--ndef >=0)
12421241
{
1243-
/*
1244-
* Found it, convert string representation to node tree.
1245-
*/
1246-
expr=stringToNode(defval[ndef].adbin);
1247-
break;
1242+
if (attrno==defval[ndef].adnum)
1243+
{
1244+
/* Found it, convert string representation to node tree. */
1245+
expr=stringToNode(defval[ndef].adbin);
1246+
break;
1247+
}
12481248
}
12491249
}
1250+
if (expr==NULL)
1251+
elog(ERROR,"default expression not found for attribute %d of relation \"%s\"",
1252+
attrno,RelationGetRelationName(rel));
12501253
}
12511254

12521255
/*

0 commit comments

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