Small bug on CREATE EXTENSION pgq...

Started by Jerry Sieversalmost 11 years ago7 messages
#1Jerry Sievers
gsievers19@comcast.net

Hackers; I noticed this trying to import a large pg_dump file with
warnings supressed.

It seems loading pgq sets client_min_messages to warning and leaves it
this way which defeats an attempt to change the setting prior and have
it stick.

I tested with several other extensions in same DB and only pgq has the
problem.

Sorry if this is known/fixed already.

Thanks

sj$ cat q
select version();

create database foo template template0;
\c foo

show client_min_messages;

create extension pgq;

show client_min_messages;
reset client_min_messages;
show client_min_messages;

create extension pgq_node;

show client_min_messages;

\c postgres

drop database foo;

sj$ /usr/local/postgresql-9.3/bin/psql -ef q --no-psqlrc
select version();
version
----------------------------------------------------------------------------------------------
PostgreSQL 9.3.4 on x86_64-unknown-linux-gnu, compiled by gcc (Debian 4.7.2-5) 4.7.2, 64-bit
(1 row)

create database foo template template0;
CREATE DATABASE
psql (9.3.5, server 9.3.4)
You are now connected to database "foo" as user "jsievers".
show client_min_messages;
client_min_messages
---------------------
notice
(1 row)

create extension pgq;
CREATE EXTENSION
show client_min_messages;
client_min_messages
---------------------
warning
(1 row)

reset client_min_messages;
RESET
show client_min_messages;
client_min_messages
---------------------
notice
(1 row)

create extension pgq_node;
CREATE EXTENSION
show client_min_messages;
client_min_messages
---------------------
notice
(1 row)

