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
*** 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
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers