Re: [PATCHES] Tablespace for temporary objects and sort files

["Jaime Casanova" <systemguards(at)gmail(dot)com>]

On 1/13/07, Albert Cervera Areny <albertca(at)hotpop(dot)com> wrote:
> It was already possible to set the guc on postgresql.conf when I posted the
> patch...

ok... fixed... the problem was that this code only let
num_temp_tablespaces be greater than zero when we are in an
interactive command (eg. a SET command) but setting the guc from
postgresql.conf at startup time is not interactive so
num_temp_tablespaces is zero and when i try to get the first temp
tablespace to use (MyProcPid % num_temp_tablespaces) causes a floatin
exception (division by zero).

+   if (source >= PGC_S_INTERACTIVE && IsTransactionState())
+   {
+       /*
+        * Verify that all the names are valid tablspace names
+        * We do not check for USAGE rights should we?
+        */
+       foreach(l, namelist)
+       {
+           char       *curname = (char *) lfirst(l);
+
+           if (get_tablespace_oid(curname) == InvalidOid)
+               ereport((source == PGC_S_TEST) ? NOTICE : ERROR,
+                       (errcode(ERRCODE_UNDEFINED_OBJECT),
+                       errmsg("tablespace \"%s\" does not exist", curname)));
+
+           num_temp_tablespaces++;
+       }
+   }


new patch added, with that piece of code refactored to let
num_temp_tablespaces get a value greater than zero always that the guc
is setted, i also add some docs.

the patch passes all 104 regression tests and all my tests as well...

i think the patch is ready to be applied to HEAD, any committer want
to review it?

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
                                      Richard Cook

Index: doc/src/sgml/config.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.101
diff -c -B -b -r1.101 config.sgml
*** doc/src/sgml/config.sgml	9 Jan 2007 22:16:46 -0000	1.101
--- doc/src/sgml/config.sgml	15 Jan 2007 04:02:13 -0000
***************
*** 3398,3403 ****
--- 3398,3432 ----
        </listitem>
       </varlistentry>
  
+      <varlistentry id="guc-temp-tablespaces" xreflabel="temp_tablespaces">
+       <term><varname>temp_tablespaces</varname> (<type>string</type>)</term>
+       <indexterm>
+        <primary><varname>temp_tablespaces</> configuration parameter</primary>
+       </indexterm>
+       <indexterm><primary>tablespace</><secondary>temp</></>
+       <listitem>
+        <para>
+         This variable specifies tablespaces in which to create temp
+         objects (temp tables and indexes on temp tables) when a 
+ 		<command>CREATE</> command does not explicitly specify a tablespace 
+ 		and temp files when necessary (eg. for sorting operations).
+        </para>
+ 
+        <para>
+         The value is either a list of names of tablespaces, or an empty 
+ 		string to specify using the default tablespace of the current database.
+         If the value does not match the name of any existing tablespace,
+         <productname>PostgreSQL</> will automatically use the default
+         tablespace of the current database.
+        </para>
+ 
+        <para>
+         For more information on tablespaces,
+         see <xref linkend="manage-ag-tablespaces">.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
       <varlistentry id="guc-check-function-bodies" xreflabel="check_function_bodies">
        <term><varname>check_function_bodies</varname> (<type>boolean</type>)</term>
        <indexterm>
Index: src/backend/commands/indexcmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.152
diff -c -B -b -r1.152 indexcmds.c
*** src/backend/commands/indexcmds.c	9 Jan 2007 02:14:11 -0000	1.152
--- src/backend/commands/indexcmds.c	15 Jan 2007 04:02:17 -0000
***************
*** 209,215 ****
--- 209,221 ----
  	}
  	else
  	{
+ 		/*
+ 		 * if the target table is temporary then use a temp_tablespace
+ 		 */
+ 		if (!rel->rd_istemp)
  			tablespaceId = GetDefaultTablespace();
+ 		else
+ 			tablespaceId = GetTempTablespace();
  		/* note InvalidOid is OK in this case */
  	}
  
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.210
diff -c -B -b -r1.210 tablecmds.c
*** src/backend/commands/tablecmds.c	5 Jan 2007 22:19:26 -0000	1.210
--- src/backend/commands/tablecmds.c	15 Jan 2007 04:02:28 -0000
***************
*** 334,339 ****
--- 334,343 ----
  					 errmsg("tablespace \"%s\" does not exist",
  							stmt->tablespacename)));
  	}
