Re: Allowing join removals for more join types

Поиск
Список
Период
Сортировка
От David Rowley
Тема Re: Allowing join removals for more join types
Дата
Msg-id CAApHDvq+ebv=uaXZYebuPoVM+St7JxT7LKqdf86RVEkDORtJoA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Allowing join removals for more join types  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Allowing join removals for more join types  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
On Tue, Jul 8, 2014 at 4:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:
> On Mon, Jul 7, 2014 at 4:15 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I poked around to see if we didn't have some code already for that, and
>> soon found that not only do we have such code (equality_ops_are_compatible)
>> but actually almost this entire patch duplicates logic that already exists
>> in optimizer/util/pathnode.c, to wit create_unique_path's subroutines
>> query_is_distinct_for et al.  So I'm thinking what this needs to turn into
>> is an exercise in refactoring to allow that logic to be used for both
>> purposes.

> Well, it seems that might just reduce the patch size a little!
> I currently have this half hacked up to use query_is_distinct_for, but I
> see there's no code that allows Const's to exist in the join condition. I
> had allowed for this in groupinglist_is_unique_on_restrictinfo() and I
> tested it worked in a regression test (which now fails). I think to fix
> this, all it would take would be to modify query_is_distinct_for to take a
> list of Node's rather than a list of column numbers then just add some
> logic that skips if it's a Const and checks it as it does now if it's a Var
> Would you see a change of this kind a valid refactor for this patch?

I'm a bit skeptical as to whether testing for that case is actually worth
any extra complexity.  Do you have a compelling use-case?  But anyway,
if we do want to allow it, why does it take any more than adding a check
for Consts to the loops in query_is_distinct_for?  It's the targetlist
entries where we'd want to allow Consts, not the join conditions.


I don't really have a compelling use-case, but you're right, it's just a Const check in query_is_distinct_for(), it seems simple enough so I've included that in my refactor of the patch to use query_is_distinct_for(). This allows the regression tests all to pass again.

I've included an updated patch and a delta patch.

Now a couple of things to note:

1. The fast path code that exited in join_is_removable() for subquery's when the subquery had no group or distinct clause is now gone. I wasn't too sure that I wanted to assume too much about what query_is_distinct_for may do in the future and I thought if I included some logic in join_is_removable() to fast path, that one day it may fast path wrongly. Perhaps we could protect against this with a small note in query_is_distinct_for().

2. The patch I submitted here http://www.postgresql.org/message-id/CAApHDvrfVkH0P3FAooGcckBy7feCJ9QFanKLkX7MWsBcxY2Vcg@mail.gmail.com if that gets accepted then it makes the check for set returning functions in join_is_removable void. 

Regards

David Rowley

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: David Rowley
Дата:
Сообщение: query_is_distinct_for does not take into account set returning functions
Следующее
От: Ali Akbar
Дата:
Сообщение: Re: [REVIEW] Re: Fix xpath() to return namespace definitions