Re: [HACKERS] get_loop_count() fails to ignore RELOPT_DEADREL rels

2014-07-27 Thread Tom Lane
David Rowley  writes:
> On Sun, Jul 27, 2014 at 2:35 AM, Tom Lane  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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] get_loop_count() fails to ignore RELOPT_DEADREL rels

2014-07-27 Thread David Rowley
On Sun, Jul 27, 2014 at 2:35 AM, Tom Lane  wrote:

> David Rowley  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.
>
>
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


Re: [HACKERS] get_loop_count() fails to ignore RELOPT_DEADREL rels

2014-07-26 Thread Tom Lane
David Rowley  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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] get_loop_count() fails to ignore RELOPT_DEADREL rels

2014-07-26 Thread David Rowley
On Sat, Jul 26, 2014 at 9:11 PM, David Rowley  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


get_loop_count_ignore_dead_rels_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] get_loop_count() fails to ignore RELOPT_DEADREL rels

2014-07-26 Thread David Rowley
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_a6tn9xu4v0cut9wab...@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


get_loop_count_ignore_dead_rels.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers