On Thursday, March 07, 2013 10:54 AM Greg Smith wrote:
> On 3/5/13 9:07 AM, Amit Kapila wrote:
> > In v11 patch, I have changed name of directory to config.
> > For file name, currently I have not changed, but if you feel it needs
> to be
> > changed, kindly suggest any one of above or if any other better you
> have in
> > mind.
>
> This seems fine for now. Hashing out exactly which name is best is not
> going to be the thing that determines whether this can be committed or
> not.
>
> Your v11 applies cleanly for me too, and the code does look a lot
> nicer.
> The code diff itself is down to 1300 lines and not nearly as
> disruptive, the rest is docs or the many regression tests you've added.
I also think the tests added for regression may be more than required.
Currently it is organized as below:
1. 3 type of parameters to use with set persistent. a. parameters for which new connection is required
(log_connections) b. parameters for which server restart is required c. parameters for which reload is required
I think in third category "c", there are many parameters, I think may be
3-4 parameters should be sufficient. The initial intention to keep more parameters is to have test it for
various type(Boolean, integer, float, string)
2. Set some of the parameters to default, then try reload and reconnect.
3. Again set those parameters to some value and re-verify by reload and
reconnect.
4. Test for set persistent command inside functions and transaction.
5. Set all parameters value to default.
I think tests for step-3 can be removed and tests for step-5 can be merged
to step-2.
If you think above optimization's to reduce number of tests are okay, then I
will
update the patch.
> That's still not the sort of small change Tom was hoping this was
> possible, but it's closer to the right range. It may not really be
> possible to make this much smaller.
>
> I'm out of time for today, but I'll try to do some functional review
> work on this tomorrow if no one beats me to it. I would find it very
> useful to me personally if this feature were committed, and we know
> there's plenty of user demand for it too.
With Regards,
Amit Kapila.
