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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
