On Thu, Jun 25, 2015 at 11:55 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 6:25 PM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
>> I have a serious open item reported 1.5 month ago then reminded
>> several times has not been fixed up yet.
>>
>> 9A28C8860F777E439AA12E8AEA7694F8010F3EA6@BPXM15GP.gisp.nec.co.jp
>>
>> Patch is less than 100 lines, entirely designed according to Tom's suggestion.
>>
>> The problem is, commit 1a8a4e5cde2b7755e11bde2ea7897bd650622d3e reverted
>> create_plan_recurse() to static function, thus, extension lost way to
>> transform Path node to Plan node if it wants to takes underlying child
>> nodes, like SeqScan, HashJoin and so on.
>>
>> Tom's suggestion is to add a list of Path nodes on CustomPath structure,
>> to be transformed by createplan.c, instead of public create_plan_recurse().
>>
>> It is nearly obvious problem, and bugfix patch already exists.
>
> Yes, I am quite unhappy with this situation. Tom promised me at PGCon
> that he would look at this soon, but there is no sign that he has, and
> he said the same thing weeks ago. I think it can't be right to let
> this sit for another month or three. Even if the API you've
> implemented is worse than something Tom can design, it is certainly
> better than the status quo. I would rather have a working but
> imperfect API and have to break compatibility later in beta than have
> a non-working API.
...given which, I have committed this. I did not like the use of
custom_children as a generic monicker, so I changed it to
custom_paths, custom_plans, or custom_ps, as appropriate in each case.
I did a bit of cosmetic cleanup as well.
This does seem much nicer than giving custom plans direct access to
create_plan_recurse(). The fact that you found various other places
of using those lists demonstrates that nicely.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
