On Wed, Feb 12, 2014 at 2:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello,
>
>> Please find new version of patch attached with this mail containing
>> above changes.
>
> This patch applies cleanly on current HEAD and build completed
> successfully on both Windows and Linux. (but master needed to be
> rewinded to some time ago for some compile errors.)
>
> This works correctly as before.
>
> Finally before send to commiters, would you mind changing the
> name of the segment "Global/PostgreSQL.%u" as
> 'Global/PostgreSQL.dsm.%u" or something mentioned in the last my
> email?
Actually in that case, to maintain consistency we need to change even in
function dsm_impl_posix() which uses segment name as "/PostgreSQL.%u".
For windows, we have added "Global" to "/PostgreSQL.%u", as it is
mandate by windows spec.
To be honest, I see no harm in changing the name as per your suggestion,
as it can improve segment naming for dynamic shared memory segments,
however there is no clear problem with current name as well, so I don't
want to change in places this patch has no relation.
I think best thing to do here is to put it as Notes To Committer, something
like:
Some suggestions for Committer to consider:
"Change the name of dsm segments from .. to .."
In general, what I see is that they consider all discussion in thread, but
putting some special notes like above will reduce the chance of getting
overlooked by them. I have done as a reviewer previously and it worked
well.
> However, it is a bit different thing from this patch so
> I have no intention to compel to do the changing.
Thanks to you for understanding my position.
Thanks for reviewing the patch so carefully, especially Windows part
which I think was bit tricky for you to setup.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com