Re: [PATCHES] [HACKERS] Custom variable class segmentation

2006-09-02 Thread Bruce Momjian

Where are we on the GUC comment/reload to defaults patch?  Do we have
multiple people objecting to its application?  Previously only Tom
objected, and Andrew partially.

---

Bruce Momjian wrote:
 Tom Lane wrote:
  Michael Fuhr [EMAIL PROTECTED] writes:
   The latest HEAD is segfaulting on startup if I have the following
   lines in postgresql.conf:
  
   custom_variable_classes = 'plperl'
   plperl.use_strict = on
  
  Bruce, please re-revert that GUC patch and don't put it back in until
  someone like Peter or me has actually reviewed it.  My faith in it has
  gone to zero, and I don't think you are able to fix it either.
 
 Peter had already reviewed it and given comments.
 
 There were three things wrong with the original patch:
 
   o  spacing, e.g. if( x =- 1 )
   o  an incorrect API for memory freeing by parse_value()
   o  verify_config_option() didn't consider custom variables
 
 These have all been corrected, so I don't see the value in removing the
 patch now that it is working.  I have attached the three GUC patches
 that were applied to CVS, so if someone wants corrections or removal
 based on specific issues, please let me know.
 
  BTW, do I need to mention that the plperl patch is breaking the
  buildfarm again?
 
 That has been reverted, because of the regression failures and Andrew's
 analysis.
 
 -- 
   Bruce Momjian   [EMAIL PROTECTED]
   EnterpriseDBhttp://www.enterprisedb.com
 
   + If your life is a hard drive, Christ can be your backup. +




 
 ---(end of broadcast)---
 TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

-- 
  Bruce Momjian   [EMAIL PROTECTED]
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] [HACKERS] Custom variable class segmentation fault

2006-08-13 Thread Bruce Momjian
Tom Lane wrote:
 Michael Fuhr [EMAIL PROTECTED] writes:
  The latest HEAD is segfaulting on startup if I have the following
  lines in postgresql.conf:
 
  custom_variable_classes = 'plperl'
  plperl.use_strict = on
 
 Bruce, please re-revert that GUC patch and don't put it back in until
 someone like Peter or me has actually reviewed it.  My faith in it has
 gone to zero, and I don't think you are able to fix it either.

Peter had already reviewed it and given comments.

There were three things wrong with the original patch:

o  spacing, e.g. if( x =- 1 )
o  an incorrect API for memory freeing by parse_value()
o  verify_config_option() didn't consider custom variables

These have all been corrected, so I don't see the value in removing the
patch now that it is working.  I have attached the three GUC patches
that were applied to CVS, so if someone wants corrections or removal
based on specific issues, please let me know.

 BTW, do I need to mention that the plperl patch is breaking the
 buildfarm again?

That has been reverted, because of the regression failures and Andrew's
analysis.

-- 
  Bruce Momjian   [EMAIL PROTECTED]
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/misc/guc-file.l
===
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.42
retrieving revision 1.43
diff -c -r1.42 -r1.43
*** src/backend/utils/misc/guc-file.l	12 Aug 2006 04:12:41 -	1.42
--- src/backend/utils/misc/guc-file.l	13 Aug 2006 01:30:17 -	1.43
***
*** 4,10 
   *
   * Copyright (c) 2000-2006, PostgreSQL Global Development Group
   *
!  * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.42 2006/08/12 04:12:41 momjian Exp $
   */
  
  %{
--- 4,10 
   *
   * Copyright (c) 2000-2006, PostgreSQL Global Development Group
   *
!  * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.43 2006/08/13 01:30:17 momjian Exp $
   */
  
  %{
***
*** 50,56 
  static bool ParseConfigFile(const char *config_file, const char *calling_file,
  			int depth, GucContext context, int elevel,
  			struct name_value_pair **head_p,
! 			struct name_value_pair **tail_p);
  static void free_name_value_list(struct name_value_pair * list);
  static char *GUC_scanstr(const char *s);
  
--- 50,57 
  static bool ParseConfigFile(const char *config_file, const char *calling_file,
  			int depth, GucContext context, int elevel,
  			struct name_value_pair **head_p,
! 			struct name_value_pair **tail_p,
! 			int *varcount);
  static void free_name_value_list(struct name_value_pair * list);
  static char *GUC_scanstr(const char *s);
  
***
*** 114,121 
  void
  ProcessConfigFile(GucContext context)
  {
! 	int			elevel;
  	struct name_value_pair *item, *head, *tail;
  
  	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
  
--- 115,124 
  void
  ProcessConfigFile(GucContext context)
  {
! 	int			elevel, i;
  	struct name_value_pair *item, *head, *tail;
+ 	bool	   *apply_list = NULL;
+ 	int			varcount = 0;
  
  	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
  
***
*** 134,158 
  
  	if (!ParseConfigFile(ConfigFileName, NULL,
  		 0, context, elevel,
! 		 head, tail))
  		goto cleanup_list;
  
  	/* Check if all options are valid */
! 	for (item = head; item; item = item-next)
  	{
! 		if (!set_config_option(item-name, item-value, context,
! 			   PGC_S_FILE, false, false))
  			goto cleanup_list;
  	}
  
  	/* If we got here all the options checked out okay, so apply them. */
! 	for (item = head; item; item = item-next)
! 	{
! 		set_config_option(item-name, item-value, context,
! 		  PGC_S_FILE, false, true);
! 	}
  
!  cleanup_list:
  	free_name_value_list(head);
  }
  
--- 137,192 
  
  	if (!ParseConfigFile(ConfigFileName, NULL,
  		 0, context, elevel,
! 		 head, tail, varcount))
  		goto cleanup_list;
  
+ 	/* Can we allocate memory here, what about leaving here prematurely? */
+ 	apply_list = (bool *) palloc(sizeof(bool) * varcount);
+ 
  	/* Check if all options are valid */
! 	for (item = head, i = 0; item; item = item-next, i++)
  	{
! 		bool isEqual, isContextOk;
! 
! 		if (!verify_config_option(item-name, item-value, context,
! 		  PGC_S_FILE, isEqual, isContextOk))
! 		{
! 			ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg(configuration file is invalid)));
  			goto cleanup_list;
+ 		}
+ 
+ 		if (isContextOk == false)
+ 		{
+ 			apply_list[i] = false;
+ 			if (context == PGC_SIGHUP)
+ 			{
+ if (isEqual == false)
+ 	ereport(elevel,
+ 		(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ 		errmsg(parameter \%s\ cannot be changed after server start; configuration file change ignored,
+ 		item-name)));
+ 			}
+ 			else
+ /* if it