[PATCH][PROPOSAL] Refuse setting toast.* reloptions when TOAST table does not exist
Hi!
This patch is part of a bigger patch I've offered before
/messages/by-id/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.
Attachments:
no-toast-no-toast-options.difftext/x-patch; charset=UTF-8; name=no-toast-no-toast-options.diffDownload
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=20} | t
(1 row)
+-- CREATE AS use different brach of code, so test it separately
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test WITH (fillfactor=75) AS (SELECT 1::INT);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+ reloptions
+-----------------
+ {fillfactor=75}
+(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=23);
+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=25}
+(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=20, 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 (fillfactor=75) 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=23);
+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);
On Wed, Jan 17, 2018 at 3:50 PM, Nikolay Shaplov <dhyan@nataraj.su> wrote:
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.
I think there is a problem with this idea, which is that the rules for
whether or not a given table has an associated TOAST table
occasionally change from one major release to the next. Suppose that,
in release X, a particular table definition causes a TOAST table to be
created, but in release X+1, it does not. If a table with that
definition has a toast.* option set, then upgrading from release X to
release X+1 via pg_dump and restore will fail. That's bad.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 17, 2018 at 3:50 PM, Nikolay Shaplov <dhyan@nataraj.su> wrote:
This patch raises error if user tries o set or change toast.* option for a
table that does not have a TOST relation.
I think there is a problem with this idea, which is that the rules for
whether or not a given table has an associated TOAST table
occasionally change from one major release to the next. Suppose that,
in release X, a particular table definition causes a TOAST table to be
created, but in release X+1, it does not. If a table with that
definition has a toast.* option set, then upgrading from release X to
release X+1 via pg_dump and restore will fail. That's bad.
Yeah --- and it doesn't even have to be a major version change; the
same version on different hardware might make different choices too.
Not to mention that there is discussion out there about making the
toasting rules more configurable.
There might be a case for raising a warning in this situation,
but I would disagree with making it a hard error in any case.
All that's going to accomplish is to break people's scripts and
get them mad at you.
regards, tom lane
В письме от 18 января 2018 18:42:01 пользователь Tom Lane написал:
This patch raises error if user tries o set or change toast.* option for
a
table that does not have a TOST relation.I think there is a problem with this idea, which is that the rules for
whether or not a given table has an associated TOAST table
occasionally change from one major release to the next. Suppose that,
in release X, a particular table definition causes a TOAST table to be
created, but in release X+1, it does not. If a table with that
definition has a toast.* option set, then upgrading from release X to
release X+1 via pg_dump and restore will fail. That's bad.Yeah --- and it doesn't even have to be a major version change; the
same version on different hardware might make different choices too.
Not to mention that there is discussion out there about making the
toasting rules more configurable.There might be a case for raising a warning in this situation,
but I would disagree with making it a hard error in any case.
All that's going to accomplish is to break people's scripts and
get them mad at you.
These all sound very reasonable, but still does not solve problem with loosing
toast option values when you set one for table without TOAST relation.
May be, if reasons for existence TOAST relation is no so much fixed thing (I
thought it is almost as fixed as a rock), may be the solution would be to
create a TOAST relation anyway if toast options is set, and gave a warning
that may be setting this option is not the thing the user really want?
This way we will not loose option values. And it would be 100% backward
compatible.
The main misfeature here is that we will have empty TOAST relation in this
case. But first I do not think it will really harm anybody, and second, we
would WARN that it is not the best way to do things, so DB user will be able
to find way around.
What do you think about it?
--
Do code for fun.
On 1/18/18 18:42, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 17, 2018 at 3:50 PM, Nikolay Shaplov <dhyan@nataraj.su> wrote:
This patch raises error if user tries o set or change toast.* option for a
table that does not have a TOST relation.
There might be a case for raising a warning in this situation,
but I would disagree with making it a hard error in any case.
All that's going to accomplish is to break people's scripts and
get them mad at you.
It might also be useful to set these reloptions before you add a new
column to a table.
So I think this change is not desirable.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jan 23, 2018 at 12:03 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
It might also be useful to set these reloptions before you add a new
column to a table.
I don't understand. AAUI, it is currently the case that if you set
the options before the TOAST table exists, they are lost.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
В письме от 23 января 2018 12:03:50 пользователь Peter Eisentraut написал:
This patch raises error if user tries o set or change toast.* option for
a table that does not have a TOST relation.There might be a case for raising a warning in this situation,
but I would disagree with making it a hard error in any case.
All that's going to accomplish is to break people's scripts and
get them mad at you.It might also be useful to set these reloptions before you add a new
column to a table.
As Robert have just said, this will not work with current master.
If we, as I've suggested in previous letter, will force TOAST creation if
toast.* option is set, then your use case will also work.
--
Do code for fun.
On 1/23/18 13:39, Robert Haas wrote:
On Tue, Jan 23, 2018 at 12:03 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:It might also be useful to set these reloptions before you add a new
column to a table.I don't understand. AAUI, it is currently the case that if you set
the options before the TOAST table exists, they are lost.
Well, that's also weird.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/23/18 13:39, Robert Haas wrote:
I don't understand. AAUI, it is currently the case that if you set
the options before the TOAST table exists, they are lost.
Well, that's also weird.
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?
Backward compatibility might be an issue, but I doubt that there
are enough people using these options that we couldn't get away
with breaking things, if we're unable to find a decent automatic
conversion method.
regards, tom lane
Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/23/18 13:39, Robert Haas wrote:
I don't understand. AAUI, it is currently the case that if you set
the options before the TOAST table exists, they are lost.Well, that's also weird.
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?
Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Tom Lane wrote:
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?
Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.
That adds a lot of overhead if you never actually need the toast table.
Still, maybe it's an appropriate amount of effort compared to the size
of the use-case for this.
regards, tom lane
В письме от 25 января 2018 11:29:34 пользователь Tom Lane написал:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Tom Lane wrote:
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.That adds a lot of overhead if you never actually need the toast table.
I think this overhead case is not that terrible if it is properly warned ;-)
Still, maybe it's an appropriate amount of effort compared to the size
of the use-case for this.
If you came to some final conclustion, please close the commiffest item with
"Return with feedback" resolution, and I write another patch...
--
Do code for fun.
On 1/25/18 09:40, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/23/18 13:39, Robert Haas wrote:
I don't understand. AAUI, it is currently the case that if you set
the options before the TOAST table exists, they are lost.Well, that's also weird.
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?
I don't think there is a case where this can currently be a problem with
the current options set, but here is what I was thinking about: Imagine
there were a setting toast.fillfactor or something like that that
affects the storage layout of the TOAST table. If you ran
ALTER TABLE mytbl ADD COLUMN foo text DEFAULT somelongvalue();
then you would conceivably want to set TOAST-related reloptions before
the TOAST table is created and have them apply as the TOAST table is
being first populated.
Again, currently not a problem, I think.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 1/25/18 12:27 PM, Nikolay Shaplov wrote:
В письме от 25 января 2018 11:29:34 пользователь Tom Lane написал:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Tom Lane wrote:
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.That adds a lot of overhead if you never actually need the toast table.
I think this overhead case is not that terrible if it is properly warned ;-)
Still, maybe it's an appropriate amount of effort compared to the size
of the use-case for this.If you came to some final conclustion, please close the commiffest item with
"Return with feedback" resolution, and I write another patch...
I think this patch should be marked Returned with Feedback since it
appears there is no consensus on whether it is useful or correct, so I
have done that.
If I got it wrong I'm happy to move it to the next CF in Waiting for
Author state instead.
Thanks,
--
-David
david@pgmasters.net
Nikolay Shaplov wrote:
В письме от 25 января 2018 11:29:34 пользователь Tom Lane написал:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.That adds a lot of overhead if you never actually need the toast table.
I think this overhead case is not that terrible if it is properly warned ;-)
Let's go with this idea, which seems to me better than the statu quo.
There are a couple of other proposals, but they seem to require a lot of
work in exchange for unclear benefits.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
В письме от 10 апреля 2018 08:55:52 пользователь David Steele написал:
On 1/25/18 12:27 PM, Nikolay Shaplov wrote:
В письме от 25 января 2018 11:29:34 пользователь Tom Lane написал:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Tom Lane wrote:
Well, maybe the right answer is to address that. It's clear to me
why that would happen if we store these things as reloptions on the
toast table, but can't they be stored on the parent table?Actually, Nikolay provided a possible solution: if you execute ALTER
TABLE SET (toast.foobar = xyz), and a toast table doesn't exist, create
one at that point.That adds a lot of overhead if you never actually need the toast table.
I think this overhead case is not that terrible if it is properly warned
;-)>Still, maybe it's an appropriate amount of effort compared to the size
of the use-case for this.If you came to some final conclustion, please close the commiffest item
with "Return with feedback" resolution, and I write another patch...I think this patch should be marked Returned with Feedback since it
appears there is no consensus on whether it is useful or correct, so I
have done that.
Exactly!
But I'd like to know what kind of feedback is it :-)
What conclusion have been reached (I did not got it)
Otherwise I would not know how to rewrite this patch.
I would suggest: create a TOAST relation whenever toast.* options is set, but
give a warning it this relation will not be used for data storage (i.e. there
is no toastable columns in the table)
But I need some confirmation, in order not to write patch in vain again :-)
If I got it wrong I'm happy to move it to the next CF in Waiting for
Author state instead.Thanks,
--
Do code for fun.
Nikolay Shaplov wrote:
But I need some confirmation, in order not to write patch in vain again :-)
Don't worry, rest assured that you will still write *many* patches in
vain, not just this one.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 4/10/18 9:17 AM, Alvaro Herrera wrote:
Nikolay Shaplov wrote:
But I need some confirmation, in order not to write patch in vain again :-)
Don't worry, rest assured that you will still write *many* patches in
vain, not just this one.
Despite the rather dubious pep talk, Álvaro is right. You will get more
response with a new patch and a new thread.
But everyone is pretty fatigued right now, so I would recommend waiting
a while. If you would like to pursue this, plan to have a new patch
ready in August for the next CF. It sounds like you have an idea about
what needs to be done.
Regards,
--
-David
david@pgmasters.net