Re: [RFC] [PATCH] Flexible "partition pruning" hook

Sorry for jumping into this thread pretty late. I have read the
messages on this thread a couple of times and...

On Fri, Jul 12, 2019 at 3:05 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
> I'm on Peter's side. Unlike other similar hooks, this hook is
> provided just to introduce arbitrary inconsistency in partition
> pruning.

...I have questions about the patch as proposed, such as what Peter
and Horiguchi-san might.

Why does this hook need to be specific to partitioning, that is, why
is this a "partition pruning" hook? IOW, why does the functionality of
this hook apply only to partitions as opposed to *any* table? I may
be missing something, but the two things -- a table's partition
constraint and security properties (or any properties that the
proposed hook's suppliers will provide) -- seem unrelated, so making
this specific to partitioning seems odd. If this was proposed as
applying to any table, then it might be easier to choose the point
from which hook will be called and there will be fewer such points to
consider. Needless to say, since the planner sees partitions as
tables, the partitioning case will be covered too.

Looking at the patch, it seems that the hook will be called *after*
opening the partition (provided it survived partition pruning); I'm
looking at this hunk:

@@ -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;
+ }

set_append_rel_size() is called *after* partitions are opened and
their RelOptInfos formed. So it's not following the design you
intended whereby the hook will avoid uselessly opening partition
files.

Also, I suspect you don't want to call the hook from here:

@@ -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;
+

...as you might see unintended consequences, because
find_inheritance_children() is called from many places that wouldn't
want arbitrary extension code to drop some children from the list. For
example, it is called when adding a column to a parent table to find
the children that will need to get the same column added. You
wouldn't want some children getting the column added while others not.

Thanks,
Amit