On 2019/01/11 11:07, Amit Langote wrote:
> On 2019/01/11 6:47, David Rowley wrote:
>> On Fri, 11 Jan 2019 at 06:56, Alvaro Herrera <alvhe...@2ndquadrant.com>
>> wrote:
>>> Pushed 0001 with some minor tweaks, thanks.
>>
>> Thanks for pushing. I had looked at 0001 last night and there wasn't
>> much to report, just:
>>
>> v12 0001
>>
>> 1. I see you've moved translate_col_privs() out of prepunion.c into
>> appendinfo.c. This required making it an external function, but it's
>> only use is in inherit.c, so would it not be better to put it there
>> and keep it static?
>
> Actually, I *was* a bit puzzled where to put it. I tend to agree with you
> now that it might be define it locally within inherit.c as it might not be
> needed elsewhere. Let's hear what Alvaro thinks. I'm attaching a patch
> anyway.
>
>> 2. The following two lines I think need to swap their order.
>>
>> +#include "utils/rel.h"
>> +#include "utils/lsyscache.h"
>
> Oops, fixed.
>
>> Both are pretty minor details but thought I'd post them anyway.
>
> Thank you for reporting.
>
> Attached find the patch.
Looking around a bit more, I started thinking even build_child_join_sjinfo
doesn't belong in appendinfo.c (just to be clear, it was defined in
prepunion.c before this commit), so maybe we should move it to joinrels.c
and instead export adjust_child_relids that's required by it from
appendinfo.c. There's already adjust_child_relids_multilevel in
appendinfo.h, so having adjust_child_relids next to it isn't too bad. At
least not as bad as appendinfo.c exporting build_child_join_sjinfo for
joinrels.c to use.
I have updated the patch.
Thanks,
Amit
diff --git a/src/backend/optimizer/path/joinrels.c
b/src/backend/optimizer/path/joinrels.c
index 38eeb23d81..db18feccfe 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -46,6 +46,9 @@ static void try_partitionwise_join(PlannerInfo *root,
RelOptInfo *rel1,
List *parent_restrictlist);
static int match_expr_to_partition_keys(Expr *expr, RelOptInfo *rel,
bool strict_op);
+static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
+ SpecialJoinInfo *parent_sjinfo,
+ Relids left_relids, Relids
right_relids);
/*
@@ -1582,3 +1585,45 @@ match_expr_to_partition_keys(Expr *expr, RelOptInfo
*rel, bool strict_op)
return -1;
}
+
+/*
+ * Construct the SpecialJoinInfo for a child-join by translating
+ * SpecialJoinInfo for the join between parents. left_relids and right_relids
+ * are the relids of left and right side of the join respectively.
+ */
+static SpecialJoinInfo *
+build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
+ Relids left_relids, Relids
right_relids)
+{
+ SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
+ AppendRelInfo **left_appinfos;
+ int left_nappinfos;
+ AppendRelInfo **right_appinfos;
+ int right_nappinfos;
+
+ memcpy(sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
+ left_appinfos = find_appinfos_by_relids(root, left_relids,
+
&left_nappinfos);
+ right_appinfos = find_appinfos_by_relids(root, right_relids,
+
&right_nappinfos);
+
+ sjinfo->min_lefthand = adjust_child_relids(sjinfo->min_lefthand,
+
left_nappinfos, left_appinfos);
+ sjinfo->min_righthand = adjust_child_relids(sjinfo->min_righthand,
+
right_nappinfos,
+
right_appinfos);
+ sjinfo->syn_lefthand = adjust_child_relids(sjinfo->syn_lefthand,
+
left_nappinfos, left_appinfos);
+ sjinfo->syn_righthand = adjust_child_relids(sjinfo->syn_righthand,
+
right_nappinfos,
+
right_appinfos);
+ sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
+
(Node *) sjinfo->semi_rhs_exprs,
+
right_nappinfos,
+
right_appinfos);
+
+ pfree(left_appinfos);
+ pfree(right_appinfos);
+
+ return sjinfo;
+}
diff --git a/src/backend/optimizer/util/appendinfo.c
b/src/backend/optimizer/util/appendinfo.c
index d48e3a09b3..37fbc3a117 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -15,13 +15,12 @@
#include "postgres.h"
#include "access/htup_details.h"
-#include "access/sysattr.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/appendinfo.h"
#include "parser/parsetree.h"
-#include "utils/rel.h"
#include "utils/lsyscache.h"
+#include "utils/rel.h"
#include "utils/syscache.h"
@@ -38,8 +37,6 @@ static void make_inh_translation_list(Relation oldrelation,
List **translated_vars);
static Node *adjust_appendrel_attrs_mutator(Node *node,
adjust_appendrel_attrs_context *context);
-static Relids adjust_child_relids(Relids relids, int nappinfos,
- AppendRelInfo **appinfos);
static List *adjust_inherited_tlist(List *tlist,
AppendRelInfo *context);
@@ -167,58 +164,6 @@ make_inh_translation_list(Relation oldrelation, Relation
newrelation,
}
/*
- * translate_col_privs
- * Translate a bitmapset representing per-column privileges from the
- * parent rel's attribute numbering to the child's.
- *
- * The only surprise here is that we don't translate a parent whole-row
- * reference into a child whole-row reference. That would mean requiring
- * permissions on all child columns, which is overly strict, since the
- * query is really only going to reference the inherited columns. Instead
- * we set the per-column bits for all inherited columns.
- */
-Bitmapset *
-translate_col_privs(const Bitmapset *parent_privs,
- List *translated_vars)
-{
- Bitmapset *child_privs = NULL;
- bool whole_row;
- int attno;
- ListCell *lc;
-
- /* System attributes have the same numbers in all tables */
- for (attno = FirstLowInvalidHeapAttributeNumber + 1; attno < 0; attno++)
- {
- if (bms_is_member(attno - FirstLowInvalidHeapAttributeNumber,
- parent_privs))
- child_privs = bms_add_member(child_privs,
-
attno - FirstLowInvalidHeapAttributeNumber);
- }
-
- /* Check if parent has whole-row reference */
- whole_row = bms_is_member(InvalidAttrNumber -
FirstLowInvalidHeapAttributeNumber,
- parent_privs);
-
- /* And now translate the regular user attributes, using the vars list */
- attno = InvalidAttrNumber;
- foreach(lc, translated_vars)
- {
- Var *var = lfirst_node(Var, lc);
-
- attno++;
- if (var == NULL) /* ignore dropped columns */
- continue;
- if (whole_row ||
- bms_is_member(attno -
FirstLowInvalidHeapAttributeNumber,
- parent_privs))
- child_privs = bms_add_member(child_privs,
-
var->varattno - FirstLowInvalidHeapAttributeNumber);
- }
-
- return child_privs;
-}
-
-/*
* adjust_appendrel_attrs
* Copy the specified query or expression and translate Vars referring
to a
* parent rel to refer to the corresponding child rel instead. We also
@@ -531,7 +476,7 @@ adjust_appendrel_attrs_mutator(Node *node,
* Substitute child relids for parent relids in a Relid set. The array of
* appinfos specifies the substitutions to be performed.
*/
-static Relids
+Relids
adjust_child_relids(Relids relids, int nappinfos, AppendRelInfo **appinfos)
{
Bitmapset *result = NULL;
@@ -754,48 +699,6 @@ adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node
*node,
}
/*
- * Construct the SpecialJoinInfo for a child-join by translating
- * SpecialJoinInfo for the join between parents. left_relids and right_relids
- * are the relids of left and right side of the join respectively.
- */
-SpecialJoinInfo *
-build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids
right_relids)
-{
- SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
- AppendRelInfo **left_appinfos;
- int left_nappinfos;
- AppendRelInfo **right_appinfos;
- int right_nappinfos;
-
- memcpy(sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
- left_appinfos = find_appinfos_by_relids(root, left_relids,
-
&left_nappinfos);
- right_appinfos = find_appinfos_by_relids(root, right_relids,
-
&right_nappinfos);
-
- sjinfo->min_lefthand = adjust_child_relids(sjinfo->min_lefthand,
-
left_nappinfos, left_appinfos);
- sjinfo->min_righthand = adjust_child_relids(sjinfo->min_righthand,
-
right_nappinfos,
-
right_appinfos);
- sjinfo->syn_lefthand = adjust_child_relids(sjinfo->syn_lefthand,
-
left_nappinfos, left_appinfos);
- sjinfo->syn_righthand = adjust_child_relids(sjinfo->syn_righthand,
-
right_nappinfos,
-
right_appinfos);
- sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
-
(Node *) sjinfo->semi_rhs_exprs,
-
right_nappinfos,
-
right_appinfos);
-
- pfree(left_appinfos);
- pfree(right_appinfos);
-
- return sjinfo;
-}
-
-/*
* find_appinfos_by_relids
* Find AppendRelInfo structures for all relations specified by
relids.
*
diff --git a/src/backend/optimizer/util/inherit.c
b/src/backend/optimizer/util/inherit.c
index 350e6afe27..db474acbc5 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/heapam.h"
+#include "access/sysattr.h"
#include "catalog/partition.h"
#include "catalog/pg_inherits.h"
#include "miscadmin.h"
@@ -38,6 +39,8 @@ static void expand_single_inheritance_child(PlannerInfo *root,
PlanRowMark
*top_parentrc, Relation childrel,
List
**appinfos, RangeTblEntry **childrte_p,
Index
*childRTindex_p);
+static Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
+ List *translated_vars);
/*
@@ -437,3 +440,55 @@ expand_single_inheritance_child(PlannerInfo *root,
RangeTblEntry *parentrte,
root->rowMarks = lappend(root->rowMarks, childrc);
}
}
+
+/*
+ * translate_col_privs
+ * Translate a bitmapset representing per-column privileges from the
+ * parent rel's attribute numbering to the child's.
+ *
+ * The only surprise here is that we don't translate a parent whole-row
+ * reference into a child whole-row reference. That would mean requiring
+ * permissions on all child columns, which is overly strict, since the
+ * query is really only going to reference the inherited columns. Instead
+ * we set the per-column bits for all inherited columns.
+ */
+static Bitmapset *
+translate_col_privs(const Bitmapset *parent_privs,
+ List *translated_vars)
+{
+ Bitmapset *child_privs = NULL;
+ bool whole_row;
+ int attno;
+ ListCell *lc;
+
+ /* System attributes have the same numbers in all tables */
+ for (attno = FirstLowInvalidHeapAttributeNumber + 1; attno < 0; attno++)
+ {
+ if (bms_is_member(attno - FirstLowInvalidHeapAttributeNumber,
+ parent_privs))
+ child_privs = bms_add_member(child_privs,
+
attno - FirstLowInvalidHeapAttributeNumber);
+ }
+
+ /* Check if parent has whole-row reference */
+ whole_row = bms_is_member(InvalidAttrNumber -
FirstLowInvalidHeapAttributeNumber,
+ parent_privs);
+
+ /* And now translate the regular user attributes, using the vars list */
+ attno = InvalidAttrNumber;
+ foreach(lc, translated_vars)
+ {
+ Var *var = lfirst_node(Var, lc);
+
+ attno++;
+ if (var == NULL) /* ignore dropped columns */
+ continue;
+ if (whole_row ||
+ bms_is_member(attno -
FirstLowInvalidHeapAttributeNumber,
+ parent_privs))
+ child_privs = bms_add_member(child_privs,
+
var->varattno - FirstLowInvalidHeapAttributeNumber);
+ }
+
+ return child_privs;
+}
diff --git a/src/include/optimizer/appendinfo.h
b/src/include/optimizer/appendinfo.h
index 16705da780..156a0e077d 100644
--- a/src/include/optimizer/appendinfo.h
+++ b/src/include/optimizer/appendinfo.h
@@ -20,21 +20,15 @@
extern AppendRelInfo *make_append_rel_info(Relation parentrel,
Relation childrel,
Index parentRTindex, Index
childRTindex);
-extern Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
- List *translated_vars);
extern Node *adjust_appendrel_attrs(PlannerInfo *root, Node *node,
int nappinfos, AppendRelInfo
**appinfos);
-
extern Node *adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node *node,
Relids
child_relids,
Relids
top_parent_relids);
-
extern AppendRelInfo **find_appinfos_by_relids(PlannerInfo *root,
Relids relids, int *nappinfos);
-
-extern SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
- SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids
right_relids);
+extern Relids adjust_child_relids(Relids relids, int nappinfos,
+ AppendRelInfo **appinfos);
extern Relids adjust_child_relids_multilevel(PlannerInfo *root, Relids relids,
Relids child_relids,
Relids top_parent_relids);
