Hi,

I'm trying to introduce myself into postgresql development and I'm working on 
the "tablespace for temporary objects and sort files" TODO item. The attached 
patch shows what I've already done. The GUC is currently 
called "temp_tablespaces". 

The tablespace changes correctly for me when creating temporary tables.  I've 
got some questions though:
        
        How can I test that the tablespace is correctly used for sort files? Is 
there 
an easy way? Or should I reduce work_mem to a minimum, populate the database 
with data and try an "ORDER BY"?

        The GetTempTablespace function correctly returns a different tablespace 
each 
time is called, but I store the position of the last tablespace used with an 
integer and iterate through the list of tablespaces each time. I tried to 
keep the iterator from call to call but I got a segfault, I imagine due to 
the memory context. Should I try to keep the iterator? How can I do it?

Hope the diff and idents are ok. Please let me know if there's something wrong 
with them.

Thanks!
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.206
diff -r1.206 tablecmds.c
336a337,340
> 	else if (stmt->relation->istemp)
> 	{
> 		tablespaceId = GetTempTablespace();
> 	}
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.39
diff -r1.39 tablespace.c
68c68
< /* GUC variable */
---
> /* GUC variables */
69a70
> char       *temp_tablespaces = NULL;
70a72,73
> int	   next_temp_tablespace;
> int	   num_temp_tablespaces;
932a936,1056
> /*
>  * 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;
> 	/*
> 	 * 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?
> 		 */
> 		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++;
> 		}
> 	}
> 
> 	pfree(rawname);
> 	list_free(namelist);
> 	next_temp_tablespace = 0;
> 	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;
> 	
> 	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;
> 	}
> 
> 	foreach(l, namelist)
> 	{
> 		curname = (char *) lfirst(l);
> 		if ( i == next_temp_tablespace )
> 			break;
> 		i++;
> 	}
> 
> 	pfree(rawname);
> 
> 	/* 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);
> 		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);
> 
> 	/*
> 	 * 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;
> }
1004a1129,1167
> /*
>  * get_tablespace_path - given a tablespace OID, look up the path
>  *
>  * Returns a palloc'd string, or NULL if no such tablespace.
>  */
> char *
> get_tablespace_path(Oid spc_oid)
> {
> 	char	   *result;
> 	Relation	rel;
> 	HeapScanDesc scandesc;
> 	HeapTuple	tuple;
> 	ScanKeyData entry[1];
> 	bool isNull;
> 	Datum pathDatum;
> 
> 	/* Search pg_tablespace */
> 	rel = heap_open(TableSpaceRelationId, AccessShareLock);
> 
> 	ScanKeyInit(&entry[0],
> 				ObjectIdAttributeNumber,
> 				BTEqualStrategyNumber, F_OIDEQ,
> 				ObjectIdGetDatum(spc_oid));
> 	scandesc = heap_beginscan(rel, SnapshotNow, 1, entry);
> 	tuple = heap_getnext(scandesc, ForwardScanDirection);
> 
> 	/* We assume that there can be at most one matching tuple */
> 	if (HeapTupleIsValid(tuple)) {
> 		pathDatum = heap_getattr( tuple, Anum_pg_tablespace_spclocation, 
> 			RelationGetDescr(rel), &isNull );
> 		result = pstrdup( DatumGetCString( pathDatum ) );
> 	} else
> 		result = NULL;
> 
> 	heap_endscan(scandesc);
> 	heap_close(rel, AccessShareLock);
> 
> 	return result;
> }
Index: src/backend/storage/file/fd.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/file/fd.c,v
retrieving revision 1.130
diff -r1.130 fd.c
48a49,50
> #include "commands/tablespace.h"
> 
881a884,898
> 	Oid		oid;
> 	char		*path;
> 
> 	/*
> 	 * Take a look what should be the path of the temporary file
> 	 */
> 	oid = GetTempTablespace();
> 	if ( oid == InvalidOid ) 
> 	{
> 		path = PG_TEMP_FILES_DIR;
> 	}
> 	else 
> 	{
> 		path = get_tablespace_path(oid);
> 	}
888c905
< 			 "%s/%s%d.%ld", PG_TEMP_FILES_DIR, PG_TEMP_FILE_PREFIX,
---
> 			 "%s/%s%d.%ld", path, PG_TEMP_FILE_PREFIX,
890a908,911
> 	/* path should be pfreed in case it  was obtained from get_tablespace_path */
> 	if ( oid != InvalidOid )
> 		pfree(path);
> 
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.357
diff -r1.357 guc.c
98a99
> extern char *temp_tablespaces;
2250a2252,2261
> 	{
> 		{"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
> 	},
> 
Index: src/include/commands/tablespace.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/commands/tablespace.h,v
retrieving revision 1.13
diff -r1.13 tablespace.h
43a44
> extern Oid	GetTempTablespace(void);
46a48
> extern char *get_tablespace_path(Oid spc_oid);
Index: src/include/utils/guc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.76
diff -r1.76 guc.h
239a240,243
> /* in commands/tablespace.c */
> extern const char *assign_temp_tablespaces(const char *newval,
> 						  bool doit, GucSource source);
> 
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Reply via email to