On Fri, Sep 18, 2009 at 1:26 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> On Thu, Sep 17, 2009 at 4:59 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Robert Haas <robertmh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
