Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Petr Jelinek <petr(at)2ndquadrant(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgreSQL(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Subject: Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)
Date: 2015-01-11 07:56:53
Message-ID: CADyhKSXkAth3ZvXKFmfj7abxZjTEAAmCsa9tonY6HXDNEEBVfg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2015-01-11 10:40 GMT+09:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:
> On 1/9/15, 8:51 PM, Kohei KaiGai wrote:
>>
>> 2015-01-10 9:56 GMT+09:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:
>>>
>>> On 1/9/15, 6:54 PM, Jim Nasby wrote:
>>>>
>>>>
>>>> On 1/9/15, 6:44 PM, Petr Jelinek wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Yep, I had a same impression when I looked at the code first time,
>>>>>> however, it is defined as below. Not a manner of custom-scan itself.
>>>>>>
>>>>>> /*
>>>>>> * ==========
>>>>>> * Scan nodes
>>>>>> * ==========
>>>>>> */
>>>>>> typedef struct Scan
>>>>>> {
>>>>>> Plan plan;
>>>>>> Index scanrelid; /* relid is index into the range
>>>>>> table
>>>>>> */
>>>>>> } Scan;
>>>>>>
>>>>>
>>>>> Yeah there are actually several places in the code where "relid" means
>>>>> index in range table and not oid of relation, it still manages to
>>>>> confuse
>>>>> me. Nothing this patch can do about that.
>>>>
>>>>
>>>>
>>>> Well, since it's confused 3 of us now... should we change it (as a
>>>> separate patch)? I'm willing to do that work but don't want to waste
>>>> time if
>>>> it'll just be rejected.
>>>>
>>>> Any other examples of this I should fix too?
>>>
>>>
>>>
>>> Sorry, to clarify... any other items besides Scan.scanrelid that I should
>>> fix?
>>>
>> This naming is a little bit confusing, however, I don't think it "should"
>> be
>> changed because this structure has been used for a long time, so reworking
>> will prevent back-patching when we find bugs around "scanrelid".
>
>
> We can still backpatch; it just requires more work. And how many bugs do we
> actually expect to find around this anyway?
>
> If folks think this just isn't worth fixing fine, but I find the
> backpatching argument rather dubious.
>
Even though here is no problem around Scan structure itself, a bugfix may
use the variable name of "scanrelid" to fix it. If we renamed it on v9.5, we
also need a little adjustment to apply this bugfix on prior versions.
It seems to me a waste of time for committers.
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-01-11 08:36:15 Re: Transactions involving multiple postgres foreign servers
Previous Message Peter Geoghegan 2015-01-11 04:32:47 INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0