On Tue, Sep 20, 2022 at 4:53 PM David Rowley <dgrowleyml@gmail.com> wrote:
> Thank you for having a look at the patch.
>
> On Tue, 20 Sept 2022 at 18:41, Amit Langote <amitlangote09@gmail.com> wrote:
> > Agreed, though the patch's changes to tests does not seem to have to
> > do with join removal? I don't really understand what the test changes
> > are all about. I wonder why the patch doesn't instead add the test
> > case that Arne showed in the file he attached with [1].
> > [1] https://www.postgresql.org/message-id/2641568c18de40e8b1528fc9d4d80127%40index.de
>
> I adjusted a test in partition_join.sql to add an additional column to
> the fract_t table. Before the change that table only had a single
> column and due to the query's join condition being USING(id), none of
> the columns from the left joined table were being used. That resulted
> in the updated code performing a left join removal as it was passing
> the checks for no columns being used in the left joined table in
> analyzejoins.c. The test in partition_join.sql claims to be testing
> "partitionwise join with fractional paths", so I thought we'd better
> not have a query that the planner removes the join when we're meant to
> be testing joins.
Ah, got it, thanks for the explanation.
> It probably wouldn't hurt to have a new test to ensure left join
> removals work with a partitioned table. That should go in join.sql
> along with the other join removal tests.
Makes sense.
> I didn't study Arne's patch
> to see what test he added. I was only interested in writing enough
> code so I could check there was no good reason not to add the
> partitioned index into RelOptInfo.indexlist. Arne sent me an off-list
> message to say he's planning on working on the patch that uses the
> existing field instead of the new one he originally added. Let's hold
> off for that patch.
Ok, sure.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
