Skip to content

Commit f95b65e

Browse files
funbringerfunbringer
committed
Merge branch 'master_inh_translation_list_bugfix' into rel_1_3_beta
2 parents d8e4d38 + 1871b04 commit f95b65e

File tree

1 file changed

+92
-54
lines changed

1 file changed

+92
-54
lines changed

‎src/pg_pathman.c

Lines changed: 92 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,8 @@ static bool pull_var_param(const WalkerContext *ctx,
7474

7575

7676
/* Misc */
77-
static List *make_inh_translation_list_simplified(Relation oldrelation,
78-
Relation newrelation,
79-
Index newvarno);
77+
static void make_inh_translation_list(Relation oldrelation, Relation newrelation,
78+
Index newvarno, List **translated_vars);
8079

8180

8281
/* Copied from allpaths.h */
@@ -165,79 +164,118 @@ _PG_init(void)
165164
}
166165

167166
/*
168-
* Build the list of translations from parent Vars to child Vars
169-
* for an inheritance child.
167+
* make_inh_translation_list
168+
* Build the list of translations from parent Vars to child Vars for
169+
* an inheritance child.
170170
*
171-
* NOTE: Inspired by function make_inh_translation_list().
171+
* For paranoia's sake, we match type/collation as well as attribute name.
172+
*
173+
* NOTE: borrowed from prepunion.c
172174
*/
173-
static List *
174-
make_inh_translation_list_simplified(Relation oldrelation,
175-
Relation newrelation,
176-
Index newvarno)
175+
static void
176+
make_inh_translation_list(Relation oldrelation, Relation newrelation,
177+
Index newvarno, List **translated_vars)
177178
{
178179
List *vars = NIL;
179180
TupleDesc old_tupdesc = RelationGetDescr(oldrelation);
180181
TupleDesc new_tupdesc = RelationGetDescr(newrelation);
181-
int oldnatts = RelationGetNumberOfAttributes(oldrelation);
182-
int newnatts = RelationGetNumberOfAttributes(newrelation);
182+
int oldnatts = old_tupdesc->natts;
183+
int newnatts = new_tupdesc->natts;
183184
int old_attno;
184185

185-
/* Amounts of attributes must match */
186-
if (oldnatts != newnatts)
187-
goto inh_translation_list_error;
188-
189-
/* We expect that parent and partition have an identical tupdesc */
190186
for (old_attno = 0; old_attno < oldnatts; old_attno++)
191187
{
192-
Form_pg_attribute old_att,
193-
new_att;
194-
Oid atttypid;
195-
int32 atttypmod;
196-
Oid attcollation;
197-
198-
old_att = old_tupdesc->attrs[old_attno];
199-
new_att = new_tupdesc->attrs[old_attno];
200-
201-
/* Attribute definitions must match */
202-
if (old_att->attisdropped != new_att->attisdropped ||
203-
old_att->atttypid != new_att->atttypid ||
204-
old_att->atttypmod != new_att->atttypmod ||
205-
old_att->attcollation != new_att->attcollation ||
206-
strcmp(NameStr(old_att->attname), NameStr(new_att->attname)) != 0)
188+
Form_pg_attribute att;
189+
char *attname;
190+
Oid atttypid;
191+
int32 atttypmod;
192+
Oid attcollation;
193+
int new_attno;
194+
195+
att = old_tupdesc->attrs[old_attno];
196+
if (att->attisdropped)
207197
{
208-
goto inh_translation_list_error;
198+
/* Just put NULL into this list entry */
199+
vars = lappend(vars, NULL);
200+
continue;
209201
}
202+
attname = NameStr(att->attname);
203+
atttypid = att->atttypid;
204+
atttypmod = att->atttypmod;
205+
attcollation = att->attcollation;
210206

211-
if (old_att->attisdropped)
207+
/*
208+
* When we are generating the "translation list" for the parent table
209+
* of an inheritance set, no need to search for matches.
210+
*/
211+
if (oldrelation == newrelation)
212212
{
213-
/* Just put NULL into this list entry */
214-
vars = lappend(vars, NULL);
213+
vars = lappend(vars, makeVar(newvarno,
214+
(AttrNumber) (old_attno + 1),
215+
atttypid,
216+
atttypmod,
217+
attcollation,
218+
0));
215219
continue;
216220
}
217221

218-
atttypid = old_att->atttypid;
219-
atttypmod = old_att->atttypmod;
220-
attcollation = old_att->attcollation;
222+
/*
223+
* Otherwise we have to search for the matching column by name.
224+
* There's no guarantee it'll have the same column position, because
225+
* of cases like ALTER TABLE ADD COLUMN and multiple inheritance.
226+
* However, in simple cases it will be the same column number, so try
227+
* that before we go groveling through all the columns.
228+
*
229+
* Note: the test for (att = ...) != NULL cannot fail, it's just a
230+
* notational device to include the assignment into the if-clause.
231+
*/
232+
if (old_attno < newnatts &&
233+
(att = new_tupdesc->attrs[old_attno]) != NULL &&
234+
!att->attisdropped && att->attinhcount != 0 &&
235+
strcmp(attname, NameStr(att->attname)) == 0)
236+
new_attno = old_attno;
237+
else
238+
{
239+
for (new_attno = 0; new_attno < newnatts; new_attno++)
240+
{
241+
att = new_tupdesc->attrs[new_attno];
242+
243+
/*
244+
* Make clang analyzer happy:
245+
*
246+
* Access to field 'attisdropped' results
247+
* in a dereference of a null pointer
248+
*/
249+
if (!att)
250+
elog(ERROR, "error in function "
251+
CppAsString(make_inh_translation_list));
252+
253+
if (!att->attisdropped && att->attinhcount != 0 &&
254+
strcmp(attname, NameStr(att->attname)) == 0)
255+
break;
256+
}
257+
if (new_attno >= newnatts)
258+
elog(ERROR, "could not find inherited attribute \"%s\" of relation \"%s\"",
259+
attname, RelationGetRelationName(newrelation));
260+
}
261+
262+
/* Found it, check type and collation match */
263+
if (atttypid != att->atttypid || atttypmod != att->atttypmod)
264+
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's type",
265+
attname, RelationGetRelationName(newrelation));
266+
if (attcollation != att->attcollation)
267+
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's collation",
268+
attname, RelationGetRelationName(newrelation));
221269

