Re: [PATCHES] patch: garbage error strings in libpq

2005-07-05 Thread jtv
Tom Lane wrote:
> [EMAIL PROTECTED] writes:
>> Another approach would have been to make libpq_gettext() preserve errno.
>
> That seems like a far easier, cleaner, and more robust fix than this.

Provided that either:

(a) the C standard has added a sequence point between the arguments in a
function call, which AFAIK wasn't there before, or the sequence point was
there all along (and the compiler implements it);
(b) the compiler is sufficiently naive;
(c) you get lucky with instruction scheduling on your particular
architecture.

This is why I called this approach was "tempting," but didn't go for it. 
I felt it was better to really fix the instances I found first, then see
what patterns emerge and refactor.

Like maybe a wrapper for printfPQExpBuffer() that takes a PGconn *, an
untranslated format string, and varargs; this in turn can do the
libpq_gettext().  That would cover all uses of printfPQExpBuffer() in
libpq--except for one of the out-of-memory errors where no translation is
done, which may have been unintentional (and this bug is again duplicated
in the code).


> Moreover I don't believe that this approach works either, as the result
> of strerror() is not guaranteed still usable after another strerror call
> (ie, it can use a static buffer repeatedly), so you'd still have the
> problem if libpq_gettext invokes strerror.  I suppose that a really
> robust solution would involve libpq_gettext saving errno, restoring
> errno, and invoking strerror() again ...

Check again.  The calls to strerror() are routed through pqStrerror()
which copies the error message to the buffer, or in the case of GNU
strerror_r(), at least ensures it is in some reusable location.


Jeroen



---(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] enable/disable trigger (Re: Fwd: [HACKERS] Open items)

2005-07-05 Thread Bruce Momjian
Satoshi Nagayasu wrote:
> Bruce Momjian wrote:
> > I am not sure what to do with this patch.  It is missing dump
> > capability, there is no clause to disable all triggers on a table, and
> > it uses a table owner check when a super user check is required (because
> > of referential integrity).
> > 
> > Satoshi, are you still working on it?  If not does someone else want to
> > complete it?  I realized you just started on it but the deadline is
> > soon.
> 
> I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
> and the superuser check has been added.  Please check it.
> 
> And, I'm going to have a business trip to Sydney this weekend,
> so I'll complete pg_dump stuffs while my flight.

OK, but once our patch queue is empty, we will need to process your
patch.  My guess is that we are several days away from that.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Quick little \h enhancement for psql

2005-07-05 Thread Bruce Momjian
Greg Sabino Mullane wrote:
> 
> http://archives.postgresql.org/pgsql-patches/2005-05/msg00197.php

Applied.

---

Attached is a patch that enhances the "\h" capability in psql. I often
find myself typing a command and then wanting to get the syntax for
it. So I do a ctrl-a and add a \h: but psql does not recognize the
command, because I have stuff attached to it (e.g. "alter table
foobar"),
so I have to scroll over and delete everything except the name of the
command itself. This patch gives \h three chances to match: if nothing
matches the complete string (current behavior), it tries to match the
first two words (e.g. "ALTER TABLE"). If that fails, it tries to match
the first word (e.g. "DELETE").

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: src/bin/psql/help.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.102
diff -c -c -r1.102 help.c
*** src/bin/psql/help.c 14 Jun 2005 02:57:41 -  1.102
--- src/bin/psql/help.c 6 Jul 2005 02:18:59 -
***
*** 305,356 
}
else
{
!   int i;
boolhelp_found = false;
FILE   *output;
!   size_t  len;
int nl_count = 0;
char   *ch;
  
!   /* don't care about trailing spaces or semicolons */
len = strlen(topic);
while (topic[len - 1] == ' ' || topic[len - 1] == ';')
!   len--;
  
!   /* Count newlines for pager */
!   for (i = 0; QL_HELP[i].cmd; i++)
{
!   if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
!   strcmp(topic, "*") == 0)
!   {
!   nl_count += 5;
!   for (ch = QL_HELP[i].syntax; *ch != '\0'; ch++)
!   if (*ch == '\n')
!   nl_count++;
!   /* If we have an exact match, exit.  Fixes \h 
SELECT */
!   if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0)
!   break;
!   }
!   }
! 
!   output = PageOutput(nl_count, pager);
! 
!   for (i = 0; QL_HELP[i].cmd; i++)
!   {
!   if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
!   strcmp(topic, "*") == 0)
!   {
!   help_found = true;
!   fprintf(output, _("Command: %s\n"
! "Description: 
%s\n"
! 
"Syntax:\n%s\n\n"),
!   QL_HELP[i].cmd,
!   _(QL_HELP[i].help),
!   _(QL_HELP[i].syntax));
!   /* If we have an exact match, exit.  Fixes \h 
SELECT */
!   if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0)
!   break;
!   }
}
  
if (!help_found)
--- 305,382 
}
else
{
!   int i,j,x=0;
boolhelp_found = false;
FILE   *output;
!   size_t  len, wordlen;
int nl_count = 0;
char   *ch;
  
!   /* User gets two chances: exact match, then the first word */
!   
!   /* First pass : strip trailing spaces and semicolons */
len = strlen(topic);
while (topic[len - 1] == ' ' || topic[len - 1] == ';')
!   len--;
  
!   for (x=1; x<=3; x++) /* Three chances to guess that word... */
{
!   if (x>1) /* Nothing on first pass - try the 
opening words */
!   {
!   wordlen=j=1;
!   while (topic[j] != ' ' && 
j++= len) /* Don't 
try again if the same word */
!   {
!   output = 
P

Re: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  writes:
> > Tom Lane wrote:
> >> I could live with that.  Or "pg_total_relation_size".
> 
> > The problem with "total", to me, is that it already is the total size of
> > the heap/index/toast.  Complete has the idea of adding additional
> > pieces, which I think fits best.
> 
> [ shrug ]  I don't care --- if you do, then do that.
> 
> I finally realized exactly what was bugging me about "dbfile_size": it
> seems to imply that we are measuring the size of one *file*, which is
> under no circumstance the definition of any of these functions (see
> file splitting behavior for relations exceeding 1GB).

Yes, that is an issue I considered.  I was more relying on the _idea_
that people thought it was a single file, but that is an implementation
detail that shouldn't be promoted.

> pg_relation_size plus pg_complete_relation_size is fine.  Ship it...

OK.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Tom Lane
Bruce Momjian  writes:
> Tom Lane wrote:
>> I could live with that.  Or "pg_total_relation_size".

> The problem with "total", to me, is that it already is the total size of
> the heap/index/toast.  Complete has the idea of adding additional
> pieces, which I think fits best.

[ shrug ]  I don't care --- if you do, then do that.

I finally realized exactly what was bugging me about "dbfile_size": it
seems to imply that we are measuring the size of one *file*, which is
under no circumstance the definition of any of these functions (see
file splitting behavior for relations exceeding 1GB).

pg_relation_size plus pg_complete_relation_size is fine.  Ship it...

regards, tom lane

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


Re: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  writes:
> > If we go pg_table_size() and pg_relation_size(), which is object-only
> > and which is heap + index + toast?  I think ideally we want
> > pg_relation_size to be the combined one, but then we have pg_table_size
> > that works on indexes and toast too, and that is confusing, and we don't
> > want to add index and toast versions.  Or is an index a relation?  And
> > TOAST?
> 
> All the backend code thinks so --- anything that has an entry in
> pg_class is a relation.  So personally I don't find "table" and
> "relation" confusing in this context.  But I can see it might be
> confusing to people not familiar with PG jargon.
> 
> > OK, how about pg_relation_size for heap/index/toast, and
> > pg_complete_relation_size for the combined total.
> 
> I could live with that.  Or "pg_total_relation_size".

The problem with "total", to me, is that it already is the total size of
the heap/index/toast.  Complete has the idea of adding additional
pieces, which I think fits best.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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] enable/disable trigger (Re: Fwd: [HACKERS] Open items)

2005-07-05 Thread Satoshi Nagayasu
Bruce Momjian wrote:
> I am not sure what to do with this patch.  It is missing dump
> capability, there is no clause to disable all triggers on a table, and
> it uses a table owner check when a super user check is required (because
> of referential integrity).
> 
> Satoshi, are you still working on it?  If not does someone else want to
> complete it?  I realized you just started on it but the deadline is
> soon.

I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
and the superuser check has been added.  Please check it.

And, I'm going to have a business trip to Sydney this weekend,
so I'll complete pg_dump stuffs while my flight.

Thank you.
-- 
NAGAYASU Satoshi <[EMAIL PROTECTED]>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c 
pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c 2005-06-28 14:08:54.0 
+0900
--- pgsql/src/backend/commands/tablecmds.c  2005-07-01 15:50:27.0 
+0900
***
*** 236,241 
--- 236,243 

  Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+  bool 
enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***
*** 1993,1998 
--- 1995,2005 
}
pass = AT_PASS_DROP;
break;
+   case AT_EnableTrig: /* ENABLE TRIGGER */
+   case AT_DisableTrig:/* DISABLE TRIGGER */
+   ATSimplePermissions(rel, false);
+   pass = AT_PASS_MISC;
+   break;
case AT_SetTableSpace:  /* SET TABLESPACE */
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd->name);
***
*** 2155,2160 
--- 2162,2173 
 * Nothing to do here; Phase 3 does the work
 */
break;
+   case AT_EnableTrig: /* ENABLE TRIGGER */
+   ATExecEnableDisableTrigger(rel, cmd->name, true);
+   break;
+   case AT_DisableTrig:/* DISABLE TRIGGER */
+   ATExecEnableDisableTrigger(rel, cmd->name, false);
+   break;
default:/* oops */
elog(ERROR, "unrecognized alter table type: %d",
 (int) cmd->subtype);
***
*** 5465,5470 
--- 5478,5492 
  }
  
  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+   EnableDisableTrigger(rel, trigname, enable);
+ }
+ 
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c 
pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c   2005-05-30 16:20:58.0 
+0900
--- pgsql/src/backend/commands/trigger.c2005-07-04 10:40:27.0 
+0900
***
*** 3063,3065 
--- 3063,3158 
afterTriggerAddEvent(new_event);
}
  }
+ 
+ /* --
+  * EnableDisableTrigger()
+  *
+  *Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * --
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+   Relation tgrel;
+   SysScanDesc tgscan;
+   ScanKeyData keys[2];
+   HeapTuple tuple;
+   int nkeys;
+   int changed;
+ 
+   /* Permissions checks */
+   if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+   aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+  RelationGetRelationName(rel));
+ 
+   if (!allowSystemTableMods && IsSystemRelation(rel))
+   ereport(ERROR,
+   (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg("permission denied: \"%s\" is a system 
catalog",
+   RelationGetRelationName(rel;
+ 
+   tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+ 
+   nkeys = 2;
+   changed = 0;
+   if ( strcmp(tgname, "*")==0 )
+   {
+   if ( !superuser() )
+   elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed 
superuser only.");
+ 
+   nkeys = 1;
+   }
+ 
+   ScanKeyInit(&keys[0],
+  

Re: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Tom Lane
Bruce Momjian  writes:
> If we go pg_table_size() and pg_relation_size(), which is object-only
> and which is heap + index + toast?  I think ideally we want
> pg_relation_size to be the combined one, but then we have pg_table_size
> that works on indexes and toast too, and that is confusing, and we don't
> want to add index and toast versions.  Or is an index a relation?  And
> TOAST?

All the backend code thinks so --- anything that has an entry in
pg_class is a relation.  So personally I don't find "table" and
"relation" confusing in this context.  But I can see it might be
confusing to people not familiar with PG jargon.

> OK, how about pg_relation_size for heap/index/toast, and
> pg_complete_relation_size for the combined total.

I could live with that.  Or "pg_total_relation_size".

regards, tom lane

---(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] Disable page writes when fsync off, add GUC

2005-07-05 Thread Bruce Momjian
Bruce Momjian wrote:
> This also adds a full_page_writes GUC to turn off page writes to WAL. 
> Some people might not want full_page_writes.

Fsync linkage removed, patch attached and applied.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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/runtime.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.335
diff -c -c -r1.335 runtime.sgml
*** doc/src/sgml/runtime.sgml   2 Jul 2005 19:16:36 -   1.335
--- doc/src/sgml/runtime.sgml   5 Jul 2005 23:15:33 -
***
*** 1660,1666 
  
 
  This option can only be set at server start or in the
! postgresql.conf file.
 

   
--- 1660,1668 
  
 
  This option can only be set at server start or in the
! postgresql.conf file.  If this option
! is off, consider also turning off 
! guc-full-page-writes.
 

   
***
*** 1687,1692 
--- 1689,1725 

   
   
+  
+   
+full_page_writes configuration parameter
+   
+   full_page_writes (boolean)
+   
+
+ A page write in process during an operating system crash might
+ be only partially written to disk, leading to an on-disk page
+ that contains a mix of old and new data. During recovery, the
+ row changes stored in WAL are not enough to completely restore
+ the page.
+
+ 
+
+ When this option is on, the PostgreSQL server
+ writes full pages to WAL when they first modified after a checkpoint
+ so full recovery is possible. Turning this option off might lead
+ to a corrupt system after an operating system crash because
+ uncorrected partial pages might contain inconsistent or corrupt
+ data. The risks are less but similar to fsync.
+
+ 
+
+ This option can only be set at server start or in the
+ postgresql.conf file.  The default is
+ on.
+
+   
+  
+  
   
wal_buffers (integer)

Index: src/backend/access/transam/xlog.c
===
RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.206
diff -c -c -r1.206 xlog.c
*** src/backend/access/transam/xlog.c   4 Jul 2005 04:51:44 -   1.206
--- src/backend/access/transam/xlog.c   5 Jul 2005 23:15:36 -
***
*** 103,108 
--- 103,109 
  char *XLogArchiveCommand = NULL;
  char *XLOG_sync_method = NULL;
  const charXLOG_sync_method_default[] = DEFAULT_SYNC_METHOD_STR;
+ bool  fullPageWrites = true;
  
  #ifdef WAL_DEBUG
  bool  XLOG_DEBUG = false;
***
*** 594,600 
{
/* OK, put it in this slot */
dtbuf[i] = rdt->buffer;
!   if (XLogCheckBuffer(rdt, 
&(dtbuf_lsn[i]), &(dtbuf_xlg[i])))
{
dtbuf_bkp[i] = true;
rdt->data = NULL;
--- 595,603 
{
/* OK, put it in this slot */
dtbuf[i] = rdt->buffer;
!   /* If fsync is off, no need to backup 
pages. */
!   if (fullPageWrites &&
!   XLogCheckBuffer(rdt, 
&(dtbuf_lsn[i]), &(dtbuf_xlg[i])))
{
dtbuf_bkp[i] = true;
rdt->data = NULL;
Index: src/backend/utils/misc/guc.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.272
diff -c -c -r1.272 guc.c
*** src/backend/utils/misc/guc.c4 Jul 2005 04:51:51 -   1.272
--- src/backend/utils/misc/guc.c5 Jul 2005 23:15:39 -
***
*** 83,88 
--- 83,89 
  extern intCommitDelay;
  extern intCommitSiblings;
  extern char *default_tablespace;
+ extern bool   fullPageWrites;
  
  static const char *assign_log_destination(const char *value,
   bool doit, GucSource source);
***
*** 483,488 
--- 484,501 
false, NULL, NULL
},
{
+   {"full_page_writes", PGC_SIGHUP, WAL_SETTINGS,
+   

Re: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Bruce Momjian
Dave Page wrote:
> > >>You are into the cycle we were in.  We discussed pg_object size (too
> > >>vague) and pg_index_size (needs pg_toast_size too, and maybe toast
> > >>indexes; too many functions).
> > > 
> > > Yeah, I read those discussions, and think you were better 
> > off then than you 
> > > are now, which is why I went back to it somewhat.  
> > 
> > To be honest, the amount of effort being expended on this naming 
> > discussion far outweighs the benefits.  Maybe it's time for a core 
> > member to step in and just resolve it - one way or the other?
> 
> Agreed. The current names were discussed (at some length!) by Bruce & I
> before I reworked the latest version of the patch. Can we just settle on
> that?

If we go pg_table_size() and pg_relation_size(), which is object-only
and which is heap + index + toast?  I think ideally we want
pg_relation_size to be the combined one, but then we have pg_table_size
that works on indexes and toast too, and that is confusing, and we don't
want to add index and toast versions.  Or is an index a relation?  And
TOAST?

OK, how about pg_relation_size for heap/index/toast, and
pg_complete_relation_size for the combined total.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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] Dependencies on shared objects

2005-07-05 Thread Alvaro Herrera
On Tue, Jul 05, 2005 at 04:32:22PM -0400, Tom Lane wrote:
> Another question about this: why bother with dependencies on
> tablespaces?  That seems to me to be isomorphic with dependencies on
> databases --- we don't need those either, because in both cases we
> count on the filesystem to provide ground truth about which objects
> live inside a database/tablespace.

Because it appeared ugly to me to require information from the
filesystem for something that should be known internally in the
database (system catalogs).  No other reason that I remember.

-- 
Alvaro Herrera ()
"You knock on that door or the sun will be shining on places inside you
that the sun doesn't usually shine" (en Death: "The High Cost of Living")

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

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Dependencies on shared objects

2005-07-05 Thread Tom Lane
Another question about this: why bother with dependencies on
tablespaces?  That seems to me to be isomorphic with dependencies on
databases --- we don't need those either, because in both cases we
count on the filesystem to provide ground truth about which objects
live inside a database/tablespace.

regards, tom lane

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

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Dependencies on shared objects

2005-07-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> On Tue, Jul 05, 2005 at 02:47:15PM -0400, Tom Lane wrote:
>> Although I don't have any particular objection to the OWNER/NORMAL
>> distinction, your explanation doesn't seem to make sense.  Don't you
>> have to poke the Acl anyway, if it's non-null?  Else the grantor values
>> will be wrong.

> Hum, we don't register a dependency on the owner when registering
> dependencies from the Acl -- we actively skip that (because we know the
> owner has an entry of the other type already).  Is this still an issue?

Not sure.  ISTM the distinction we want to capture is more along the
lines of DEPENDENCY_NORMAL vs DEPENDENCY_AUTO --- that is, we should be
willing to auto-drop grants to a user when dropping the user, but not be
willing to auto-drop objects unless the drop is with CASCADE.  Also,
grants from the user to someone else (using grant options) probably
shouldn't go away without CASCADE either, though this is maybe
debatable.  If you believe the latter then the OWNER/ACL division is
clearly the wrong way to think about it.  So I'd be inclined to use
the NORMAL/AUTO terminology instead.

On the other hand: we might need to be able to express that "this
object's ACL depends on that user" as opposed to "this object depends on
that user" --- if you're really using the dependency type as a flag of
this kind, then we might have to keep it.  I've not gotten that far
in reading the patch yet.

regards, tom lane

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


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Matthew T. O'Connor
While we are at it (assuming the autovacuum patch gets included in 8.1) 
we have a few autovacuum related todo items. 


These are the ones I can think of right now:
* XID Wraparound improvement, moving to per-table vacuuming rather than 
per database. (8.2)

* Alter table commands to set per table autovacuum threshold settings. (8.2)
* Incorporate FSM data to improve vacuum decision making (8.2)
* Deal with stats reset better, possibly force stats reset to false if 
autovacuum is enabled. (8.2)

* Add the concept of a maintenance window to autovacuum. (maybe 8.1?)
* Have the VACUUM and ANALYZE commands update the pg_autovacuum table 
itself.  This will allow autovacuum to work in harmony with manually 
issued VACUUM's. (I would like to see this done for 8.1, but I will 
understand if people demand that it wait for 8.2)

* Add some regression tests?  Not sure what would be appropriate here. (8.1)
* Improve autovacuum threshold defaults (8.1)

Anyone have anything to add to the list?

Matthew



Bruce Momjian wrote:


TODO item?




---(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] Dependencies on shared objects

2005-07-05 Thread Alvaro Herrera
On Tue, Jul 05, 2005 at 02:47:15PM -0400, Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > I attach a patch to implement dependencies on shared objects.
> > As some of you may remember, the purpose of this patch is to record
> > dependencies on shared objects, such as roles and tablespaces, from
> > regular database objects.  This is done on a new shared system catalog
> > called pg_shdepend, so that when a backend wants to drop any shared
> > object, it can easily verify whether it is referenced in other database.
> 
> Will work on applying this next.

Cool.

> > - added a dependency type.  There are three types: PIN, same as normal
> >   dependencies; OWNER, for roles that own objects; NORMAL, all the rest
> >   (roles in the Acl and tablespaces).
> >   I needed to separate the OWNER entries to support changing ownership
> >   of objects without having to poke the whole Acl for the object.
> 
> Although I don't have any particular objection to the OWNER/NORMAL
> distinction, your explanation doesn't seem to make sense.  Don't you
> have to poke the Acl anyway, if it's non-null?  Else the grantor values
> will be wrong.

Hum, we don't register a dependency on the owner when registering
dependencies from the Acl -- we actively skip that (because we know the
owner has an entry of the other type already).  Is this still an issue?

-- 
Alvaro Herrera ()
"La verdad no siempre es bonita, pero el hambre de ella sí"

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


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Bruce Momjian
Matthew T. O'Connor wrote:
> I think so.  Something like: Improve autovacuum xid wraparound detection 
> by moving to a pertable solution rather than per database.

Thanks, added.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Matthew T. O'Connor
I think so.  Something like: Improve autovacuum xid wraparound detection 
by moving to a pertable solution rather than per database.


Matt


Bruce Momjian wrote:


TODO item?

 




---(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] Dependencies on shared objects

2005-07-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> I attach a patch to implement dependencies on shared objects.
> As some of you may remember, the purpose of this patch is to record
> dependencies on shared objects, such as roles and tablespaces, from
> regular database objects.  This is done on a new shared system catalog
> called pg_shdepend, so that when a backend wants to drop any shared
> object, it can easily verify whether it is referenced in other database.

Will work on applying this next.

> - added a dependency type.  There are three types: PIN, same as normal
>   dependencies; OWNER, for roles that own objects; NORMAL, all the rest
>   (roles in the Acl and tablespaces).
>   I needed to separate the OWNER entries to support changing ownership
>   of objects without having to poke the whole Acl for the object.

Although I don't have any particular objection to the OWNER/NORMAL
distinction, your explanation doesn't seem to make sense.  Don't you
have to poke the Acl anyway, if it's non-null?  Else the grantor values
will be wrong.

regards, tom lane

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

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Bruce Momjian

TODO item?

---

Alvaro Herrera wrote:
> On Tue, Jul 05, 2005 at 01:00:50PM -0400, Tom Lane wrote:
> > "Matthew T. O'Connor"  writes:
> > > Tom Lane wrote:
> > >> No, you're wrong.  VACUUMing of individual tables is perfectly good
> > >> enough as far as XID wrap protection goes, it's just that we chose to
> > >> track whether it had been done at the database level.  If we tracked it
> > >> in, say, a new pg_class column then in principle you could protect
> > >> against XID wrap with only table-at-a-time VACUUMs.
> > 
> > > Good, I'm glad I'm wrong on this.  This will be another nice advantage 
> > > of autovacuum then and should be fairly easy to do.  Any thoughts on 
> > > this being a change we can get in for 8.1?
> > 
> > I'd say this is probably a tad too late --- there's a fair amount of
> > code change that would be needed, none of which has been written, and
> > we are past the feature-freeze deadline for new code.
> 
> Right.  I've written a small, non-intrusive patch that handles the Xid
> wraparound just as pg_autovacuum used to, checking the Xid from
> pg_database.
> 
> -- 
> Alvaro Herrera ()
> "Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)
> 
> ---(end of broadcast)---
> TIP 4: Don't 'kill -9' the postmaster
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (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] Python setof patch

2005-07-05 Thread Bruce Momjian

OK, patch backed out, new and regression file removed.

---

Andrew Dunstan wrote:
> 
> 
> Michael Fuhr wrote:
> 
> >On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
> >  
> >
> >>Aside from minor problems like being broken and undocumented, there is a
> >>more serious question here: is this even the functionality we want?
> >>
> >>
> >
> >I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
> >return_next.
> >
> >  
> >
> 
> Agreed. My rudimentary testing shows that plperl's shiny new return_next 
> functionality not only avoids memory blowup but has a 40% speed 
> improvement over the old 'return the lot at once' API.
> 
> cheers
> 
> andrew
> 
> ---(end of broadcast)---
> TIP 7: don't forget to increase your free space map settings
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: src/pl/plpython/plpython.c
===
RCS file: /cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.62
retrieving revision 1.63
diff -c -r1.62 -r1.63
*** src/pl/plpython/plpython.c  6 May 2005 17:24:55 -   1.62
--- src/pl/plpython/plpython.c  4 Jul 2005 18:59:42 -   1.63
***
*** 29,35 
   * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
   *
   * IDENTIFICATION
!  *$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.62 2005/05/06 
17:24:55 tgl Exp $
   *
   *
   */
--- 29,35 
   * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
   *
   * IDENTIFICATION
!  *$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.63 2005/07/04 
18:59:42 momjian Exp $
   *
   *
   */
***
*** 286,291 
--- 286,294 
  static PyObject *PLy_exc_fatal = NULL;
  static PyObject *PLy_exc_spi_error = NULL;
  
+ /* End-of-set Indication */
+ static PyObject *PLy_endofset = NULL;
+ 
  /* some globals for the python module
   */
  static char PLy_plan_doc[] = {
***
*** 770,775 
--- 773,788 
fcinfo->isnull = true;
rv = (Datum) NULL;
}
+   /* test for end-of-set condition */
+   else if (fcinfo->flinfo->fn_retset && plrv == PLy_endofset)
+   {
+  ReturnSetInfo *rsi;
+ 
+  fcinfo->isnull = true;
+  rv = (Datum)NULL;
+  rsi = (ReturnSetInfo *)fcinfo->resultinfo;
+  rsi->isDone = ExprEndResult;
+   }
else
{
fcinfo->isnull = false;
***
*** 2317,2325 
--- 2330,2340 
PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+   PLy_endofset = PyErr_NewException("plpy.EndOfSet",NULL,NULL);
PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error);
PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal);
PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error);
+   PyDict_SetItemString(plpy_dict, "EndOfSet", PLy_endofset);
  
/*
 * initialize main module, and add plpy

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


Re: [PATCHES] Python setof patch

2005-07-05 Thread Andrew Dunstan



Michael Fuhr wrote:


On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
 


Aside from minor problems like being broken and undocumented, there is a
more serious question here: is this even the functionality we want?
   



I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
return_next.

 



Agreed. My rudimentary testing shows that plperl's shiny new return_next 
functionality not only avoids memory blowup but has a 40% speed 
improvement over the old 'return the lot at once' API.


cheers

andrew

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


Re: [PATCHES] Python setof patch

2005-07-05 Thread Michael Fuhr
On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
> 
> Aside from minor problems like being broken and undocumented, there is a
> more serious question here: is this even the functionality we want?

I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
return_next.

-- 
Michael Fuhr
http://www.fuhr.org/~mfuhr/

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


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Alvaro Herrera
On Tue, Jul 05, 2005 at 01:00:50PM -0400, Tom Lane wrote:
> "Matthew T. O'Connor"  writes:
> > Tom Lane wrote:
> >> No, you're wrong.  VACUUMing of individual tables is perfectly good
> >> enough as far as XID wrap protection goes, it's just that we chose to
> >> track whether it had been done at the database level.  If we tracked it
> >> in, say, a new pg_class column then in principle you could protect
> >> against XID wrap with only table-at-a-time VACUUMs.
> 
> > Good, I'm glad I'm wrong on this.  This will be another nice advantage 
> > of autovacuum then and should be fairly easy to do.  Any thoughts on 
> > this being a change we can get in for 8.1?
> 
> I'd say this is probably a tad too late --- there's a fair amount of
> code change that would be needed, none of which has been written, and
> we are past the feature-freeze deadline for new code.

Right.  I've written a small, non-intrusive patch that handles the Xid
wraparound just as pg_autovacuum used to, checking the Xid from
pg_database.

-- 
Alvaro Herrera ()
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)

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


Re: [PATCHES] Python setof patch

2005-07-05 Thread Tom Lane
Michael Fuhr <[EMAIL PROTECTED]> writes:
>> This patch allows the PL/Python module to do (SRF) functions.

> Does this patch work?

Aside from minor problems like being broken and undocumented, there is a
more serious question here: is this even the functionality we want?

The proposed test case is:

CREATE or replace FUNCTION test_setof() returns setof text
AS
'if GD.has_key("calls"):
GD["calls"] = GD["calls"] + 1
if GD["calls"] > 2:
del GD["calls"]
return plpy.EndOfSet
else:
GD["calls"] = 1
return str(GD["calls"])'
LANGUAGE plpythonu;

which is essentially exposing the old value-per-call SRF implementation
to the Python programmer.  Do we really want to do that?  plperl and
plpgsql have not taken that tack.  One reason this doesn't seem a
particularly great idea is that the above example would likely fail
if invoked twice in one query, due to it using only one global state
variable for both invocations.

regards, tom lane

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

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Tom Lane
"Matthew T. O'Connor"  writes:
> Tom Lane wrote:
>> No, you're wrong.  VACUUMing of individual tables is perfectly good
>> enough as far as XID wrap protection goes, it's just that we chose to
>> track whether it had been done at the database level.  If we tracked it
>> in, say, a new pg_class column then in principle you could protect
>> against XID wrap with only table-at-a-time VACUUMs.

> Good, I'm glad I'm wrong on this.  This will be another nice advantage 
> of autovacuum then and should be fairly easy to do.  Any thoughts on 
> this being a change we can get in for 8.1?

I'd say this is probably a tad too late --- there's a fair amount of
code change that would be needed, none of which has been written, and
we are past the feature-freeze deadline for new code.

regards, tom lane

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


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Matthew T. O'Connor

Tom Lane wrote:


"Matthew T. O'Connor"  writes:
 


The current implementation of XID wraparound requires that the vacuum
command be run against the entire database, you can not run it on a per
table basis and have it work.  At least that is my understanding,
   



No, you're wrong.  VACUUMing of individual tables is perfectly good
enough as far as XID wrap protection goes, it's just that we chose to
track whether it had been done at the database level.  If we tracked it
in, say, a new pg_class column then in principle you could protect
against XID wrap with only table-at-a-time VACUUMs.  (I think you'd
still want the pg_database column, but you'd update it to be the minimum
of the per-table values at the completion of any VACUUM.)

At the time this didn't seem particularly worth the complication since
no one would be likely to try to do that manually --- but with
autovacuum handling the work, it starts to sound more realistic.



Good, I'm glad I'm wrong on this.  This will be another nice advantage 
of autovacuum then and should be fairly easy to do.  Any thoughts on 
this being a change we can get in for 8.1?


Matt


---(end of broadcast)---
TIP 6: Have you searched our list archives?

  http://archives.postgresql.org


Re: [PATCHES] Autovacuum integration patch

2005-07-05 Thread Tom Lane
"Matthew T. O'Connor"  writes:
>>> Hmm.  Yes, this patch doesn't handle Xid wraparound.  This should be
>>> easy to add though.  Anyway, I was thinking that we could add a "last
>>> vacuum Xid" to pg_autovacuum, and handle Xid wraparound for each table
>>> separately -- this means you don't have to issue huge whole-database
>>> VACUUMs, because it will be handled nicely for each table.  Storing the
>>> last vacuum Xid in pg_database would have to be rethought.

> The current implementation of XID wraparound requires that the vacuum
> command be run against the entire database, you can not run it on a per
> table basis and have it work.  At least that is my understanding,

No, you're wrong.  VACUUMing of individual tables is perfectly good
enough as far as XID wrap protection goes, it's just that we chose to
track whether it had been done at the database level.  If we tracked it
in, say, a new pg_class column then in principle you could protect
against XID wrap with only table-at-a-time VACUUMs.  (I think you'd
still want the pg_database column, but you'd update it to be the minimum
of the per-table values at the completion of any VACUUM.)

At the time this didn't seem particularly worth the complication since
no one would be likely to try to do that manually --- but with
autovacuum handling the work, it starts to sound more realistic.

regards, tom lane

---(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] INSERT ... RETURNING

2005-07-05 Thread Tom Lane
[EMAIL PROTECTED] writes:
> Attached is a patch (by Gavin Sherry, fixed up to apply to 8.1 by me) that
> implements INSERT ... RETURNING functionality.

> It does work for the common case of RETURNING the value of a serial/sequence
> column, but gets confused when returning results out-of-order (CREATE TABLE x
> (a int, b int), INSERT ... RETURNING b, a) and doesn't let you specify the 
> same
> column multiple times (INSERT ... RETURNING b, b). These will be addressed
> soon.

This is pretty considerably shy of what I thought had been agreed to
anyway:

- should allow expressions not only column names

- should work for UPDATE and DELETE too

regards, tom lane

---(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] Python setof patch

2005-07-05 Thread Tom Lane
Michael Fuhr <[EMAIL PROTECTED]> writes:
> I'm running HEAD (8.1devel), which is where the patch was committed.
> Could somebody else test HEAD and see what they get?

Same as you --- ie, this patch is utterly broken.

> I don't see the setof functionality in the regression tests,
> presumably because there's no expected/plpython_setof.sql file:

That's because it wasn't added to the Makefile.  However, a test that
only defines a function and doesn't actually exercise it seems a bit
useless anyway :-(

regards, tom lane

---(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] Error handling fix in interfaces/libpq/fe-secure.c

2005-07-05 Thread Tom Lane
BTW, I read at

http://www.gnu.org/software/libc/manual/html_node/Translation-with-gettext.html

  The gettext function does not modify the value of the global errno
  variable. This is necessary to make it possible to write something like 

 printf (gettext ("Operation failed: %m\n"));

which is pretty much what I expected to find.  Ergo, this entire
discussion is wrong, and whatever bug you are concerned about will
not be solved this way.

What you may actually be running into is the problem that there are two
different definitions of strerror_r() out there, the SUS spec and the
GNU spec, and pre-8.0 we failed to distinguish these.  The 7.4 coding
will yield garbage messages in some cases when GNU strerror_r is in use.

regards, tom lane

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


Re: [PATCHES] Python setof patch

2005-07-05 Thread Michael Fuhr
On Tue, Jul 05, 2005 at 04:23:42PM +0200, Gerrit van Dyk wrote:
> 
> Ok, I just looked at the code again and the results I am getting, is 
> what you expect.
> 
> 2 rows every time returning 1 and 2.
> 
> What version of postgres are you running, I am running 8.0.3

I'm running HEAD (8.1devel), which is where the patch was committed.
Could somebody else test HEAD and see what they get?

-- 
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---(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] Error handling fix in interfaces/libpq/fe-secure.c

2005-07-05 Thread Tom Lane
[EMAIL PROTECTED] writes:
> Here's another one similar to what I described in my previous message.

More or less proves my point, no?  Even if you manage to fix every
occurence of this issue now, it'll keep popping up as people change
the code.  This approach is just not maintainable.

regards, tom lane

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] patch: garbage error strings in libpq

2005-07-05 Thread Tom Lane
[EMAIL PROTECTED] writes:
> Another approach would have been to make libpq_gettext() preserve errno. 

That seems like a far easier, cleaner, and more robust fix than this.

Moreover I don't believe that this approach works either, as the result
of strerror() is not guaranteed still usable after another strerror call
(ie, it can use a static buffer repeatedly), so you'd still have the
problem if libpq_gettext invokes strerror.  I suppose that a really
robust solution would involve libpq_gettext saving errno, restoring
errno, and invoking strerror() again ...

regards, tom lane

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


[PATCHES] pgcrypto volatility and strictness changes

2005-07-05 Thread Michael Fuhr
This patch updates the DDL for contrib/pgcrypto to create all
functions as STRICT, and all functions except gen_salt() as IMMUTABLE.
gen_salt() is VOLATILE.

Although the functions are now STRICT, I left their PG_ARGISNULL()
checks in place as a protective measure for users who install the
new code but use old (non-STRICT) catalog entries (e.g., restored
from a dump).  Per recent discussion in pgsql-hackers.

-- 
Michael Fuhr
http://www.fuhr.org/~mfuhr/
Index: contrib/pgcrypto/pgcrypto.sql.in
===
RCS file: /projects/cvsroot/pgsql/contrib/pgcrypto/pgcrypto.sql.in,v
retrieving revision 1.9
diff -c -r1.9 pgcrypto.sql.in
*** contrib/pgcrypto/pgcrypto.sql.in14 May 2003 03:25:56 -  1.9
--- contrib/pgcrypto/pgcrypto.sql.in5 Jul 2005 14:11:45 -
***
*** 4,74 
  CREATE OR REPLACE FUNCTION digest(text, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_digest'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION digest(bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_digest'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION digest_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_digest_exists'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION hmac(text, text, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_hmac'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION hmac(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_hmac'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION hmac_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_hmac_exists'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION crypt(text, text)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_crypt'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION gen_salt(text)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_gen_salt'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION gen_salt(text, int4)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_gen_salt_rounds'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION encrypt(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_encrypt'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION decrypt(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_decrypt'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION encrypt_iv(bytea, bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_encrypt_iv'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION decrypt_iv(bytea, bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_decrypt_iv'
! LANGUAGE 'C';
  
  CREATE OR REPLACE FUNCTION cipher_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_cipher_exists'
! LANGUAGE 'C';
  
  
--- 4,74 
  CREATE OR REPLACE FUNCTION digest(text, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_digest'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION digest(bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_digest'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION digest_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_digest_exists'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION hmac(text, text, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_hmac'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION hmac(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_hmac'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION hmac_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_hmac_exists'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION crypt(text, text)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_crypt'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION gen_salt(text)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_gen_salt'
! LANGUAGE 'C' VOLATILE STRICT;
  
  CREATE OR REPLACE FUNCTION gen_salt(text, int4)
  RETURNS text
  AS 'MODULE_PATHNAME', 'pg_gen_salt_rounds'
! LANGUAGE 'C' VOLATILE STRICT;
  
  CREATE OR REPLACE FUNCTION encrypt(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_encrypt'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION decrypt(bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_decrypt'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION encrypt_iv(bytea, bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_encrypt_iv'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION decrypt_iv(bytea, bytea, bytea, text)
  RETURNS bytea
  AS 'MODULE_PATHNAME', 'pg_decrypt_iv'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  CREATE OR REPLACE FUNCTION cipher_exists(text)
  RETURNS bool
  AS 'MODULE_PATHNAME', 'pg_cipher_exists'
! LANGUAGE 'C' IMMUTABLE STRICT;
  
  

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


Re: [PATCHES] Python setof patch

2005-07-05 Thread Michael Fuhr
On Mon, Jul 04, 2005 at 03:04:51PM -0400, Bruce Momjian wrote:
> 
> Patch applied.  Thanks.
> 
> Gerrit van Dyk wrote:
> > 
> > This patch allows the PL/Python module to do (SRF) functions.

Does this patch work?  The test_setof() function in sql/plpython_setof.sql
gives me the following:

SELECT * FROM test_setof();
 test_setof 

 1
(1 row)

If I call the function again I get this:

SELECT * FROM test_setof();
 test_setof 

 2
(1 row)

Calling the function a third time gives this:

SELECT * FROM test_setof();
 test_setof 

(0 rows)

Am I misreading the code, or shouldn't the function return two rows
on each invocation?

I don't see the setof functionality in the regression tests,
presumably because there's no expected/plpython_setof.sql file:

== running regression test queries==
test plpython_schema  ... ok
test plpython_populate... ok
test plpython_function... ok
test plpython_test... ok
test plpython_error   ... ok
test plpython_drop... ok

=
 All 6 tests passed. 
=

What about documentation updates?  Still in the works?

-- 
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


[PATCHES] Error handling fix in interfaces/libpq/fe-secure.c

2005-07-05 Thread jtv
Here's another one similar to what I described in my previous message.  In
libpq's pqsecure_read(), if SSL_read() returns -1 and sets an error of
SSL_ERROR_SYSCALL, errno may be polluted by libpq_gettext() before a
human-readable string is derived from it.  Also, pqReadData() will see the
wrong errno value after the call.

The attached patch fixes both by introducing a named variable to hold the
significant value of errno.


Jeroen
--- fe-secure.c.org	2005-07-05 19:45:19.0 +0700
+++ fe-secure.c	2005-07-05 19:55:26.0 +0700
@@ -340,9 +340,13 @@
 	char		sebuf[256];
 
 	if (n == -1)
+	{
+		const int errcode = SOCK_ERRNO;
 		printfPQExpBuffer(&conn->errorMessage,
 libpq_gettext("SSL SYSCALL error: %s\n"),
-		SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+		SOCK_STRERROR(errcode, sebuf, sizeof(sebuf)));
+		SOCK_ERRNO_SET(errcode);
+	}
 	else
 	{
 		printfPQExpBuffer(&conn->errorMessage,
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


[PATCHES] patch: garbage error strings in libpq

2005-07-05 Thread jtv
Several libpqxx users have been reporting odd problems with certain error
messages generated by libpq.  One of them was the inclusion of garbage
data.

As it turns out, src/interfaces/libpq/fe-misc.c contains several instances
of this construct:

  printfPQExpBuffer(&conn->ErrorMessage,
libpq_gettext("error: %s"),
SOCK_STRERROR(SOCK_ERRNO, buffer, sizeof(buffer)));

This may occur in other source files as well.  On Unix-like systems,
SOCK_ERRNO defines to plain errno--which is likely to be overwritten by
the libpq_gettext().  I'm attaching a patch that fixes these instances by
introducing a named pointer to the SOCK_STRERROR message, initialized
before either of the other function calls.

Another approach would have been to make libpq_gettext() preserve errno. 
It's tempting, but I'm not sure it would be valid from a language-lawyer
point of view.  There is no sequence point between the evaluations of
libpq_gettext() and SOCK_STRERROR().  From what I vaguely remember hearing
somewhere in the distant past, that means that theoretically they may be
evaluated not just in any order but even in parallel.  I guess it may
actually happen if both inlining and scheduling are sufficiently
aggressive.  Even if libpq_gettext() is made to restore errno, it will
still have to pollute errno at some points during its execution.


Jeroen
--- fe-misc.c.org	2005-07-05 17:48:25.0 +0700
+++ fe-misc.c	2005-07-05 18:13:03.0 +0700
@@ -23,7 +23,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.113 2005/02/22 04:42:20 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.114 2005/06/12 00:00:21 neilc Exp $
  *
  *-
  */
@@ -175,7 +175,8 @@
 	conn->inCursor += len;
 
 	if (conn->Pfdebug)
-		fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"), (unsigned long) len, (int) len, s);
+		fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"),
+(unsigned long) len, (int) len, s);
 
 	return 0;
 }
@@ -590,6 +591,7 @@
 		  conn->inBufSize - conn->inEnd);
 	if (nread < 0)
 	{
+		const char *errstr;
 		if (SOCK_ERRNO == EINTR)
 			goto retry3;
 		/* Some systems return EAGAIN/EWOULDBLOCK for no data */
@@ -606,9 +608,10 @@
 		if (SOCK_ERRNO == ECONNRESET)
 			goto definitelyFailed;
 #endif
+		errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf));
 		printfPQExpBuffer(&conn->errorMessage,
 			   libpq_gettext("could not receive data from server: %s\n"),
-		SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+		errstr);
 		return -1;
 	}
 	if (nread > 0)
@@ -681,6 +684,7 @@
 		  conn->inBufSize - conn->inEnd);
 	if (nread < 0)
 	{
+		const char *errstr;
 		if (SOCK_ERRNO == EINTR)
 			goto retry4;
 		/* Some systems return EAGAIN/EWOULDBLOCK for no data */
@@ -697,9 +701,10 @@
 		if (SOCK_ERRNO == ECONNRESET)
 			goto definitelyFailed;
 #endif
+		errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf));
 		printfPQExpBuffer(&conn->errorMessage,
 			   libpq_gettext("could not receive data from server: %s\n"),
-		SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+		errstr);
 		return -1;
 	}
 	if (nread > 0)
@@ -759,6 +764,7 @@
 
 		if (sent < 0)
 		{
+			const char *errstr;
 			/*
 			 * Anything except EAGAIN/EWOULDBLOCK/EINTR is trouble. If
 			 * it's EPIPE or ECONNRESET, assume we've lost the backend
@@ -799,9 +805,10 @@
 	return -1;
 
 default:
+	errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf));
 	printfPQExpBuffer(&conn->errorMessage,
 	libpq_gettext("could not send data to server: %s\n"),
-		SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+		errstr);
 	/* We don't assume it's a fatal error... */
 	conn->outCount = 0;
 	return -1;
@@ -986,10 +993,12 @@
 	if (result < 0)
 	{
 		char		sebuf[256];
+		const char *errstr;
 
+		errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf));
 		printfPQExpBuffer(&conn->errorMessage,
 		  libpq_gettext("select() failed: %s\n"),
-		SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+		  errstr);
 	}
 
 	return result;
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster


Re: [HACKERS] [PATCHES] [PATCH] pgcrypto: pgp_encrypt v3

2005-07-05 Thread Marko Kreen
On Tue, Jul 05, 2005 at 10:20:17AM +1000, Neil Conway wrote:
> Bruce Momjian wrote:
> >Your patch has been added to the PostgreSQL unapplied patches list
> 
> That is not the latest version of Marko's patch.

Bruce got v3, thats indeed the latest.

Also, http://momjian.postgresql.org/cgi-bin/pgpatches shows v3.


> But in any case, the 
> patch is not yet ready for application:
> 
> http://archives.postgresql.org/pgsql-patches/2005-07/msg00077.php

Now I did fresh rebuild of CVS and played with it.  Result
is that it is only partly my error.  It just happens that I did
initdb with option '-E unicode'.  Now, can anybody explain the
following difference:


$ psql -c '\l' template1; psql -c "select 'a\nxx'::text as
x;" template1
   List of databases
Name| Owner | Encoding
+---+---
 contrib_regression | marko | SQL_ASCII
 postgres   | marko | SQL_ASCII
 template0  | marko | SQL_ASCII
 template1  | marko | SQL_ASCII
(4 rows)

x
--
 a
xx
(1 row)

$ psql -c '\l' template1; psql -c "select 'a\nxx'::text as
x;" template1
   List of databases
Name| Owner | Encoding
+---+--
 contrib_regression | marko | UTF8
 postgres   | marko | UTF8
 template0  | marko | UTF8
 template1  | marko | UTF8
(4 rows)

 x
---
 a
xx
(1 row)

=

I can send new regression test for SQL_ASCII, but it still
would not work for all users.

-- 
marko



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


Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)

2005-07-05 Thread Andreas Pflug

Bruce Momjian wrote:


I am not sure what to do with this patch.  It is missing dump
capability, there is no clause to disable all triggers on a table, and
it uses a table owner check when a super user check is required (because
of referential integrity).
 

From a user's view, a trigger implementing RI isn't a trigger but an 
implementation detail he shouldn't need to care about. So for std 
triggers, owner check should be sufficient, requiring superuser for RI 
triggers only.
This impacts EN/DISABLE TRIGGER ALL too. To touch RI triggers as well, 
an additional keyword is needed.


Regards,
Andreas


---(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: [HACKERS] [PATCHES] Dbsize backend integration

2005-07-05 Thread Dave Page
 

> -Original Message-
> From: Christopher Kings-Lynne [mailto:[EMAIL PROTECTED] 
> Sent: 05 July 2005 02:39
> To: Robert Treat
> Cc: Bruce Momjian; Dave Page; Tom Lane; Dawid Kuroczko; 
> Andreas Pflug; PostgreSQL-patches; PostgreSQL-development
> Subject: Re: [HACKERS] [PATCHES] Dbsize backend integration
> 
> >>You are into the cycle we were in.  We discussed pg_object size (too
> >>vague) and pg_index_size (needs pg_toast_size too, and maybe toast
> >>indexes; too many functions).
> > 
> > Yeah, I read those discussions, and think you were better 
> off then than you 
> > are now, which is why I went back to it somewhat.  
> 
> To be honest, the amount of effort being expended on this naming 
> discussion far outweighs the benefits.  Maybe it's time for a core 
> member to step in and just resolve it - one way or the other?

Agreed. The current names were discussed (at some length!) by Bruce & I
before I reworked the latest version of the patch. Can we just settle on
that?

Regards, Dave.

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