partition -> partitioned
Hi.
Commit 499be013de6 used 'partition' where it really meant 'partitioned' in
a few places including in a variable name. For example, what almost all
places call 'partitioned_rels', make_partition_pruneinfo called
'partition_rels'.
Attached a patch to make that uniform to avoid confusion.
Thanks,
Amit
Attachments:
use-partitioned-not-partition.patchtext/plain; charset=UTF-8; name=use-partitioned-not-partition.patchDownload
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 58ec2a684d..c39b618cee 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -188,7 +188,7 @@ static bool partkey_datum_from_expr(PartitionPruneContext *context,
* pruning would be useless.
*/
List *
-make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
+make_partition_pruneinfo(PlannerInfo *root, List *partitioned_rels,
List *subpaths, List *prunequal)
{
RelOptInfo *targetpart = NULL;
@@ -218,9 +218,9 @@ make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
relid_subnode_map[pathrel->relid] = i++;
}
- /* Likewise for the partition_rels */
+ /* Likewise for the partitioned_rels */
i = 1;
- foreach(lc, partition_rels)
+ foreach(lc, partitioned_res)
{
Index rti = lfirst_int(lc);
@@ -229,8 +229,8 @@ make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
relid_subpart_map[rti] = i++;
}
- /* We now build a PartitionPruneInfo for each partition_rels */
- foreach(lc, partition_rels)
+ /* We now build a PartitionPruneInfo for each partitioned_rels */
+ foreach(lc, partitioned_rels)
{
Index rti = lfirst_int(lc);
RelOptInfo *subpart = find_base_rel(root, rti);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index f90aa7b2a1..6bec45a2a3 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1593,7 +1593,7 @@ typedef struct PartitionPruneStepCombine
typedef struct PartitionPruneInfo
{
NodeTag type;
- Oid reloid; /* Oid of partition rel */
+ Oid reloid; /* Oid of the partitioned table */
List *pruning_steps; /* List of PartitionPruneStep */
Bitmapset *present_parts; /* Indexes of all partitions which subnodes
* are present for. */
diff --git a/src/include/partitioning/partprune.h b/src/include/partitioning/partprune.h
index 3d114b4c71..ff034206f7 100644
--- a/src/include/partitioning/partprune.h
+++ b/src/include/partitioning/partprune.h
@@ -62,7 +62,8 @@ typedef struct PartitionPruneContext
#define PruneCxtStateIdx(partnatts, step_id, keyno) \
((partnatts) * (step_id) + (keyno))
-extern List *make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
+extern List *make_partition_pruneinfo(PlannerInfo *root,
+ List *partitioned_rels,
List *subpaths, List *prunequal);
extern Relids prune_append_rel_partitions(RelOptInfo *rel);
extern Bitmapset *get_matching_partitions(PartitionPruneContext *context,
On 17 May 2018 at 13:52, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Commit 499be013de6 used 'partition' where it really meant 'partitioned' in
a few places including in a variable name. For example, what almost all
places call 'partitioned_rels', make_partition_pruneinfo called
'partition_rels'.Attached a patch to make that uniform to avoid confusion.
Looks good to me.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 2018/05/17 11:40, David Rowley wrote:
On 17 May 2018 at 13:52, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Commit 499be013de6 used 'partition' where it really meant 'partitioned' in
a few places including in a variable name. For example, what almost all
places call 'partitioned_rels', make_partition_pruneinfo called
'partition_rels'.Attached a patch to make that uniform to avoid confusion.
Looks good to me.
Thanks for taking a look at it.
Regards,
Amit
On 2018/05/17 11:48, Amit Langote wrote:
On 2018/05/17 11:40, David Rowley wrote:
On 17 May 2018 at 13:52, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Commit 499be013de6 used 'partition' where it really meant 'partitioned' in
a few places including in a variable name. For example, what almost all
places call 'partitioned_rels', make_partition_pruneinfo called
'partition_rels'.Attached a patch to make that uniform to avoid confusion.
Looks good to me.
Thanks for taking a look at it.
Did this perhaps get forgotten?
Thanks,
Amit
On 2018/06/19 17:51, Amit Langote wrote:
On 2018/05/17 11:48, Amit Langote wrote:
On 2018/05/17 11:40, David Rowley wrote:
On 17 May 2018 at 13:52, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Commit 499be013de6 used 'partition' where it really meant 'partitioned' in
a few places including in a variable name. For example, what almost all
places call 'partitioned_rels', make_partition_pruneinfo called
'partition_rels'.Attached a patch to make that uniform to avoid confusion.
Looks good to me.
Thanks for taking a look at it.
Did this perhaps get forgotten?
Noticed that the relevant code changed, so I rebased the patch. Also,
made a minor update to a nearby comment.
Thanks,
Amit
Attachments:
use-partitioned-not-partition-v2.patchtext/plain; charset=UTF-8; name=use-partitioned-not-partition-v2.patchDownload
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 0a76b77d3e..aad75f113d 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -176,9 +176,9 @@ static bool partkey_datum_from_expr(PartitionPruneContext *context,
/*
* make_partition_pruneinfo
- * Build List of PartitionPruneInfos, one for each 'partitioned_rels'.
- * These can be used in the executor to allow additional partition
- * pruning to take place.
+ * Build List of PartitionPruneInfos, one for each relation mentioned
+ * in 'partitioned_rels'. These can be used in the executor to allow
+ * additional partition pruning to take place.
*
* Here we generate partition pruning steps for 'prunequal' and also build a
* data structure which allows mapping of partition indexes into 'subpaths'
@@ -190,7 +190,7 @@ static bool partkey_datum_from_expr(PartitionPruneContext *context,
* pruning done during planning will have pruned everything that can be.
*/
List *
-make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
+make_partition_pruneinfo(PlannerInfo *root, List *partitioned_rels,
List *subpaths, List *prunequal)
{
RelOptInfo *targetpart = NULL;
@@ -229,11 +229,11 @@ make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
/*
* relid_subpart_map maps relid of a non-leaf partition to the index in
- * 'partition_rels' of that rel (which will also be the index in the
+ * 'partitioned_rels' of that rel (which will also be the index in the
* returned PartitionPruneInfo list of the info for that partition).
*/
i = 1;
- foreach(lc, partition_rels)
+ foreach(lc, partitioned_rels)
{
Index rti = lfirst_int(lc);
@@ -246,8 +246,8 @@ make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
relid_subpart_map[rti] = i++;
}
- /* We now build a PartitionPruneInfo for each rel in partition_rels */
- foreach(lc, partition_rels)
+ /* We now build a PartitionPruneInfo for each rel in partitioned_rels */
+ foreach(lc, partitioned_rels)
{
Index rti = lfirst_int(lc);
RelOptInfo *subpart = find_base_rel(root, rti);
diff --git a/src/include/partitioning/partprune.h b/src/include/partitioning/partprune.h
index 09147b532c..9944d2832f 100644
--- a/src/include/partitioning/partprune.h
+++ b/src/include/partitioning/partprune.h
@@ -74,7 +74,8 @@ typedef struct PartitionPruneContext
#define PruneCxtStateIdx(partnatts, step_id, keyno) \
((partnatts) * (step_id) + (keyno))
-extern List *make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
+extern List *make_partition_pruneinfo(PlannerInfo *root,
+ List *partitioned_rels,
List *subpaths, List *prunequal);
extern Relids prune_append_rel_partitions(RelOptInfo *rel);
extern Bitmapset *get_matching_partitions(PartitionPruneContext *context,
On Tue, Jun 19, 2018 at 06:02:22PM +0900, Amit Langote wrote:
Noticed that the relevant code changed, so I rebased the patch. Also,
made a minor update to a nearby comment.
That looks right to me as we speak about non-leaf partitions here.
Alvaro, as 499be013 is yours, would you fix this inconsistency or should
I? I could understand why things are confusing on HEAD, "partitioned"
and "partition" have opposite meanings.
--
Michael
On 2018-Jun-20, Michael Paquier wrote:
On Tue, Jun 19, 2018 at 06:02:22PM +0900, Amit Langote wrote:
Noticed that the relevant code changed, so I rebased the patch. Also,
made a minor update to a nearby comment.That looks right to me as we speak about non-leaf partitions here.
Alvaro, as 499be013 is yours, would you fix this inconsistency or should
I? I could understand why things are confusing on HEAD, "partitioned"
and "partition" have opposite meanings.
Hmm, will look.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Jun-19, Amit Langote wrote:
Noticed that the relevant code changed, so I rebased the patch. Also,
made a minor update to a nearby comment.
Pushed, thanks. I made a couple of comments one or two words shorter
while (IMO) not losing clarity.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018/06/21 0:45, Alvaro Herrera wrote:
On 2018-Jun-19, Amit Langote wrote:
Noticed that the relevant code changed, so I rebased the patch. Also,
made a minor update to a nearby comment.Pushed, thanks. I made a couple of comments one or two words shorter
while (IMO) not losing clarity.
Thank you.
Regards,
Amit