Hi.
On 2018/04/06 7:35, Alvaro Herrera wrote:
> I seems pretty clear that putting get_matching_partitions() in
> catalog/partition.c is totally the wrong thing; it belongs wholly in
> partprune. I think the reason you put it there is that it requires
> access to a lot of internals that are static in partition.c. In the
> attached not yet cleaned version of the patch, I have moved a whole lot
> of what you added to partition.c to partprune.c; and for the functions
> and struct declarations that were required to make it work, I created
> catalog/partition_internal.h.
Yes, I really wanted for most of the new code that this patch adds to land
in the planner, especially after Robert's comments here:
https://www.postgresql.org/message-id/CA%2BTgmoabi-29Vs8H0xkjtYB%3DcU%2BGVCrNwPz7okpa3KsoLmdEUQ%40mail.gmail.com
It would've been nice if we'd gotten the "reorganizing partitioning code"
thread resolved sooner.
> I changed a lot of code also, but cosmetic changes only.
>
> I'll clean this up a bit more now, and try to commit shortly (or early
> tomorrow); wanted to share current status now in case I have to rush
> out.
Some comments on the code reorganizing part of the patch:
* Did you intentionally not put PartitionBoundInfoData and its accessor
macros in partition_internal.h. partprune.c would not need to include
partition.h if we do that.
* Also, I wonder why you left PartitionPruneContext in partition.h. Isn't
it better taken out to partprune.h?
I have done that in the attached.
* Why isn't gen_partprune_steps() in partprune.h? I see only
prune_append_rel_partitions() exported out of partprune.c, but the runtime
patch needs gen_partprune_steps() to be called from createplan.c.
* I don't see get_matching_partitions() exported either. Runtime pruning
patch needs that too.
Maybe you've thought something about these two items though.
Thanks,
Amit
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index d5e91b111d..f89c99f544 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -24,7 +24,6 @@
#include "catalog/indexing.h"
#include "catalog/objectaddress.h"
#include "catalog/partition.h"
-#include "catalog/partition_internal.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_inherits.h"
#include "catalog/pg_inherits_fn.h"
diff --git a/src/backend/optimizer/util/partprune.c
b/src/backend/optimizer/util/partprune.c
index b0638d5aa6..93553b5d13 100644
--- a/src/backend/optimizer/util/partprune.c
+++ b/src/backend/optimizer/util/partprune.c
@@ -19,8 +19,6 @@
#include "access/hash.h"
#include "access/nbtree.h"
-#include "catalog/partition.h"
-#include "catalog/partition_internal.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
@@ -35,6 +33,7 @@
#include "parser/parse_coerce.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteManip.h"
+#include "utils/array.h"
#include "utils/lsyscache.h"
/*
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 0bcaa36165..1c17553917 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -13,7 +13,7 @@
#ifndef PARTITION_H
#define PARTITION_H
-#include "fmgr.h"
+#include "catalog/partition_internal.h"
#include "executor/tuptable.h"
#include "nodes/execnodes.h"
#include "parser/parse_node.h"
@@ -23,65 +23,6 @@
#define HASH_PARTITION_SEED UINT64CONST(0x7A5B22367996DCFD)
/*
- * PartitionBoundInfoData encapsulates a set of partition bounds. It is
- * usually associated with partitioned tables as part of its partition
- * descriptor, but may also be used to represent a virtual partitioned
- * table such as a partitioned joinrel within the planner.
- *
- * A list partition datum that is known to be NULL is never put into the
- * datums array. Instead, it is tracked using the null_index field.
- *
- * In the case of range partitioning, ndatums will typically be far less than
- * 2 * nparts, because a partition's upper bound and the next partition's lower
- * bound are the same in most common cases, and we only store one of them (the
- * upper bound). In case of hash partitioning, ndatums will be same as the
- * number of partitions.
- *
- * For range and list partitioned tables, datums is an array of datum-tuples
- * with key->partnatts datums each. For hash partitioned tables, it is an
array
- * of datum-tuples with 2 datums, modulus and remainder, corresponding to a
- * given partition.
- *
- * The datums in datums array are arranged in increasing order as defined by
- * functions qsort_partition_rbound_cmp(), qsort_partition_list_value_cmp() and
- * qsort_partition_hbound_cmp() for range, list and hash partitioned tables
- * respectively. For range and list partitions this simply means that the
- * datums in the datums array are arranged in increasing order as defined by
- * the partition key's operator classes and collations.
- *
- * In the case of list partitioning, the indexes array stores one entry for
- * every datum, which is the index of the partition that accepts a given datum.
- * In case of range partitioning, it stores one entry per distinct range
- * datum, which is the index of the partition for which a given datum
- * is an upper bound. In the case of hash partitioning, the number of the
- * entries in the indexes array is same as the greatest modulus amongst all
- * partitions. For a given partition key datum-tuple, the index of the
- * partition which would accept that datum-tuple would be given by the entry
- * pointed by remainder produced when hash value of the datum-tuple is divided
- * by the greatest modulus.
- */
-
-typedef struct PartitionBoundInfoData
-{
- char strategy; /* hash, list or range? */
- int ndatums; /* Length of the datums
following array */
- Datum **datums;
- PartitionRangeDatumKind **kind; /* The kind of each range bound datum;
- * NULL
for hash and list partitioned
- *
tables */
- int *indexes; /* Partition indexes */
- int null_index; /* Index of the
null-accepting partition; -1
- * if there
isn't one */
- int default_index; /* Index of the default
partition; -1 if there
- * isn't one */
-} PartitionBoundInfoData;
-
-typedef struct PartitionBoundInfoData *PartitionBoundInfo;
-
-#define partition_bound_accepts_nulls(bi) ((bi)->null_index != -1)
-#define partition_bound_has_default(bi) ((bi)->default_index != -1)
-
-/*
* Information about partitions of a partitioned table.
*/
typedef struct PartitionDescData
@@ -93,28 +34,6 @@ typedef struct PartitionDescData
typedef struct PartitionDescData *PartitionDesc;
-/*
- * PartitionPruneContext
- *
- * Information about a partitioned table needed to perform partition pruning.
- */
-typedef struct PartitionPruneContext
-{
- /* Partition key information */
- char strategy;
- int partnatts;
- Oid *partopfamily;
- Oid *partopcintype;
- Oid *partcollation;
- FmgrInfo *partsupfunc;
-
- /* Number of partitions */
- int nparts;
-
- /* Partition boundary info */
- PartitionBoundInfo boundinfo;
-} PartitionPruneContext;
-
extern void RelationBuildPartitionDesc(Relation relation);
extern bool partition_bounds_equal(int partnatts, int16 *parttyplen,
bool *parttypbyval,
PartitionBoundInfo b1,
diff --git a/src/include/catalog/partition_internal.h
b/src/include/catalog/partition_internal.h
index b21d422e75..e3b9123a2e 100644
--- a/src/include/catalog/partition_internal.h
+++ b/src/include/catalog/partition_internal.h
@@ -11,6 +11,9 @@
#ifndef PARTITION_INTERNAL_H
#define PARTITION_INTERNAL_H
+#include "fmgr.h"
+#include "nodes/parsenodes.h"
+
/*
* When qsort'ing partition bounds after reading from the catalog, each bound
* is represented with one of the following structs.
@@ -40,6 +43,63 @@ typedef struct PartitionRangeBound
bool lower; /* this is the lower (vs upper)
bound */
} PartitionRangeBound;
+/*
+ * PartitionBoundInfoData encapsulates a set of partition bounds. It is
+ * usually associated with partitioned tables as part of its partition
+ * descriptor, but may also be used to represent a virtual partitioned
+ * table such as a partitioned joinrel within the planner.
+ *
+ * A list partition datum that is known to be NULL is never put into the
+ * datums array. Instead, it is tracked using the null_index field.
+ *
+ * In the case of range partitioning, ndatums will typically be far less than
+ * 2 * nparts, because a partition's upper bound and the next partition's lower
+ * bound are the same in most common cases, and we only store one of them (the
+ * upper bound). In case of hash partitioning, ndatums will be same as the
+ * number of partitions.
+ *
+ * For range and list partitioned tables, datums is an array of datum-tuples
+ * with key->partnatts datums each. For hash partitioned tables, it is an
array
+ * of datum-tuples with 2 datums, modulus and remainder, corresponding to a
+ * given partition.
+ *
+ * The datums in datums array are arranged in increasing order as defined by
+ * functions qsort_partition_rbound_cmp(), qsort_partition_list_value_cmp() and
+ * qsort_partition_hbound_cmp() for range, list and hash partitioned tables
+ * respectively. For range and list partitions this simply means that the
+ * datums in the datums array are arranged in increasing order as defined by
+ * the partition key's operator classes and collations.
+ *
+ * In the case of list partitioning, the indexes array stores one entry for
+ * every datum, which is the index of the partition that accepts a given datum.
+ * In case of range partitioning, it stores one entry per distinct range
+ * datum, which is the index of the partition for which a given datum
+ * is an upper bound. In the case of hash partitioning, the number of the
+ * entries in the indexes array is same as the greatest modulus amongst all
+ * partitions. For a given partition key datum-tuple, the index of the
+ * partition which would accept that datum-tuple would be given by the entry
+ * pointed by remainder produced when hash value of the datum-tuple is divided
+ * by the greatest modulus.
+ */
+typedef struct PartitionBoundInfoData
+{
+ char strategy; /* hash, list or range? */
+ int ndatums; /* Length of the datums
following array */
+ Datum **datums;
+ PartitionRangeDatumKind **kind; /* The kind of each range bound datum;
+ * NULL
for hash and list partitioned
+ *
tables */
+ int *indexes; /* Partition indexes */
+ int null_index; /* Index of the
null-accepting partition; -1
+ * if there
isn't one */
+ int default_index; /* Index of the default
partition; -1 if there
+ * isn't one */
+} PartitionBoundInfoData;
+
+typedef struct PartitionBoundInfoData *PartitionBoundInfo;
+
+#define partition_bound_accepts_nulls(bi) ((bi)->null_index != -1)
+#define partition_bound_has_default(bi) ((bi)->default_index != -1)
extern int get_hash_partition_greatest_modulus(PartitionBoundInfo b);
extern int partition_list_bsearch(FmgrInfo *partsupfunc, Oid *partcollation,
diff --git a/src/include/optimizer/partprune.h
b/src/include/optimizer/partprune.h
index 027016b32c..093ade24c3 100644
--- a/src/include/optimizer/partprune.h
+++ b/src/include/optimizer/partprune.h
@@ -14,8 +14,31 @@
#ifndef PARTPRUNE_H
#define PARTPRUNE_H
+#include "catalog/partition_internal.h"
#include "nodes/relation.h"
+/*
+ * PartitionPruneContext
+ *
+ * Information about a partitioned table needed to perform partition pruning.
+ */
+typedef struct PartitionPruneContext
+{
+ /* Partition key information */
+ char strategy;
+ int partnatts;
+ Oid *partopfamily;
+ Oid *partopcintype;
+ Oid *partcollation;
+ FmgrInfo *partsupfunc;
+
+ /* Number of partitions */
+ int nparts;
+
+ /* Partition boundary info */
+ PartitionBoundInfo boundinfo;
+} PartitionPruneContext;
+
extern Relids prune_append_rel_partitions(RelOptInfo *rel);
#endif /* PARTPRUNE_H */
