On Fri, Aug 8, 2014 at 1:19 PM, Amit Kapila <[email protected]> wrote:
> On Thu, Aug 7, 2014 at 12:36 PM, Fujii Masao <[email protected]> wrote:
>> On Thu, Aug 7, 2014 at 12:36 PM, Amit Kapila <[email protected]>
>> wrote:
>> > On Wed, Aug 6, 2014 at 1:32 PM, Fujii Masao <[email protected]>
>> > wrote:
>
>> >> What about picking up only data_directory at the first pass?
>> >
>> > I think that will workout and for that I think we might need to add
>> > few checks during ProcessConfigFile. Do you want to address it
>> > during parsing of config file or during setting of values?
>>
>> I prefer "during paring". The attached patch does that. In this patch,
>> the first call of ProcessConfigFile() picks up only data_directory. One
>> concern of this patch is that the logic is a bit ugly.
>
> I think handling 'data_directory' in ParseConfigFp() looks bit out of
> place as this API is used to parse other config files as well like
> recovery.conf. I agree that for all other paths DataDir might be
> already set due to which this new path will never be executed, still
> from code maintenance point of view it would have been better if we
> can find a way to handle it in a place where we are processing
> the server's main config file (postgresql.conf).
Yep, right. ParseConfigFp() is not good place to pick up data_directory.
What about the attached patch which makes ProcessConfigFile() instead of
ParseConfigFp() pick up data_directory just after the configuration file is
parsed?
Regards,
--
Fujii Masao
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
***************
*** 162,167 **** ProcessConfigFile(GucContext context)
--- 162,209 ----
}
/*
+ * Pick up only the data_directory if DataDir is not set, which
+ * means that the configuration file is read for the first time and
+ * PG_AUTOCONF_FILENAME file cannot be read yet. In this case,
+ * we should not pick up all the settings except the data_directory
+ * from postgresql.conf yet because they might be overwritten
+ * with the settings in PG_AUTOCONF_FILENAME file which will be
+ * read later. OTOH, since it's ensured that data_directory doesn't
+ * exist in PG_AUTOCONF_FILENAME file, it will never be overwritten
+ * later. Now only the data_directory needs to be picked up to
+ * read PG_AUTOCONF_FILENAME file later.
+ */
+ if (DataDir == NULL)
+ {
+ ConfigVariable *prev = NULL;
+
+ for (item = head; item;)
+ {
+ ConfigVariable *ptr = item;
+
+ item = item->next;
+ if (strcmp(ptr->name, "data_directory") != 0)
+ {
+ if (prev == NULL)
+ head = ptr->next;
+ else
+ {
+ prev->next = ptr->next;
+ /*
+ * On removing last item in list, we need to update tail
+ * to ensure that list will be maintianed.
+ */
+ if (prev->next == NULL)
+ tail = prev;
+ }
+ pfree(ptr);
+ }
+ else
+ prev = ptr;
+ }
+ }
+
+ /*
* Mark all extant GUC variables as not present in the config file.
* We need this so that we can tell below which ones have been removed
* from the file since we last processed it.
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 4342,4347 **** SelectConfigFiles(const char *userDoption, const char *progname)
--- 4342,4354 ----
return false;
}
+ /*
+ * Read the configuration file for the first time. This time only
+ * data_directory parameter is picked up to determine the data directory
+ * so that we can read PG_AUTOCONF_FILENAME file next time. Then don't
+ * forget to read the configuration file again later to pick up all the
+ * parameters.
+ */
ProcessConfigFile(PGC_POSTMASTER);
/*
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
