Hi,

Please find attached an updated patch (context diffs) improving the
comments related to ALTER SYSTEM. This patch does nothing for the
suffix tmp/temp used in a couple of places of the code, it only
corrects some typos and makes the comments more consistent with
current code.

The inconsistencies with the temporary file suffixes should be tackled
in a separate patch/thread.
Thanks,
-- 
Michael
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
***************
*** 149,155 **** ProcessConfigFile(GucContext context)
        }
  
        /*
!        * Parse postgresql.auto.conf file after postgresql.conf to replace
         * parameters set by ALTER SYSTEM command. This file is present in
         * data directory, however when called during initdb data directory is 
not
         * set till this point, so use ConfigFile path which will be same.
--- 149,155 ----
        }
  
        /*
!        * Parse file PG_AUTOCONF_FILENAME after postgresql.conf to replace
         * parameters set by ALTER SYSTEM command. This file is present in
         * data directory, however when called during initdb data directory is 
not
         * set till this point, so use ConfigFile path which will be same.
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 6457,6465 **** flatten_set_variable_args(const char *name, List *args)
  }
  
  /*
!  * Writes updated configuration parameter values into
!  * postgresql.auto.conf.temp file. It traverses the list of parameters
!  * and quote the string values before writing them to temporaray file.
   */
  static void
  write_auto_conf_file(int fd, const char *filename, ConfigVariable **head_p)
--- 6457,6465 ----
  }
  
  /*
!  * Write updated configuration parameter values into a temporary file.
!  * this function traverses the list of parameters and quotes the string
!  * values before writing them.
   */
  static void
  write_auto_conf_file(int fd, const char *filename, ConfigVariable **head_p)
***************
*** 6468,6475 **** write_auto_conf_file(int fd, const char *filename, 
ConfigVariable **head_p)
        StringInfoData buf;
  
        initStringInfo(&buf);
!       appendStringInfoString(&buf, "# Do not edit this file manually! \n");
!       appendStringInfoString(&buf, "# It will be overwritten by ALTER SYSTEM 
command. \n");
  
        /*
         * write the file header message before contents, so that if there is no
--- 6468,6475 ----
        StringInfoData buf;
  
        initStringInfo(&buf);
!       appendStringInfoString(&buf, "# Do not edit this file manually!\n");
!       appendStringInfoString(&buf, "# It will be overwritten by ALTER SYSTEM 
command.\n");
  
        /*
         * write the file header message before contents, so that if there is no
***************
*** 6517,6523 **** write_auto_conf_file(int fd, const char *filename, 
ConfigVariable **head_p)
  
  /*
   * This function takes list of all configuration parameters in
!  * postgresql.auto.conf and parameter to be updated as input arguments and
   * replace the updated configuration parameter value in a list. If the
   * parameter to be updated is new then it is appended to the list of
   * parameters.
--- 6517,6523 ----
  
  /*
   * This function takes list of all configuration parameters in
!  * PG_AUTOCONF_FILENAME and parameter to be updated as input arguments and
   * replace the updated configuration parameter value in a list. If the
   * parameter to be updated is new then it is appended to the list of
   * parameters.
***************
*** 6595,6607 **** replace_auto_config_value(ConfigVariable **head_p, 
ConfigVariable **tail_p,
   * and write them all to the automatic configuration file.
   *
   * The configuration parameters are written to a temporary
!  * file then renamed to the final name. The template for the
!  * temporary file is postgresql.auto.conf.temp.
   *
   * An LWLock is used to serialize writing to the same file.
   *
   * In case of an error, we leave the original automatic
!  * configuration file (postgresql.auto.conf) intact.
   */
  void
  AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
--- 6595,6606 ----
   * and write them all to the automatic configuration file.
   *
   * The configuration parameters are written to a temporary
!  * file then renamed to the final name.
   *
   * An LWLock is used to serialize writing to the same file.
   *
   * In case of an error, we leave the original automatic
!  * configuration file (PG_AUTOCONF_FILENAME) intact.
   */
  void
  AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
***************
*** 6664,6671 **** AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
  
  
        /*
!        * Use data directory as reference path for postgresql.auto.conf and 
it's
!        * corresponding temp file
         */
        join_path_components(AutoConfFileName, data_directory, 
PG_AUTOCONF_FILENAME);
        canonicalize_path(AutoConfFileName);
--- 6663,6670 ----
  
  
        /*
!        * Use data directory as reference path for PG_AUTOCONF_FILENAME and its
!        * corresponding temporary file.
         */
        join_path_components(AutoConfFileName, data_directory, 
PG_AUTOCONF_FILENAME);
        canonicalize_path(AutoConfFileName);
***************
*** 6674,6683 **** AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
                         "temp");
  
        /*
!        * one backend is allowed to operate on postgresql.auto.conf file, to
         * ensure that we need to update the contents of the file with
         * AutoFileLock. To ensure crash safety, first the contents are written 
to
!        * temporary file and then rename it to postgresql.auto.conf. In case
         * there exists a temp file from previous crash, that can be reused.
         */
  
