CREATE TABLESPACE SET

Started by Vik Fearingabout 12 years ago10 messages
#1Vik Fearing
vik.fearing@dalibo.com
1 attachment(s)

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

If people think this is a good idea, I'll clean it up, add
documentation, and submit it to the next commitfest.

--
Vik

Attachments:

create_tablespace_set_v1.patchtext/x-diff; name=create_tablespace_set_v1.patchDownload
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 234,239 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 234,240 ----
  	Oid			tablespaceoid;
  	char	   *location;
  	Oid			ownerId;
+ 	Datum		newOptions;
  
  	/* Must be super user */
  	if (!superuser())
***************
*** 317,323 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 	nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
--- 318,333 ----
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 
! 	/* Generate new proposed spcoptions (text array) */
! 	newOptions = transformRelOptions((Datum) 0,
! 									 stmt->options,
! 									 NULL, NULL, false, false);
! 	(void) tablespace_reloptions(newOptions, true);
! 	if (newOptions != (Datum) 0)
! 		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
! 	else
! 		nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 3370,3375 **** _copyCreateTableSpaceStmt(const CreateTableSpaceStmt *from)
--- 3370,3376 ----
  	COPY_STRING_FIELD(tablespacename);
  	COPY_STRING_FIELD(owner);
  	COPY_STRING_FIELD(location);
+ 	COPY_NODE_FIELD(options);
  
  	return newnode;
  }
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 1610,1615 **** _equalCreateTableSpaceStmt(const CreateTableSpaceStmt *a, const CreateTableSpace
--- 1610,1616 ----
  	COMPARE_STRING_FIELD(tablespacename);
  	COMPARE_STRING_FIELD(owner);
  	COMPARE_STRING_FIELD(location);
+ 	COMPARE_NODE_FIELD(options);
  
  	return true;
  }
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 325,331 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  %type <list>	stmtblock stmtmulti
  				OptTableElementList TableElementList OptInherit definition
  				OptTypedTableElementList TypedTableElementList
! 				reloptions opt_reloptions
  				OptWith opt_distinct opt_definition func_args func_args_list
  				func_args_with_defaults func_args_with_defaults_list
  				aggr_args aggr_args_list
--- 325,331 ----
  %type <list>	stmtblock stmtmulti
  				OptTableElementList TableElementList OptInherit definition
  				OptTypedTableElementList TypedTableElementList
! 				reloptions opt_reloptions opt_setreloptions
  				OptWith opt_distinct opt_definition func_args func_args_list
  				func_args_with_defaults func_args_with_defaults_list
  				aggr_args aggr_args_list
***************
*** 2275,2280 **** opt_reloptions:		WITH reloptions					{ $$ = $2; }
--- 2275,2284 ----
  			 |		/* EMPTY */						{ $$ = NIL; }
  		;
  
+ opt_setreloptions:		SET reloptions					{ $$ = $2; }
+ 			 |		/* EMPTY */						{ $$ = NIL; }
+ 		;
+ 
  reloption_list:
  			reloption_elem							{ $$ = list_make1($1); }
  			| reloption_list ',' reloption_elem		{ $$ = lappend($1, $3); }
***************
*** 3588,3599 **** opt_procedural:
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
  					$$ = (Node *) n;
  				}
  		;
--- 3592,3604 ----
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_setreloptions
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
+ 					n->options = $7;
  					$$ = (Node *) n;
  				}
  		;
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1669,1674 **** typedef struct CreateTableSpaceStmt
--- 1669,1675 ----
  	char	   *tablespacename;
  	char	   *owner;
  	char	   *location;
+ 	List	   *options;
  } CreateTableSpaceStmt;
  
  typedef struct DropTableSpaceStmt
