Is this a bug?
Hi all,
Shouldn't the "ALTER" statements below raise an exception?
fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY);
CREATE TABLE
fabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~ '^foo';
relname | reloptions
-------------+------------
foo |
foo_bar_seq |
foo_pkey |
(3 rows)
fabrizio=# ALTER TABLE foo RESET (noname);
ALTER TABLE
fabrizio=# ALTER INDEX foo_pkey RESET (noname);
ALTER INDEX
fabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname);
ALTER TABLE
If I try to "SET" an option called "noname" obviously will raise an
exception:
fabrizio=# ALTER TABLE foo SET (noname=1);
ERROR: unrecognized parameter "noname"
fabrizio=# ALTER INDEX foo_pkey SET (noname=1);
ERROR: unrecognized parameter "noname"
fabrizio=# ALTER TABLE foo ALTER COLUMN bar SET (noname=1);
ERROR: unrecognized parameter "noname"
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
On Wed, Mar 12, 2014 at 11:11 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
Hi all,
Shouldn't the "ALTER" statements below raise an exception?
fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY);
CREATE TABLEfabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~ '^foo';
relname | reloptions
-------------+------------
foo |
foo_bar_seq |
foo_pkey |
(3 rows)fabrizio=# ALTER TABLE foo RESET (noname);
ALTER TABLEfabrizio=# ALTER INDEX foo_pkey RESET (noname);
ALTER INDEXfabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname);
ALTER TABLEIf I try to "SET" an option called "noname" obviously will raise an
exception:fabrizio=# ALTER TABLE foo SET (noname=1);
ERROR: unrecognized parameter "noname"
Well, it's fairly harmless, but it might not be a bad idea to tighten that up.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 13-03-2014 00:11, Fabr�zio de Royes Mello wrote:
Shouldn't the "ALTER" statements below raise an exception?
For consistency, yes. Who cares? I mean, there is no harm in resetting
an unrecognized parameter. Have in mind that tighten it up could break
scripts. In general, I'm in favor of validating things.
euler@euler=# reset noname;
ERROR: 42704: unrecognized configuration parameter "noname"
LOCAL: set_config_option, guc.c:5220
--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 13, 2014 at 10:34 AM, Euler Taveira <euler@timbira.com.br>
wrote:
On 13-03-2014 00:11, Fabrízio de Royes Mello wrote:
Shouldn't the "ALTER" statements below raise an exception?
For consistency, yes. Who cares? I mean, there is no harm in resetting
an unrecognized parameter. Have in mind that tighten it up could break
scripts. In general, I'm in favor of validating things.
I know this could break scripts, but I think a consistent behavior should
be raise an exception when an option doesn't exists.
euler@euler=# reset noname;
ERROR: 42704: unrecognized configuration parameter "noname"
LOCAL: set_config_option, guc.c:5220
This is a consistent behavior.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
fabriziomello wrote
On Thu, Mar 13, 2014 at 10:34 AM, Euler Taveira <
euler@.com
>
wrote:On 13-03-2014 00:11, Fabrízio de Royes Mello wrote:
Shouldn't the "ALTER" statements below raise an exception?
For consistency, yes. Who cares? I mean, there is no harm in resetting
an unrecognized parameter. Have in mind that tighten it up could break
scripts. In general, I'm in favor of validating things.I know this could break scripts, but I think a consistent behavior should
be raise an exception when an option doesn't exists.euler@euler=# reset noname;
ERROR: 42704: unrecognized configuration parameter "noname"
LOCAL: set_config_option, guc.c:5220This is a consistent behavior.
Regards,
Probably shouldn't back-patch but a fix and release comment in 9.4 is
warranted.
Scripts resetting invalid parameters are probably already broken, they just
haven't discovered their mistake yet.
Do we need an "IF EXISTS" feature on these as well? ;)
David J.
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Is-this-a-bug-tp5795831p5795943.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Mar 12, 2014 at 11:11 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Hi all,
Shouldn't the "ALTER" statements below raise an exception?
fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY);
CREATE TABLEfabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~
'^foo';
relname | reloptions
-------------+------------
foo |
foo_bar_seq |
foo_pkey |
(3 rows)fabrizio=# ALTER TABLE foo RESET (noname);
ALTER TABLEfabrizio=# ALTER INDEX foo_pkey RESET (noname);
ALTER INDEXfabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname);
ALTER TABLEIf I try to "SET" an option called "noname" obviously will raise an
exception:fabrizio=# ALTER TABLE foo SET (noname=1);
ERROR: unrecognized parameter "noname"Well, it's fairly harmless, but it might not be a bad idea to tighten
that up.
The attached patch tighten that up.
Grettings,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Attachments:
alter-object-reset-exception-v1.patchtext/x-diff; charset=US-ASCII; name=alter-object-reset-exception-v1.patchDownload
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 530a1ae..4a5b767 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -307,6 +307,8 @@ static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+static bool is_valid_reloption(char *name);
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
@@ -382,6 +384,25 @@ initialize_reloptions(void)
}
/*
+ * is_valid_reloption
+ * check if a reloption exists
+ *
+ */
+static bool
+is_valid_reloption(char *name)
+{
+ int i;
+
+ for (i = 0; relOpts[i]; i++)
+ {
+ if (pg_strcasecmp(relOpts[i]->name, name) == 0)
+ return true;
+ }
+
+ return false;
+}
+
+/*
* add_reloption_kind
* Create a new relopt_kind value, to be used in custom reloptions by
* user-defined AMs.
@@ -674,6 +695,11 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
if (isReset)
{
+ if (!is_valid_reloption(def->defname))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized parameter \"%s\"", def->defname)));
+
if (def->arg != NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0f0c638..195103e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -7,6 +7,14 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
ERROR: relation "tmp_wrong" does not exist
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ALTER TABLE tmp SET (fillfactor=70);
+ALTER TABLE tmp RESET (fillfactor, noname);
+ERROR: unrecognized parameter "noname"
+ALTER TABLE tmp RESET (fillfactor=70);
+ERROR: RESET must not include values for parameters
+ALTER TABLE tmp RESET (fillfactor);
+ALTER TABLE tmp RESET (noname);
+ERROR: unrecognized parameter "noname"
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
ERROR: column name "xmin" conflicts with a system column name
ALTER TABLE tmp ADD COLUMN a int4 default 3;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 87973c1..2cb31f2 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -9,6 +9,16 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ALTER TABLE tmp SET (fillfactor=70);
+
+ALTER TABLE tmp RESET (fillfactor, noname);
+
+ALTER TABLE tmp RESET (fillfactor=70);
+
+ALTER TABLE tmp RESET (fillfactor);
+
+ALTER TABLE tmp RESET (noname);
+
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
ALTER TABLE tmp ADD COLUMN a int4 default 3;
On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.
Meh. There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote:
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, Mar 18, 2014 at 10:24 AM, Fabr�zio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.Meh. There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.
Do we want this patch for 9.5? It throws an error for invalid reloption
specifications.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote:
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.Meh. There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.Do we want this patch for 9.5? It throws an error for invalid reloption
specifications.
Fine with me. But I have a vague recollection of seeing pg_upgrade
doing this on purpose to create TOAST tables or something... am I
misremembering?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote:
On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote:
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, Mar 18, 2014 at 10:24 AM, Fabr�zio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.Meh. There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.Do we want this patch for 9.5? It throws an error for invalid reloption
specifications.Fine with me. But I have a vague recollection of seeing pg_upgrade
doing this on purpose to create TOAST tables or something... am I
misremembering?
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 12:53:30PM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote:
On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote:
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, Mar 18, 2014 at 10:24 AM, Fabr�zio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea to tighten that
up.The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO, somewhat
grouping with what has been done in a6542a4 for SET and ABORT.Meh. There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.Do we want this patch for 9.5? It throws an error for invalid reloption
specifications.Fine with me. But I have a vague recollection of seeing pg_upgrade
doing this on purpose to create TOAST tables or something... am I
misremembering?Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.
Looking at the ALTER TABLE options, I am going to put this check in a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On August 22, 2014 8:33:57 PM CEST, Bruce Momjian <bruce@momjian.us> wrote:
On Fri, Aug 22, 2014 at 12:53:30PM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote:
On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian <bruce@momjian.us>
wrote:
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote:
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas
<robertmhaas@gmail.com> wrote:
Well, it's fairly harmless, but it might not be a bad idea
to tighten that
up.
The attached patch tighten that up.
Hm... It might be interesting to include it in 9.4 IMO,
somewhat
grouping with what has been done in a6542a4 for SET and ABORT.
Meh. There will always be another thing we could squeeze in; I
don't
think this is particularly urgent, and it's late to the party.
Do we want this patch for 9.5? It throws an error for invalid
reloption
specifications.
Fine with me. But I have a vague recollection of seeing pg_upgrade
doing this on purpose to create TOAST tables or something... am I
misremembering?Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check in a
!IsBinaryUpgrade block so pg_upgrade can still use it?
Why not simply not do anything? This doesn't prevent any bugs and requiring pg-upgrade specific checks in there seems absurd. Also somebody might use it for similar purposes.
---
Please excuse brevity and formatting - I am writing this on my mobile phone.--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian <bruce@momjian.us> wrote:
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check in a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.
-1, that's really ugly.
Maybe the right solution is to add a form of ALTER TABLE that is
specifically defined to do only this check. This is an ongoing need,
so that might not be out of line.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote:
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian <bruce@momjian.us> wrote:
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check in a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.-1, that's really ugly.
Maybe the right solution is to add a form of ALTER TABLE that is
specifically defined to do only this check. This is an ongoing need,
so that might not be out of line.
Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I
will use that.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote:
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian <bruce@momjian.us> wrote:
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check in a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.-1, that's really ugly.
Maybe the right solution is to add a form of ALTER TABLE that is
specifically defined to do only this check. This is an ongoing need,
so that might not be out of line.Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I
will use that.
OK, attached patch applied, with pg_upgrade adjustments. I didn't
think the original regression tests for this were necessary.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
option.difftext/x-diff; charset=us-asciiDownload
diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
new file mode 100644
index e623a22..29a68c0
*** a/contrib/pg_upgrade/dump.c
--- b/contrib/pg_upgrade/dump.c
*************** optionally_create_toast_tables(void)
*** 115,120 ****
--- 115,124 ----
"c.relkind IN ('r', 'm') AND "
"c.reltoastrelid = 0");
+ /* Suppress NOTICE output from non-existant constraints */
+ PQclear(executeQueryOrDie(conn, "SET client_min_messages = warning;"));
+ PQclear(executeQueryOrDie(conn, "SET log_min_messages = warning;"));
+
ntups = PQntuples(res);
i_nspname = PQfnumber(res, "nspname");
i_relname = PQfnumber(res, "relname");
*************** optionally_create_toast_tables(void)
*** 125,137 ****
OPTIONALLY_CREATE_TOAST_OID));
/* dummy command that also triggers check for required TOAST table */
! PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s RESET (binary_upgrade_dummy_option);",
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
quote_identifier(PQgetvalue(res, rowno, i_relname))));
}
PQclear(res);
PQfinish(conn);
}
--- 129,144 ----
OPTIONALLY_CREATE_TOAST_OID));
/* dummy command that also triggers check for required TOAST table */
! PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s DROP CONSTRAINT IF EXISTS binary_upgrade_dummy_constraint;",
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
quote_identifier(PQgetvalue(res, rowno, i_relname))));
}
PQclear(res);
+ PQclear(executeQueryOrDie(conn, "RESET client_min_messages;"));
+ PQclear(executeQueryOrDie(conn, "RESET log_min_messages;"));
+
PQfinish(conn);
}
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
new file mode 100644
index e0b81b9..97a4e22
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
*************** static void initialize_reloptions(void);
*** 307,312 ****
--- 307,314 ----
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+ static bool is_valid_reloption(char *name);
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
*************** initialize_reloptions(void)
*** 382,387 ****
--- 384,408 ----
}
/*
+ * is_valid_reloption
+ * check if a reloption exists
+ *
+ */
+ static bool
+ is_valid_reloption(char *name)
+ {
+ int i;
+
+ for (i = 0; relOpts[i]; i++)
+ {
+ if (pg_strcasecmp(relOpts[i]->name, name) == 0)
+ return true;
+ }
+
+ return false;
+ }
+
+ /*
* add_reloption_kind
* Create a new relopt_kind value, to be used in custom reloptions by
* user-defined AMs.
*************** transformRelOptions(Datum oldOptions, Li
*** 672,677 ****
--- 693,703 ----
if (isReset)
{
+ if (!is_valid_reloption(def->defname))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized parameter \"%s\"", def->defname)));
+
if (def->arg != NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
On Mon, Aug 25, 2014 at 6:07 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote:
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian <bruce@momjian.us>
wrote:
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check in
a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.
-1, that's really ugly.
Maybe the right solution is to add a form of ALTER TABLE that is
specifically defined to do only this check. This is an ongoing need,
so that might not be out of line.Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I
will use that.OK, attached patch applied, with pg_upgrade adjustments. I didn't
think the original regression tests for this were necessary.
Hi,
Why this patch was reverted one day after applied [1]http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465? I didn't see any
discussion around it.
Regards,
[1]: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On 26 August 2015 at 20:24, Fabrízio de Royes Mello <fabriziomello@gmail.com
wrote:
On Mon, Aug 25, 2014 at 6:07 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote:
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian <bruce@momjian.us>
wrote:
Yes, you remember well. I will have to find a different way for
pg_upgrade to call a no-op ALTER TABLE, which is fine.Looking at the ALTER TABLE options, I am going to put this check
in a
!IsBinaryUpgrade block so pg_upgrade can still use its trick.
-1, that's really ugly.
Maybe the right solution is to add a form of ALTER TABLE that is
specifically defined to do only this check. This is an ongoing need,
so that might not be out of line.Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I
will use that.OK, attached patch applied, with pg_upgrade adjustments. I didn't
think the original regression tests for this were necessary.Hi,
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.Regards,
The discussion was here:
/messages/by-id/20140826000757.GE14956@momjian.us
Thom
On 2015-08-26 16:24:31 -0300, Fabr�zio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.
/messages/by-id/23918.1409010246@sss.pgh.pa.us
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Fabr�zio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.
Contributors whose patches are getting committed should really subscribe
to pgsql-committers.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Aug 26, 2015 at 4:30 PM, Andres Freund <andres@anarazel.de> wrote:
On 2015-08-26 16:24:31 -0300, Fabrízio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.
Thanks.... I'm not subscribed to pgsql-commiters so I didn't see it.
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Wed, Aug 26, 2015 at 4:31 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:
Fabrízio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.Contributors whose patches are getting committed should really subscribe
to pgsql-committers.
Ok. I sent a subscribe to pgsql-committers.
Thanks,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Fabrízio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.Contributors whose patches are getting committed should really subscribe
to pgsql-committers.
I would have thought discussion of committed patches should be moved
to -hackers. The description for the -committers list says:
"Notification of git commits are sent to this list. Do not post here!"
So, it's understandable that people would not expect other traffic there.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas wrote:
On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fabr�zio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see any
discussion around it.Contributors whose patches are getting committed should really subscribe
to pgsql-committers.I would have thought discussion of committed patches should be moved
to -hackers.
I agree, but it happens anyway quite frequently. Since recently, I make
a point of changing the CC from -committers to -hackers, but due to
force of habit I often forget.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 4, 2015 at 3:52 AM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:
Robert Haas wrote:
On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fabrízio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see
any
discussion around it.
Contributors whose patches are getting committed should really
subscribe
to pgsql-committers.
I would have thought discussion of committed patches should be moved
to -hackers.I agree, but it happens anyway quite frequently. Since recently, I make
a point of changing the CC from -committers to -hackers, but due to
force of habit I often forget.
Noted. I usually don't do that.
--
Michael
On Fri, Sep 4, 2015 at 09:40:10AM +0900, Michael Paquier wrote:
Robert Haas wrote:
On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fabr�zio de Royes Mello wrote:
Why this patch was reverted one day after applied [1]? I didn't see
any
discussion around it.
Contributors whose patches are getting committed should really
subscribe
to pgsql-committers.
I would have thought discussion of committed patches should be moved
to -hackers.I agree, but it happens anyway quite frequently.� Since recently, I make
a point of changing the CC from -committers to -hackers, but due to
force of habit I often forget.�
Noted. I usually don't do that.
I am thinking we should all agree if we should redirect commit comments
to hackers, or not, so we are consistent.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers