[PBCKP-874] Addeded default unit output for config-show

- config-show command now has --default-units parameter
- If it is provided, all time and memory
- configuration options will be printed in their default units
- test added, test_help_1 corrected

Author: Oleg Gurev

src/configure.c

@@ -269,18 +269,21 @@ static const char *current_group = NULL;
  * Show configure options including default values.
  */
 void
-do_show_config(void)
+do_show_config(bool show_default_units)
 {
 	int			i;
 
 	show_configure_start();
 
 	for (i = 0; instance_options[i].type; i++)
 	{
+		if (show_default_units && strchr("bBiIuU", instance_options[i].type) && instance_options[i].get_value == *option_get_value)
+			instance_options[i].flags |= GET_VAL_IN_DEFAULT_UNITS;	/* Set flag */
 		if (show_format == SHOW_PLAIN)
 			show_configure_plain(&instance_options[i]);
 		else
 			show_configure_json(&instance_options[i]);
+		instance_options[i].flags &= ~(GET_VAL_IN_DEFAULT_UNITS); /* Reset flag. It was resetted in option_get_value(). Probably this reset isn't needed */
 	}
 
 	show_configure_end();
@@ -801,6 +804,6 @@ show_configure_json(ConfigOption *opt)
 		return;
 
 	json_add_value(&show_buf, opt->lname, value, json_level,
-				   true);
+				   !(opt->flags & GET_VAL_IN_DEFAULT_UNITS));
 	pfree(value);
 }

src/help.c

@@ -121,6 +121,7 @@ help_pg_probackup(void)
 
 	printf(_("\n  %s show-config -B backup-path --instance=instance_name\n"), PROGRAM_NAME);
 	printf(_("                 [--format=format]\n"));
+	printf(_("                 [--default-units]\n"));
 	printf(_("                 [--help]\n"));
 
 	printf(_("\n  %s backup -B backup-path -b backup-mode --instance=instance_name\n"), PROGRAM_NAME);
@@ -953,7 +954,8 @@ help_show_config(void)
 
 	printf(_("  -B, --backup-path=backup-path    location of the backup storage area\n"));
 	printf(_("      --instance=instance_name     name of the instance\n"));
-	printf(_("      --format=format              show format=PLAIN|JSON\n\n"));
+	printf(_("      --format=format              show format=PLAIN|JSON\n"));
+	printf(_("      --default-units              show memory and time values in default units\n\n"));
 }
 
 static void

src/pg_probackup.c

@@ -164,6 +164,7 @@ bool no_validate_wal = false;
 /* show options */
 ShowFormat show_format = SHOW_PLAIN;
 bool show_archive = false;
+static bool show_default_units = false;
 
 /* set-backup options */
 int64 ttl = -1;
@@ -275,6 +276,8 @@ static ConfigOption cmd_options[] =
 	/* show options */
 	{ 'f', 165, "format",			opt_show_format,	SOURCE_CMD_STRICT },
 	{ 'b', 166, "archive",			&show_archive,		SOURCE_CMD_STRICT },
+	/* show-config options */
+	{ 'b', 167, "default-units",	&show_default_units,SOURCE_CMD_STRICT },
 	/* set-backup options */
 	{ 'I', 170, "ttl", &ttl, SOURCE_CMD_STRICT, SOURCE_DEFAULT, 0, OPTION_UNIT_S, option_get_value},
 	{ 's', 171, "expire-time",		&expire_time_string,	SOURCE_CMD_STRICT },
@@ -1049,7 +1052,7 @@ main(int argc, char *argv[])
 			do_merge(instanceState, current.backup_id, no_validate, no_sync);
 			break;
 		case SHOW_CONFIG_CMD:
-			do_show_config();
+			do_show_config(show_default_units);
 			break;
 		case SET_CONFIG_CMD:
 			do_set_config(instanceState, false);

src/pg_probackup.h

@@ -939,7 +939,7 @@ extern void do_archive_get(InstanceState *instanceState, InstanceConfig *instanc
 						   char *wal_file_name, int batch_size, bool validate_wal);
 
 /* in configure.c */
-extern void do_show_config(void);
+extern void do_show_config(bool show_default_units);
 extern void do_set_config(InstanceState *instanceState, bool missing_ok);
 extern void init_config(InstanceConfig *config, const char *instance_name);
 extern InstanceConfig *readInstanceConfigFile(InstanceState *instanceState);

src/utils/configuration.c

@@ -678,6 +678,8 @@ config_set_opt(ConfigOption options[], void *var, OptionSource source)
 /*
  * Return value of the function in the string representation. Result is
  * allocated string.
+ * We can set  GET_VAL_IN_DEFAULT_UNITS flag in opt->flags
+ * before call option_get_value() to get option value in default units
  */
 char *
 option_get_value(ConfigOption *opt)