#2David Fetter
david@fetter.org
In reply to: Vik Fearing (#1)
Re: CREATE TABLESPACE SET

On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote:

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

That should probably be WITH instead of SET for consistency with other
similar DDL.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Vik Fearing
vik.fearing@dalibo.com
In reply to: David Fetter (#2)
Re: CREATE TABLESPACE SET

On 12/26/2013 06:10 PM, David Fetter wrote:

On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote:

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

That should probably be WITH instead of SET for consistency with other
similar DDL.

It would make a simpler patch, too, but I wanted to be consistent with
ALTER TABLESPACE. I have no firm opinion on the subject, so I'll switch
it to WITH if no one else says anything.

--
Vik

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Vik Fearing
vik.fearing@dalibo.com
In reply to: David Fetter (#2)
1 attachment(s)
Re: CREATE TABLESPACE WITH

On 12/26/2013 06:10 PM, David Fetter wrote:

On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote:

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

That should probably be WITH instead of SET for consistency with other
similar DDL.

Here is version 2 of the patch, which uses WITH instead of SET, and also
adds to the documentation.

--
Vik

Attachments:

create_tablespace_with_v2.patchtext/x-diff; name=create_tablespace_with_v2.patchDownload
*** a/doc/src/sgml/ref/create_tablespace.sgml
--- b/doc/src/sgml/ref/create_tablespace.sgml
***************
*** 21,27 **** PostgreSQL documentation
  
   <refsynopsisdiv>
  <synopsis>
! CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [ OWNER <replaceable class="parameter">user_name</replaceable> ] LOCATION '<replaceable class="parameter">directory</replaceable>'
  </synopsis>
   </refsynopsisdiv>
  
--- 21,30 ----
  
   <refsynopsisdiv>
  <synopsis>
! CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable>
!     [ OWNER <replaceable class="parameter">user_name</replaceable> ]
!     LOCATION '<replaceable class="parameter">directory</replaceable>'
!     [ WITH ( <replaceable class="PARAMETER">tablespace_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
  </synopsis>
   </refsynopsisdiv>
  
***************
*** 87,92 **** CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [
--- 90,113 ----
         </para>
        </listitem>
       </varlistentry>
+ 
+      <varlistentry>
+       <term><replaceable class="parameter">tablespace_option</replaceable></term>
+       <listitem>
+        <para>
+         A tablespace parameter to be set or reset.  Currently, the only
+         available parameters are <varname>seq_page_cost</> and
+         <varname>random_page_cost</>.  Setting either value for a particular
+         tablespace will override the planner's usual estimate of the cost of
+         reading pages from tables in that tablespace, as established by
+         the configuration parameters of the same name (see
+         <xref linkend="guc-seq-page-cost">,
+         <xref linkend="guc-random-page-cost">).  This may be useful if one
+         tablespace is located on a disk which is faster or slower than the
+         remainder of the I/O subsystem.
+        </para>
+       </listitem>
+      </varlistentry>
    </variablelist>
   </refsect1>
  
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 234,239 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 234,240 ----
  	Oid			tablespaceoid;
  	char	   *location;
  	Oid			ownerId;
+ 	Datum		newOptions;
  
  	/* Must be super user */
  	if (!superuser())
***************
*** 317,323 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 	nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
--- 318,333 ----
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 
! 	/* Generate new proposed spcoptions (text array) */
! 	newOptions = transformRelOptions((Datum) 0,
! 									 stmt->options,
! 									 NULL, NULL, false, false);
! 	(void) tablespace_reloptions(newOptions, true);
! 	if (newOptions != (Datum) 0)
! 		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
! 	else
! 		nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 3370,3375 **** _copyCreateTableSpaceStmt(const CreateTableSpaceStmt *from)
--- 3370,3376 ----
  	COPY_STRING_FIELD(tablespacename);
  	COPY_STRING_FIELD(owner);
  	COPY_STRING_FIELD(location);
+ 	COPY_NODE_FIELD(options);
  
  	return newnode;
  }
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 1610,1615 **** _equalCreateTableSpaceStmt(const CreateTableSpaceStmt *a, const CreateTableSpace
--- 1610,1616 ----
  	COMPARE_STRING_FIELD(tablespacename);
  	COMPARE_STRING_FIELD(owner);
  	COMPARE_STRING_FIELD(location);
+ 	COMPARE_NODE_FIELD(options);
  
  	return true;
  }
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 3588,3599 **** opt_procedural:
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
  					$$ = (Node *) n;
  				}
  		;
--- 3588,3600 ----
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_reloptions
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
+ 					n->options = $7;
  					$$ = (Node *) n;
  				}
  		;
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1669,1674 **** typedef struct CreateTableSpaceStmt
--- 1669,1675 ----
  	char	   *tablespacename;
  	char	   *owner;
  	char	   *location;
+ 	List	   *options;
  } CreateTableSpaceStmt;
  
  typedef struct DropTableSpaceStmt
#5Michael Paquier
michael.paquier@gmail.com
In reply to: Vik Fearing (#4)
Re: CREATE TABLESPACE WITH

On Wed, Jan 15, 2014 at 10:27 AM, Vik Fearing <vik.fearing@dalibo.com> wrote:

On 12/26/2013 06:10 PM, David Fetter wrote:

On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote:

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

That should probably be WITH instead of SET for consistency with other
similar DDL.

Here is version 2 of the patch, which uses WITH instead of SET, and also
adds to the documentation.

I just had a quick look at this patch, no testing at all. I am seeing
that regression tests are still missing here, they should be added in
src/test/regress/input/tablespace.source. Then, for the use of either
WITH or SET... For example CREATE FUNCTION uses SET for a
configuration parameter, and ALTER FUNCTION is consistent with that.
So SET makes more sense to be consistent with CREATE TABLESPACE? This
patch should not be modified once again as long as there are no more
opinions though...
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Jim Nasby
jim@nasby.net
In reply to: Michael Paquier (#5)
Re: CREATE TABLESPACE WITH

On 1/14/14, 8:07 PM, Michael Paquier wrote:

On Wed, Jan 15, 2014 at 10:27 AM, Vik Fearing <vik.fearing@dalibo.com> wrote:

On 12/26/2013 06:10 PM, David Fetter wrote:

On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote:

I was recently annoyed that I had to do

CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);

The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.

CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);

That should probably be WITH instead of SET for consistency with other
similar DDL.

Here is version 2 of the patch, which uses WITH instead of SET, and also
adds to the documentation.

I just had a quick look at this patch, no testing at all. I am seeing
that regression tests are still missing here, they should be added in
src/test/regress/input/tablespace.source. Then, for the use of either
WITH or SET... For example CREATE FUNCTION uses SET for a
configuration parameter, and ALTER FUNCTION is consistent with that.
So SET makes more sense to be consistent with CREATE TABLESPACE? This
patch should not be modified once again as long as there are no more
opinions though...

You know, this doesn't do much to encourage people to submit patches since it was just suggested that we use WITH instead of SET. :(

Anyone have an easy way to see which is more prevalent? I'd be stuck with \h or trying to grep SGML... I'm hoping someone else has an easier way...
--
Jim C. Nasby, Data Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Michael Paquier
michael.paquier@gmail.com
In reply to: Jim Nasby (#6)
Re: CREATE TABLESPACE WITH

On Wed, Jan 15, 2014 at 2:02 PM, Jim Nasby <jim@nasby.net> wrote:

You know, this doesn't do much to encourage people to submit patches since
it was just suggested that we use WITH instead of SET. :(

Sorry, you are right.

Anyone have an easy way to see which is more prevalent? I'd be stuck with \h
or trying to grep SGML... I'm hoping someone else has an easier way...

I am seeing that the "SET ()" pattern is used in those files:
$ cd postgres/doc/src/sgml/ref
$ git grep -e " SET (" --and -e "replaceable" --name-only
alter_foreign_table.sgml
alter_index.sgml
alter_materialized_view.sgml
alter_table.sgml
alter_tablespace.sgml
alter_view.sgml

While the WITH () pattern is used in those files:
$ git grep -e " WITH (" --and -e "replaceable" --name-only
create_function.sgml
create_index.sgml
create_materialized_view.sgml
create_table.sgml
create_table_as.sgml
create_view.sgml

So yes WITH makes sense for a CREATE command.
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Vik Fearing
vik.fearing@dalibo.com
In reply to: Michael Paquier (#5)
1 attachment(s)
Re: CREATE TABLESPACE WITH

On 01/15/2014 03:07 AM, Michael Paquier wrote:

I just had a quick look at this patch, no testing at all.

Thank you for looking at it.

I am seeing that regression tests are still missing here, they should be added in
src/test/regress/input/tablespace.source.

New patch attached, with regression tests.

Then, for the use of either
WITH or SET... For example CREATE FUNCTION uses SET for a
configuration parameter, and ALTER FUNCTION is consistent with that.
So SET makes more sense to be consistent with CREATE TABLESPACE? This
patch should not be modified once again as long as there are no more
opinions though...

Based on your analysis of current behavior elsewhere, and the messier
patch with SET, I have switched my vote from ambivalent to preferring WITH.

--
Vik

Attachments:

create_tablespace_with_v3.patchtext/x-diff; name=create_tablespace_with_v3.patchDownload
*** a/doc/src/sgml/ref/create_tablespace.sgml
--- b/doc/src/sgml/ref/create_tablespace.sgml
***************
*** 21,27 **** PostgreSQL documentation
  
   <refsynopsisdiv>
  <synopsis>
! CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [ OWNER <replaceable class="parameter">user_name</replaceable> ] LOCATION '<replaceable class="parameter">directory</replaceable>'
  </synopsis>
   </refsynopsisdiv>
  
--- 21,30 ----
  
   <refsynopsisdiv>
  <synopsis>
! CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable>
!     [ OWNER <replaceable class="parameter">user_name</replaceable> ]
!     LOCATION '<replaceable class="parameter">directory</replaceable>'
!     [ WITH ( <replaceable class="PARAMETER">tablespace_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
  </synopsis>
   </refsynopsisdiv>
  
***************
*** 87,92 **** CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [
--- 90,113 ----
         </para>
        </listitem>
       </varlistentry>
+ 
+      <varlistentry>
+       <term><replaceable class="parameter">tablespace_option</replaceable></term>
+       <listitem>
+        <para>
+         A tablespace parameter to be set or reset.  Currently, the only
+         available parameters are <varname>seq_page_cost</> and
+         <varname>random_page_cost</>.  Setting either value for a particular
+         tablespace will override the planner's usual estimate of the cost of
+         reading pages from tables in that tablespace, as established by
+         the configuration parameters of the same name (see
+         <xref linkend="guc-seq-page-cost">,
+         <xref linkend="guc-random-page-cost">).  This may be useful if one
+         tablespace is located on a disk which is faster or slower than the
+         remainder of the I/O subsystem.
+        </para>
+       </listitem>
+      </varlistentry>
    </variablelist>
   </refsect1>
  
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 234,239 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 234,240 ----
  	Oid			tablespaceoid;
  	char	   *location;
  	Oid			ownerId;
+ 	Datum		newOptions;
  
  	/* Must be super user */
  	if (!superuser())
***************
*** 317,323 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 	nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
--- 318,333 ----
  	values[Anum_pg_tablespace_spcowner - 1] =
  		ObjectIdGetDatum(ownerId);
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
! 
! 	/* Generate new proposed spcoptions (text array) */
! 	newOptions = transformRelOptions((Datum) 0,
! 									 stmt->options,
! 									 NULL, NULL, false, false);
! 	(void) tablespace_reloptions(newOptions, true);
! 	if (newOptions != (Datum) 0)
! 		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
! 	else
! 		nulls[Anum_pg_tablespace_spcoptions - 1] = true;
  
  	tuple = heap_form_tuple(rel->rd_att, values, nulls);
  
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 3370,3375 **** _copyCreateTableSpaceStmt(const CreateTableSpaceStmt *from)
--- 3370,3376 ----
  	COPY_STRING_FIELD(tablespacename);
  	COPY_STRING_FIELD(owner);
  	COPY_STRING_FIELD(location);
+ 	COPY_NODE_FIELD(options);
  
  	return newnode;
  }
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 1610,1615 **** _equalCreateTableSpaceStmt(const CreateTableSpaceStmt *a, const CreateTableSpace
--- 1610,1616 ----
  	COMPARE_STRING_FIELD(tablespacename);
  	COMPARE_STRING_FIELD(owner);
  	COMPARE_STRING_FIELD(location);
+ 	COMPARE_NODE_FIELD(options);
  
  	return true;
  }
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 3588,3599 **** opt_procedural:
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
  					$$ = (Node *) n;
  				}
  		;
--- 3588,3600 ----
   *
   *****************************************************************************/
  
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_reloptions
  				{
  					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
  					n->tablespacename = $3;
  					n->owner = $4;
  					n->location = $6;
+ 					n->options = $7;
  					$$ = (Node *) n;
  				}
  		;
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1669,1674 **** typedef struct CreateTableSpaceStmt
--- 1669,1675 ----
  	char	   *tablespacename;
  	char	   *owner;
  	char	   *location;
+ 	List	   *options;
  } CreateTableSpaceStmt;
  
  typedef struct DropTableSpaceStmt
*** a/src/test/regress/input/tablespace.source
--- b/src/test/regress/input/tablespace.source
***************
*** 1,3 ****
--- 1,13 ----
+ -- create a tablespace using WITH clause
+ CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
+ CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
+ 
+ -- check to see the parameter was used
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspacewith';
+ 
+ -- drop the tablespace so we can re-use the location
+ DROP TABLESPACE testspacewith;
+ 
  -- create a tablespace we can use
  CREATE TABLESPACE testspace LOCATION '@testtablespace@';
  
*** a/src/test/regress/output/tablespace.source
--- b/src/test/regress/output/tablespace.source
***************
*** 1,3 ****
--- 1,16 ----
+ -- create a tablespace using WITH clause
+ CREATE TABLESPACE testspacewith LOCATION '/home/vik/postgresql/9.4/git/src/test/regress/testtablespace' WITH (some_nonexistent_parameter = true); -- fail
+ ERROR:  unrecognized parameter "some_nonexistent_parameter"
+ CREATE TABLESPACE testspacewith LOCATION '/home/vik/postgresql/9.4/git/src/test/regress/testtablespace' WITH (random_page_cost = 3.0); -- ok
+ -- check to see the parameter was used
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspacewith';
+        spcoptions       
+ ------------------------
+  {random_page_cost=3.0}
+ (1 row)
+ 
+ -- drop the tablespace so we can re-use the location
+ DROP TABLESPACE testspacewith;
  -- create a tablespace we can use
  CREATE TABLESPACE testspace LOCATION '@testtablespace@';
  -- try setting and resetting some properties for the new tablespace
#9Michael Paquier
michael.paquier@gmail.com
In reply to: Vik Fearing (#8)
1 attachment(s)
Re: CREATE TABLESPACE WITH

On Thu, Jan 16, 2014 at 9:03 AM, Vik Fearing <vik.fearing@dalibo.com> wrote:

New patch attached, with regression tests.

Thanks for the new version, I have spent some time looking at it:
- Patch compiles without warnings.
- Done some manual testing with CREATE/ALTER TABLESPACE WITH and
checked pg_tablespace, it worked fine.
- However, regression tests are failing because
src/test/regress/output/tablespace.source has an incorrect output, it
is necessary to replace that:
/home/vik/postgresql/9.4/git/src/test/regress/testtablespace
by that:
@testtablespace@
This is something I have actually fixed that in the attached patch.

So, except for the regression output problem, I think that the code is
clean so marking it as "Ready for committer" on the commit fest app.
Thanks,
--
Michael

Attachments:

20140118_create_tablespace_with_v4.patchtext/x-diff; charset=US-ASCII; name=20140118_create_tablespace_with_v4.patchDownload
diff --git a/doc/src/sgml/ref/create_tablespace.sgml b/doc/src/sgml/ref/create_tablespace.sgml
index 89c8907..04c5fb8 100644
--- a/doc/src/sgml/ref/create_tablespace.sgml
+++ b/doc/src/sgml/ref/create_tablespace.sgml
@@ -21,7 +21,10 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [ OWNER <replaceable class="parameter">user_name</replaceable> ] LOCATION '<replaceable class="parameter">directory</replaceable>'
+CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable>
+    [ OWNER <replaceable class="parameter">user_name</replaceable> ]
+    LOCATION '<replaceable class="parameter">directory</replaceable>'
+    [ WITH ( <replaceable class="PARAMETER">tablespace_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
 </synopsis>
  </refsynopsisdiv>
 
@@ -87,6 +90,24 @@ CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> [
        </para>
       </listitem>
      </varlistentry>
+
+     <varlistentry>
+      <term><replaceable class="parameter">tablespace_option</replaceable></term>
+      <listitem>
+       <para>
+        A tablespace parameter to be set or reset.  Currently, the only
+        available parameters are <varname>seq_page_cost</> and
+        <varname>random_page_cost</>.  Setting either value for a particular
+        tablespace will override the planner's usual estimate of the cost of
+        reading pages from tables in that tablespace, as established by
+        the configuration parameters of the same name (see
+        <xref linkend="guc-seq-page-cost">,
+        <xref linkend="guc-random-page-cost">).  This may be useful if one
+        tablespace is located on a disk which is faster or slower than the
+        remainder of the I/O subsystem.
+       </para>
+      </listitem>
+     </varlistentry>
   </variablelist>
  </refsect1>
 
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 07f5221..c7aedc0 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -234,6 +234,7 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 	Oid			tablespaceoid;
 	char	   *location;
 	Oid			ownerId;
+	Datum		newOptions;
 
 	/* Must be super user */
 	if (!superuser())
@@ -317,7 +318,16 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 	values[Anum_pg_tablespace_spcowner - 1] =
 		ObjectIdGetDatum(ownerId);
 	nulls[Anum_pg_tablespace_spcacl - 1] = true;
-	nulls[Anum_pg_tablespace_spcoptions - 1] = true;
+
+	/* Generate new proposed spcoptions (text array) */
+	newOptions = transformRelOptions((Datum) 0,
+									 stmt->options,
+									 NULL, NULL, false, false);
+	(void) tablespace_reloptions(newOptions, true);
+	if (newOptions != (Datum) 0)
+		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
+	else
+		nulls[Anum_pg_tablespace_spcoptions - 1] = true;
 
 	tuple = heap_form_tuple(rel->rd_att, values, nulls);
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index fb4ce2c..506ceb8 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3370,6 +3370,7 @@ _copyCreateTableSpaceStmt(const CreateTableSpaceStmt *from)
 	COPY_STRING_FIELD(tablespacename);
 	COPY_STRING_FIELD(owner);
 	COPY_STRING_FIELD(location);
+	COPY_NODE_FIELD(options);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index ccf7267..ae68b86 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1610,6 +1610,7 @@ _equalCreateTableSpaceStmt(const CreateTableSpaceStmt *a, const CreateTableSpace
 	COMPARE_STRING_FIELD(tablespacename);
 	COMPARE_STRING_FIELD(owner);
 	COMPARE_STRING_FIELD(location);
+	COMPARE_NODE_FIELD(options);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index f0b9507..9b76ac9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3588,12 +3588,13 @@ opt_procedural:
  *
  *****************************************************************************/
 
-CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst
+CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_reloptions
 				{
 					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
 					n->tablespacename = $3;
 					n->owner = $4;
 					n->location = $6;
+					n->options = $7;
 					$$ = (Node *) n;
 				}
 		;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9a3a5d7..6e721c2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1669,6 +1669,7 @@ typedef struct CreateTableSpaceStmt
 	char	   *tablespacename;
 	char	   *owner;
 	char	   *location;
+	List	   *options;
 } CreateTableSpaceStmt;
 
 typedef struct DropTableSpaceStmt
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 4f17b09..c25b838 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -1,3 +1,13 @@
+-- create a tablespace using WITH clause
+CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
+CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
+
+-- check to see the parameter was used
+SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspacewith';
+
+-- drop the tablespace so we can re-use the location
+DROP TABLESPACE testspacewith;
+
 -- create a tablespace we can use
 CREATE TABLESPACE testspace LOCATION '@testtablespace@';
 
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 2868169..cc5648e 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -1,3 +1,16 @@
+-- create a tablespace using WITH clause
+CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
+ERROR:  unrecognized parameter "some_nonexistent_parameter"
+CREATE TABLESPACE testspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
+-- check to see the parameter was used
+SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspacewith';
+       spcoptions       
+------------------------
+ {random_page_cost=3.0}
+(1 row)
+
+-- drop the tablespace so we can re-use the location
+DROP TABLESPACE testspacewith;
 -- create a tablespace we can use
 CREATE TABLESPACE testspace LOCATION '@testtablespace@';
 -- try setting and resetting some properties for the new tablespace
#10Vik Fearing
vik.fearing@dalibo.com
In reply to: Michael Paquier (#9)
Re: CREATE TABLESPACE WITH

On 01/18/2014 03:21 PM, Michael Paquier wrote:

So, except for the regression output problem, I think that the code is
clean so marking it as "Ready for committer" on the commit fest app.

Thank you for the review. Sorry about the regression thing. I tried to
randomize it a bit so that I would catch it but it ended up breaking all
the other tablespace tests, so I decided it's not worth it.

--
Vik

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers