Hi Amit , On 19 January 2018 at 04:03, David Rowley <david.row...@2ndquadrant.com> wrote: > On 18 January 2018 at 23:56, Amit Langote <langote_amit...@lab.ntt.co.jp> > wrote: >> So, I've been assuming that the planner changes in the run-time pruning >> patch have to do with selecting clauses (restriction clauses not >> containing Consts and/or join clauses) to be passed to the executor by >> recording them in the Append node. Will they be selected by the planner >> calling into partition.c? > > I had thought so. I only have a rough idea in my head and that's that > the PartitionPruneInfo struct that I wrote for the run-time pruning > patch would have the clause List replaced with this new > PartScanClauseInfo struct (which likely means it needs to go into > primnodes.h), this struct would contain all the partition pruning > clauses in a more structured form so that no matching of quals to the > partition key wouldn't be required during execution. The idea is that > we'd just need to call; remove_redundant_clauses, > extract_bounding_datums and get_partitions_for_keys. I think this is > the bare minimum of work that can be done in execution since we can't > remove the redundant clauses until we know the values of the Params. > > Likely this means there will need to be 2 functions externally > accessible for this in partition.c. I'm not sure exactly how best to > do this. Maybe it's fine just to have allpaths.c call > extract_partition_key_clauses to generate the PartScanClauseInfo then > call some version of get_partitions_from_clauses which does do the > extract_partition_key_clauses duties. This is made more complex by the > code that handles adding the default partition bound to the quals. I'm > not yet sure where that should live. > > I've also been thinking of having some sort of PartitionPruneContext > struct that we can pass around the functions. Runtime pruning had to > add structs which store the Param values to various functions which I > didn't like. It would be good to just add those to the context and > have them passed down without having to bloat the parameters in the > functions. I might try and do that tomorrow too. This should make the > footprint of the runtime pruning patch is a bit smaller.
Attached is what I had in mind about how to do this. Only the planner will need to call populate_partition_clause_info. The planner and executor can call get_partitions_from_clauseinfo. I'll just need to change the run-time prune patch to pass the PartitionClauseInfo into the executor in the Append node. I've also added the context struct that I mentioned above. It's currently not carrying much weight, but the idea is that this context will be passed around a bit more with the run-time pruning patch and it will also carry the details about parameter values. I'll need to modify a few signatures of functions like partkey_datum_from_expr to pass the context there too. I didn't do that here because currently, those functions have no use for the context with the fields that they currently have. I've also fixed a bug where when you built the commutator OpExpr in what's now called extract_partition_key_clauses the inputcollid was not being properly set. The changes I made there go much further than just that, I've completely removed the OpExpr from the PartClause struct as only two members were ever used. I thought it was better just to add those to PartClause instead. I also did some renaming of variables that always assumed that the Expr being compared to the partition key was a constant. This is true now, but with run-time pruning patch, it won't be, so I thought it was better to do this here rather than having to rename them in the run-time pruning patch. One thing I don't yet understand about the patch is the use of predicate_refuted_by() in get_partitions_from_or_clause_args(). I did adjust the comment above that code, but I'm still not certain I fully understand why that function has to be used instead of building the clauses for the OR being processed along with the remaining clauses. Is it that this was too hard to extract that you ended up using predicate_refuted_by()? I've also removed the makeBoolExpr call that you were encapsulating the or_clauses in. I didn't really see the need for this since you just removed it again when looping over the or_clauses. The only other changes are just streamlining code and comment changes. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
faster_partition_prune_v21_delta_drowley_v1.patch
Description: Binary data