@@ -692,20 +694,33 @@ option_get_value(ConfigOption *opt)
 	 */
 	if (opt->flags & OPTION_UNIT)
 	{
-		if (opt->type == 'i')
-			convert_from_base_unit(*((int32 *) opt->var),
-								   opt->flags & OPTION_UNIT, &value, &unit);
-		else if (opt->type == 'I')
-			convert_from_base_unit(*((int64 *) opt->var),
-								   opt->flags & OPTION_UNIT, &value, &unit);
-		else if (opt->type == 'u')
-			convert_from_base_unit_u(*((uint32 *) opt->var),
-									 opt->flags & OPTION_UNIT, &value_u, &unit);
-		else if (opt->type == 'U')
-			convert_from_base_unit_u(*((uint64 *) opt->var),
-									 opt->flags & OPTION_UNIT, &value_u, &unit);
+		if (opt->flags & GET_VAL_IN_DEFAULT_UNITS){
+			if (opt->type == 'i')
+				value = *((int32 *) opt->var);
+			else if (opt->type == 'I')
+				value = *((int64 *) opt->var);
+			else if (opt->type == 'u')
+				value_u = *((uint32 *) opt->var);
+			else if (opt->type == 'U')
+				value_u = *((uint64 *) opt->var);
+			unit = "";
+		}
+		else
+		{
+			if (opt->type == 'i')
+				convert_from_base_unit(*((int32 *) opt->var),
+									opt->flags & OPTION_UNIT, &value, &unit);
+			else if (opt->type == 'I')
+				convert_from_base_unit(*((int64 *) opt->var),
+									opt->flags & OPTION_UNIT, &value, &unit);
+			else if (opt->type == 'u')
+				convert_from_base_unit_u(*((uint32 *) opt->var),
+										opt->flags & OPTION_UNIT, &value_u, &unit);
+			else if (opt->type == 'U')
+				convert_from_base_unit_u(*((uint64 *) opt->var),
+										opt->flags & OPTION_UNIT, &value_u, &unit);
+		}
 	}
-
 	/* Get string representation itself */
 	switch (opt->type)
 	{

src/utils/configuration.h

@@ -100,6 +100,7 @@ struct ConfigOption
 #define OPTION_UNIT_TIME			0xF0000	/* mask for time-related units */
 
 #define OPTION_UNIT					(OPTION_UNIT_MEMORY | OPTION_UNIT_TIME)
+#define GET_VAL_IN_DEFAULT_UNITS 	0x80000000	/* bitflag to get memory and time values in default units*/
 
 extern ProbackupSubcmd parse_subcmd(char const * const subcmd_str);
 extern char const *get_subcmd_name(ProbackupSubcmd const subcmd);

tests/expected/option_help.out

@@ -39,6 +39,7 @@ pg_probackup - utility to manage backup/recovery of PostgreSQL database.
 
   pg_probackup show-config -B backup-path --instance=instance_name
                  [--format=format]
+                 [--default-units]
                  [--help]
 
   pg_probackup backup -B backup-path -b backup-mode --instance=instance_name

tests/expected/option_help_ru.out

@@ -39,6 +39,7 @@ pg_probackup - утилита для управления резервным к
 
   pg_probackup show-config -B backup-path --instance=instance_name
                  [--format=format]
+                 [--default-units]
                  [--help]
 
   pg_probackup backup -B backup-path -b backup-mode --instance=instance_name

tests/option_test.py

@@ -230,6 +230,25 @@ def test_help_6(self):
             self.skipTest(
                 'You need configure PostgreSQL with --enabled-nls option for this test')
 
+    # @unittest.skip("skip")
+    def test_options_default_units(self):
+        """check --default-units option"""
+        backup_dir =  os.path.join(self.tmp_path, self.module_name, self.fname, 'backup')
+        node = self.make_simple_node(
+            base_dir=os.path.join(self.module_name, self.fname, 'node'))
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        # check that --default-units option works correctly
+        output = self.run_pb(["show-config", "--backup-path", backup_dir, "--instance=node"])
+        self.assertIn(container=output, member="archive-timeout = 5min")
+        output = self.run_pb(["show-config", "--backup-path", backup_dir, "--instance=node", "--default-units"])
+        self.assertIn(container=output, member="archive-timeout = 300")
+        self.assertNotIn(container=output, member="archive-timeout = 300s")
+        # check that we have now quotes ("") in json output
+        output = self.run_pb(["show-config", "--backup-path", backup_dir, "--instance=node", "--default-units", "--format=json"])
+        self.assertIn(container=output, member='"archive-timeout": 300,')
+        self.assertIn(container=output, member='"retention-redundancy": 0,')
+        self.assertNotIn(container=output, member='"archive-timeout": "300",')
 
 def check_locale(locale_name):
    ret=True