psql (9.3.5, server 9.3.4)
You are now connected to database "postgres" as user "jsievers".
drop database foo;
DROP DATABASE
sj$
--
Jerry Sievers
Postgres DBA/Development Consulting
e: postgres.consulting@comcast.net
p: 312.241.7800

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2David G Johnston
david.g.johnston@gmail.com
In reply to: Jerry Sievers (#1)
Re: Small bug on CREATE EXTENSION pgq...

Jerry Sievers-3 wrote

Hackers; I noticed this trying to import a large pg_dump file with
warnings supressed.

It seems loading pgq sets client_min_messages to warning and leaves it
this way which defeats an attempt to change the setting prior and have
it stick.

I tested with several other extensions in same DB and only pgq has the
problem.

Sorry if this is known/fixed already.

This is not part of PostgreSQL proper and thus not supported by -hackers;
you should report this directly to the authors.

David J.

--
View this message in context: http://postgresql.nabble.com/Small-bug-on-CREATE-EXTENSION-pgq-tp5835899p5835900.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

#3Stephen Frost
sfrost@snowman.net
In reply to: David G Johnston (#2)
Re: Small bug on CREATE EXTENSION pgq...

* David G Johnston (david.g.johnston@gmail.com) wrote:

Jerry Sievers-3 wrote

Hackers; I noticed this trying to import a large pg_dump file with
warnings supressed.

It seems loading pgq sets client_min_messages to warning and leaves it
this way which defeats an attempt to change the setting prior and have
it stick.

I tested with several other extensions in same DB and only pgq has the
problem.

Sorry if this is known/fixed already.

This is not part of PostgreSQL proper and thus not supported by -hackers;
you should report this directly to the authors.

Ehh.. Shouldn't we try to take a bit more care that we reset things
after a CREATE EXTENSION is run? Not really sure how much effort we
want to put into it, but I see a bit of blame on both sides here.

Thanks,

Stephen

#4David Johnston
david.g.johnston@gmail.com
In reply to: Stephen Frost (#3)
Re: Small bug on CREATE EXTENSION pgq...

On Wednesday, January 28, 2015, Stephen Frost <sfrost@snowman.net> wrote:

* David G Johnston (david.g.johnston@gmail.com <javascript:;>) wrote:

Jerry Sievers-3 wrote

Hackers; I noticed this trying to import a large pg_dump file with
warnings supressed.

It seems loading pgq sets client_min_messages to warning and leaves it
this way which defeats an attempt to change the setting prior and have
it stick.

I tested with several other extensions in same DB and only pgq has the
problem.

Sorry if this is known/fixed already.

This is not part of PostgreSQL proper and thus not supported by -hackers;
you should report this directly to the authors.

Ehh.. Shouldn't we try to take a bit more care that we reset things
after a CREATE EXTENSION is run? Not really sure how much effort we
want to put into it, but I see a bit of blame on both sides here.

Fair enough but "reset" to what? I don't know the internal mechanics but
if the session default is "warning" and a local change sets it to "notice"
then an unconditional reset would not get us back to the intended value.

Now, if extensions can be handled similarly to how functions operate, where
one can define function/extension -local settings and have them revert
after resolution that might be ok.

That said, while there isn't any way to prevent it the log_min GUCs should
not be changed by extensions; that decision should be left up to the user.
The complaint is not that it should be reset but that the installation
script should not even care what the setting is.

David J.

#5Stephen Frost
sfrost@snowman.net
In reply to: David Johnston (#4)
Re: Small bug on CREATE EXTENSION pgq...

* David Johnston (david.g.johnston@gmail.com) wrote:

On Wednesday, January 28, 2015, Stephen Frost <sfrost@snowman.net> wrote:

Ehh.. Shouldn't we try to take a bit more care that we reset things
after a CREATE EXTENSION is run? Not really sure how much effort we
want to put into it, but I see a bit of blame on both sides here.

Fair enough but "reset" to what? I don't know the internal mechanics but
if the session default is "warning" and a local change sets it to "notice"
then an unconditional reset would not get us back to the intended value.

Yeah, we'd really want to reset it to "what it was before."

Now, if extensions can be handled similarly to how functions operate, where
one can define function/extension -local settings and have them revert
after resolution that might be ok.

That, imv, is really what should be happening inside the create
extension script.. I'm not anxious to look into how to make that happen
though.

That said, while there isn't any way to prevent it the log_min GUCs should
not be changed by extensions; that decision should be left up to the user.
The complaint is not that it should be reset but that the installation
script should not even care what the setting is.

I agree that the script really shouldn't be changing it.

Thanks!

Stephen

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#5)
Re: Small bug on CREATE EXTENSION pgq...

Stephen Frost <sfrost@snowman.net> writes:

* David Johnston (david.g.johnston@gmail.com) wrote:

Fair enough but "reset" to what? I don't know the internal mechanics but
if the session default is "warning" and a local change sets it to "notice"
then an unconditional reset would not get us back to the intended value.

Yeah, we'd really want to reset it to "what it was before."

An extension script runs as a single transaction, so SET LOCAL could've
been used to accomplish the result without trashing the session-lifespan
setting.

I'm not sure whether or not there was good reason to be changing the
setting at all, but it's entirely on the extension script's head that
it didn't do this in a less invasive way.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Tom Lane (#6)
Re: Small bug on CREATE EXTENSION pgq...

On 1/28/15 10:25 PM, Tom Lane wrote:

Stephen Frost <sfrost@snowman.net> writes:

* David Johnston (david.g.johnston@gmail.com) wrote:

Fair enough but "reset" to what? I don't know the internal mechanics but
if the session default is "warning" and a local change sets it to "notice"
then an unconditional reset would not get us back to the intended value.

Yeah, we'd really want to reset it to "what it was before."

An extension script runs as a single transaction, so SET LOCAL could've
been used to accomplish the result without trashing the session-lifespan
setting.

I'm not sure whether or not there was good reason to be changing the
setting at all, but it's entirely on the extension script's head that
it didn't do this in a less invasive way.

+1

One thing I have wished for is something akin to SET LOCAL that reverts at the end of a subtransaction. Typically I only need to tweak something like client_min_messages for a single command, so I'd prefer to

SAVEPOINT old_setting;
SET LOCAL blah;
command;
RELEASE old_setting;
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers