Re: [HACKERS] Runtime Partition Pruning

Hello Robert,
Thank you for the comments. I have started working on it.

On Fri, Dec 8, 2017 at 9:27 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 2:22 AM, Beena Emerson <memissemerson@gmail.com> wrote:
>> I have added the partition quals that are used for pruning.
>>
>> PFA the updated patch. I have changed the names of variables to make
>> it more appropriate, along with adding more code comments and doing
>> some refactoring and other code cleanups.
>
> - initClauses() seems to be a duplicate of the existing function
> ExecInitExprList(), except for the extra NULL test, which isn't
> necessary.

The initClauses has been removed and ExecInitExprList has been used.

> - The executor already has a system for making sure that relations get
> opened and locked, and it's different from the new one scheme which
> set_append_subplan_indexes() implements. Relations should be locked
> during the executor initialization phase (i.e. ExecInitAppend) and not
> when the first tuple is requested (i.e. ExecAppend).  Also, there's
> already code to lock both child relations (i.e. the scans of those
> relations, see InitScanRelation, ExecInitIndexScan) and non-leaf
> partitions (ExecLockNonLeafAppendTables).   The call to
> find_all_inheritors() will lock all of that same stuff again *plus*
> the leaf partitions that were pruned during planning - moreover, if
> the Append is rescanned, we'll walk the partitioning structure again
> for every rescan.  I think RelationGetPartitionDispatchInfo should be
> called directly from ExecInitAppend after the existing code to take
> locks has been called, and store a pointer to the PartitionDispatch
> object in the AppendState for future use.

I have moved the call to ExecInitAppend. This still uses the previous
locking method, I will work on it in the next version of the patch.


> - I am surprised that set_append_subplan_indexes() needs to worry
> about multi-level partitioning directly.  I would have thought that
> Amit's patch would take care of that, just returning a Bitmapset of
> indexes which this function could use directly.  It also doesn't seem
> like a very good idea to convert the Bitmapset (subplans) into a list
> of integers (node->subplan_indexes), as set_append_subplan_indexes()
> does at the bottom.  The Bitmapset will be a lot more efficient; we
> should be able to just iterate over that directly rather than
> converting it into a List.  Note that a Bitmapset can be created with
> a single palloc, but an integer list needs one per list element plus
> one for the list itself.

The function get_partitions_from_clauses returns the Bitmap set of
partitions for a level of partition. So when the BitmapSet that
indicates a child partitioned table, set_append_subplan_indexes loops
throgh again till it gets the list of all leaf indexes.

I am working on the other comments and will post the patch along with
rebasing to v14 of Amit's patch soon.


-- 

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company