--- 6673,6682 ----
                         "temp");
  
        /*
!        * One backend is allowed to operate on file PG_AUTOCONF_FILENAME, to
         * ensure that we need to update the contents of the file with
         * AutoFileLock. To ensure crash safety, first the contents are written 
to
!        * a temporary file which is then renameed to PG_AUTOCONF_FILENAME. In 
case
         * there exists a temp file from previous crash, that can be reused.
         */
  
***************
*** 6694,6707 **** AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
        {
                if (stat(AutoConfFileName, &st) == 0)
                {
!                       /* open postgresql.auto.conf file */
                        infile = AllocateFile(AutoConfFileName, "r");
                        if (infile == NULL)
                                ereport(ERROR,
                                                (errmsg("failed to open auto 
conf file \"%s\": %m ",
                                                                
AutoConfFileName)));
  
!                       /* Parse the postgresql.auto.conf file */
                        ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, 
&tail);
  
                        FreeFile(infile);
--- 6693,6706 ----
        {
                if (stat(AutoConfFileName, &st) == 0)
                {
!                       /* open file PG_AUTOCONF_FILENAME */
                        infile = AllocateFile(AutoConfFileName, "r");
                        if (infile == NULL)
                                ereport(ERROR,
                                                (errmsg("failed to open auto 
conf file \"%s\": %m ",
                                                                
AutoConfFileName)));
  
!                       /* parse it */
                        ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, 
&tail);
  
                        FreeFile(infile);
***************
*** 6713,6719 **** AlterSystemSetConfigFile(AlterSystemStmt * altersysstmt)
                 */
                replace_auto_config_value(&head, &tail, AutoConfFileName, name, 
value);
  
!               /* Write and sync the New contents to postgresql.auto.conf.temp 
file */
                write_auto_conf_file(Tmpfd, AutoConfTmpFileName, &head);
  
                close(Tmpfd);
--- 6712,6718 ----
                 */
                replace_auto_config_value(&head, &tail, AutoConfFileName, name, 
value);
  
!               /* Write and sync the new contents to the temporary file */
                write_auto_conf_file(Tmpfd, AutoConfTmpFileName, &head);
  
                close(Tmpfd);
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
***************
*** 1300,1307 **** setup_config(void)
         * file will override the value of parameters that exists before parse 
of
         * this file.
         */
!       autoconflines[0] = pg_strdup("# Do not edit this file manually! \n");
!       autoconflines[1] = pg_strdup("# It will be overwritten by the ALTER 
SYSTEM command. \n");
        autoconflines[2] = NULL;
  
        sprintf(path, "%s/%s", pg_data, PG_AUTOCONF_FILENAME);
--- 1300,1307 ----
         * file will override the value of parameters that exists before parse 
of
         * this file.
         */
!       autoconflines[0] = pg_strdup("# Do not edit this file manually!\n");
!       autoconflines[1] = pg_strdup("# It will be overwritten by the ALTER 
SYSTEM command.\n");
        autoconflines[2] = NULL;
  
        sprintf(path, "%s/%s", pg_data, PG_AUTOCONF_FILENAME);
*** a/src/include/pg_config_manual.h
--- b/src/include/pg_config_manual.h
***************
*** 304,309 ****
  /*
   * Automatic configuration file name for ALTER SYSTEM.
   * This file will be used to store values of configuration parameters
!  * set by ALTER SYSTEM command
   */
  #define PG_AUTOCONF_FILENAME          "postgresql.auto.conf"
--- 304,309 ----
  /*
   * Automatic configuration file name for ALTER SYSTEM.
   * This file will be used to store values of configuration parameters
!  * set by ALTER SYSTEM command.
   */
  #define PG_AUTOCONF_FILENAME          "postgresql.auto.conf"
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to