Обсуждение: get_loop_count() fails to ignore RELOPT_DEADREL rels
I've just been hacking away a bit more at the WIP patch that I posted a while back which allows join removals for SEMI and ANTI joins that could be proved useless due to the existence of a foreign key which matched the join condition (here http://www.postgresql.org/message-id/CAApHDvq0NAi8cEqTNNdqG6mhFH__7_A6Tn9XU4V0cut9wab4gA@mail.gmail.com)
On testing I found that the Assert(outer_rel->rows > 0) was failing in get_loop_count(). The relation which it was failing on was one that I had declared dead in remove_useless_joins(). I've not done very much work so far in the costing part of the planner, however I see that set_rel_size() seems to be in charge of dishing out row estimates in this case and that it naturally does nothing for rels marked with RELOPT_DEADREL.
I've not yet determined if I can exploit this with the existing join removal code, but I can give it a try if required.
In order to get my patch working with an Assert enabled build I've had to apply the attached patch.
Regards
David Rowley
Вложения
On Sat, Jul 26, 2014 at 9:11 PM, David Rowley <dgrowleyml@gmail.com> wrote:
In order to get my patch working with an Assert enabled build I've had to apply the attached patch.
Actually I meant to attach this patch instead.
Regards
David Rowley
Вложения
David Rowley <dgrowleyml@gmail.com> writes:
> In order to get my patch working with an Assert enabled build I've had to
> apply the attached patch.
That patch is entirely bogus. What you should be asking is why
get_loop_count is being applied to a relation that's supposedly been
removed from the query. It should only get applied to rels that are
required outer rels for a parameterized path, and thus certainly
not dead.
regards, tom lane
On Sun, Jul 27, 2014 at 2:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:That patch is entirely bogus. What you should be asking is why
> In order to get my patch working with an Assert enabled build I've had to
> apply the attached patch.
get_loop_count is being applied to a relation that's supposedly been
removed from the query. It should only get applied to rels that are
required outer rels for a parameterized path, and thus certainly
not dead.
hmm ok. After further investigation it seems that this is down to the EquivalenceClass still containing references to the dead rel. What seems to be happening is match_eclass_clauses_to_index() is calling generate_implied_equalities_for_column() which is generating RestrictInfo clauses which make reference to the dead relation.
With the existing left join removal code, in all the test cases I've thrown at it, match_eclass_clauses_to_index() seems to early exit on if (!index->rel->has_eclass_joins), but this is not the case when I'm removing SEMI and ANTI joins. So likely this is something I'll have to tackle in that patch, perhaps by stripping out the equivalence class members which belong to the newly dead rel in remove_rel_from_query().
Apologies for the noise
Regards
David Rowley
David Rowley <dgrowleyml@gmail.com> writes:
> On Sun, Jul 27, 2014 at 2:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That patch is entirely bogus. What you should be asking is why
>> get_loop_count is being applied to a relation that's supposedly been
>> removed from the query.
> hmm ok. After further investigation it seems that this is down to the
> EquivalenceClass still containing references to the dead rel. What seems to
> be happening is match_eclass_clauses_to_index() is calling
> generate_implied_equalities_for_column() which is generating RestrictInfo
> clauses which make reference to the dead relation.
Hm. Maybe we need to improve the join removal code so that it cleans out
the eclasses better? But, as you say, it's not clear why it's not failing
for the existing left-join cases if that's the problem. You should
probably spend a bit of time to understand exactly how the difference is
arising.
regards, tom lane