On Tue, Nov 5, 2013 at 3:20 PM, Rajeev rastogi
<rajeev.rastogi@huawei.com> wrote:
> On execution of pg_resetxlog using the option -n
>
> 1. It will display values in two section.
>
> 2. First section will be called as "Current pg_control
> values or Guess pg_control values".
>
> 3. In first section, it will display all current (i.e.
> before change) values of control file or guessed values.
>
> 4. Second section will be called as "Values to be used after
> reset".
>
> 5. In second section, it will display new values of
> parameters to be reset as per user request.
>
>
>
> Please provide your opinion or expectation out of this patch.
Your approach in patch seems to be inline with Todo item. On a quick
glance, I observed few things which can make your patch better:
1. The purpose was to print pg_control values in one section and any
other reset values in different section, so in that regard, should we display below in new section, as here
newXlogSegNo is not directly from pg_control.
PrintControlValues()
{
..
XLogFileName(fname, ControlFile.checkPointCopy.ThisTimeLineID, newXlogSegNo);
printf(_("First log segment after reset: %s\n"), fname);
}
2. why to have extra flags for changedParam, can't we do without it.
For example, we already use set_xid value to check if user has provided xid.
3.
+ static void
+ PrintNewControlValues(int changedParam)
+ {
+ if (changedParam)
+ printf(_("\n\nValues to be used after reset:\n\n"));
Even after first if check fails, still you continue to check other
values, it is better if you can have check if (changedParam) before
calling this function
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
