Final /contrib cleanup -- yes/no?

Started by Josh Berkusabout 17 years ago10 messages
#1Josh Berkus
josh@agliodbs.com

All,

Looking at my old thread I realized I never got an answer on whether
people agreed with these two items:

1) Take the "SET search_path=public" out of all contrib SQL scripts so
that DBAs can determine the correct schema by using PGOPTIONS.

2) Add BEGIN/COMMIT to all SQL scripts.

--Josh

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#1)
Re: Final /contrib cleanup -- yes/no?

Josh Berkus <josh@agliodbs.com> writes:

1) Take the "SET search_path=public" out of all contrib SQL scripts so
that DBAs can determine the correct schema by using PGOPTIONS.

I don't recall that having been proposed, and I don't think it's really
a good idea. We intentionally put those SETs in, not that long ago.

2) Add BEGIN/COMMIT to all SQL scripts.

The effects of that haven't been debated, either. Are you sure none of
those scripts rely on surviving errors? What about the possibility of
other scripts including them when already inside a BEGIN block?

The thing we really need to make that stuff nice is a proper module
facility. Changing stuff at the margins in the meantime doesn't really
do much except create more different possible behaviors that people will
have to deal with.

regards, tom lane

#3Josh Berkus
josh@agliodbs.com
In reply to: Tom Lane (#2)
Re: Final /contrib cleanup -- yes/no?

Tom,

I don't recall that having been proposed, and I don't think it's really
a good idea. We intentionally put those SETs in, not that long ago.

I haven't been able to find any reasoning on any list why those SETs
where a good idea. Bruce put them in, but apparently without
discussion. Unless you have a link for something I can't find in search?

The way the SQL scripts currently work, there is no way to manage what
schema the contrib modules get built in *except* to edit the scripts.
In fact, because of the SET statements, a DBA who might *reasonably*
expect that setting PGOPTIONS would allow him to determine that will be
unpleasantly surprised when the module ends up in "public" anyway.

For that matter, I really don't see the point of explicitly setting the
default schema ("public") in the scripts. Why bother?

The effects of that haven't been debated, either. Are you sure none of
those scripts rely on surviving errors? What about the possibility of
other scripts including them when already inside a BEGIN block?

Hmmm, I can see that. Not that important given that we have the remove
scripts. I need to finish testing whether the remove scripts actually
remove everything, though.

The thing we really need to make that stuff nice is a proper module
facility. Changing stuff at the margins in the meantime doesn't really
do much except create more different possible behaviors that people will
have to deal with.

Yeah, but we're clearly not getting that done for 8.4, so I'm trying to
do a little admin cleanup to live with for the next year. This isn't
based on idle conjecture; this came up again because I'm writing scripts
to automatically build PostgreSQL servers, and the SET search_path thing
keeps biting me on the tuchas.

--Josh

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#3)
Re: Final /contrib cleanup -- yes/no?

Josh Berkus <josh@agliodbs.com> writes:

The way the SQL scripts currently work, there is no way to manage what
schema the contrib modules get built in *except* to edit the scripts.

Right, that's the intended and documented way to do it.

In fact, because of the SET statements, a DBA who might *reasonably*
expect that setting PGOPTIONS would allow him to determine that will be
unpleasantly surprised when the module ends up in "public" anyway.

I don't see that this is a reasonable expectation; it has never worked
in any previous release, and the documentation explicitly says to do the
other. Also, at least some of the proposed forms of a module facility
would have the effect of overriding any such approach anyhow.

Again, I'm not for whacking around the procedures for dealing with
contrib each time we make a release. We should change it once when we
have a shot at getting it right.

regards, tom lane

#5Joshua D. Drake
jd@commandprompt.com
In reply to: Tom Lane (#4)
Re: Final /contrib cleanup -- yes/no?

On Thu, 2008-11-06 at 17:24 -0500, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

The way the SQL scripts currently work, there is no way to manage what
schema the contrib modules get built in *except* to edit the scripts.

Right, that's the intended and documented way to do it.

I believe the intention is a bad one. They should be installed per the
settings of the user installing them. Whether that be through an ALTER
ROLE/USER or PGOPTIONS.

Joshua D. Drake

--

#6Josh Berkus
josh@agliodbs.com
In reply to: Joshua D. Drake (#5)
Re: Final /contrib cleanup -- yes/no?

Joshua D. Drake wrote:

On Thu, 2008-11-06 at 17:24 -0500, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

The way the SQL scripts currently work, there is no way to manage what
schema the contrib modules get built in *except* to edit the scripts.

Right, that's the intended and documented way to do it.

I believe the intention is a bad one. They should be installed per the
settings of the user installing them. Whether that be through an ALTER
ROLE/USER or PGOPTIONS.

Eh, Tom has a point. If we build module loading for 8.5, we shouldn't
change the functionality in the interim for 8.4. Annoying as it is.

--Josh

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#6)
Re: Final /contrib cleanup -- yes/no?

Josh Berkus <josh@agliodbs.com> writes:

Eh, Tom has a point. If we build module loading for 8.5, we shouldn't
change the functionality in the interim for 8.4. Annoying as it is.

The main reason I'm concerned about it is that when we do modules
(which I certainly hope happens for 8.5) we would then have two
different old behaviors to worry about compatibility with.
I'm afraid of painting ourselves into a corner.

regards, tom lane

#8Robert Treat
xzilla@users.sourceforge.net
In reply to: Josh Berkus (#6)
Re: Final /contrib cleanup -- yes/no?

On Thursday 06 November 2008 17:35:58 Josh Berkus wrote:

Joshua D. Drake wrote:

On Thu, 2008-11-06 at 17:24 -0500, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

The way the SQL scripts currently work, there is no way to manage what
schema the contrib modules get built in *except* to edit the scripts.

Right, that's the intended and documented way to do it.

I believe the intention is a bad one. They should be installed per the
settings of the user installing them. Whether that be through an ALTER
ROLE/USER or PGOPTIONS.

Eh, Tom has a point. If we build module loading for 8.5, we shouldn't
change the functionality in the interim for 8.4. Annoying as it is.

Also be aware that some of those modules really do require being in the public
schema (which I have learned the hard way). It's probably possible to make
them work in different schemas, but just changing the install schema will
break them right now (i think intarray is an example of one)

--
Robert Treat
Conjecture: http://www.xzilla.net
Consulting: http://www.omniti.com

#9Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#7)
Re: Final /contrib cleanup -- yes/no?

Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Eh, Tom has a point. If we build module loading for 8.5, we shouldn't
change the functionality in the interim for 8.4. Annoying as it is.

The main reason I'm concerned about it is that when we do modules
(which I certainly hope happens for 8.5) we would then have two
different old behaviors to worry about compatibility with.
I'm afraid of painting ourselves into a corner.

I personally find the current way *very* annoying, because it keeps
sticking things in public when I don't want to - yes, that's because I
keep forgetting to edit the scripts, but it sucks that I should have to.

But. I think this argument is a winner. *IF* we get modules, which we
have been talking about for a long time, in the next version. But there
seems to be more interest now than before, so I think the probability is
better than it used to be ;-) Which means the argument for not creating
"yet another way" wins in my book.

//Magnus

#10Bruce Momjian
bruce@momjian.us
In reply to: Josh Berkus (#3)
Re: Final /contrib cleanup -- yes/no?

Josh Berkus wrote:

Tom,

I don't recall that having been proposed, and I don't think it's really
a good idea. We intentionally put those SETs in, not that long ago.

I haven't been able to find any reasoning on any list why those SETs
where a good idea. Bruce put them in, but apparently without
discussion. Unless you have a link for something I can't find in search?

The SET search_path was added to create a consistent install experience
for all the /contrib modules. We did kind of talked about it in
relation to the file label:

http://archives.postgresql.org/pgsql-hackers/2007-11/msg00560.php

I can't find any specific mention of it but some files had it and some
didn't so I added it to all of them.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +