From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: updated join removal patch |
Date: | 2009-09-18 17:48:02 |
Message-ID: | 603c8f070909181048jac93afaw71a8734d30011466@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Sep 18, 2009 at 1:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Thu, Sep 17, 2009 at 4:59 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>> You're the committer; I'm not. But I completely disagree. There
>>>> isn't any reason at all to duplicate this logic in two separate
>>>> places, let alone three. I'd actually be in favor of merging the
>>>> existing two cases even if we weren't adding join removal.
>>>
>>> No, I still think this was a bad idea. There are *parts* of those
>>> tests that are similar, but combining them all into one function is
>>> just a recipe for bugs.
>
>> Having read your commit, it makes more sense to me. The fact that
>> we're now looking at innerrel->baserestrictinfo also is a pretty
>> powerful argument for your way.
>
> Looking at it some more, I think that there is some value in factoring
> out the tests to see if the clause has the right variable membership,
> so I did that.
Mmm, I like that. Putting that bunch of hairy logic in a subroutine
instead of repeating it in several places definitely seems better. I
don't really like the name "clause_matches_join", though. It's more
like "clause has well-defined sides, and mark which is which as a
side-effect".
...Robert
From | Date | Subject | |
---|---|---|---|
Next Message | Emmanuel Cecchet | 2009-09-18 17:54:23 | Re: generic copy options |
Previous Message | Dan Colish | 2009-09-18 17:39:55 | Re: generic copy options |