At Mon, 28 Aug 2017 18:28:07 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI
<horiguchi.kyot...@lab.ntt.co.jp> wrote in
<20170828.182807.98097766.horiguchi.kyot...@lab.ntt.co.jp>
> I'll add this to CF2017-09.
This patch got deadly atack from the commit 30833ba. I changed
the signature of expand_single_inheritance_child in addition to
make_inh_translation_list to notify that the specified child is
no longer a child of the parent.
This passes regular regression test and fixed the the problem.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
*** a/src/backend/catalog/pg_inherits.c
--- b/src/backend/catalog/pg_inherits.c
***************
*** 42,47 **** typedef struct SeenRelsEntry
--- 42,49 ----
ListCell *numparents_cell; /* corresponding list cell */
} SeenRelsEntry;
+ static bool is_descendent_of_internal(Oid parentId, Oid childId,
+ HTAB *seen_rels);
/*
* find_inheritance_children
*
***************
*** 400,402 **** typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId)
--- 402,472 ----
return result;
}
+
+ /*
+ * Check if the child is really a descendent of the parent
+ */
+ bool
+ is_descendent_of(Oid parentId, Oid childId)
+ {
+ HTAB *seen_rels;
+ HASHCTL ctl;
+ bool ischild = false;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(Oid);
+ ctl.entrysize = sizeof(Oid);
+ ctl.hcxt = CurrentMemoryContext;
+
+ seen_rels = hash_create("is_descendent_of temporary table",
+ 32, /* start small and extend */
+ &ctl,
+ HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+
+ ischild = is_descendent_of_internal(parentId, childId, seen_rels);
+
+ hash_destroy(seen_rels);
+
+ return ischild;
+ }
+
+ static bool
+ is_descendent_of_internal(Oid parentId, Oid childId, HTAB *seen_rels)
+ {
+ Relation inhrel;
+ SysScanDesc scan;
+ ScanKeyData key[1];
+ bool ischild = false;
+ HeapTuple inheritsTuple;
+
+ inhrel = heap_open(InheritsRelationId, AccessShareLock);
+ ScanKeyInit(&key[0], Anum_pg_inherits_inhparent,
+ BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(parentId));
+ scan = systable_beginscan(inhrel, InheritsParentIndexId, true,
+ NULL, 1, key);
+
+ while ((inheritsTuple = systable_getnext(scan)) != NULL)
+ {
+ bool found;
+ Oid inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid;
+
+ hash_search(seen_rels, &inhrelid, HASH_ENTER, &found);
+
+ /*
+ * Recursively check into children. Although there can't theoretically
+ * be any cycles in the inheritance graph, check the cycles following
+ * find_all_inheritors.
+ */
+ if (inhrelid == childId ||
+ (!found && is_descendent_of_internal(inhrelid, childId, seen_rels)))
+ {
+ ischild = true;
+ break;
+ }
+ }
+
+ systable_endscan(scan);
+ heap_close(inhrel, AccessShareLock);
+
+ return ischild;
+ }
*** a/src/backend/optimizer/prep/prepunion.c
--- b/src/backend/optimizer/prep/prepunion.c
***************
*** 108,123 **** static void expand_partitioned_rtentry(PlannerInfo *root,
LOCKMODE lockmode,
bool *has_child, List **appinfos,
List **partitioned_child_rels);
! static void expand_single_inheritance_child(PlannerInfo *root,
RangeTblEntry *parentrte,
Index parentRTindex, Relation parentrel,
PlanRowMark *parentrc, Relation childrel,
bool *has_child, List **appinfos,
List **partitioned_child_rels);
! static void make_inh_translation_list(Relation oldrelation,
Relation newrelation,
! Index newvarno,
! List **translated_vars);
static Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
List *translated_vars);
static Node *adjust_appendrel_attrs_mutator(Node *node,
--- 108,122 ----
LOCKMODE lockmode,
bool *has_child, List **appinfos,
List **partitioned_child_rels);
! static bool expand_single_inheritance_child(PlannerInfo *root,
RangeTblEntry *parentrte,
Index parentRTindex, Relation parentrel,
PlanRowMark *parentrc, Relation childrel,
bool *has_child, List **appinfos,
List **partitioned_child_rels);
! static List *make_inh_translation_list(Relation oldrelation,
Relation newrelation,
! Index newvarno);
static Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
List *translated_vars);
static Node *adjust_appendrel_attrs_mutator(Node *node,
***************
*** 1476,1481 **** expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
--- 1475,1482 ----
* in which they appear in the PartitionDesc. But first, expand the
* parent itself.
*/
+
+ /* ignore the return value since this doesn't exclude the parent */
expand_single_inheritance_child(root, rte, rti, oldrelation, oldrc,
oldrelation,
&has_child, &appinfos,
***************
*** 1497,1502 **** expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
--- 1498,1504 ----
{
Oid childOID = lfirst_oid(l);
Relation newrelation;
+ bool expanded = false;
/* Open rel if needed; we already have required locks */
if (childOID != parentOID)
***************
*** 1516,1529 **** expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
continue;
}
! expand_single_inheritance_child(root, rte, rti, oldrelation, oldrc,
newrelation,
&has_child, &appinfos,
&partitioned_child_rels);
! /* Close child relations, but keep locks */
if (childOID != parentOID)
! heap_close(newrelation, NoLock);
}
}
--- 1518,1532 ----
continue;
}
! expanded = expand_single_inheritance_child(root,
! rte, rti, oldrelation, oldrc,
newrelation,
&has_child, &appinfos,
&partitioned_child_rels);
! /* Close child relations, but keep locks if expanded */
if (childOID != parentOID)
! heap_close(newrelation, expanded ? NoLock : lockmode);
}
}
***************
*** 1581,1586 **** expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
--- 1584,1590 ----
{
Oid childOID = partdesc->oids[i];
Relation childrel;
+ bool expanded = false;
/* Open rel; we already have required locks */
childrel = heap_open(childOID, NoLock);
***************
*** 1592,1598 **** expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
continue;
}
! expand_single_inheritance_child(root, parentrte, parentRTindex,
parentrel, parentrc, childrel,
has_child, appinfos,
partitioned_child_rels);
--- 1596,1603 ----
continue;
}
! expanded = expand_single_inheritance_child(root,
! parentrte, parentRTindex,
parentrel, parentrc, childrel,
has_child, appinfos,
partitioned_child_rels);
***************
*** 1606,1613 **** expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
has_child, appinfos,
partitioned_child_rels);
! /* Close child relation, but keep locks */
! heap_close(childrel, NoLock);
}
}
--- 1611,1618 ----
has_child, appinfos,
partitioned_child_rels);
! /* Close child relation, but keep locks if expanded */
! heap_close(childrel, expanded ? NoLock : lockmode);
}
}
***************
*** 1619,1626 **** expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
* anything at all. Otherwise, we'll set "has_child" to true, build a
* RangeTblEntry and either a PartitionedChildRelInfo or AppendRelInfo as
* appropriate, plus maybe a PlanRowMark.
*/
! static void
expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
Index parentRTindex, Relation parentrel,
PlanRowMark *parentrc, Relation childrel,
--- 1624,1634 ----
* anything at all. Otherwise, we'll set "has_child" to true, build a
* RangeTblEntry and either a PartitionedChildRelInfo or AppendRelInfo as
* appropriate, plus maybe a PlanRowMark.
+ *
+ * Returns false if the child has been found that no longer a child of the
+ * parent.
*/
! static bool
expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
Index parentRTindex, Relation parentrel,
PlanRowMark *parentrc, Relation childrel,
***************
*** 1661,1666 **** expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
--- 1669,1692 ----
*/
if (childrte->relkind != RELKIND_PARTITIONED_TABLE)
{
+ List *translated_vars =
+ make_inh_translation_list(parentrel, childrel, childRTindex);
+
+ /*
+ * It may happen that the childOID is no longer a child of the
+ * parent.
+ */
+ if (!translated_vars)
+ {
+ /*
+ * This childrel is no longer a child of the parent. Return doing
+ * nothing. Halfway built childrte is abandoned but this happens
+ * really rarely.
+ */
+ Assert (childOID != parentOID);
+ return false;
+ }
+
/* Remember if we saw a real child. */
if (childOID != parentOID)
*has_child = true;
***************
*** 1670,1677 **** expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
appinfo->child_relid = childRTindex;
appinfo->parent_reltype = parentrel->rd_rel->reltype;
appinfo->child_reltype = childrel->rd_rel->reltype;
! make_inh_translation_list(parentrel, childrel, childRTindex,
! &appinfo->translated_vars);
appinfo->parent_reloid = parentOID;
*appinfos = lappend(*appinfos, appinfo);
--- 1696,1702 ----
appinfo->child_relid = childRTindex;
appinfo->parent_reltype = parentrel->rd_rel->reltype;
appinfo->child_reltype = childrel->rd_rel->reltype;
! appinfo->translated_vars = translated_vars;
appinfo->parent_reloid = parentOID;
*appinfos = lappend(*appinfos, appinfo);
***************
*** 1726,1744 **** expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
root->rowMarks = lappend(root->rowMarks, childrc);
}
}
/*
* make_inh_translation_list
* Build the list of translations from parent Vars to child Vars for
! * an inheritance child.
*
* For paranoia's sake, we match type/collation as well as attribute name.
*/
! static void
make_inh_translation_list(Relation oldrelation, Relation newrelation,
! Index newvarno,
! List **translated_vars)
{
List *vars = NIL;
TupleDesc old_tupdesc = RelationGetDescr(oldrelation);
--- 1751,1771 ----
root->rowMarks = lappend(root->rowMarks, childrc);
}
+
+ return true;
}
/*
* make_inh_translation_list
* Build the list of translations from parent Vars to child Vars for
! * an inheritance child. Returns NULL if the two relations are no longer in
! * a inheritance relationship.
*
* For paranoia's sake, we match type/collation as well as attribute name.
*/
! static List *
make_inh_translation_list(Relation oldrelation, Relation newrelation,
! Index newvarno)
{
List *vars = NIL;
TupleDesc old_tupdesc = RelationGetDescr(oldrelation);
***************
*** 1808,1815 **** make_inh_translation_list(Relation oldrelation, Relation newrelation,
--- 1835,1852 ----
break;
}
if (new_attno >= newnatts)
+ {
+ /*
+ * It's possible that newrelation is no longer a child of the
+ * oldrelation. We just ingore it for the case.
+ */
+ if (!is_descendent_of(oldrelation->rd_id, newrelation->rd_id))
+ return NULL;
+
+ /* If still a child, something's wrong. */
elog(ERROR, "could not find inherited attribute \"%s\" of relation \"%s\"",
attname, RelationGetRelationName(newrelation));
+ }
}
/* Found it, check type and collation match */
***************
*** 1828,1834 **** make_inh_translation_list(Relation oldrelation, Relation newrelation,
0));
}
! *translated_vars = vars;
}
/*
--- 1865,1871 ----
0));
}
! return vars;
}
/*
*** a/src/include/catalog/pg_inherits_fn.h
--- b/src/include/catalog/pg_inherits_fn.h
***************
*** 23,27 **** extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode,
--- 23,28 ----
extern bool has_subclass(Oid relationId);
extern bool has_superclass(Oid relationId);
extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId);
+ extern bool is_descendent_of(Oid parentId, Oid childId);
#endif /* PG_INHERITS_FN_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
