On 2018/08/09 0:48, Tom Lane wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 8 August 2018 at 17:28, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> Attached is a patch which modifies the if test to compare relids instead
>>> of RelOptInfo pointers.
>
>> Thanks for investigating and writing a patch. I agree with the fix.
>
> I changed this to compare the relid sets not just rel->relid, since
> rel->relid is only reliable for baserels. The partitioned rel could
> safely be assumed to be a baserel, but I'm less comfortable with
> supposing that the parentrel always will be. Otherwise, added a
> test case based on Rushabh's example and pushed. (I'm not quite
> sure if the plan will be stable enough to satisfy the buildfarm,
> but we'll soon find out ...)
Thank you for committing, agreed that comparing relid sets for equality
might be more future-proof.
About the test case, wondering if we should, like David seemed to suggest,
add a test case that would actually use run-time pruning? Maybe, even
better if the new test also had partitioned parent under UNION ALL parent
under ModifyTable. Something like in the attached?
One reason why we should adapt such a test case is that, in the future, we
may arrange for make_partitionedrel_pruneinfo(), whose code we just fixed,
to not be called if we know that run-time pruning is not needed. It seems
that that's true for the test added by the commit, that is, it doesn't
need run-time pruning.
Regards,
Amit