+ 	else if (stmt->relation->istemp)
+ 	{
+ 		tablespaceId = GetTempTablespace();
+ 	}
  	else
  	{
  		tablespaceId = GetDefaultTablespace();
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.40
diff -c -B -b -r1.40 tablespace.c
*** src/backend/commands/tablespace.c	5 Jan 2007 22:19:26 -0000	1.40
--- src/backend/commands/tablespace.c	15 Jan 2007 04:02:30 -0000
***************
*** 65,73 ****
  #include "utils/lsyscache.h"
  
  
! /* GUC variable */
  char	   *default_tablespace = NULL;
  
  
  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
  static void set_short_version(const char *path);
--- 65,76 ----
  #include "utils/lsyscache.h"
  
  
! /* GUC variables */
  char	   *default_tablespace = NULL;
+ char       *temp_tablespaces = NULL;
  
+ int	   next_temp_tablespace;
+ int	   num_temp_tablespaces;
  
  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
  static void set_short_version(const char *path);
***************
*** 930,935 ****
--- 933,1074 ----
  	return result;
  }
  
+ /*
+  * Routines for handling the GUC variable 'temp_tablespaces'.
+  */
+ 
+ /* assign_hook: validate new temp_tablespaces, do extra actions as needed */
+ const char *
+ assign_temp_tablespaces(const char *newval, bool doit, GucSource source)
+ {
+ 	char	   *rawname;
+ 	List	   *namelist;
+ 	ListCell   *l;
+ 
+ 	/* Need a modifiable copy of string */
+ 	rawname = pstrdup(newval);
+ 
+ 	/* Parse string into list of identifiers */
+ 	if (!SplitIdentifierString(rawname, ',', &namelist))
+ 	{
+ 		/* syntax error in name list */
+ 		pfree(rawname);
+ 		list_free(namelist);
+ 		return NULL;
+ 	}
+ 
+ 	num_temp_tablespaces = 0;
+ 	foreach(l, namelist)
+ 	{
+ 		char	   *curname = (char *) lfirst(l);
+ 
+ 		/*
+ 		 * If we aren't inside a transaction, we cannot do database access so
+ 		 * cannot verify the individual names.	Must accept the list on faith.
+ 		 */
+ 		if (source >= PGC_S_INTERACTIVE && IsTransactionState())
+ 		{
+ 			/*
+ 			 * Verify that all the names are valid tablspace names 
+ 			 * We do not check for USAGE rights should we?
+ 			 */
+ 			if (get_tablespace_oid(curname) == InvalidOid)
+ 				ereport((source == PGC_S_TEST) ? NOTICE : ERROR,
+ 						(errcode(ERRCODE_UNDEFINED_OBJECT),
+ 						errmsg("tablespace \"%s\" does not exist", curname)));
+ 		}
+ 		num_temp_tablespaces++;
+ 	}
+ 
+ 	/*
+ 	 * Select the first tablespace to use
+ 	 */
+ 	next_temp_tablespace = MyProcPid % num_temp_tablespaces;
+ 
+ 	pfree(rawname);
+ 	list_free(namelist);
+ 	return newval;
+ }
+ 
+ /*
+  * GetTempTablespace -- get the OID of the tablespace for temporary objects
+  *
+  * May return InvalidOid to indicate "use the database's default tablespace"
+  *
+  * This exists to hide the temp_tablespace GUC variable.
+  */
+ Oid
+ GetTempTablespace(void)
+ {
+ 	Oid			result;
+ 	char *curname = NULL;
+ 	char *rawname;
+ 	List *namelist;
+ 	ListCell *l;
+ 	int i = 0;
+ 	
+ 	if ( temp_tablespaces == NULL )
+ 		return InvalidOid;
+ 
+ 	/* Need a modifiable version of temp_tablespaces */
+ 	rawname = pstrdup(temp_tablespaces);
+ 
+ 	/* Parse string into list of identifiers */
+ 	if (!SplitIdentifierString(rawname, ',', &namelist))
+ 	{
+ 		/* syntax error in name list */
+ 		pfree(rawname);
+ 		list_free(namelist);
+ 		return InvalidOid;
+ 	}
+ 
+ 	/* 
+ 	 * Iterate through the list of namespaces until the one we need 
+ 	 * (next_temp_tablespace) 
+ 	 */
+ 	foreach(l, namelist)
+ 	{
+ 		curname = (char *) lfirst(l);
+ 		if ( i == next_temp_tablespace )
+ 			break;
+ 		i++;
+ 	}
+ 
+ 
+ 	/* Prepare for the next time the function is called */
+ 	next_temp_tablespace++;
+ 	if (next_temp_tablespace == num_temp_tablespaces)
+ 		next_temp_tablespace = 0;
+ 
+ 	/* Fast path for temp_tablespaces == "" */
+ 	if ( curname == NULL || curname[0] == '\0') {
+ 		list_free(namelist);
+ 		pfree(rawname);
+ 		return InvalidOid;
+ 	}
+ 
+ 	/*
+ 	 * It is tempting to cache this lookup for more speed, but then we would
+ 	 * fail to detect the case where the tablespace was dropped since the GUC
+ 	 * variable was set.  Note also that we don't complain if the value fails
+ 	 * to refer to an existing tablespace; we just silently return InvalidOid,
+ 	 * causing the new object to be created in the database's tablespace.
+ 	 */
+ 	result = get_tablespace_oid(curname);
+ 
+ 	/* We don't free rawname before because curname points to a part of it */
+ 	pfree(rawname);
+ 
+ 	/*
+ 	 * Allow explicit specification of database's default tablespace in
+ 	 * default_tablespace without triggering permissions checks.
+ 	 */
+ 	if (result == MyDatabaseTableSpace)
+ 		result = InvalidOid;
+ 	
+ 	list_free(namelist);
+ 	return result;
+ }
  
  /*
   * get_tablespace_oid - given a tablespace name, look up the OID
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.283
diff -c -B -b -r1.283 execMain.c
*** src/backend/executor/execMain.c	5 Jan 2007 22:19:27 -0000	1.283
--- src/backend/executor/execMain.c	15 Jan 2007 04:02:34 -0000
***************
*** 2409,2414 ****
--- 2409,2418 ----
  					 errmsg("tablespace \"%s\" does not exist",
  							parseTree->intoTableSpaceName)));
  	}
+ 	else if (parseTree->into->istemp)
+ 	{
+ 		tablespaceId = GetTempTablespace();
+ 	}
  	else
  	{
  		tablespaceId = GetDefaultTablespace();
Index: src/backend/storage/file/fd.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/file/fd.c,v
retrieving revision 1.134
diff -c -B -b -r1.134 fd.c
*** src/backend/storage/file/fd.c	9 Jan 2007 22:03:51 -0000	1.134
--- src/backend/storage/file/fd.c	15 Jan 2007 04:02:38 -0000
***************
*** 46,51 ****
--- 46,53 ----
  #include <unistd.h>
  #include <fcntl.h>
  
+ #include "commands/tablespace.h"
+ 
  #include "miscadmin.h"
  #include "access/xact.h"
  #include "storage/fd.h"
***************
*** 76,81 ****
--- 78,84 ----
   */
  #define FD_MINFREE				10
  
