Hi David. On 2018/02/01 8:57, David Rowley wrote: > On 31 January 2018 at 21:03, Amit Langote <langote_amit...@lab.ntt.co.jp> > wrote: >> Update patch set attached. Thanks again. > > (My apologies for being slow to respond here. I've been on leave this > week and I'm off again next week. I now have a little time to reply)
No worries. > Thanks for incorporating my changes into the patchset. A while ago I > was rebasing the run-time pruning patch on top of this but ran into a > few problems which are all results of my changes. > > 1. remove_redundant_clauses collapses the PartClause list into the > most restrictive set of clauses. This disallows multiple evaluations > of the PartitionClauseInfo during runtime pruning. I've written a > proposed fix for this and attached it. I've incorporated it in the latest patch I posted today. > 2. PartitionClauseInfo->keyclauses is a list of PartClause which is > not a node type. This will cause _copyPartitionClauseInfo() to fail. > > I'm still not quite sure the best way to fix #2 since PartClause > contains a FmgrInfo. I do have a local fix which moves PartClause to > primnodes.h and makes it a proper node type. I also added a copy > function which does not copy any of the cache fields in PartClause. It > just sets valid_cache to false. I didn't particularly think this was > the correct fix. I just couldn't think of how exactly this should be > done at the time. > > The attached patch also adds the missing nodetag from > PartitionClauseInfo and also fixes up other code so as we don't memset > the node memory to zero, as that would destroy the tag. I ended up > just having extract_partition_key_clauses do the makeNode call. This > also resulted in populate_partition_clauses being renamed to > generate_partition_clauses I started wondering if it's not such a good idea to make PartitionClauseInfo a Node at all? I went back to your earlier message [1] where you said that it's put into the Append node for run-time pruning to use, but it doesn't sound nice that we'd be putting into the plan something that's looks more like scratchpad for the partition.c code. I think we should try to keep PartitionClauseInfo in partition.h and put only the list of matched bare clauses into Append. Thanks, Amit [1] https://www.postgresql.org/message-id/CAKJS1f8T_efuAgPWtyGdfwD1kBLR-giFvoez7raYsQ4P1i2OYw%40mail.gmail.com