222270
vars = lappend(vars, makeVar(newvarno,
223-
(AttrNumber) (old_attno + 1),
271+
(AttrNumber) (new_attno + 1),
224272
atttypid,
225273
atttypmod,
226274
attcollation,
227275
0));
228276
}
229277

230-
/* Everything's ok */
231-
return vars;
232-
233-
/* We end up here if any attribute differs */
234-
inh_translation_list_error:
235-
elog(ERROR, "partition \"%s\" must have exact"
236-
"same structure as parent \"%s\"",
237-
RelationGetRelationName(newrelation),
238-
RelationGetRelationName(oldrelation));
239-
240-
return NIL; /* keep compiler happy */
278+
*translated_vars = vars;
241279
}
242280

243281
/*
@@ -295,9 +333,9 @@ append_child_relation(PlannerInfo *root, Relation parent_relation,
295333
appinfo->parent_relid = parent_rti;
296334
appinfo->child_relid = childRTindex;
297335
appinfo->parent_reloid = parent_rte->relid;
298-
appinfo->translated_vars = make_inh_translation_list_simplified(parent_relation,
299-
child_relation,
300-
childRTindex);
336+
337+
make_inh_translation_list(parent_relation, child_relation, childRTindex,
338+
&appinfo->translated_vars);
301339

302340
/* Now append 'appinfo' to 'root->append_rel_list' */
303341
root->append_rel_list = lappend(root->append_rel_list, appinfo);

0 commit comments

Comments
 (0)