+ #define OIDCHARS        10                      /* max chars printed by %u */
  
  /*
   * A number of platforms allow individual processes to open many more files
***************
*** 880,892 ****
  {
  	char		tempfilepath[MAXPGPATH];
  	File		file;
  
  	/*
  	 * Generate a tempfile name that should be unique within the current
  	 * database instance.
  	 */
  	snprintf(tempfilepath, sizeof(tempfilepath),
! 			 "%s/%s%d.%ld", PG_TEMP_FILES_DIR, PG_TEMP_FILE_PREFIX,
  			 MyProcPid, tempFileCounter++);
  
  	/*
--- 883,933 ----
  {
  	char		tempfilepath[MAXPGPATH];
  	File		file;
+ 	Oid		oid;
+ 	char		*path;
+ 	int		pathlen;
+ 
+ 	/*
+ 	 * Take a look what should be the path of the temporary file
+ 	 */
+ 	oid = GetTempTablespace();
+ 	if ( oid != InvalidOid )
+ 	{
+ 		/*
+ 		 * As we got a valid tablespace, try to create the
+ 		 * file there
+ 		 */
+ 
+ 		pathlen = 10 + OIDCHARS + 1;
+ 		path = (char *) palloc(pathlen);
+ 		snprintf(path, pathlen, "pg_tblspc/%u", oid );
  
  		/*
  		 * Generate a tempfile name that should be unique within the current
  		 * database instance.
  		 */
  		snprintf(tempfilepath, sizeof(tempfilepath),
! 				 "%s/%s%d.%ld", path, PG_TEMP_FILE_PREFIX,
! 				 MyProcPid, tempFileCounter++);
! 		pfree(path);
! 		file = PathNameOpenFile(tempfilepath,
! 							O_RDWR | O_CREAT | O_TRUNC | PG_BINARY,
! 							0600);
! 	}
! 
! 	/*
! 	 * Create a normal temporary file if no tablespace returned or
! 	 * couldn't create the file in the tablespace "oid"
! 	 */
! 	if (oid == InvalidOid || file <= 0) 
! 	{
! 		path = PG_TEMP_FILES_DIR;
! 		/*
! 		 * Generate a tempfile name that should be unique within the current
! 		 * database instance.
! 		 */
! 		snprintf(tempfilepath, sizeof(tempfilepath),
! 				 "%s/%s%d.%ld", path, PG_TEMP_FILE_PREFIX,
  				 MyProcPid, tempFileCounter++);
  
  		/*
***************
*** 919,924 ****
--- 960,967 ----
  				elog(ERROR, "could not create temporary file \"%s\": %m",
  					 tempfilepath);
  		}
+ 	}
+ 
  
  	/* Mark it for deletion at close */
  	VfdCache[file].fdstate |= FD_TEMPORARY;
***************
*** 1292,1297 ****
--- 1335,1354 ----
  		errno = save_errno;
  	}
  
