On Thu, Jul 11, 2019 at 8:49 AM Thomas Munro <thomas.mu...@gmail.com> wrote:
> <snip>
>
> Here are some comments on superficial aspects of the patch:
>
> +/* Custom partition child access hook. Provides further partition pruning
> given
> + * child OID.
> + */
>
> Should be like:
>
> /*
> * Multi-line comment...
> */
Fixed in attached patch.
>
> Why "child"? Don't you really mean "Partition pruning hook. Provides
> custom pruning given partition OID." or something?
>
> +typedef bool (*partitionChildAccess_hook_type) (Oid childOID);
> +PGDLLIMPORT partitionChildAccess_hook_type partitionChildAccess_hook;
>
> Hmm, I wonder if this could better evoke the job that it's doing...
> partition_filter_hook?
> partition_access_filter_hook? partition_prune_hook?
Ended up going with partition_prune_hook. Good call.
>
> +/* Macro to use partitionChildAccess_hook. Handles NULL-checking. */
>
> It's not a macro, it's a function.
Copy-pasta. Fixed.
>
> +static inline bool InvokePartitionChildAccessHook (Oid childOID)
> +{
> + if (partitionChildAccess_hook && enable_partition_pruning && childOID)
> + {
>
> Normally we write OidIsValid(childOID) rather than comparing with 0.
> I wonder if you should call the variable relId? Single line if
> branches don't usually get curly braces.
Fixed.
>
> + return (*partitionChildAccess_hook) (childOID);
>
> The syntax we usually use for calling function pointers is just
> partitionChildAccess_hook(childOID).
Fixed.
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 7a2494c8829df95dc75d037cf54000d350c25d62 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto <mike.palmio...@crunchydata.com>
Date: Mon, 8 Jul 2019 12:46:21 +0000
Subject: [PATCH] Flexible partition pruning hook
This hook allows partition pruning to be performed for entire partitions which
are deemed inaccessible by RLS policy prior to those relations being opened on
disk.
---
src/backend/catalog/pg_inherits.c | 5 +++++
src/backend/optimizer/path/allpaths.c | 7 +++++++
src/include/partitioning/partprune.h | 17 +++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 00f7957..644cd59 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -25,6 +25,7 @@
#include "catalog/indexing.h"
#include "catalog/pg_inherits.h"
#include "parser/parse_type.h"
+#include "partitioning/partprune.h"
#include "storage/lmgr.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -92,6 +93,10 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
while ((inheritsTuple = systable_getnext(scan)) != NULL)
{
inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid;
+
+ if (!InvokePartitionPruneHook(inhrelid))
+ continue;
+
if (numoids >= maxoids)
{
maxoids *= 2;
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index b772348..b45e086 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1038,6 +1038,13 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
continue;
}
+ if (!InvokePartitionPruneHook(childRTE->relid))
+ {
+ /* Implement custom partition pruning filter*/
+ set_dummy_rel_pathlist(childrel);
+ continue;
+ }
+
/*
* Constraint exclusion failed, so copy the parent's join quals and
* targetlist to the child, with appropriate variable substitutions.
diff --git a/src/include/partitioning/partprune.h b/src/include/partitioning/partprune.h
index b3e9268..c3e30f9 100644
--- a/src/include/partitioning/partprune.h
+++ b/src/include/partitioning/partprune.h
@@ -15,6 +15,7 @@
#define PARTPRUNE_H
#include "nodes/execnodes.h"
+#include "optimizer/cost.h"
#include "partitioning/partdefs.h"
struct PlannerInfo; /* avoid including pathnodes.h here */
@@ -68,6 +69,22 @@ typedef struct PartitionPruneContext
#define PruneCxtStateIdx(partnatts, step_id, keyno) \
((partnatts) * (step_id) + (keyno))
+/*
+ * Custom partition pruning hook. Provides further partition pruning given
+ * partition OID.
+ */
+typedef bool (*partition_prune_hook_type) (Oid relid);
+PGDLLIMPORT partition_prune_hook_type partition_prune_hook;
+
+/* Inline function for calling partition_prune_hook with NULL-checking. */
+static inline bool InvokePartitionPruneHook (Oid relid)
+{
+ if (partition_prune_hook && enable_partition_pruning && OidIsValid(relid))
+ return partition_prune_hook(relid);
+
+ return true;
+}
+
extern PartitionPruneInfo *make_partition_pruneinfo(struct PlannerInfo *root,
struct RelOptInfo *parentrel,
List *subpaths,
--
1.8.3.1
