Re: join removal

On Sun, Mar 28, 2010 at 2:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sun, Mar 28, 2010 at 12:19 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> * I left join_is_removable where it was, mainly so that it was easy to
>>> compare how much it changed for this usage (not a lot).  I'm not sure
>>> that joinpath.c is an appropriate place for it anymore, though I can't
>>> see any obviously better place either.  Any thoughts on that?
>
>> I dislike the idea of leaving it in joinpath.c.  I don't even think it
>> properly belongs in the path subdirectory since it no longer has
>> anything to do with paths.  Also worth thinking about where we would
>> put the logic I pontificated about here:
>> http://archives.postgresql.org/pgsql-hackers/2009-10/msg01012.php
>
> The only argument I can see for leaving it where it is is that it
> depends on clause_sides_match_join, which we'd have to either duplicate
> or global-ize in order to continue sharing that code.  However, since
> join_is_removable now needs a slightly different API for that anyway
> (cf changes in draft patch), it's probably better to not try to share it.
> So let's put the join removal code somewhere else.  The reasonable
> alternatives seem to be:
>
> * in a new file in prep/.  Although this clearly has the flavor of
> preprocessing, all the other work in prep/ is done before we get into
> query_planner().  So this choice seems a bit dubious.
>
> * directly in plan/planmain.c.  Has the advantage of being where the
> caller is, so no globally visible function declaration needed.  No other
> redeeming social value though.
>
> * in plan/initsplan.c.  Somewhat reasonable, although that file is
> rather large already.
>
> * in a new file in plan/.  Not sure if it's worth this, though your
> thought that we might add more logic later makes it more defensible.

I sort of like the last of these ideas though I'm at a loss for what
to call it. Otherwise I kind of like planmain.c.

...Robert