Hi.
On 2018/04/06 7:35, Alvaro Herrera wrote:
> I seems pretty clear that putting get_matching_partitions() in
> catalog/partition.c is totally the wrong thing; it belongs wholly in
> partprune. I think the reason you put it there is that it requires
> access to a lot of internals that are static in partition.c. In the
> attached not yet cleaned version of the patch, I have moved a whole lot
> of what you added to partition.c to partprune.c; and for the functions
> and struct declarations that were required to make it work, I created
> catalog/partition_internal.h.
Yes, I really wanted for most of the new code that this patch adds to land
in the planner, especially after Robert's comments here:
https://www.postgresql.org/message-id/CA%2BTgmoabi-29Vs8H0xkjtYB%3DcU%2BGVCrNwPz7okpa3KsoLmdEUQ%40mail.gmail.com
It would've been nice if we'd gotten the "reorganizing partitioning code"
thread resolved sooner.
> I changed a lot of code also, but cosmetic changes only.
>
> I'll clean this up a bit more now, and try to commit shortly (or early
> tomorrow); wanted to share current status now in case I have to rush
> out.
Some comments on the code reorganizing part of the patch:
* Did you intentionally not put PartitionBoundInfoData and its accessor
macros in partition_internal.h. partprune.c would not need to include
partition.h if we do that.
* Also, I wonder why you left PartitionPruneContext in partition.h. Isn't
it better taken out to partprune.h?
I have done that in the attached.
* Why isn't gen_partprune_steps() in partprune.h? I see only
prune_append_rel_partitions() exported out of partprune.c, but the runtime
patch needs gen_partprune_steps() to be called from createplan.c.
* I don't see get_matching_partitions() exported either. Runtime pruning
patch needs that too.
Maybe you've thought something about these two items though.
Thanks,
Amit