From: | Beena Emerson <memissemerson(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, amul sul <sulamul(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Subject: | Re: [HACKERS] Runtime Partition Pruning |
Date: | 2017-12-12 11:27:10 |
Message-ID: | CAOG9ApHfkE3Empvn4b1oAVJwaeJormAAL70EPQLU6z3RJM8ECA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(at)gmail(dot)com> wrote:
> On Thu, Dec 7, 2017 at 2:22 AM, Beena Emerson <memissemerson(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Beena Emerson | 2017-12-12 11:33:29 | Re: [HACKERS] Runtime Partition Pruning |
Previous Message | Teodor Sigaev | 2017-12-12 11:21:06 | Re: Fwd: [BUGS] pg_trgm word_similarity inconsistencies or bug |