Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Matthew T. O'Connor
On Mon, 2004-08-02 at 19:26, Tom Lane wrote:
> I looked over this patch (sorry for the delay), and found a number of
> problems.

Thanks for the feedback, hopefully we can still get something in place
for 7.5.

> Bigger problems:
> 
> * I don't think you've thought through system shutdown at all.  The
> postmaster code as given will not try to shutdown the autovac daemon
> until the same time it shuts down the bgwriter, which is no good for
> a couple of reasons:

[ snip: lots of good reasons autovac should shutdown before bgwriter ]

I agree.  The thought had crossed my mind that autovac should shut down
first, but I'm really not sure how to make that happen.  I was only able
todo the backend integration at all by cribbing off the bgwriter example
(as it was was suggested I do).  So

I'll take a stab at this, but some direction would be appreciated. 

> * I hadn't quite focused before on the fact that this patch requires
> statically binding frontend libpq into the backend.  

[ snip ]

Based on the other part of this thread, I think this is solved by
dynamic linking?  But as I said in the other email, I will need help
making this happen.

> * AFAICS there is no provision for setting pg_user or pg_user_password,
> which means that the daemon won't actually be able to connect in
> non-TRUST environments.  I don't know what we do about this: putting
> such a password in a GUC variable is right out (because any user could
> read it) and putting it on the postmaster command line is no better
> (anyone on the same machine can see it).  Right at the moment we do not
> have any secure place for postmaster parameters.

Right, I didn't want to use GUC vars for this.  As I said in the other
email, I will take a whack at using a new special .autovacpasswd file in
the $PGDATA dir.

BTW, is this really something that needs to get solved?  It could just
be considered a limitation of the 7.5 implementation that it requires
local trust or ident authentication.

> Smaller problems:
> 
> * It still contains too much code copied-and-pasted from bgwriter,
> such as
>   ShutdownXLOG(0, 0);
>   DumpFreeSpaceMap(0, 0);
> autovac has *no* business doing that.  I don't have time to go through
> it line-by-line to see what else shouldn't have been copied.

Yeah, as I said above, I cribbed heavily off the bgwriter example.  I
left in most of the things that I was unsure about thinking that it
would be more obvious to remove the code then to readd it.  I'll remove
the above code and try to see if there is anything else that should go
away.

> * The patch reverts some recent changes in postmaster.c (write_stderr
> changes for instance).

I'm sure this is just due to the fact that the patch was submitted prior
to that work going in.  I'll update the patch.

> * Emitting this warning on every single iteration of the postmaster idle
> loop is excessive:
>   elog(WARNING, "pg_autovacuum: autovac is enabled, but requires stats_row_level 
> which is not enabled");
> and this one even more so:
>   if (!autovacuum_start_daemon)
>   elog(DEBUG1, "pg_autovacuum: not enabled");

I left it this way on purpose as it didn't want the admin to miss it,
and I thought they might if it only emits the warning once on postmaster
startup.  I figure that the admin see this while setting up their server
initially and never see it again.   I'm more then happy to change this
if you still think I should. 

> * Any message that's actually likely to be seen by users should be an
> ereport, not elog.  elog is okay for debugging messages and can't-happen
> errors, but not for messages that will be seen by ordinary users,
> because there's no support for message translation in elog.

I looked into changing my elog calls to ereport, but I thought it wasn't
necessary since ordinary users won't see these messages, they will only
be in the postmaster log file.

> * I don't think you've actually thought very carefully about elog-based
> error handling; for instance this bit:
> dbs->conn = db_connect(dbs);
> if (dbs->conn == NULL)
> {/* Serious problem: We can't connect to
>  * template1 */
> elog(ERROR, "pg_autovacuum: Cannot connect to template1, exiting.");
> exit(1);
> }
> Control won't make it to the exit(1)  (which is a darn good thing in
> itself, seeing that you are connected to shared memory and hence had
> better be using proc_exit()).  What *will* happen in the code as given
> is that control will bounce back to the sigsetjmp, which will recover
> and re-call AutoVacLoop, which would be okay except you just forgot
> any open backend connections you have (plus leaked all the memory in
> your datastructures).  Maybe it would be better if you did not have
> an error catcher but just aborted the process on any serious error,
> letting the postmaster start a new one.  (Thi

Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Matthew T. O'Connor
On Mon, 2004-08-02 at 21:53, Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > As far as libpq, can't pg_autovacuum dynamically load libpq like dblink
> > does?
> 
> Hmm, make the bulk of the autovac daemon be a shlib that is dynamically
> linked by just that subprocess?  Yeah, that might work.

Ok, but I will need someone else to help with this.

> > On the password issue, can't we use .pgpass in the postgres home
> > directory?
> 
> I thought of that after sending my initial email.  It's a fairly ugly
> solution, because it confuses logins/passwords that would be appropriate
> for interactive use with what you'd want the daemon to use.  But it
> might be sufficient as a stopgap.  Or possibly better, we could hack the
> autovac code to read the user and password from some protected file
> under $PGDATA.

Ok, I can do this.

> Both of these issues are things that would probably go away in the long
> run, since I doubt that we want to keep the fire-up-an-ordinary-backend
> model forever.  The thing that's troubling me at the moment is that we
> might need to spend a fair amount of work on turning what's only an
> intermediate code state into something that's usable and doesn't break
> any other stuff.  It might be better to hold off and spend that same
> work on the longer-range goal of direct integration.

I understand the concern.  I'm just pushing to get autovac into 7.5, I'm
sure there are lots of people on this list who could have done a better
job, but nobody working on it...

Anyway, I'm very willing to do as much of the lifting as I can. 

Matthew



---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Matthew T. O'Connor
On Mon, 2004-08-02 at 21:36, Bruce Momjian wrote:
> Tom Lane wrote:
> > I'm not sure what we do now.  I can't apply this in its current state,
> > and I do not have time to fix it.  I don't really want to push it in
> > and assume we can fix the problems during beta ...
> 
> I see.  :-(
> 
> I know Matthew just got back from being away so perhaps he has time to
> address some of these.  I say we see if he can and move on the other
> open items and see where this is when they are complete.

Yes I'm here, and I'm quite willing to put some more work into this.  I
really want to see autovac in 7.5/8.0.

I have been pushing for a while to get some feedback on my patch since I
knew it would be considered a little "rough around the edges" to put it
mildly.

I will do what I can to improve the patch as long as people think there
is a decent chance of getting it applied.

Matthew 


---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


[PATCHES] Epoch to timestamp conversion function patch

2004-08-02 Thread Michael Glaesemann
Please find attached two patches (one for pg_proc.h and another for 
supporting documentation) for two SQL functions: 
epoch_to_timestamp(integer) and epoch_to_timestamptz(double precision), 
which convert from UNIX epoch to the native PostgreSQL timestamp and 
timestamptz data types. The equivalent SQL code is

create function epoch_to_timestamp(integer)
	returns timestamp
	language sql as '
	select (\'epoch\'::timestamptz + $1 * \'1 
second\'::interval)::timestamp
	';

create function epoch_to_timestamptz(double precision)
	returns timestamptz
	language sql as '
	select (\'epoch\'::timestamp + $1 * \'1 second\'::interval) at time 
zone \'UTC\'
	';

Some very simple tests (all should return TRUE):
test=# select epoch_to_timestamp(extract(epoch from 
current_timestamp)::integer) = current_timestamp::timestamp(0);
 ?column?
--
 t
(1 row)

test=# select epoch_to_timestamptz(extract(epoch from 
current_timestamp)::integer) = current_timestamp(0);
 ?column?
--
 t
(1 row)

test=# select epoch_to_timestamptz(extract(epoch from 
current_timestamp)) = current_timestamp;
 ?column?
--
 t
(1 row)

If regression tests are desired, I'll work some up. Any feedback 
appreciated.

Michael Glaesemann
grzm myrealbox com


func.sgml.diff
Description: Binary data


pg_proc.h.diff
Description: Binary data

---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES] psql latex bugfixes

2004-08-02 Thread Christopher Kings-Lynne
OK, when I find a moment.
Bruce Momjian wrote:
If you would like to review it I will apply it.

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
  http://www.postgresql.org/docs/faqs/FAQ.html


Re: [PATCHES] psql latex bugfixes

2004-08-02 Thread Bruce Momjian

If you would like to review it I will apply it.

---

Christopher Kings-Lynne wrote:
> Surely this is a really good bug fix and should be in 7.5?
> 
> Bruce Momjian wrote:
> 
> > This has been saved for the 7.6 release:
> > 
> > http:/momjian.postgresql.org/cgi-bin/pgpatches2
> > 
> > ---
> > 
> > Roger Leigh wrote:
> > 
> >>I have noticed that the latex format in psql has some bugs:
> >>
> >>? "_" is not escaped, and causes TeX to abort, thinking it's a
> >>  subscript outside of maths mode.  Most of my table and field names
> >>  use underscores, so this is a really nasty one.
> >>? The column count is calculated using the contents of opt_align.  But
> >>  opt_align has one extra element, and so it's always one too many.  I
> >>  changed it to count the column headings, like all the other output
> >>  formats.  There may be a bug in computing opt_align that this patch
> >>  does not address, but I'm not yet familiar enough with the psql
> >>  source to fix this as well.
> >>? The line drawing rules for each border setting (0-3) and expanded
> >>  mode didn't always match the documented behaviour and what other
> >>  formats (e.g. aligned) did.  I made it as conformant as possible,
> >>  and also tidied the alignment of the first line of the footer, which
> >>  was incorrectly indented.
> >>
> >>I've attached some example output with this patch applied.
> >>
> >>Regards,
> >>Roger
> >>
> >>
> >>Index: src/bin/psql/print.c
> >>===
> >>RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
> >>retrieving revision 1.48
> >>diff -u -r1.48 print.c
> >>--- src/bin/psql/print.c23 May 2004 22:20:10 -  1.48
> >>+++ src/bin/psql/print.c1 Aug 2004 22:54:22 -
> >>@@ -769,7 +769,7 @@
> >> 
> >> 
> >> /*/
> >>-/* LaTeX*/
> >>+/* LaTeX*/
> >> /*/
> >> 
> >> 
> >>@@ -790,6 +790,9 @@
> >>case '$':
> >>fputs("\\$", fout);
> >>break;
> >>+   case '_':
> >>+   fputs("\\_", fout);
> >>+   break;
> >>case '{':
> >>fputs("\\{", fout);
> >>break;
> >>@@ -817,7 +820,6 @@
> >> {
> >>unsigned int col_count = 0;
> >>unsigned int i;
> >>-   const char *cp;
> >>const char *const * ptr;
> >> 
> >> 
> >>@@ -829,42 +831,39 @@
> >>fputs("\n\\end{center}\n\n", fout);
> >>}
> >> 
> >>+   /* count columns */
> >>+   for (ptr = headers; *ptr; ptr++)
> >>+   col_count++;
> >>+
> >>/* begin environment and set alignments and borders */
> >>fputs("\\begin{tabular}{", fout);
> >>-   if (opt_border == 0)
> >>-   fputs(opt_align, fout);
> >>-   else if (opt_border == 1)
> >>-   {
> >>-   for (cp = opt_align; *cp; cp++)
> >>-   {
> >>-   if (cp != opt_align)
> >>-   fputc('|', fout);
> >>-   fputc(*cp, fout);
> >>-   }
> >>-   }
> >>-   else if (opt_border == 2)
> >>+
> >>+   if (opt_border == 2)
> >>+ fputs("| ", fout);
> >>+for (i = 0; i < col_count; i++)
> >>{
> >>-   for (cp = opt_align; *cp; cp++)
> >>-   {
> >>-   fputc('|', fout);
> >>-   fputc(*cp, fout);
> >>-   }
> >>-   fputc('|', fout);
> >>+ fputc(*(opt_align + i), fout);
> >>+ if (opt_border != 0 && i < col_count - 1)
> >>+   fputs (" | ", fout);
> >>}
> >>+   if (opt_border == 2)
> >>+ fputs(" |", fout);
> >>+
> >>fputs("}\n", fout);
> >> 
> >>if (!opt_barebones && opt_border == 2)
> >>fputs("\\hline\n", fout);
> >> 
> >>/* print headers and count columns */
> >>-   for (i = 0, ptr = headers; *ptr; i++, ptr++)
> >>+   for (i = 0, ptr = headers; i < col_count; i++, ptr++)
> >>{
> >>-   col_count++;
> >>if (!opt_barebones)
> >>{
> >>if (i != 0)
> >>fputs(" & ", fout);
> >>+fputs("\\textit{", fout);
> >>latex_escaped_print(*ptr, fout);
> >>+fputc('}', fout);
> >>}
> >>}
> >> 
> >>@@ -888,7 +887,7 @@
> >>if (opt_border == 2)
> >>fputs("\\hline\n", fout);
> >> 
> >>-   fputs("\\end{tabular}\n\n", fout);
> >>+   fputs("\\end{tabular}\n\n\\noindent ", fout);
> >> 
> >> 
> >>/* print footers */
> >>@@ -951,8 +950,12 @@
> >>if (!opt_barebones)
> >>{
> >>if (opt_border == 2)
> >>+   {
> >>fputs("\\

Re: [PATCHES] psql latex bugfixes

2004-08-02 Thread Christopher Kings-Lynne
Surely this is a really good bug fix and should be in 7.5?
Bruce Momjian wrote:
This has been saved for the 7.6 release:
http:/momjian.postgresql.org/cgi-bin/pgpatches2
---
Roger Leigh wrote:
I have noticed that the latex format in psql has some bugs:
? "_" is not escaped, and causes TeX to abort, thinking it's a
 subscript outside of maths mode.  Most of my table and field names
 use underscores, so this is a really nasty one.
? The column count is calculated using the contents of opt_align.  But
 opt_align has one extra element, and so it's always one too many.  I
 changed it to count the column headings, like all the other output
 formats.  There may be a bug in computing opt_align that this patch
 does not address, but I'm not yet familiar enough with the psql
 source to fix this as well.
? The line drawing rules for each border setting (0-3) and expanded
 mode didn't always match the documented behaviour and what other
 formats (e.g. aligned) did.  I made it as conformant as possible,
 and also tidied the alignment of the first line of the footer, which
 was incorrectly indented.
I've attached some example output with this patch applied.
Regards,
Roger
Index: src/bin/psql/print.c
===
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.48
diff -u -r1.48 print.c
--- src/bin/psql/print.c23 May 2004 22:20:10 -  1.48
+++ src/bin/psql/print.c1 Aug 2004 22:54:22 -
@@ -769,7 +769,7 @@
/*/
-/* LaTeX*/
+/* LaTeX*/
/*/
@@ -790,6 +790,9 @@
case '$':
fputs("\\$", fout);
break;
+   case '_':
+   fputs("\\_", fout);
+   break;
case '{':
fputs("\\{", fout);
break;
@@ -817,7 +820,6 @@
{
unsigned int col_count = 0;
unsigned int i;
-   const char *cp;
const char *const * ptr;
@@ -829,42 +831,39 @@
fputs("\n\\end{center}\n\n", fout);
}
+   /* count columns */
+   for (ptr = headers; *ptr; ptr++)
+   col_count++;
+
/* begin environment and set alignments and borders */
fputs("\\begin{tabular}{", fout);
-   if (opt_border == 0)
-   fputs(opt_align, fout);
-   else if (opt_border == 1)
-   {
-   for (cp = opt_align; *cp; cp++)
-   {
-   if (cp != opt_align)
-   fputc('|', fout);
-   fputc(*cp, fout);
-   }
-   }
-   else if (opt_border == 2)
+
+   if (opt_border == 2)
+ fputs("| ", fout);
+for (i = 0; i < col_count; i++)
{
-   for (cp = opt_align; *cp; cp++)
-   {
-   fputc('|', fout);
-   fputc(*cp, fout);
-   }
-   fputc('|', fout);
+ fputc(*(opt_align + i), fout);
+ if (opt_border != 0 && i < col_count - 1)
+   fputs (" | ", fout);
}
+   if (opt_border == 2)
+ fputs(" |", fout);
+
fputs("}\n", fout);
if (!opt_barebones && opt_border == 2)
fputs("\\hline\n", fout);
/* print headers and count columns */
-   for (i = 0, ptr = headers; *ptr; i++, ptr++)
+   for (i = 0, ptr = headers; i < col_count; i++, ptr++)
{
-   col_count++;
if (!opt_barebones)
{
if (i != 0)
fputs(" & ", fout);
+fputs("\\textit{", fout);
latex_escaped_print(*ptr, fout);
+fputc('}', fout);
}
}
@@ -888,7 +887,7 @@
if (opt_border == 2)
fputs("\\hline\n", fout);
-   fputs("\\end{tabular}\n\n", fout);
+   fputs("\\end{tabular}\n\n\\noindent ", fout);
/* print footers */
@@ -951,8 +950,12 @@
if (!opt_barebones)
{
if (opt_border == 2)
+   {
fputs("\\hline\n", fout);
-   fprintf(fout, "\\multicolumn{2}{c}{Record %d} \n", 
record++);
+   fprintf(fout, 
"\\multicolumn{2}{|c|}{\\textit{Record %d}} \n", record++);
+   }
+   else
+   fprintf(fout, "\\multicolumn{2}{c}{\\textit{Record 
%d}} \n", record++);
}

Re: [PATCHES] Troff -ms output for psql

2004-08-02 Thread Bruce Momjian

Perhaps.  :-)

---

Christopher Kings-Lynne wrote:
> You mean 8.1 release :P
> 
> Bruce Momjian wrote:
> 
> > This has been saved for the 7.6 release:
> > 
> > http:/momjian.postgresql.org/cgi-bin/pgpatches2
> 
> 
> ---(end of broadcast)---
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match


Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > As far as libpq, can't pg_autovacuum dynamically load libpq like dblink
> > does?
> 
> Hmm, make the bulk of the autovac daemon be a shlib that is dynamically
> linked by just that subprocess?  Yeah, that might work.

We certainly don't want to put any of these issues into a normal
backend.

> 
> > On the password issue, can't we use .pgpass in the postgres home
> > directory?
> 
> I thought of that after sending my initial email.  It's a fairly ugly
> solution, because it confuses logins/passwords that would be appropriate
> for interactive use with what you'd want the daemon to use.  But it
> might be sufficient as a stopgap.  Or possibly better, we could hack the
> autovac code to read the user and password from some protected file
> under $PGDATA.
> 
> Both of these issues are things that would probably go away in the long
> run, since I doubt that we want to keep the fire-up-an-ordinary-backend
> model forever.  The thing that's troubling me at the moment is that we
> might need to spend a fair amount of work on turning what's only an
> intermediate code state into something that's usable and doesn't break
> any other stuff.  It might be better to hold off and spend that same
> work on the longer-range goal of direct integration.

Well, it is going to be a local connect so hopefully some can use
local/ident and not worry about the passswords.

The big question is how much flexibility do we have on getting this into
7.5 while still being fair to Matthew?

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [PATCHES] Troff -ms output for psql

2004-08-02 Thread Christopher Kings-Lynne
You mean 8.1 release :P
Bruce Momjian wrote:
This has been saved for the 7.6 release:
	http:/momjian.postgresql.org/cgi-bin/pgpatches2

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
   (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> As far as libpq, can't pg_autovacuum dynamically load libpq like dblink
> does?

Hmm, make the bulk of the autovac daemon be a shlib that is dynamically
linked by just that subprocess?  Yeah, that might work.

> On the password issue, can't we use .pgpass in the postgres home
> directory?

I thought of that after sending my initial email.  It's a fairly ugly
solution, because it confuses logins/passwords that would be appropriate
for interactive use with what you'd want the daemon to use.  But it
might be sufficient as a stopgap.  Or possibly better, we could hack the
autovac code to read the user and password from some protected file
under $PGDATA.

Both of these issues are things that would probably go away in the long
run, since I doubt that we want to keep the fire-up-an-ordinary-backend
model forever.  The thing that's troubling me at the moment is that we
might need to spend a fair amount of work on turning what's only an
intermediate code state into something that's usable and doesn't break
any other stuff.  It might be better to hold off and spend that same
work on the longer-range goal of direct integration.

regards, tom lane

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Bruce Momjian
Tom Lane wrote:
> I'm not sure what we do now.  I can't apply this in its current state,
> and I do not have time to fix it.  I don't really want to push it in
> and assume we can fix the problems during beta ...

I see.  :-(

I know Matthew just got back from being away so perhaps he has time to
address some of these.  I say we see if he can and move on the other
open items and see where this is when they are complete.

As far as libpq, can't pg_autovacuum dynamically load libpq like dblink
does?  On the password issue, can't we use .pgpass in the postgres home
directory?

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [PATCHES] Patch for pg_dump: Multiple -t options and new -T option

2004-08-02 Thread Bruce Momjian

This has been saved for the 7.6 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---

Tom Lane wrote:
> "David F. Skoll" <[EMAIL PROTECTED]> writes:
> > On Wed, 21 Jul 2004, Tom Lane wrote:
> >> pg_dump -t s1.t1 -t s2.t2  -- Dump s1.t1 and s2.t2
> 
> > That's a good idea, but then it's questionable whether we need the -n
> > switch at all.
> 
> Sure we do --- for backwards compatibility if nothing else.
> 
> > It might be simpler to extend the -t switch to accept:
> > pg-dump -t 's1.*'
> 
> That would not be the same thing --- that would mean to dump *only tables*
> from s1, rather than objects of all types.  Anyway, I think it's a bit
> late in this cycle to be proposing to implement wild-card matching.
> Maybe for next time someone can do that, but for 7.5 I think we should
> limit ourselves to cleaning up any design flaws of the already-submitted
> patch.
> 
>   regards, tom lane
> 
> ---(end of broadcast)---
> TIP 7: don't forget to increase your free space map settings
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster


Re: [PATCHES] psql latex bugfixes

2004-08-02 Thread Bruce Momjian

This has been saved for the 7.6 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---

Roger Leigh wrote:
> I have noticed that the latex format in psql has some bugs:
> 
> ? "_" is not escaped, and causes TeX to abort, thinking it's a
>   subscript outside of maths mode.  Most of my table and field names
>   use underscores, so this is a really nasty one.
> ? The column count is calculated using the contents of opt_align.  But
>   opt_align has one extra element, and so it's always one too many.  I
>   changed it to count the column headings, like all the other output
>   formats.  There may be a bug in computing opt_align that this patch
>   does not address, but I'm not yet familiar enough with the psql
>   source to fix this as well.
> ? The line drawing rules for each border setting (0-3) and expanded
>   mode didn't always match the documented behaviour and what other
>   formats (e.g. aligned) did.  I made it as conformant as possible,
>   and also tidied the alignment of the first line of the footer, which
>   was incorrectly indented.
> 
> I've attached some example output with this patch applied.
> 
> Regards,
> Roger
> 
> 
> Index: src/bin/psql/print.c
> ===
> RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
> retrieving revision 1.48
> diff -u -r1.48 print.c
> --- src/bin/psql/print.c  23 May 2004 22:20:10 -  1.48
> +++ src/bin/psql/print.c  1 Aug 2004 22:54:22 -
> @@ -769,7 +769,7 @@
>  
>  
>  /*/
> -/* LaTeX  */
> +/* LaTeX  */
>  /*/
>  
>  
> @@ -790,6 +790,9 @@
>   case '$':
>   fputs("\\$", fout);
>   break;
> + case '_':
> + fputs("\\_", fout);
> + break;
>   case '{':
>   fputs("\\{", fout);
>   break;
> @@ -817,7 +820,6 @@
>  {
>   unsigned int col_count = 0;
>   unsigned int i;
> - const char *cp;
>   const char *const * ptr;
>  
>  
> @@ -829,42 +831,39 @@
>   fputs("\n\\end{center}\n\n", fout);
>   }
>  
> + /* count columns */
> + for (ptr = headers; *ptr; ptr++)
> + col_count++;
> +
>   /* begin environment and set alignments and borders */
>   fputs("\\begin{tabular}{", fout);
> - if (opt_border == 0)
> - fputs(opt_align, fout);
> - else if (opt_border == 1)
> - {
> - for (cp = opt_align; *cp; cp++)
> - {
> - if (cp != opt_align)
> - fputc('|', fout);
> - fputc(*cp, fout);
> - }
> - }
> - else if (opt_border == 2)
> +
> + if (opt_border == 2)
> +   fputs("| ", fout);
> +for (i = 0; i < col_count; i++)
>   {
> - for (cp = opt_align; *cp; cp++)
> - {
> - fputc('|', fout);
> - fputc(*cp, fout);
> - }
> - fputc('|', fout);
> +   fputc(*(opt_align + i), fout);
> +   if (opt_border != 0 && i < col_count - 1)
> + fputs (" | ", fout);
>   }
> + if (opt_border == 2)
> +   fputs(" |", fout);
> +
>   fputs("}\n", fout);
>  
>   if (!opt_barebones && opt_border == 2)
>   fputs("\\hline\n", fout);
>  
>   /* print headers and count columns */
> - for (i = 0, ptr = headers; *ptr; i++, ptr++)
> + for (i = 0, ptr = headers; i < col_count; i++, ptr++)
>   {
> - col_count++;
>   if (!opt_barebones)
>   {
>   if (i != 0)
>   fputs(" & ", fout);
> +fputs("\\textit{", fout);
>   latex_escaped_print(*ptr, fout);
> +fputc('}', fout);
>   }
>   }
>  
> @@ -888,7 +887,7 @@
>   if (opt_border == 2)
>   fputs("\\hline\n", fout);
>  
> - fputs("\\end{tabular}\n\n", fout);
> + fputs("\\end{tabular}\n\n\\noindent ", fout);
>  
>  
>   /* print footers */
> @@ -951,8 +950,12 @@
>   if (!opt_barebones)
>   {
>   if (opt_border == 2)
> + {
>   fputs("\\hline\n", fout);
> - fprintf(fout, "\\multicolumn{2}{c}{Record %d} \n", 
> record++);
> + fprintf(fout, 
> "\\multicolumn{2}{|c|}{\\textit{Record %d}} \n", record++);
> + }
> + else
> + fprintf(fout, 
> "\\multicolumn{2}{c}{

Re: [PATCHES] Troff -ms output for psql

2004-08-02 Thread Bruce Momjian

This has been saved for the 7.6 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---

Roger Leigh wrote:
> Hello,
> 
> I've created a patch which adds support for troff "-ms" output to
> psql.  i.e. "\pset format troff-ms".  The patch also corrects some
> problems with the "latex" format, notably defining an extra column in
> the output table, and correcting some alignment issues; it also
> changes the output to match the border setting as documented in the
> manual page and as shown with the "aligned" format.
> 
> The troff-ms output is mostly identical to the latex output allowing
> for the differences between the two typesetters.
> 
> The output should be saved in a file and piped as follows:
> 
>   cat file | tbl | troff -T ps -ms > file.ps
> or
>   tbl file | troff -T ps -ms > file.ps
> 
> Because it contains tabs, you'll need to redirect psql output or use
> "script", rather than pasting from a terminal window, due to the tabs
> which can be replaced with spaces.
> 
> I've attached the patch (against the current mainline), and example
> output of each for each of border=[123] and expanded mode on and off
> (both source and printable copy).
> 
> 
> Regards,
> Roger
> 
> 
> PS.  I'm not subscribed, so I would appreciate a CC on any replies.
> Thanks!
> 
> 
> -- 
> Roger Leigh
> 
> Printing on GNU/Linux?  http://gimp-print.sourceforge.net/
> GPG Public Key: 0x25BFB848.  Please sign and encrypt your mail.

Content-Description: patch to add troff output to psql

[ text/x-patch is unsupported, treating like TEXT/PLAIN ]

> Index: doc/src/sgml/ref/psql-ref.sgml
> ===
> RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/psql-ref.sgml,v
> retrieving revision 1.119
> diff -u -r1.119 psql-ref.sgml
> --- doc/src/sgml/ref/psql-ref.sgml15 Jul 2004 03:56:04 -  1.119
> +++ doc/src/sgml/ref/psql-ref.sgml1 Aug 2004 17:36:46 -
> @@ -1372,9 +1372,10 @@
>
>
>Sets the output format to one of unaligned,
> -  aligned, html, or
> -  latex. Unique abbreviations are allowed.
> -  (That would mean one letter is enough.)
> +  aligned, html,
> +  latex, or troff-ms.
> +   Unique abbreviations are allowed.  (That would mean one letter
> +   is enough.)
>
>  
>
> Index: src/bin/psql/command.c
> ===
> RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/command.c,v
> retrieving revision 1.122
> diff -u -r1.122 command.c
> --- src/bin/psql/command.c15 Jul 2004 03:56:06 -  1.122
> +++ src/bin/psql/command.c1 Aug 2004 17:36:53 -
> @@ -1307,6 +1307,9 @@
>   case PRINT_LATEX:
>   return "latex";
>   break;
> + case PRINT_TROFF_MS:
> + return "troff-ms";
> + break;
>   }
>   return "unknown";
>  }
> @@ -1335,9 +1338,11 @@
>   popt->topt.format = PRINT_HTML;
>   else if (pg_strncasecmp("latex", value, vallen) == 0)
>   popt->topt.format = PRINT_LATEX;
> + else if (pg_strncasecmp("troff-ms", value, vallen) == 0)
> + popt->topt.format = PRINT_TROFF_MS;
>   else
>   {
> - psql_error("\\pset: allowed formats are unaligned, aligned, 
> html, latex\n");
> + psql_error("\\pset: allowed formats are unaligned, aligned, 
> html, latex, troff-ms\n");
>   return false;
>   }
>  
> Index: src/bin/psql/print.c
> ===
> RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
> retrieving revision 1.48
> diff -u -r1.48 print.c
> --- src/bin/psql/print.c  23 May 2004 22:20:10 -  1.48
> +++ src/bin/psql/print.c  1 Aug 2004 17:36:56 -
> @@ -769,7 +769,7 @@
>  
>  
>  /*/
> -/* LaTeX  */
> +/* LaTeX  */
>  /*/
>  
>  
> @@ -790,6 +790,9 @@
>   case '$':
>   fputs("\\$", fout);
>   break;
> + case '_':
> + fputs("\\_", fout);
> + break;
>   case '{':
>   fputs("\\{", fout);
>   break;
> @@ -817,7 +820,6 @@
>  {
>   unsigned int col_count = 0;
>   unsigned int i;
> - const char *cp;
>   const char *const * ptr;
>  
>  
> @@ -829,42 +831,39 @@
>   fputs("\n\\end{center}\n\n", fout);
>   }
>  
> + /* count columns */
> + for (ptr = headers

Re: [PATCHES] autovauum integration patch: Attempt #4

2004-08-02 Thread Tom Lane
"Matthew T. O'Connor" <[EMAIL PROTECTED]> writes:
> Please apply to CVS or tell me what I need to change to get it applied.

I looked over this patch (sorry for the delay), and found a number of
problems.

Bigger problems:

* I don't think you've thought through system shutdown at all.  The
postmaster code as given will not try to shutdown the autovac daemon
until the same time it shuts down the bgwriter, which is no good for
a couple of reasons:

(a) None of this code will trigger as long as there is any ordinary
backend running; like say the one(s) invoked by autovac itself.
This means that autovac-driven vacuuming is considered just as good a
reason to keep the system going as an ordinary user query, which I think
is not cool.  IMHO a SIGTERM to the postmaster should result in
canceling whatever autovacuum operation is currently running.

(b) It's really not cool that autovac isn't shut down *before* we start
shutting down bgwriter.  I think that it might not make too much
difference right at the moment, since the autovac daemon isn't actually
making any use of its connection to shared memory, but the moment that
autovac tries to do anything on its own behalf rather than via a backend
this is going to be a serious risk.  There can't be anything going on in
parallel with the shutdown checkpoint.

I think really we want autovac to shut down at the beginning of the
shutdown cycle, not the end, and not to start bgwriter shutdown until
autovac is gone.

* I hadn't quite focused before on the fact that this patch requires
statically binding frontend libpq into the backend.  There are a number
of issues with that, the most risky being that if libpq.so is compiled
thread-aware then it's going to create problems on platforms where
there's a difference between thread-aware and non-thread-aware C library
code.  Even without threading worries there are conflicts: if someone
calls a dllist.c routine, which instance will they get?  I'm also
concerned about the implications for modules like contrib/dblink, which
expect to load libpq.so dynamically.  There could be conflicts between
the dynamically linked libpq and the inbuilt one.

* AFAICS there is no provision for setting pg_user or pg_user_password,
which means that the daemon won't actually be able to connect in
non-TRUST environments.  I don't know what we do about this: putting
such a password in a GUC variable is right out (because any user could
read it) and putting it on the postmaster command line is no better
(anyone on the same machine can see it).  Right at the moment we do not
have any secure place for postmaster parameters.

Smaller problems:

* It still contains too much code copied-and-pasted from bgwriter,
such as
ShutdownXLOG(0, 0);
DumpFreeSpaceMap(0, 0);
autovac has *no* business doing that.  I don't have time to go through
it line-by-line to see what else shouldn't have been copied.

* The patch reverts some recent changes in postmaster.c (write_stderr
changes for instance).

* Emitting this warning on every single iteration of the postmaster idle
loop is excessive:
elog(WARNING, "pg_autovacuum: autovac is enabled, but requires stats_row_level 
which is not enabled");
and this one even more so:
if (!autovacuum_start_daemon)
elog(DEBUG1, "pg_autovacuum: not enabled");

* Any message that's actually likely to be seen by users should be an
ereport, not elog.  elog is okay for debugging messages and can't-happen
errors, but not for messages that will be seen by ordinary users,
because there's no support for message translation in elog.

* I don't think you've actually thought very carefully about elog-based
error handling; for instance this bit:
dbs->conn = db_connect(dbs);
if (dbs->conn == NULL)
{/* Serious problem: We can't connect to
 * template1 */
elog(ERROR, "pg_autovacuum: Cannot connect to template1, exiting.");
exit(1);
}
Control won't make it to the exit(1)  (which is a darn good thing in
itself, seeing that you are connected to shared memory and hence had
better be using proc_exit()).  What *will* happen in the code as given
is that control will bounce back to the sigsetjmp, which will recover
and re-call AutoVacLoop, which would be okay except you just forgot
any open backend connections you have (plus leaked all the memory in
your datastructures).  Maybe it would be better if you did not have
an error catcher but just aborted the process on any serious error,
letting the postmaster start a new one.  (This ties into your FIXME
note about how the postmaster should react to autovac crashes...)

* autovacuum.h exports way too much stuff that is not relevant to
other modules.  (Rule #1: you don't declare static functions in a
header file; these *will* provoke warnings.)


I'm not sure what we do now.  I can't apply this in its current

Re: [PATCHES] win32 readline

2004-08-02 Thread Peter Eisentraut
Marc G. Fournier wrote:
> Stupid question, but for Windows ... how many will actually use psql,
> vs something like PgAdmin?

I don't see any reason to believe that it would be significantly less 
than now.

> I'd say revisit it later myself ... it
> isn't critical to the operation of the server, only a 'luxury item'
> that we've all gotten used to being there :)

By that logic, approximately 37% of all features count as luxury items. 
:)

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [PATCHES] win32 readline

2004-08-02 Thread Marc G. Fournier
On Mon, 2 Aug 2004, Bruce Momjian wrote:
Where are we on this?  Right now readline is disabled on Win32.  psql
works fine for me.  In fact it is more native because the delete key
deletes, and control-d doesn't exit.
I am inclined to leave the code unchanged and we can revisit this later
after we figure out why readline doesn't work on some Win32 versions.
Stupid question, but for Windows ... how many will actually use psql, vs 
something like PgAdmin?  I'd say revisit it later myself ... it isn't 
critical to the operation of the server, only a 'luxury item' that we've 
all gotten used to being there :)


---
Peter Eisentraut wrote:
Magnus Hagander wrote:
Just for reference, what features are we losing without readline. Tab
completion, but what more?
Command history, customizable key bindings, cut and paste, cursor keys
come to mind.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly
--
 Bruce Momjian|  http://candle.pha.pa.us
 [EMAIL PROTECTED]   |  (610) 359-1001
 +  If your life is a hard drive, |  13 Roberts Road
 +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
 subscribe-nomail command to [EMAIL PROTECTED] so that your
 message can get through to the mailing list cleanly

Marc G. Fournier   Hub.Org Networking Services (http://www.hub.org)
Email: [EMAIL PROTECTED]   Yahoo!: yscrappy  ICQ: 7615664
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
 joining column's datatypes do not match


Re: [PATCHES] win32 readline

2004-08-02 Thread Bruce Momjian

Where are we on this?  Right now readline is disabled on Win32.  psql
works fine for me.  In fact it is more native because the delete key
deletes, and control-d doesn't exit.

I am inclined to leave the code unchanged and we can revisit this later
after we figure out why readline doesn't work on some Win32 versions.

---

Peter Eisentraut wrote:
> Magnus Hagander wrote:
> > Just for reference, what features are we losing without readline. Tab
> > completion, but what more?
> 
> Command history, customizable key bindings, cut and paste, cursor keys 
> come to mind.
> 
> -- 
> Peter Eisentraut
> http://developer.postgresql.org/~petere/
> 
> 
> ---(end of broadcast)---
> TIP 3: if posting/reading through Usenet, please send an appropriate
>   subscribe-nomail command to [EMAIL PROTECTED] so that your
>   message can get through to the mailing list cleanly
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly


Re: [PATCHES]Traditional Chinese postgres-zh_TW.po for 7.5

2004-08-02 Thread Peter Eisentraut
Zhenbang Wei wrote:
> Translate more messages in addition to fix existing ones.

Installed.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES]Traditional Chinese initdb-zh_TW.po for 7.5

2004-08-02 Thread Peter Eisentraut
Zhenbang Wei wrote:
> 3 new messages translated.

Installed.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES] Random regression failures

2004-08-02 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> I ran some loops myself and couldn't reproduce it anymore.  I will wait
> to see if it happens again. 

Hm.  I let mine go for about 400 "make check" cycles and didn't see
anything.  Doesn't prove the 401st wouldn't have failed though :-(

I'm wondering whether you may have seen a bug that was fixed in the
recent nested-xact mopup patch.  Do you have time to pull CVS from 
say Friday or Saturday and see if the problem comes back?

regards, tom lane

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [PATCHES] Random regression failures

2004-08-02 Thread Bruce Momjian

I ran some loops myself and couldn't reproduce it anymore.  I will wait
to see if it happens again. 

Sorry.

---

Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > I am seeing random parallel regression failures.  I would say 20% of
> > the time I see a failure on the regression test.
> 
> FWIW, my other machine has been running repeated parallel regress tests
> for several hours.  It's now up to 130 completed cycles with no failures
> (except one that I deliberately induced to make sure the shell script
> would notice...)  This is a clean build from a CVS pull at about 14:30
> EDT today.  All parameters default except for
>   configure --enable-debug --enable-cassert
> 
>   regards, tom lane
> 
> ---(end of broadcast)---
> TIP 4: Don't 'kill -9' the postmaster
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [PATCHES] pg_config

2004-08-02 Thread Bruce Momjian

OK, addition made, and I added documentation for -pgxs.

---

Andrew Dunstan wrote:
> 
> 
> Peter Eisentraut wrote:
> 
> >Bruce Momjian wrote:
> >  
> >
> >>Oops, sorry, done.
> >>
> >>
> >
> >The --pgxs option seems to have gotten lost in the conversion.
> >
> >  
> >
> 
> It wasn't there when I did the conversion. Usual problem of duelling 
> patches. Can you fix it, or do you need me to send in a patch?
> 
> cheers
> 
> andrew
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/pg_config-ref.sgml
===
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/pg_config-ref.sgml,v
retrieving revision 1.17
diff -c -c -r1.17 pg_config-ref.sgml
*** doc/src/sgml/ref/pg_config-ref.sgml 29 Nov 2003 19:51:39 -  1.17
--- doc/src/sgml/ref/pg_config-ref.sgml 2 Aug 2004 12:31:43 -
***
*** 25,30 
--- 25,31 
  --includedir-server
  --libdir
  --pkglibdir
+ --pgxs
  --configure
  --version
 
***
*** 101,106 
--- 102,116 
  
  
  
+  --pgxs
+  
+   
+Print the location of extension makefiles.
+  
+  
+ 
+ 
+ 
   --configure
   

Index: src/bin/pg_config/pg_config.c
===
RCS file: /cvsroot/pgsql-server/src/bin/pg_config/pg_config.c,v
retrieving revision 1.2
diff -c -c -r1.2 pg_config.c
*** src/bin/pg_config/pg_config.c   1 Aug 2004 14:01:36 -   1.2
--- src/bin/pg_config/pg_config.c   2 Aug 2004 12:31:48 -
***
*** 43,48 
--- 43,49 
printf(_("  --includedir-server   show location of C header files for the 
server\n"));
printf(_("  --libdir  show location of object code libraries\n"));
printf(_("  --pkglibdir   show location of dynamically loadable 
modules\n"));
+   printf(_("  --pgxsshow location of extension makefile\n"));
printf(_("  --configure   show options given to 'configure' script 
when\n"));
printf(_("PostgreSQL was built\n"));
printf(_("  --version show the PostgreSQL version, then exit\n"));
***
*** 81,86 
--- 82,88 
strcmp(argv[i],"--includedir-server") == 0 ||
strcmp(argv[i],"--libdir") == 0 ||
strcmp(argv[i],"--pkglibdir") == 0 ||
+   strcmp(argv[i],"--pgxs") == 0 ||
strcmp(argv[i],"--configure") == 0)
{
/* come back to these later */
***
*** 136,141 
--- 138,148 
get_lib_path(mypath,otherpath);
else if (strcmp(argv[i],"--pkglibdir") == 0)
get_pkglib_path(mypath,otherpath);
+   else if (strcmp(argv[i],"--pgxs") == 0)
+   {
+   get_pkglib_path(mypath,otherpath);
+   strncat(otherpath, "/pgxs", MAXPGPATH-1);
+   }
  
printf("%s\n",otherpath);
}

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faqs/FAQ.html


Re: [PATCHES] fix schema ownership on first connection preliminary patch

2004-08-02 Thread Bruce Momjian

Is there a TODO here?

---

Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Fabien COELHO wrote:
> >> Please find attached a preliminary patch to fix schema ownership on first
> >> connection. It is for comments and advices as I still have doubts about
> >> various how-and-where issues, thus it is not submitted to the patch list.
> 
> > I have added the v2 version of this patch to the patch queue (attached).
> 
> I do apologize for not having looked at this patch sooner, but it's been
> at the bottom of the priority queue :-(
> 
> In general I do not like the approach of using SPI for this, because
> it has to execute before the system is really quite fully up.  Just
> looking at InitPostgres, I note that the namespace search path isn't
> set yet, for example, and I'm not real sure what that implies for
> execution of these queries.  I'm also uncomfortable with the fact that
> RelationCacheInitializePhase3 isn't done yet --- the SPI execution could
> pollute the relcache with stuff we don't really want written into
> the relcache cache file.  (Much of this could be dodged by having
> ReverifyMyDatabase just pass back the datinit flag, and then taking
> action on it (if any) at the bottom of InitPostgres instead of where
> the patch puts it.  But I'm still against using SPI for it.)
> 
> Also, since we already have AlterSchemaOwner coded at the C level,
> the original rationale of not wanting to add code has been rendered
> moot.
> 
> I do not like the hardwired assumption that userID 1 exists and is
> a superuser.  The code is also broken to assume that ID 1 is the owner
> of the public schema in the source database (though this part is at
> least easy to fix, and would go away anyway if using AlterSchemaOwner).
> 
> Lastly, I'm unconvinced that the (lack of) locking is safe.  Two
> backends trying to connect at the same time would make conflicting
> attempts to UPDATE pg_database, and if the default transaction isolation
> mode is SERIALIZABLE then one of them is going to get an actual failure,
> not just decide it has nothing to do.  A possible alternative is to take
> out ExclusiveLock on pg_namespace before re-examining pg_database.
> 
> However, enough about implementation deficiencies.  Did we really have
> consensus that the system should do this in the first place?  I was
> only about halfway sold on the notion of changing public's ownership.
> I especially dislike the detail that it will alter the ownership of
> *all* non-builtin schemas, not only "public".  If someone has added
> special-purpose schemas to template1, it seems unlikely that this is
> the behavior they'd want.
> 
> I'm also wondering about what side-effects this will have on pg_dump
> behavior.  In particular, will pg_dump try to "ALTER OWNER public",
> and if so will that be appropriate?  We haven't previously needed to
> assume that we are restoring into a database with the same datowner as
> we dumped from...
> 
> I think we should leave the behavior alone, at least for this release
> cycle.
> 
>   regards, tom lane
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 8: explain analyze is your friend