Is this a bug?

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 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"

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

fabriziomello wrote

On Thu, Mar 13, 2014 at 10:34 AM, Euler Taveira &lt;

euler@.com

&gt;
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,

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 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"

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

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. +

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

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,

[1]
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465

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