Hi! This patch is part of a bigger patch I've offered before https://www.postgresql.org/message-id/flat/2146419.veIEZdk4E4@x200m#2146419.veIEZdk4E4@x200m as we agreed I am trying to commit it by smaller bits
This patch raises error if user tries o set or change toast.* option for a table that does not have a TOST relation. I believe it is the only right thing to do, as now if you set toast reloption for table that does not have TOAST table, the value of this option will be lost without any warning. You will not get it back with pg_dump, it will not be effective when you add varlen attributes to the table later. So you offer DB some value to store, it accepts it without errors, and immediately loses it. I would consider it a bad behavior. I also think that we should not change this error to warning, as toast.* options are usually used by very experienced users for precised DB tunning. I hardly expect them to do TOAST tuning for tables without TOAST relations. So chances to get problem with existing SQL code are minimal. So I would suggest to throw an error in this case. Possible flaws: I tied to write error messages according to guide lines. But I suppose it is still not prefect enough as I am not so good with English. May be somebody who knows the language well, can make it better. -- Do code for fun.
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 0b4b5631..2551023 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -36,7 +36,7 @@
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_toast_pg_type_oid = InvalidOid;
-static void CheckAndCreateToastTable(Oid relOid, Datum reloptions,
+static bool CheckAndCreateToastTable(Oid relOid, Datum reloptions,
LOCKMODE lockmode, bool check);
static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
Datum reloptions, LOCKMODE lockmode, bool check);
@@ -67,23 +67,26 @@ NewHeapCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode)
CheckAndCreateToastTable(relOid, reloptions, lockmode, false);
}
-void
+bool
NewRelationCreateToastTable(Oid relOid, Datum reloptions)
{
- CheckAndCreateToastTable(relOid, reloptions, AccessExclusiveLock, false);
+ return CheckAndCreateToastTable(relOid, reloptions,
+ AccessExclusiveLock, false);
}
-static void
+static bool
CheckAndCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode, bool check)
{
Relation rel;
+ bool success;
rel = heap_open(relOid, lockmode);
/* create_toast_table does all the work */
- (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions, lockmode, check);
+ success = create_toast_table(rel, InvalidOid, InvalidOid, reloptions, lockmode, check);
heap_close(rel, NoLock);
+ return success;
}
/*
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 3d82edb..7745aa5 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -89,6 +89,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
ObjectAddress intoRelationAddr;
+ bool toast_created;
/* This code supports both CREATE TABLE AS and CREATE MATERIALIZED VIEW */
is_matview = (into->viewQuery != NULL);
@@ -130,7 +131,15 @@ create_ctas_internal(List *attrList, IntoClause *into)
(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
- NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
+ toast_created = NewRelationCreateToastTable(intoRelationAddr.objectId,
+ toast_options);
+ if (!toast_created && toast_options)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("can't set TOAST relation options for a new table"),
+ errdetail("TOAST relation were not created"),
+ errhint("do not specify toast.* options, or add some variable length attributes to the table")
+ ));
/* Create the "view" part of a materialized view. */
if (is_matview)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f2a928b..a169e14 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10557,6 +10557,19 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
heap_close(toastrel, NoLock);
}
+ else
+ {
+ newOptions = transformRelOptions((Datum) 0, defList, "toast",
+ validnsps, false, false);
+ if (newOptions)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("can't alter TOAST relation options for \"%s\" table",
+ RelationGetRelationName(rel)),
+ errdetail("TOAST relation does not exist"),
+ errhint("do not specify toast.* options, or add some variable length attributes to the table")
+ ));
+ }
heap_close(pgclass, RowExclusiveLock);
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index ec98a61..73f547b 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1007,6 +1007,7 @@ ProcessUtilitySlow(ParseState *pstate,
{
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
+ bool success;
/* Create the table itself */
address = DefineRelation((CreateStmt *) stmt,
@@ -1037,8 +1038,16 @@ ProcessUtilitySlow(ParseState *pstate,
toast_options,
true);
- NewRelationCreateToastTable(address.objectId,
- toast_options);
+ success = NewRelationCreateToastTable(address.objectId,
+ toast_options);
+
+ if (!success && toast_options)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("can't set TOAST relation options for a new table"),
+ errdetail("TOAST relation were not created"),
+ errhint("do not specify toast.* options, or add some variable length attributes to the table")
+ ));
}
else if (IsA(stmt, CreateForeignTableStmt))
{
diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index f6387ae..88d221a 100644
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -19,7 +19,7 @@
/*
* toasting.c prototypes
*/
-extern void NewRelationCreateToastTable(Oid relOid, Datum reloptions);
+extern bool NewRelationCreateToastTable(Oid relOid, Datum reloptions);
extern void NewHeapCreateToastTable(Oid relOid, Datum reloptions,
LOCKMODE lockmode);
extern void AlterTableCreateToastTable(Oid relOid, Datum reloptions,
diff --git a/src/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out
index c4107d5..7d3a8dd 100644
--- a/src/test/regress/expected/reloptions.out
+++ b/src/test/regress/expected/reloptions.out
@@ -95,6 +95,15 @@ SELECT reloptions, relhasoids FROM pg_class WHERE oid = 'reloptions_test'::regcl
{fillfactor } | t
(1 row)
+-- CREATE AS use different brach of code, so test it separately
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (fillfactoru) AS (SELECT 1::INT);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+ reloptions
+-----------------
+ {fillfactoru}
+(1 row)
+
-- Test toast.* options
DROP TABLE reloptions_test;
CREATE TABLE reloptions_test (s VARCHAR)
@@ -124,8 +133,53 @@ SELECT reloptions FROM pg_class WHERE oid = :toast_oid;
-- Fail on non-existent options in toast namespace
CREATE TABLE reloptions_test2 (i int) WITH (toast.not_existing_option = 42);
ERROR: unrecognized parameter "not_existing_option"
--- Mix TOAST & heap
+-- Fail on setting reloption to a table that does not have a TOAST relation
+CREATE TABLE reloptions_test2 (i int)
+ WITH (toast.autovacuum_vacuum_cost_delay#);
+ERROR: can't set TOAST relation options for a new table
+DETAIL: TOAST relation were not created
+HINT: do not specify toast.* options, or add some variable length attributes to the table
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT);
+ALTER TABLE reloptions_test SET (toast.autovacuum_vacuum_cost_delay = 23);
+ERROR: can't alter TOAST relation options for "reloptions_test" table
+DETAIL: TOAST relation does not exist
+HINT: do not specify toast.* options, or add some variable length attributes to the table
+ALTER TABLE reloptions_test RESET (toast.autovacuum_vacuum_cost_delay);
+ERROR: can't alter TOAST relation options for "reloptions_test" table
+DETAIL: TOAST relation does not exist
+HINT: do not specify toast.* options, or add some variable length attributes to the table
+-- autovacuum_analyze_scale_factor and autovacuum_analyze_threshold should be
+-- accepted by heap but rejected by toast (special case)
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test (s VARCHAR)
+ WITH (autovacuum_analyze_scale_factor=1, autovacuum_analyze_threshold=1);
+CREATE TABLE reloptions_test2 (s VARCHAR)
+ WITH (toast.autovacuum_analyze_scale_factor=1);
+ERROR: unrecognized parameter "autovacuum_analyze_scale_factor"
+CREATE TABLE reloptions_test2 (s VARCHAR)
+ WITH (toast.autovacuum_analyze_threshold=1);
+ERROR: unrecognized parameter "autovacuum_analyze_threshold"
+-- CREATE AS use different brach of code, so test it separately
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (toast.autovacuum_vacuum_cost_delay = 25)
+ AS (SELECT 'some text value'::VARCHAR);
+SELECT reltoastrelid as toast_oid
+ FROM pg_class WHERE oid = 'reloptions_test'::regclass \gset
+SELECT reloptions FROM pg_class WHERE oid = :toast_oid;
+ reloptions
+-----------------------------------
+ {autovacuum_vacuum_cost_delay%}
+(1 row)
+
+-- Fail while setting toast.* options for table without TOAST relation using CREATE AS
DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (toast.autovacuum_vacuum_cost_delay = 25)
+ AS (SELECT 1::INT);
+ERROR: can't set TOAST relation options for a new table
+DETAIL: TOAST relation were not created
+HINT: do not specify toast.* options, or add some variable length attributes to the table
+-- Mix TOAST & heap
CREATE TABLE reloptions_test (s VARCHAR) WITH
(toast.autovacuum_vacuum_cost_delay = 23,
autovacuum_vacuum_cost_delay = 24, fillfactor = 40);
diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql
index c9119fd..3ac56ea 100644
--- a/src/test/regress/sql/reloptions.sql
+++ b/src/test/regress/sql/reloptions.sql
@@ -57,6 +57,11 @@ DROP TABLE reloptions_test;
CREATE TABLE reloptions_test(i INT) WITH (fillfactor , oids=true);
SELECT reloptions, relhasoids FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+-- CREATE AS use different brach of code, so test it separately
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (fillfactoru) AS (SELECT 1::INT);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+
-- Test toast.* options
DROP TABLE reloptions_test;
@@ -75,9 +80,40 @@ SELECT reloptions FROM pg_class WHERE oid = :toast_oid;
-- Fail on non-existent options in toast namespace
CREATE TABLE reloptions_test2 (i int) WITH (toast.not_existing_option = 42);
--- Mix TOAST & heap
+-- Fail on setting reloption to a table that does not have a TOAST relation
+CREATE TABLE reloptions_test2 (i int)
+ WITH (toast.autovacuum_vacuum_cost_delay#);
+DROP TABLE reloptions_test;
+
+CREATE TABLE reloptions_test(i INT);
+ALTER TABLE reloptions_test SET (toast.autovacuum_vacuum_cost_delay = 23);
+ALTER TABLE reloptions_test RESET (toast.autovacuum_vacuum_cost_delay);
+
+-- autovacuum_analyze_scale_factor and autovacuum_analyze_threshold should be
+-- accepted by heap but rejected by toast (special case)
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test (s VARCHAR)
+ WITH (autovacuum_analyze_scale_factor=1, autovacuum_analyze_threshold=1);
+
+CREATE TABLE reloptions_test2 (s VARCHAR)
+ WITH (toast.autovacuum_analyze_scale_factor=1);
+CREATE TABLE reloptions_test2 (s VARCHAR)
+ WITH (toast.autovacuum_analyze_threshold=1);
+
+-- CREATE AS use different brach of code, so test it separately
DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (toast.autovacuum_vacuum_cost_delay = 25)
+ AS (SELECT 'some text value'::VARCHAR);
+SELECT reltoastrelid as toast_oid
+ FROM pg_class WHERE oid = 'reloptions_test'::regclass \gset
+SELECT reloptions FROM pg_class WHERE oid = :toast_oid;
+
+-- Fail while setting toast.* options for table without TOAST relation using CREATE AS
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (toast.autovacuum_vacuum_cost_delay = 25)
+ AS (SELECT 1::INT);
+-- Mix TOAST & heap
CREATE TABLE reloptions_test (s VARCHAR) WITH
(toast.autovacuum_vacuum_cost_delay = 23,
autovacuum_vacuum_cost_delay = 24, fillfactor = 40);
signature.asc
Description: This is a digitally signed message part.