+ 	/*
+ 	 * TEMPORARY hack to log the Windows error code on fopen failures, in
+ 	 * hopes of diagnosing some hard-to-reproduce problems.
+ 	 */
+ #ifdef WIN32
+ 	{
+ 		int			save_errno = errno;
+ 
+ 		elog(LOG, "Windows fopen(\"%s\",\"%s\") failed: code %lu, errno %d",
+ 			 name, mode, GetLastError(), save_errno);
+ 		errno = save_errno;
+ 	}
+ #endif
+ 
  	return NULL;
  }
  
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.367
diff -c -B -b -r1.367 guc.c
*** src/backend/utils/misc/guc.c	9 Jan 2007 22:16:46 -0000	1.367
--- src/backend/utils/misc/guc.c	15 Jan 2007 04:02:48 -0000
***************
*** 98,103 ****
--- 98,104 ----
  extern int	CommitDelay;
  extern int	CommitSiblings;
  extern char *default_tablespace;
+ extern char *temp_tablespaces;
  extern bool fullPageWrites;
  
  #ifdef TRACE_SORT
***************
*** 2279,2284 ****
--- 2280,2295 ----
  		NULL, assign_canonical_path, NULL
  	},
  
+ 	{
+ 		{"temp_tablespaces", PGC_SUSET, PGC_S_FILE,
+ 			gettext_noop("Sets the tablespaces suitable for creating new objects and sort files."),
+ 			NULL,
+ 			GUC_LIST_INPUT | GUC_LIST_QUOTE 
+ 		},
+ 		&temp_tablespaces,
+ 		NULL, assign_temp_tablespaces, NULL
+ 	},
+ 
  	/* End-of-list marker */
  	{
  		{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.201
diff -c -B -b -r1.201 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample	9 Jan 2007 22:16:46 -0000	1.201
--- src/backend/utils/misc/postgresql.conf.sample	15 Jan 2007 04:02:49 -0000
***************
*** 396,401 ****
--- 396,404 ----
  #search_path = '"$user",public'		# schema names
  #default_tablespace = ''		# a tablespace name, '' uses
  						# the default
+ #temp_tablespaces = ''		# a list of tablespace names, 
+ 							# '' uses default_tablespace
+ 					
  #check_function_bodies = on
  #default_transaction_isolation = 'read committed'
  #default_transaction_read_only = off
Index: src/include/commands/tablespace.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/commands/tablespace.h,v
retrieving revision 1.14
diff -c -B -b -r1.14 tablespace.h
*** src/include/commands/tablespace.h	5 Jan 2007 22:19:54 -0000	1.14
--- src/include/commands/tablespace.h	15 Jan 2007 04:02:50 -0000
***************
*** 41,46 ****
--- 41,47 ----
  extern void TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo);
  
  extern Oid	GetDefaultTablespace(void);
+ extern Oid	GetTempTablespace(void);
  
  extern Oid	get_tablespace_oid(const char *tablespacename);
  extern char *get_tablespace_name(Oid spc_oid);
Index: src/include/utils/guc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.78
diff -c -B -b -r1.78 guc.h
*** src/include/utils/guc.h	9 Jan 2007 21:31:17 -0000	1.78
--- src/include/utils/guc.h	15 Jan 2007 04:02:51 -0000
***************
*** 238,241 ****
--- 238,245 ----
  extern const char *assign_xlog_sync_method(const char *method,
  						bool doit, GucSource source);
  
+ /* in commands/tablespace.c */
+ extern const char *assign_temp_tablespaces(const char *newval,
+ 						  bool doit, GucSource source);
+ 
  #endif   /* GUC_H */