[PATCHES] libpq events patch

2008-09-03 Thread Andrew Chernow

This is an updated version pf the libpqevents patch.  See

http://archives.postgresql.org/pgsql-hackers/2008-09/msg00153.php

for details.  The only change I didn't make yet is the event 'name'.  I 
have put it in and taken it out twice now, so a firm 'put it in there' 
would be appreciated.


Go here for libpqtypes using events.  pgfoundry is still using the older 
object hooks version.


http://libpqtypes.esilo.com/libpqtypes-events.tar.gz

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.166
diff -C6 -r1.166 Makefile
*** src/interfaces/libpq/Makefile   16 Apr 2008 14:19:56 -  1.166
--- src/interfaces/libpq/Makefile   3 Sep 2008 16:06:49 -
***
*** 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
!   md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o 
strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
--- 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
!   md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o 
libpq-events.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o 
strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
***
*** 103,114 
--- 103,115 
  
  $(top_builddir)/src/port/pg_config_paths.h:
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
  
  install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
+   $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h 
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample 
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  installdirs: installdirs-lib
$(mkinstalldirs) '$(DESTDIR)$(includedir)' 
'$(DESTDIR)$(includedir_internal)'
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt19 Mar 2008 00:39:33 -  1.19
--- src/interfaces/libpq/exports.txt3 Sep 2008 16:06:49 -
***
*** 138,143 
--- 138,154 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQcopyResult  142
+ PQsetResultAttrs  143
+ PQsetvalue144
+ PQresultAlloc 145
+ PQregisterEventProc   146
+ PQinstanceData147
+ PQsetInstanceData 148
+ PQresultInstanceData  149
+ PQresultSetInstanceData   150
+ PQpassThroughData 151
+ PQresultPassThroughData   152
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.359
diff -C6 -r1.359 fe-connect.c
*** src/interfaces/libpq/fe-connect.c   29 May 2008 22:02:44 -  1.359
--- src/interfaces/libpq/fe-connect.c   3 Sep 2008 16:06:49 -
***
*** 1971,1982 
--- 1971,2000 
   * release data that is to be held for the life of the PGconn structure.
   * If a value ought to be cleared/freed during PQreset(), do it there not 
here.
   */
  static void
  freePGconn(PGconn *conn)
  {
+   int i;
+   PGEventConnDestroy evt;
+ 
+   /* Let the event procs cleanup their state data */
+   for(i=0; i  conn-nEvents; i++)
+   {
+   evt.conn = conn;
+   (void)conn-events[i].proc(PGEVT_CONNDESTROY, evt);
+   }
+ 
+   /* free the PGEvent array */
+   if(conn-events)
+   {
+   free(conn-events);
+   conn-events = NULL;
+   conn-nEvents = conn-eventArrSize = 0;
+   }
+ 
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr

Re: [PATCHES] libpq events patch

2008-09-03 Thread Andrew Chernow

Andrew Chernow wrote:

This is an updated version pf the libpqevents patch.  See

http://archives.postgresql.org/pgsql-hackers/2008-09/msg00153.php

for details.  The only change I didn't make yet is the event 'name'.  I 
have put it in and taken it out twice now, so a firm 'put it in there' 
would be appreciated.


Go here for libpqtypes using events.  pgfoundry is still using the older 
object hooks version.


http://libpqtypes.esilo.com/libpqtypes-events.tar.gz




Patch update again.  This one includes an optional event name for 
debugging purposes.  It also includes the changes made by Alvaro that I 
missed.


PQregisterEventProc now takes a name argumet.  If the name is NULL, the 
error message will identify the event procedure by its address ... 
addr:%p.  If its not NULL, error messages will indicate the provided name.


I updated the styling in libpq-events.c and a couple places in fe-exec.c 
that Alvaro missed.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.166
diff -C6 -r1.166 Makefile
*** src/interfaces/libpq/Makefile   16 Apr 2008 14:19:56 -  1.166
--- src/interfaces/libpq/Makefile   3 Sep 2008 21:55:06 -
***
*** 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
!   md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o 
strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
--- 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
!   md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o 
libpq-events.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o 
strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
***
*** 103,123 
  
  $(top_builddir)/src/port/pg_config_paths.h:
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
  
  install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h 
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample 
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  installdirs: installdirs-lib
$(mkinstalldirs) '$(DESTDIR)$(includedir)' 
'$(DESTDIR)$(includedir_internal)'
  
  uninstall: uninstall-lib
!   rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' 
'$(DESTDIR)$(includedir_internal)/libpq-int.h' 
'$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' 
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  clean distclean: clean-lib
rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c 
noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c md5.c 
ip.c encnames.c wchar.c win32error.c pgsleep.c pthread.h libpq.rc
  # Might be left over from a Win32 client-only build
rm -f pg_config_paths.h
  
--- 103,124 
  
  $(top_builddir)/src/port/pg_config_paths.h:
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
  
  install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
+   $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h 
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample 
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  installdirs: installdirs-lib
$(mkinstalldirs) '$(DESTDIR)$(includedir)' 
'$(DESTDIR)$(includedir_internal)'
  
  uninstall: uninstall-lib
!   rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' 
'$(DESTDIR)$(includedir)/libpq-events.h' 
'$(DESTDIR)$(includedir_internal)/libpq-int.h' 
'$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' 
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  clean distclean: clean-lib
rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c 
noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c md5.c 
ip.c encnames.c wchar.c win32error.c pgsleep.c pthread.h libpq.rc
  # Might be left over from a Win32 client

Re: [HACKERS][PATCHES] odd output in restore mode

2008-07-28 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andrew Dunstan wrote:


- or maybe provide a .bat file or perl script that would work as na 
archive_command on Windows.


We're not talking about archive_command. We're talking about the thing 
that copies files to the directory that pg_standby polls.


Er, that's what the archive_command is. Look at the pg_standby docs and 
you'll see that that's where we're currently recommending use of windows 
copy. Perhaps you're confusing this with the restore_command?


cheers

andrew

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


Re: [HACKERS][PATCHES] odd output in restore mode

2008-07-28 Thread Andrew Dunstan



Greg Smith wrote:

On Wed, 23 Jul 2008, Kevin Grittner wrote:


In our scripts we handle this by copying to a temp directory on the
same mount point as the archive directory and doing a mv to the
archive location when the copy is successfully completed. I think
that this even works on Windows. Could that just be documented as a
strong recommendation for the archive script?


This is exactly what I always do. I think the way cp is shown in the 
examples promotes what's really a bad practice for lots of reasons, 
this particular problem being just one of them.


I've been working on an improved archive_command shell script that I 
expect to submit for comments and potential inclusion in the 
documentation as a better base for other people to build on. This is 
one of the options for how it can operate. It would be painful but not 
impossible to convert a subset of that script to run under Windows as 
well, at least enough to cover this particular issue.





A Perl script using the (standard) File::Copy module along with the 
builtin function rename() should be moderately portable. It would to be 
nice not to have to maintain two scripts.


cheers

andrew

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


Re: [PATCHES] pg_dump additional options for performance

2008-07-27 Thread Andrew Dunstan



Joshua D. Drake wrote:


Agreed but that is a problem I understand with a solution I don't. I 
am all eyes on a way to fix that. One thought I had and please, be 
gentle in response was some sort of async transaction capability. I 
know that libpq has the ability to send async queries. Is it possible 
to do this:


send async(copy table to foo)
send async(copy table to bar)
send async(copy table to baz)

Where all three copies are happening in the background?




IIRC, libpq doesn't let you have more than one async query active at one 
time.


cheers

andrew

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


Re: [HACKERS][PATCHES] odd output in restore mode

2008-07-23 Thread Andrew Dunstan



Kevin Grittner wrote:
Heikki Linnakangas [EMAIL PROTECTED] wrote: 

 
  

We really need a more reliable way of detecting that a file has been



  
fully copied. 

 
In our scripts we handle this by copying to a temp directory on the

same mount point as the archive directory and doing a mv to the
archive location when the copy is successfully completed.  I think
that this even works on Windows.  Could that just be documented as a
strong recommendation for the archive script?
 



  


Needs testing at least. If it does in fact work then we can just adjust 
the docs and be done - or maybe provide a .bat file or perl script that 
would work as na archive_command on Windows.


cheers

andrew

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


Re: [PATCHES] pg_dump additional options for performance

2008-07-21 Thread Andrew Dunstan



Tom Lane wrote:

Simon Riggs [EMAIL PROTECTED] writes:
  

I also suggested having three options
--want-pre-schema
--want-data
--want-post-schema
so we could ask for any or all parts in the one dump. --data-only and
--schema-only are negative options so don't allow this.
(I don't like those names either, just thinking about capabilities)



Maybe invert the logic?

--omit-pre-data
--omit-data
--omit-post-data

Not wedded to these either, just tossing out an idea...


  


Please, no. Negative logic seems likely to cause endless confusion.

I'd even be happier with --schema-part-1 and --schema-part-2 if we can't 
find some more expressive way of designating them.


cheers

andrew

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


Re: [PATCHES] [HACKERS] Solaris ident authentication using unix domain sockets

2008-07-08 Thread Andrew Dunstan



Josh Berkus wrote:

Tom,

  

Indeed.  If the Solaris folk feel that getupeercred() is insecure,
they had better explain why their kernel is that broken.  This is
entirely unrelated to the known shortcomings of the ident IP
protocol.



The Solaris security  kernel folks do, actually.  However, there's no 
question that TRUST is inherently insecure, and that's what people are going 
to use if they can't get IDENT to work.


  



I think I'd pose a slightly different question from Tom. Do the Solaris 
devs think that their getupeercred() is more insecure than the more or 
less equivalent calls that we are doing on Linux and *BSD for example? I 
suspect they probably don't ;-)


cheers

andrew



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


Re: [PATCHES] [HACKERS] Solaris ident authentication using unix domain sockets

2008-07-05 Thread Andrew Dunstan



Robert Treat wrote:

On Thursday 03 July 2008 14:01:22 Tom Lane wrote:
  

Garick Hamlin [EMAIL PROTECTED] writes:


I have a patch that I have been using to support postgresql's
notion of ident authentication when using unix domain sockets on
Solaris.  This patch basically just adds support for using
getupeercred() on Solaris so unix sockets and ident auth works just
like it does on Linux and elsewhere.
  

Cool.




Hmm... I've always been told that Solaris didn't support this because the 
Solaris developers feel that IDENT is inherently insecure. If that is more 
than just a philosphical opinion, I wonder if there should be additional 
hurdles in place to enable this on that platform. Note that isn't an 
objection from me, though I'm curious if any of the Sun guys want to chime in 
on this. 

  



We don't actually use the Ident protocol for Unix sockets on any 
platform. AIUI, this patch just implements what we do on platforms like 
Linux or *BSD.


cheers

andrew

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


Re: [PATCHES] variadic function support

2008-06-25 Thread Andrew Dunstan



Pavel Stehule wrote:

I afraid so Java syntax isn't good  inspiration
http://www.java-tips.org/java-se-tips/java.lang/using-the-varargs-language-feature.html
http://www.clanproductions.com/java5.html

they use symbol ... like specific synonym to [].
public Method getMethod(String name, Class... parameterTypes)

  



Well, ... is really more the equivalent of your variadic keyword, I think.


public Method getMethod(String name, Class[] ... parameterTypes)


would mean each variadic argument would be an array of Class.

cheers

andrew








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


Re: [PATCHES] variadic function support

2008-06-23 Thread Andrew Dunstan



Pavel Stehule wrote:

Hello

this patch enhance current syntax of CREATE FUNCTION statement. It
allows creating functions with variable number of arguments. This
version is different than last my patches. It doesn't need patching
PL. Basic idea is transformation of real arguments (related to
declared variadic argument) to array. All changes are mostly in
parser.

Demo:
CREATE FUNCTION public.least(double precision[]) RETURNS double precision AS $$
  SELECT min($1[i])
 FROM generate_subscripts($1,1) g(i)
$$ LANGUAGE SQL VARIADIC;

SELECT public.least(3,2,1);
 least
---
 1
(1 row)

SELECT public.least(3,2,1,0,-1);
 least
---
-1
CREATE FUNCTION concat(varchar, anyarray) RETURNS varchar AS $$
  SELECT array_to_string($2, $1);
$$ LANGUAGE SQL VARIADIC;

SELECT concat('-',2008,10,12);
   concat

 2008-10-12
(1 row)


  
  


And what about a function that takes 2 arrays as arguments?

This proposal strikes me as half-baked. Either we need proper and full 
support for variadic functions, or we don't, but I don't think we need 
syntactic sugar like the above (or maybe in this case it's really 
syntactic saccharine).


cheers

andrew

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


Re: [PATCHES] variadic function support

2008-06-23 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan [EMAIL PROTECTED] writes:
  

Tom Lane wrote:


What would you consider proper and full support?
  
I don't know. But this doesn't feel like it. 



That's a fairly weak argument for rejecting a patch that provides a
feature many people have asked for.
  


OK. Let me be a bit more specific. I think (forcing myself to be a bit 
more analytic than I have been up to now) my main objection is that the 
variadic part of the parameters should be marked explicitly in the 
formal parameter list.


I don't mind having it limited to a single typed array - as you say we 
probably don't want someone implementing sprintf.


But if I have

 foo( a text, b int[])

it looks odd if both these calls are legal:

 foo('a',1,2,3,)
 foo('a',ARRAY[1,2,3])

which I understand would be the case with the current patch.

I'm also still curious to know how the following would be handled:

 foo(a text[], b text[])

cheers

andrew



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


Re: [PATCHES] variadic function support

2008-06-23 Thread Andrew Dunstan



Tom Lane wrote:

Your point about the syntax is good though.  It would be better if
the syntax were like

create function foo (a text, variadic b int[])

or maybe even better

create function foo (a text, variadic b int)

since (a) this makes it much more obvious to the reader what the
function might match, and (b) it leaves the door open for marking
multiple parameters as variadic, if we can figure out what that means.


  


Yes. I understand from the family Java expert that (surface syntax 
issues aside) the second is similar to the way Java does this, in fact, 
so there's some precedent. That would mean that your first would 
actually mean each variadic arg has to be an array of ints, which we 
might well want to provide for.


So with that modification I'll be lots happier with the feature.

cheers

andrew

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


Re: [PATCHES] doc patch - archive/restore_command on windows

2008-06-20 Thread Andrew Dunstan



ITAGAKI Takahiro wrote:

I found the examples of documentation about archive_command and
restore_command for Windows are incorrect or improper.

 - copy doesn't accept / (slash) as a path separator.
   We should use \\ (double backslash) for the purpose.
 - Windows path is typically starts with a drive letter (C:\\).
 - We'd better to quote a whole path, not only the last filename
   with double-quotes. It can work, but is not a windows manner.



  


As previously discussed, we should probably stop recommending use of the 
Windows copy command altogether, and recommend use of GnuWin32 cp 
instead, for archive_command. The latter does behave sanely w.r.t. 
forward slashes.


cheers

andrew

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


Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL

2008-06-14 Thread Andrew Dunstan



Tom Lane wrote:

multi-object DROP IF NOT EXISTS would fail to perform as expected.


  


Surely this would be a noop :-)

cheers

andrew

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


Re: [PATCHES] Feature: give pg_dump a WHERE clause expression

2008-06-01 Thread Andrew Dunstan



Tom Lane wrote:

Davy Durham [EMAIL PROTECTED] writes:
  

So, if this patch is not acceptable as-is, what would you feel about
this: 
I could enhance the -t/--table=NAME option to accept more than a

simple NAME.  Rather it could accept something in the form:


  

--table=table_name:where-clause expression



Well, that would at least address the complaint that it doesn't scale
to multiple tables, but the whole thing still seems like a frammish
that will never see enough use to justify maintaining it.

(BTW, what will you do with a table whose name contains a colon?)


  


ISTM this would be better off waiting until we turn large parts of 
pg_dump into a library, as has been often discussed, at which point it 
should be relatively simple to write a custom client to do what the OP 
wants. I agree that it does not at all belong in pg_dump.


cheers

andrew

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


Re: [PATCHES] Feature: give pg_dump a WHERE clause expression

2008-06-01 Thread Andrew Dunstan



daveg wrote:



ISTM this would be better off waiting until we turn large parts of 
pg_dump into a library, as has been often discussed, at which point it 
should be relatively simple to write a custom client to do what the OP 
wants. I agree that it does not at all belong in pg_dump.



I can't imagine many of my clients ever writing another C program or even
being willing to pay me to do so. While modularizing pg_dump is a fine idea,
I don't think it addresses the same set of use cases and users as this
proposal.


  


It's not clear to me that your use case is very compelling. Does your 
foreign database not support import via CSV or XML? Postgres can now 
produce both of these for any arbitrary query.


cheers

andrew

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


Re: [PATCHES] libpq object hooks (libpq events)

2008-05-19 Thread Andrew Chernow

Will make all of those changes.  We appreciate the help.

1. remove global registration :(

2. New Name: PGCallback

3. use instanceData and passThrough names (passThrough with upper 'T')

3. separate getters for conn/result instanceData and passthrough

4. add a setter for result instance data
  - There should also be a PQsetInstanceData(PGconn*, ...)
  - I see no need for a passThrough setter

5. move callback stuff to its own header, maybe pgcallback.h?

No issue with any of them.  Although, small issue below:


Maybe instead of having the ResultCreate
callback scribble on the event data structure, provide an additional
API routine to store the pointer:
PQresultSetInstanceData(PGresult *res, PGeventProc proc,
void *instanceData);


Adding PQresultSetInstanceData doesn't removes the need for a resultcreate 
callback event.  This is an event the callbacks are informed about (instanceData 
or not).  It does remove the need for an instance data member in all event info 
structures, just use the getter/setter when desired.  If the passThrough is 
needed, one can use the public getters.


 hooks registered.  Also, meseems you need such a callback anyway:
 what if the hook library desires to realloc its instance data
 larger?


With your suggestions, this would work:

res = PQexec(conn, blah);
data = PQresultInstanceData(res, cbfunc);
data = realloc(data, 1024);
PQresultSetInstanceData(res, cbfunc, data);

The API user should have a valid instanceData whenever libpq returns a result, 
assuming they registered a callback that allocates instance data during a 
resultcreate event.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks (libpq events)

2008-05-19 Thread Andrew Chernow

Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:

4. add a setter for result instance data
   - There should also be a PQsetInstanceData(PGconn*, ...)
   - I see no need for a passThrough setter


Check, though I assume we're not expecting PQsetInstanceData to
propagate to previously created PGresults?



No, not at all.  Already created results are on their own.  If you want to 
modify the instanceData, you can use PQresultSetInstanceData.



5. move callback stuff to its own header, maybe pgcallback.h?


Should be libpq-something.  I was considering libpq-hooks.h or
libpq-events.h, but libpq-callback.h would be OK too.



I like events.  It sounds like you wanted PGCallback, although I am starting to 
think PGEventProc is better than PGcallback, only because it is more consistent 
with the term events.


BTW, my suggestion to call this libpq events was not directly referring to the 
callback/proc.  It was a term for describing the whole #!  I think the idea is 
to notify interested parties about libpq events, the callback is just an 
implementaion for doing that.


Adding PQresultSetInstanceData doesn't removes the need for a resultcreate 
callback event.


No, of course not.  What I was imagining was that the resultcreate
callback would call PQresultSetInstanceData.



Sorry, my mistake.  You were actually very clear, my reading skills are in 
question.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks (libpq events)

2008-05-19 Thread Andrew Chernow
Attached is the latest patch.  It has addressed the requested changes found 
here: http://archives.postgresql.org/pgsql-patches/2008-05/msg00389.php


Its a tarball because there are two new files, libpq-events.c and 
libpq-events.h.  The patch is in the tarball as well as attached to the email.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/


libpq-events.tgz
Description: application/compressed
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.166
diff -C6 -r1.166 Makefile
*** src/interfaces/libpq/Makefile	16 Apr 2008 14:19:56 -	1.166
--- src/interfaces/libpq/Makefile	20 May 2008 04:18:07 -
***
*** 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! 	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
  	$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
--- 29,41 
  # the object files from libpgport, this would not be true on all
  # platforms.
  LIBS := $(LIBS:-lpgport=)
  
  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! 	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o libpq-events.o \
  	$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
***
*** 103,114 
--- 103,115 
  
  $(top_builddir)/src/port/pg_config_paths.h:
  	$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
  
  install: all installdirs install-lib
  	$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
+ 	$(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
  	$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
  	$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
  	$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  installdirs: installdirs-lib
  	$(mkinstalldirs) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)'
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt	19 Mar 2008 00:39:33 -	1.19
--- src/interfaces/libpq/exports.txt	20 May 2008 04:18:07 -
***
*** 138,143 
--- 138,153 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid		  141
+ PQcopyResult  142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQregisterEventProc   145
+ PQinstanceData146
+ PQsetInstanceData 147
+ PQresultInstanceData  148
+ PQresultSetInstanceData   149
+ PQpassThroughData 150
+ PQresultPassThroughData   151
\ No newline at end of file
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.358
diff -C6 -r1.358 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	16 May 2008 18:30:53 -	1.358
--- src/interfaces/libpq/fe-connect.c	20 May 2008 04:18:08 -
***
*** 1970,1981 
--- 1970,1999 
   * release data that is to be held for the life of the PGconn structure.
   * If a value ought to be cleared/freed during PQreset(), do it there not here.
   */
  static void
  freePGconn(PGconn *conn)
  {
+ 	int i;
+ 	PGEventConnDestroy evt;
+ 
+ 	/* Let the event procs cleanup their state data */
+ 	for(i=0; i  conn-nEvents; i++)
+ 	{
+ 		evt.conn = conn;
+ 		(void)conn-events[i].proc(PGEVT_CONNDESTROY, evt);
+ 	}
+ 
+ 	/* free the PGEvent array */
+ 	if(conn-events)
+ 	{
+ 		free(conn-events);
+ 		conn-events = NULL;
+ 		conn-nEvents = conn-eventArrSize = 0;
+ 	}
+ 
  	if (conn-pghost)
  		free(conn-pghost);
  	if (conn-pghostaddr)
  		free(conn-pghostaddr);
  	if (conn-pgport)
  		free(conn-pgport);
***
*** 2151,2164 
  PQreset(PGconn *conn)
  {
  	if (conn)
  	{
  		closePGconn(conn);
  
! 		if (connectDBStart(conn))
! 			(void) connectDBComplete(conn);
  	}
  }
  
  
  /*
   * PQresetStart:
--- 2169,2199 
  PQreset(PGconn *conn)
  {
  	if (conn)
  	{
  		closePGconn(conn);
  
! 		if (connectDBStart

Re: [PATCHES] [HACKERS] use of pager on Windows psql

2008-05-18 Thread Andrew Dunstan



Bruce Momjian wrote:

Andrew Dunstan wrote:
  

Not sure why ware are not.  Should we enabled that code on Win32 and see
how it works?  Can you test it? Was it some MinGW limitation?  I do see
isatty() being used on lots of platforms.

This is kind of odd.  Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there.  I don't see the Win32 checks around
isatty() anywhere else.

  
  
In fact, it looks to me like it would be much more sensible to #include 
settings.h and then simply test pset.notty for all platforms.



Yes, we could do that but does the isatty() value ever change while psql
is running?  When you do '\g filename' does stdout then have isatty as
false?
  
  
Good point. I think the best thing would just be to remove the #ifndef 
WIN32 / #endif lines



OK, patch applied to remove the Win32 test in both places.

  



This broke the buildfarm and finally explains the following kluge which 
has been puzzling me for four years:


   /*
* for some reason MinGW (and MSVC) outputs an extra newline, so 
this

* suppresses it
*/
#ifndef WIN32
   fputc('\n', fout);
#endif

I have removed the kluge (and yes, I tested it).

cheers

andrew


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


Re: [PATCHES] [HACKERS] use of pager on Windows psql

2008-05-18 Thread Andrew Dunstan



Bruce Momjian wrote:


  
This broke the buildfarm and finally explains the following kluge which 
has been puzzling me for four years:


/*
 * for some reason MinGW (and MSVC) outputs an extra newline, so 
this

 * suppresses it
 */
#ifndef WIN32
fputc('\n', fout);
#endif

I have removed the kluge (and yes, I tested it).



Oh, that kluge.  Why did the isatty() addition fix this?  Was the pager
being used on Win32 for the regression tests and somehow eating a line
or something?
  


It apparently produced an extra line which we had compensated for with 
the kluge (without really understanding why we had to).


Anyway, all is good now, as the buildfarm shows.

cheers

andrew

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


Re: [PATCHES] libpq object hooks (libpq events)

2008-05-18 Thread Andrew Chernow

Merlin Moncure wrote:

On Sat, May 17, 2008 at 8:28 AM, Andrew Chernow [EMAIL PROTECTED] wrote:

Here is an updated patch for what was called object hooks.  This is now
called libpq events.  If someone has a better name or hates ours, let us
know.



Let's decide where to go with this.  We have no objections to pushing
this back to the next fest (tom's comments on the wiki were noted).
If that's the case though, we would like to wrap up a consenus on the
general implementation in the meantime.  Just give us a heads up and
I'll update the wiki.

merlin




Yeah, it would be nice to avoid another push back into July by getting some 
feedback now for June; it would be great to squeeze it into May.  I'm not 
talking about a review, but maybe a couple of I hate this or This works well 
or Give up coding :)  The implementation of the events (as well as when they 
were object hooks) is actually very simple, so a quick glance can probably raise 
most concerns.  There is very little going on.


In the end, something like this can be done several different ways (choice here 
is a matter of taste  style).  It would be nice to iron out the API and focus 
on other aspects; namely is this general concept appealing enough.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks (libpq events)

2008-05-17 Thread Andrew Chernow
Here is an updated patch for what was called object hooks.  This is now 
called libpq events.  If someone has a better name or hates ours, let us 
know.


I am continuing to use the object hooks thread to avoid confusing anyone.

Terminology:
I got rid of calling it object events because it is possible to add 
other non-object-related events in the future; maybe a query event. 
This system can be used for any type of event, I think it is pretty generic.


event proc - the callback procedure/function implemented outside of 
libpq ... PGEventProc.  The address of the event proc is used as a 
lookup key for getting a conn/result's event data.


event data - the state data managed by the event proc but tracked by 
libpq.  This allows the event proc implementor to basically add a 
dynamic struct member to a conn or result.  This is an instance based 
value, unlike the arg pointer.


arg pointer - this is the passthrough/user pointer.  I called it 'arg' 
as other libpq callbacks used this term for this type of value.  This 
value can be retrieved via PQeventData, PQresultEventData ... its an 
optional double pointer argument.


event state - an internal structure, PGEventState, which contains the 
event data, event proc and the 'arg' pointer.  Internally, PGconn and 
PGresult contain arrays of event states.


event id - PGEventId enum values, PGEVT_xxx.  This tells the event proc 
which event has occurred.


event info - These are the structures for event ids, like 
PGEVT_RESULTDESTROY has a corresponding PGEventResultDestroy structure. 
 The PGEventProc function's 2nd argument is void *info.  The first 
argument is an event id which tells the proc implementor how to cast the 
void *.  This replaced the initial va_arg idea.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 -  1.19
--- exports.txt 17 May 2008 12:06:10 -
***
*** 138,143 
--- 138,149 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQcopyResult  142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQregisterEventProc   145
+ PQeventData   146
+ PQresultEventData 147
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.358
diff -C6 -r1.358 fe-connect.c
*** fe-connect.c16 May 2008 18:30:53 -  1.358
--- fe-connect.c17 May 2008 12:06:11 -
***
*** 998,1009 
--- 998,1014 
 * We really shouldn't have been polled in these two 
cases, but we
 * can handle it.
 */
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+   if(!pqRegisterGlobalEvents(conn))
+   {
+   conn-status = CONNECTION_BAD;
+   return PGRES_POLLING_FAILED;
+   }
return PGRES_POLLING_OK;
  
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827 
--- 1821,1837 
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
  
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+   if(!pqRegisterGlobalEvents(conn))
+   {
+   conn-status = CONNECTION_BAD;
+   return PGRES_POLLING_FAILED;
+   }
return PGRES_POLLING_OK;
}
  
case CONNECTION_SETENV:
  
/*
***
*** 1848,1859 
--- 1858,1874 
default:
goto error_return;
}
  
/* We are open for business! */
conn-status = CONNECTION_OK;
+   if(!pqRegisterGlobalEvents(conn))
+   {
+   conn-status = CONNECTION_BAD;
+   return PGRES_POLLING_FAILED

Re: [PATCHES] libpq thread-locking

2008-05-16 Thread Andrew Chernow

! int
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
*mp = CreateMutex(0, 0, 0);
+   if (*mp == NULL)
+   return 1;
+   return 0;
  }

Maybe it would be better to emulate what pthreads does.  Instead of 
returing 1 to indicate an error, return an errno.  In the above case, 
ENOMEM seems like a good fit.


Also, maybe you should check the passed in mutex pointer.  If its NULL, 
you could return EINVAL.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-16 Thread Andrew Dunstan



Merlin Moncure wrote:
  

Also, even if varargs are safe they'd be notationally unpleasant
in the extreme.  varargs are just a PITA to work with --- you'd have
to do all the decoding in the first-level hook routine, even for
items you weren't going to use.  With something like the above
all you need is a switch() and some pointer casts.



Switch, plus struct (basically a union) will do the trick nicely.  Can
it be a formal union, or is it better as a void*?

The main issue was how what we called the 'hook data' was passed back
and forth.  We'll get a patch up.


  


All of this is getting quite a long way from what was in the commitfest 
queue. Do we still want to try to get this in this cycle, or should it 
be marked returned to author for more work?


cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-16 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan [EMAIL PROTECTED] writes:
  
All of this is getting quite a long way from what was in the commitfest 
queue. Do we still want to try to get this in this cycle, or should it 
be marked returned to author for more work?



So far I think it still falls within the category of allowing the author
to revise his work.  I don't want to hold commitfest open waiting on
revisions of this patch, but as long as there's still other stuff being
worked through I don't see why they can't keep trying.

Just for the record, I would really like to close this fest before
PGCon starts.  We still have a couple more days to get it done.


  


Apart from this one only two have not been claimed:

. Map Forks
. TRUNCATE TABLE with IDENTITY

cheers

andrew



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


Re: [PATCHES] libpq object hooks

2008-05-16 Thread Andrew Chernow

Tom Lane wrote:


typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo,
 void *passthrough);

int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough);





The above prototypes will work and we will add our 'event instance 
pointer' to the event info structures.  Should have a patch shortly.


libpqtypes doesn't need a passthrough/user-pointer.  The object 
events/hooks allocate memory when the object is created part of a 
conn/result object instance, it is not supplied by the API user 
registering the event/hook callback.


I think this is where some confusion has been occurring, there are two 
different pointers: user pointer and event instance pointer.


BTW, PQeventData and PQresultEventData return the event instance 
pointer, not the passthrough.  At least that is how we were using these 
functions, being how our previous patches do not include a 
passthrough/user-pointer feature because libpqtypes didn't need it.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-16 Thread Andrew Chernow

Tom Lane wrote:

Where
exactly does the hook hand off the storage pointer to libpq?  What
are you going to do if the hook fails to create the storage
(ie, out of memory during PGresult creation)?


The current submitted patch has 3 of its function callbacks returning a 
void*, initHookData after the creation of a conn, resultCreate, and 
resultCopy.  We have recently changed this design so all hooks, now 
called events, go through a single callback ... PGEventProc.  The old 
function callbacks are just eventIds now.


/
// The libpq side (looping registered event procs)
PGEventResultCreate info;
info.stateData = NULL; /* our event data ptr */
info.conn = conn;
info.result = result;

if(!result-evtState[i].proc(PGEVT_RESULTCREATE,
  (void *)info, result-evtState[i].passThrough)
{
  PQclear(result); // destroy result? ... not sure
  return error;// previously, we ignored it
}

// assign event data created by event proc.
result-evtState[i].data = info.stateData;


///
// example of what the event proc does
int my_evtproc(PGEventId evtId, void *evtInfo, void *passThrough)
{
  switch(eventId)
  {
case PGEVT_RESULTCREATE:
{
  void *data = makeresultdata()
  if(!data)
return FALSE;
  ((PGEventResultCreate *)evtInfo)-stateData = data;
  break;
}
  }

  return TRUE;
}


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-16 Thread Andrew Chernow

Tom Lane wrote:

 ISTM the hook
ought to be able to request that libpq return an out-of-memory failure
for the query, just as would happen if the malloc failure had happened
directly to libpq.




I am working on this new patch and that is how I have been implementing it.  If 
the eventProc function returns FALSE for creation events (not destruction 
events), the libpq call that triggered it will fail.  For instance: for the 
creation of result objects PGEVT_RESULTCREATE, I am clearing the result and 
returning an error value.


I think that is the expected behavior when one calls PQexec and an out-of-memory 
failure occurs.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] Patch to change psql default banner v6

2008-05-15 Thread Andrew Dunstan



Joshua D. Drake wrote:


O.k. I am not trying to start an argument here but... I already sent 6 
revisions of this patch that received comments and had thorough review 
via Alvaro. I even took into account Tom's original comments from the 
previous thread.


This much effort on something so simple makes it not worth the effort 
in the first place.





Welcome to UI development. There is always *far* more argument of minor 
matters of appearance than over anything else, in my experience.


cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Dunstan



Tom Lane wrote:

Merlin Moncure [EMAIL PROTECTED] writes:
  

The problem is the functions PQhookData(conn, hookname) and
PQresultHookData(result, hookName).  We need these to work in
functions that are not callbacks.  If we eliminate hookname
completely, there is no way for libpq to know which private state we
are asking for.



Well, depending on a hook name for this is broken-by-design anyway,
because there is no way for two independently written libraries to
be sure they don't choose conflicting hook names.  So the need for
a hook name has to go away.

It might work to use the address of the hook callback function as
a key for retrieving the associated void * pointer.  You'd need to
not register the same callback function more than once per object,
but from what I gather here you don't need to.


  


Or else have the library return a unique handle when registering hooks, 
rather than supplying a hook name.


cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Chernow

Tom Lane wrote:

Merlin Moncure [EMAIL PROTECTED] writes:

The problem is the functions PQhookData(conn, hookname) and
PQresultHookData(result, hookName).  We need these to work in
functions that are not callbacks.  If we eliminate hookname
completely, there is no way for libpq to know which private state we
are asking for.


Well, depending on a hook name for this is broken-by-design anyway,
because there is no way for two independently written libraries to
be sure they don't choose conflicting hook names.  So the need for
a hook name has to go away.

It might work to use the address of the hook callback function as
a key for retrieving the associated void * pointer.  You'd need to
not register the same callback function more than once per object,
but from what I gather here you don't need to.

regards, tom lane




There can be cases to use the same callbacks, although unlikely.  To 
completely avoid collisions, the below would work:


Use the address of a static, maybe an 'int', as a hook hanlde.  Provide 
the user with a macro that can make a hook handle.


typedef void *PGhookHandle;

// Declare an int and point tokname at it.  The value doesn't
// matter, its the pointer address we are interested in.
#define PQ_MAKE_HOOK_HANDLE(tokname) \
  static int hh__ ## tokname = 0; \
  static const PGhookHandle tokname = hh__ ## tokname

As an example, here is what libpqtypes would do:

// libpqtypes hooks.c
PQ_MAKE_HOOK_HANDLE(pqthookhandle);

Now the handle replaces the hookName.  The const char *hookName member 
of the PQobjectHooks structure is changed to const PGhookHanlde 
hookHandle.  This allows for all the flexibility of a const char * w/o 
the collision issues.


// these function prototypes change as well
void *PQhookData(PGconn *, const PGhookHandle);
void *PQresultHookData(PGresult *, const PGhookHandle);

We will send in an updated patch.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Chernow

Andrew Dunstan wrote:



Tom Lane wrote:


It might work to use the address of the hook callback function as
a key for retrieving the associated void * pointer.  You'd need to
not register the same callback function more than once per object,
but from what I gather here you don't need to.

   
  


Or else have the library return a unique handle when registering hooks, 
rather than supplying a hook name.


cheers

andrew




The problem with this is that hooks can be registered on a per-conn 
basis.  Is there a way to ensure the libpq returned handle would be the 
unique across every registration of a given PGobjectHooks?  ISTM that 
the hook handle needs to be constant and unique somehow.  Tom's idea 
would work with the very small limitation of not being able to reuse 
hook callbacks.  I throw out an idea of using the address of a static, 
which is constant and unique.


app_func(PGresult *res)
{
  PQresultHookData(res, ?handle?);
}

app_func is not aware of what PGconn generated the result so it has no 
idea what libpq returned handle to use.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Chernow

Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:
There can be cases to use the same callbacks, although unlikely.  To 
completely avoid collisions, the below would work:


Still looks like overdesign to me.  If we use the hook function address
we solve the problem with no extra notation and no extra storage.

Note that if you want N fixed keys, you can just have N hook functions
(all calling a shared workhorse routine, no doubt).  So your proposal
adds no functionality whatever if the usage involves a fixed number of
static handles.  Now it could possibly allow a variable-at-runtime
number of handles, but I'd want to see a worked-out use case before
designing for that much complexity.  In particular, it seems to me that
the problem would then shift to how do you know which handle to use for
the lookup, thus you've just introduced another layer of complexity
without buying anything.

I think the typical use case is just that you need to distinguish your
hook from anyone else's hooks, so the function address is plenty
sufficient.

It should also be noted that this whole problem *can* be solved without
any PQhookData at all: as long as you have hooks to get control at
creation and destruction of PGconns and PGresults, you can maintain your
own index data structure.  I'm willing to grant some amount of extra
API-stuff to save users having to do that in simple cases, but I don't
think we need to try to support infinitely complex cases.

regards, tom lane




Okay.  No problem over here.

Which callback do we use as the key?  Currently, none are required (only 
the name was required).  We have to choose one callback that must be 
provided.  Maybe initHookData() must be provided?  If the end-user 
doesn't need it they can just return NULL.


This is what is passed to PQaddObjectHooks, along with a conn:

typedef struct
{
  //char *name; REMOVED

  void *data;

  /* Invoked when PQsetObjectHook is called.  The pointer returned
   * by the hook implementation is stored in the private storage of
   * the PGconn, accessible via PQhookData(PGconn*).  If no
   * storage is needed, return NULL or set this hook to NULL.
   */
  void *(*initHookData)(const PGconn *conn);

  /* Invoked on PQreset and PQresetPoll */
  void (*connReset)(const PGconn *conn);

  /* Invoked on PQfinish. */
  void (*connDestroy)(const PGconn *conn);

  /* Invoked on PQgetResult, internally called by all exec funcs */
  void *(*resultCreate)(const PGconn *conn, const PGresult *result);

  /* Invoked on PQcopyResult */
  void *(*resultCopy)(PGresult *dest, const PGresult *src);

  /* Invoked when PQclear is called */
  void (*resultDestroy)(const PGresult *result);
} PGobjectHooks;

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Chernow

Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:
Which callback do we use as the key?  Currently, none are required (only 
the name was required).  We have to choose one callback that must be 
provided.


What?  I thought what you wanted back was the void * pointer that had
been registered with a particular callback function.  So you use that
callback function.  If it's not actually registered, you get a NULL.


This is what is passed to PQaddObjectHooks, along with a conn:


This is all wrong IMHO, not least because it creates ABI problems if you
want to add another hook type later.  Register each hook separately, eg

typedef void (*PGCRHook) (PGconn *conn, void *passthrough);

void PQregisterConnResetHook(PGconn *conn, PQCRHook func, void *passthrough);

... repeat for each possible hook ...

regards, tom lane




One can make a case to break apart the obj hooks structure into 
individual register functions, but I think you have a different idea in 
your head than what is being proposed.  For starters, there is no 
passthru pointer to register with a callback (there could be but that is 
different than hook data...your register looks more like a user_ptr). 
The passthru pointer, what we call hookData, is allocated with a PGconn 
(not provided by the user).  This is the point of the initHookData callback.


typedef void *(*PGinitHookData)(const PGconn *conn);

PQregisterInitHookData((PGconn *)NULL, (PGinitHookData)func);
PQregisterConnResetHook((PGconn *)NULL, (PGCRHook)func);
//etc...
conn = PQconnectdb();

When connectdb returns, initHookData has already been called.  So, a 
call to PQhookData(conn, ) will work.  BUT, what is still missing 
from the equation is how to uniquely reference hookData on a conn.


What I was previously suggesting was to use the address of initHookData, 
since w/o this address there wouldn't be any hook data to get.  Seemed 
like a logical choice.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] Patch to change psql default banner v6

2008-05-15 Thread Andrew Dunstan



David Fetter wrote:


I hate to bike-shed this even further, but I'd like to make those
incompatibility messages just go away by making 8.4's psql (and all
those going forward) support every living version of Postgres at the
time of their release, so 8.4's psql would be able to talk seamlessly
to Postgres 7.4 :)

  


I think you must have been out in the sun too long.

Just look at the pg_dump code if you want something of an idea of what 
this would involve.


cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-15 Thread Andrew Chernow

Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:
Which callback do we use as the key?  Currently, none are required (only 
the name was required).  We have to choose one callback that must be 
provided.


What?  I thought what you wanted back was the void * pointer that had
been registered with a particular callback function.  So you use that
callback function.  If it's not actually registered, you get a NULL.


This is what is passed to PQaddObjectHooks, along with a conn:


This is all wrong IMHO, not least because it creates ABI problems if you
want to add another hook type later.  Register each hook separately, eg

typedef void (*PGCRHook) (PGconn *conn, void *passthrough);

void PQregisterConnResetHook(PGconn *conn, PQCRHook func, void *passthrough);

... repeat for each possible hook ...

regards, tom lane




I am starting to think we have not clarified what it is we are trying to 
do; maybe hook is the wrong terminology.


We need to add members to a conn and result, that's pretty much it.  To 
do this, an api user can register callbacks to receive notifications 
about created/destroyed states of objects.  PQhookData is just like 
PQerrorMessage in that both are public accessor functions to private 
object data.  The difference is that there can be more than one hookData 
dynamic struct member on a conn/result at a time, unlike errorMessage; 
 thus the need for an additional lookup value when getting hook data 
(what was hookName).


A solution is to only have one function with an eventId, instead of a 
register function per event.  I am playing with using the name 'event' 
rather than hook.


typedef enum
{
  PQEVTID_INITDATA,
  PQEVTID_CONNRESET,
  // resultcreate, resultcopy, etc...
} PGobjectEventId;

typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...);

int PQregisterObjectEventProc(PGconn*, PGobjectEventProc);

// no more key (hookName), use PGobjectEventProc address
void *PQeventData(PGconn *, PGobjectEventProc);
void *PQresultEventData(PGresult *, PGobjectEventProc);

Now there is just one libpq register function and an enum; very 
resilient to future additions and ABI friendly.  The API user will 
follow a convention of: if you don't care about an event or don't know 
what it is, just return NULL from the eventProc function (ignore it).


The implementation of a PGobjectEventProc would most likely be a switch 
on the PGobjectEventId with a couple va_arg() calls (which would be very 
well documented).


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] Patch to change psql default banner v6

2008-05-15 Thread Andrew Dunstan



David Fetter wrote:

On Thu, May 15, 2008 at 06:55:31PM -0400, Andrew Dunstan wrote:
  

David Fetter wrote:


I hate to bike-shed this even further, but I'd like to make those
incompatibility messages just go away by making 8.4's psql (and
all those going forward) support every living version of Postgres
at the time of their release, so 8.4's psql would be able to talk
seamlessly to Postgres 7.4 :)
  

I think you must have been out in the sun too long.



One thing I really treasure about working on the Postgres project is
frank feedback. :)
  


I know you know me well enough to realise there was an implied smiley ;-)

  

Just look at the pg_dump code if you want something of an idea of
what this would involve.



Given that each previous version tied backslash commands to some
particular chunk of SQL, what would be the problem with either
immediately or lazily setting those to the chunks of SQL already
present in previous versions?


  


First, this is not a cost free exercise - it increases code complexity 
enormously.


Second, it's not nearly as easy as that:
. new commands have been added
. postgres features have been added
. catalogs have changed

Among other things, help and indeed the available command set would have 
to become server version sensitive.


And you would greatly increase the bar for anyone wanting to add a new 
command - now they (or someone) would have to work out how the command 
would or might work n versions back, not just with the current dev version.


Doing it lazily isn't acceptable - if we promise \command compatibility 
with previous server versions then we need to deliver it to the maximum 
extent possible, and if we don't promise it there's no point in doing this.


And, as Tom says, it has nothing really to do with this patch.

cheers

andrew



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


Re: [PATCHES] Patch to change psql default banner v6

2008-05-15 Thread Andrew Dunstan



Alvaro Herrera wrote:

Andrew Dunstan wrote:

  

Second, it's not nearly as easy as that:
. new commands have been added
. postgres features have been added
. catalogs have changed



Well, this just means a different piece of SQL needs to be sent for a
command depending on the server version, right?  It's not like that's
tremendously different.  The nice thing about most \X commands is that
they embed everything they need in a bunch of SQL, and they don't need
much else in C code.  So it's not all that difficult.

And for commands that have been added later, an initial version could
just say this server version does not support this command.  It would
be already a huge improvement.

Probably the biggest change would be to support versions that did not
have schemas, but I think it would be OK to punt on that.  We already
stopped supporting 7.2 anyway.
  


Have at it then. Prove me wrong.

cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-14 Thread Andrew Dunstan



Alvaro Herrera wrote:

Andrew Dunstan escribió:

  
The thing that is a bit disturbing is that the whole style of this  
scheme is very different from the fairly simple APIs that the rest of  
libpq presents. It's going to make libpq look rather odd, I think. I  
would have felt happier if the authors had been able to come up with a  
simple scheme to add API calls to export whatever information they  
needed, rather than using this callback scheme.



I'm not sure I understand this point.  Remember that this is here to
support the libpqtypes library.  There doesn't seem to be a way for an
API such as you describe to work.
  


That might well be true. The issue then becomes Do we want to add 
something with this flavor to libpq? I take it Bruce's answer is No, 
at least until he has seen more evidence of general usefulness. I think 
we need to make a decision on this before anyone wastes any more time.


It should be noted that while this feels slightly foreign, it isn't 
hugely invasive, unlike the previous effort - it's only a few hundred 
lines of new code.


If we reject this, presumably the authors will have no alternative than 
to offer libpqtypes as a patch to libpq. ISTM that we're then asking 
them to climb over a fairly high hurdle. On the one hand we want them to 
demonstrate that there's demand for their tool and on the other we make 
it difficult to distribute and deploy.


  
Second, the hook names are compared case insensitively and by linear  
search. I don't see any justification for using case insensitive names  
for hooks in a C program, so I think that part should go. And if we  
expect to keep anything other than trivial numbers of hooks we should  
look at some sort of binary or hashed search.



Keep in mind that the original patch supported a single hook being
registered.  Perhaps we could dream about having a couple of hooks
registered, but turning into hashed search would seem to be overkill, at
least for now.  (If hooking into libpq is truly successful we can always
improve it later -- it's not an exported detail of the API after all.)

  


Right, it was more the case insensitive part that bothered me.

cheers

andrew

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


Re: [PATCHES] libpq object hooks

2008-05-14 Thread Andrew Chernow

I'm wondering why the hooks need names at all.  AFAICS all that
libpq needs to know about a hook is a callback function address
and a void * passthrough pointer.



In question is:
+ void *
+ PQhookData(const PGconn *conn, const char *hookName)

Basically, libpqtypes has various functions that take a PGconn that
need the private data that is stored in libpq with the connection.
PQhookData just does simple linear search and returns the data.

[thinks]
are you suggesting something like
+ void *
+ PQhookData(const PGconn *conn, const void *hookHandle)
?

I would have to take a quick look at the code with Andrew C (he'll be
in in a bit)...but this might be doable.



The hook callback functions allow the hook implementor to receive 
created/destroyed events about a PGconn and PGresult (PQreset as well).  The 
hook implementor has the option of associating some memory with either.  But, 
that memory pointer is worthless unless there is a way of referencing it at a 
later time.


HookName would not be needed if the libpq hook API only supported a single 
Object Hook to be registered per conn (or library-wide).  It was requested of us 
to allow a list of hooks per conn.  This requries a way of referencing the item.


Functions outside the hook callback functions:
- PQparamCreate needs libpq-hook-func void *PQhookData(conn, hookName)
- PQgetf needs libpq-hook-func void *PQresultHookData(res, hookName)
- Also, the void *resultCreate(...) hook callback implementation inside 
libpqtypes must use PQhookData on the provided conn so it can copy some 
conn.hookData properties to the result.hookData.  The created result.hookData is 
returned (one can return NULL if no hookData is needed).


I have no issue with merlin's idea of a void *handle, but that doesn't really 
change the concept at all ... just allows someone registering hooks with libpq 
to use something other than a string.  The hookName string idea feels a little 
more natural and simple.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-14 Thread Andrew Chernow

Merlin Moncure wrote:

On Wed, May 14, 2008 at 10:44 AM, Tom Lane [EMAIL PROTECTED] wrote:

I'm wondering why the hooks need names at all.  AFAICS all that
libpq needs to know about a hook is a callback function address
and a void * passthrough pointer.


For reference...here is what libpqtypes has to do to bind via the
hooking interface:
http://libpqtypes.esilo.com/browse_source.html?file=hooks.c

Are you proposing something substantially different (not my handle
suggestion)? How would it work exactly?

merlin




It is important to see how NON-hook-callback functions in libpqtypes 
make use of the hook data.


PQparamCreate must get a pointer to the conn hook data
http://libpqtypes.esilo.com/browse_source.html?file=param.c#line24

PQgetf must get a pointer to the result hook data
http://libpqtypes.esilo.com/browse_source.html?file=exec.c#line65

These are NOT hook callbacks.  The hook data is NOT isolated to callback 
functions.  It is memory that is publically accessible, outside hook 
implementations.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq object hooks

2008-05-14 Thread Andrew Chernow
Attached is an updated patch.  The only change to this patch is that 
hookNames are now compared with strcmp rather than strcasecmp.  The 
comparisons occur in fe-mics.c (bottom of file) during PQhookData and 
PQresultHookData.


Not sure this needs clarification, but PQcopyResult, PQresultAlloc and 
PQsetvalue are not part of the object hooks API.  They are part of 
libpq's result management functions (at least that is what I call them).


Hook API
- PQaddObjectHooks
- PQaddGlobalObjectHooks ** CAN BE REMOVED, SEE BELOW
- PQhookData
- PQresultHookData

Result Management (I would put PQmakeEmptyResult here)
- PQcopyResult
- PQsetvalue
- PQresultAlloc (light wrapper to internal pqResultAlloc)

PROPOSAL:
PQaddGlobalObjectHooks can be removed by handling per-conn and global 
hook registeration through PQaddObjectHooks.  If the provided PGconn is 
NULL, add hooks to global libpq list:


int
PQaddObjectHooks(PGconn *conn, PGobjectHooks *hooks)
{
  if(conn == NULL)
;// global hook register
  else
;// per-conn register
}

This would make the Object Hooks API 3 functions instead of 4.  The same 
rules apply to global hook registeration, do this from main() before 
using libpq as the ObjectHooks list is not thread-safe (no locking).


NOTE: The patch is called objhooks.patch which is a misleading.  The 
patch is really comprised of the API calls needed to make libpqtypes 
work.  If anyone feels this should be broken into two patches (like 
objhooks.patch and resmgnt.patch), let us know.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 -  1.19
--- exports.txt 14 May 2008 17:47:59 -
***
*** 138,143 
--- 138,150 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQcopyResult  142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks  145
+ PQaddGlobalObjectHooks146
+ PQhookData147
+ PQresultHookData  148
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 -  1.357
--- fe-connect.c14 May 2008 17:47:59 -
***
*** 241,253 
 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 char *username);
  static void default_threadlock(int acquire);
  
- 
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
   *Connecting to a Database
--- 241,252 
***
*** 979,990 
--- 978,990 
   * o  If your backend wants to use Kerberos authentication then you 
must
   *supply both a host name and a host address, otherwise this 
function
   *may block on gethostname.
   *
   * 
   */
+ 
  PostgresPollingStatusType
  PQconnectPoll(PGconn *conn)
  {
PGresult   *res;
charsebuf[256];
  
***
*** 998,1009 
--- 998,1010 
 * We really shouldn't have been polled in these two 
cases, but we
 * can handle it.
 */
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+   pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
  
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827 
--- 1817,1829 
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
  
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+   pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
}
  
case CONNECTION_SETENV:
  
/*
***
*** 1848,1859 
--- 1850,1862 
default:
goto

Re: [PATCHES] column level privileges

2008-05-06 Thread Andrew Dunstan



Tom Lane wrote:


I'm not sure where we go from here.  Your GSOC student has disappeared,
right?  Is anyone else willing to take up the patch and work on it?


  


No, he has not disappeared at all. He is going to work on fixing issues 
and getting the work up to SQL99 level.


Your review should help enormously.

Stephen, perhaps you would like to work with him.

cheers

andrew

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


Re: [PATCHES] Fix \dT enum in psql

2008-05-04 Thread Andrew Dunstan



David Fetter wrote:

Folks,

In psql, \dT doesn't show the elements for enums.  Please find patch
vs. CVS TIP attached which fixes this per the following TODO item:

http://archives.postgresql.org/pgsql-hackers/2008-01/msg00826.php

  
  


I notice that this patch adds an Elements column to the output of \dT, 
which will only be used by enum types. That seems rather ... cluttered.


cheers

andrew

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


Re: [PATCHES] Fix \dT enum in psql

2008-05-04 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan [EMAIL PROTECTED] writes:
  
I notice that this patch adds an Elements column to the output of \dT, 
which will only be used by enum types. That seems rather ... cluttered.



But it'll only be in \dT+ anyway, no?


  


Not in this patch.

cheers

andrew

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


Re: [PATCHES] Fix \dT enum in psql

2008-05-04 Thread Andrew Dunstan



David Fetter wrote:

On Sun, May 04, 2008 at 07:49:25PM -0400, Tom Lane wrote:
  

Andrew Dunstan [EMAIL PROTECTED] writes:


Tom Lane wrote:
  

But it'll only be in \dT+ anyway, no?


Not in this patch.
  

Hmmm ... given that a long list of enum members would bloat the
output quite a lot, I think I'd vote for putting it in \dT+.



Here's one where it's only in \dT+

  


Yeah. Committed.

cheers

andrew

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


Re: [PATCHES] [HACKERS] Multiline privileges in \z

2008-05-04 Thread Andrew Dunstan



Brendan Jurd wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Fri, Apr 18, 2008 at 2:37 AM, Tom Lane  wrote:
  

 The function names in the patch need schema-qualification in case
 pg_catalog is not first in the search path.




Ah, yes.  I should have seen that.  Thanks Tom.

New version attached with schema-qualification.

  
  


Committed with slight editorialization and a consequent docs change.

cheers

andrew

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


Re: [PATCHES] [COMMITTERS] pgsql: Sigh ...

2008-05-03 Thread Andrew Dunstan



Peter Eisentraut wrote:

Andrew Dunstan wrote:
  

This patch should fix things for both sets of changes. And it
demonstrates pretty much what you need to do for config options for MSVC.



Btw., it is quite easily possible to use the autom4te tracing facility to 
parse the configure.ac file, in case you are interested in generating some of 
the Windows build code automatically.


For example:

$ autoconf -t 'AC_ARG_ENABLE:$1' configure.in
integer-datetimes
nls
shared
rpath
spinlocks
debug
profiling
dtrace
segmented-files
depend
cassert
thread-safety
thread-safety
thread-safety-force
largefile

Let me know if you want to do something with that.

  


Yeah, maybe. Let's fix the issue pg_config.h.win32 that Tom raised first.

cheers

andrew

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


Re: [PATCHES] [HACKERS] Multiline privileges in \z

2008-05-03 Thread Andrew Dunstan



Brendan Jurd wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Fri, Apr 18, 2008 at 2:37 AM, Tom Lane  wrote:
  

 The function names in the patch need schema-qualification in case
 pg_catalog is not first in the search path.




Ah, yes.  I should have seen that.  Thanks Tom.

New version attached with schema-qualification.
  


Wouldn't this expression:


pg_catalog.array_to_string(c.relacl, chr(10))


be better expressed as


pg_catalog.array_to_string(c.relacl, E'\n')

?

Quoted inside a C literal, the backslash would have to be doubled, of course.

cheers

andrew




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


Re: [PATCHES] [COMMITTERS] pgsql: Sigh ...

2008-05-02 Thread Andrew Dunstan



Tom Lane wrote:
However, all the values are hardcoded, so nothing in it should relate to 
settings that come from configure, I believe. These should be dealt with 
in src/tools/msvc/Solution.pm (mostly in GenerateFiles() ).



FYI, I'm about to commit changes moving XLOG_BLCKSZ and XLOG_SEG_SIZE
into the domain of configurable stuff, too.


  


This patch should fix things for both sets of changes. And it 
demonstrates pretty much what you need to do for config options for MSVC.


If there's no objection I will apply shortly.

cheers

andrew
Index: src/include/pg_config.h.win32
===
RCS file: /cvsroot/pgsql/src/include/pg_config.h.win32,v
retrieving revision 1.54
diff -c -r1.54 pg_config.h.win32
*** src/include/pg_config.h.win32	2 May 2008 03:41:46 -	1.54
--- src/include/pg_config.h.win32	2 May 2008 22:04:37 -
***
*** 37,51 
  /* The alignment requirement of a `short'. */
  #define ALIGNOF_SHORT 2
  
- /* Size of a disk block --- this also limits the size of a tuple. You can set
-it bigger if you need bigger tuples (although TOAST should reduce the need
-to have large tuples, since fields can be spread across multiple tuples).
-BLCKSZ must be a power of 2. The maximum possible value of BLCKSZ is
-currently 2^15 (32768). This is determined by the 15-bit widths of the
-lp_off and lp_len fields in ItemIdData (see include/storage/itemid.h).
-Changing BLCKSZ requires an initdb. */
- #define BLCKSZ 8192
- 
  /* Define to the default TCP port number on which the server listens and to
 which clients will try to connect. This can be overridden at run-time, but
 it's convenient if your clients have the right default compiled in.
--- 37,42 
***
*** 600,618 
 your system. */
  /* #undef PTHREAD_CREATE_JOINABLE */
  
- /* RELSEG_SIZE is the maximum number of blocks allowed in one disk file. Thus,
-the maximum size of a single file is RELSEG_SIZE * BLCKSZ; relations bigger
-than that are divided into multiple files. RELSEG_SIZE * BLCKSZ must be
-less than your OS' limit on file size. This is often 2 GB or 4GB in a
-32-bit operating system, unless you have large file support enabled. By
-default, we make the limit 1 GB to avoid any possible integer-overflow
-problems within the OS. A limit smaller than necessary only means we divide
-a large relation into more chunks than necessary, so it seems best to err
-in the direction of a small limit. A power-of-2 value is recommended to
-save a few cycles in md.c, but is not absolutely required. Changing
-RELSEG_SIZE requires an initdb. */
- #define RELSEG_SIZE 131072
- 
  /* The size of a `size_t', as computed by sizeof. */
  #define SIZEOF_SIZE_T 4
  
--- 591,596 
Index: src/tools/msvc/Solution.pm
===
RCS file: /cvsroot/pgsql/src/tools/msvc/Solution.pm,v
retrieving revision 1.40
diff -c -r1.40 Solution.pm
*** src/tools/msvc/Solution.pm	21 Apr 2008 18:37:28 -	1.40
--- src/tools/msvc/Solution.pm	2 May 2008 22:04:39 -
***
*** 34,39 
--- 34,56 
  die XML requires both XSLT and ICONV\n;
  }
  }
+ 	$options-{blocksize} = 8
+ 		unless $options-{blocksize}; # undef or 0 means default
+ 	die Bad blocksize $options-{blocksize}
+ 		unless grep {$_ == $options-{blocksize}} (1,2,4,8,16,32);
+ 	$options-{segsize} = 1
+ 		unless $options-{segsize}; # undef or 0 means default
+ 	# only allow segsize 1 for now, as we can't do large files yet in windows
+ 	die Bad segsize $options-{segsize}
+ 		unless $options-{segsize} == 1;
+ 	$options-{wal_blocksize} = 8
+ 		unless $options-{wal_blocksize}; # undef or 0 means default
+ 	die Bad wal_blocksize $options-{wal_blocksize}
+ 		unless grep {$_ == $options-{wal_blocksize}} (1,2,4,8,16,32,64);
+ 	$options-{wal_segsize} = 16
+ 		unless $options-{wal_segsize}; # undef or 0 means default
+ 	die Bad wal_segsize $options-{wal_segsize}
+ 		unless grep {$_ == $options-{wal_segsize}} (1,2,4,8,16,32,64);
  return $self;
  }
  
***
*** 116,122 
  print O #define USE_LDAP 1\n if ($self-{options}-{ldap});
  print O #define HAVE_LIBZ 1\n if ($self-{options}-{zlib});
  print O #define USE_SSL 1\n if ($self-{options}-{openssl});
! print O #define ENABLE_NLS 1\n if ($self-{options}-{nls});
  
  if ($self-{options}-{float4byval}) 
  {
--- 133,148 
  print O #define USE_LDAP 1\n if ($self-{options}-{ldap});
  print O #define HAVE_LIBZ 1\n if ($self-{options}-{zlib});
  print O #define USE_SSL 1\n if ($self-{options}-{openssl});
! 		print O #define ENABLE_NLS 1\n if ($self-{options}-{nls});
! 
! 		print O #define BLCKSZ ,1024 * $self-{options}-{blocksize},\n;
! 		print O #define RELSEG_SIZE ,
! 			(1024 / $self-{options}-{blocksize

Re: [PATCHES] [COMMITTERS] pgsql: Sigh ...

2008-05-02 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan [EMAIL PROTECTED] writes:
  

!   print O #define RELSEG_SIZE ,
! 			(1024 / $self-{options}-{blocksize}) * 
! $self-{options}-{segsize} * 1024, \n;



This doesn't look quite right; unless the arithmetic is being done in
floating point?  I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`
  


blocksize is one of (1,2,4,8,16,32)  so it should always be a factor of 
1024 unless my arithmetic is awry. I did it that way because I dislike 
expressions with  unbracketed mixed operations - they make me think too 
much.



Also it looks like you missed adding segsize to the config.pl comments.


  


That's deliberate - we are currently only allowing a value of 1 here, so 
I don't see any point in putting it in the sample config file, even as a 
comment. When we enable other seg sizes we can add it to the sample file.


cheers

andrew



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


Re: [PATCHES] [COMMITTERS] pgsql: Sigh ...

2008-05-02 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan [EMAIL PROTECTED] writes:
  

Tom Lane wrote:


This doesn't look quite right; unless the arithmetic is being done in
floating point?  I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`
  


  
blocksize is one of (1,2,4,8,16,32)  so it should always be a factor of 
1024 unless my arithmetic is awry. I did it that way because I dislike 
expressions with  unbracketed mixed operations - they make me think too 
much.



Well, if you dislike the original on style grounds, you should change it
to match.  Doing the same thing in two different ways in two places
isn't good.
  


OK, done. Patch applied with that addition (it was time I deployed 
autoconf 2.61 anyway).


cheers

andrew


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


Re: [PATCHES] Fix \dT enum in psql

2008-05-01 Thread Andrew Dunstan



David Fetter wrote:

Folks,

In psql, \dT doesn't show the elements for enums.  Please find patch
vs. CVS TIP attached which fixes this per the following TODO item:

http://archives.postgresql.org/pgsql-hackers/2008-01/msg00826.php

  


I don't have a particular problem with this patch - indeed the query in 
it looks eerily familiar :-)


However, I'm wondering if we should wait until a possible rework of the 
mechanics of enums as recently discussed? Or we could put it in and that 
way it would have to be redone when enums are rejigged.


cheers

andrew



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


[PATCHES] column level privileges

2008-04-21 Thread Andrew Dunstan


(try 2)

Here is an updated patch that applies to HEAD.

I will update the wiki.

(What is the maximum attachment size that -patches will accept?)

cheers

andrew


I wrote:


This patch by Golden Lui was his work for the last Google SoC. I was 
his mentor for the project. I have just realised that he didn't send 
his final patch to the list.


I guess it's too late for the current commit-fest, but it really needs 
to go on a patch queue (my memory on this was jogged by Tom's recent 
mention of $Subject).


I'm going to see how much bitrot there is and see what changes are 
necessary to get it to apply.






-
Here is a README for the whole patch.

According to the SQL92 standard, there are four levels in the 
privilege hierarchy, i.e. database, tablespace, table, and column. 
Most commercial DBMSs support all the levels, but column-level 
privilege is hitherto unaddressed in the PostgreSQL, and this patch 
try to implement it.


What this patch have done:
1. The execution of GRANT/REVOKE for column privileges. Now only 
INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified. 
SELECT privilege is now not supported. This part includes:
   1.1 Add a column named 'attrel' in pg_attribute catalog to store 
column privileges. Now all column privileges are stored, no matter 
whether they could be implied from table-level privilege.

   1.2 Parser for the new kind of GRANT/REVOKE commands.
   1.3 Execution of GRANT/REVOKE for column privileges. Corresponding 
column privileges will be added/removed automatically if no column is 
specified, as SQL standard specified.

2. Column-level privilege check.
   Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be 
done ONLY on column level. Table-level privilege check was done in the 
function InitPlan. Now in this patch, these three kind of privilege 
are checked during the parse phase.
   2.1 For UPDATE/INSERT commands. Privilege check is done in the 
function transformUpdateStmt/transformInsertStmt.
   2.2 For REFERENCES, privilege check is done in the function 
ATAddForeignKeyConstraint. This function will be called whenever a 
foreign key constraint is added, like create table, alter table, etc.
   2.3 For COPY command, INSERT privilege is check in the function 
DoCopy. SELECT command is checked in DoCopy too.
3. While adding a new column to a table using ALTER TABLE command, set 
appropriate privilege for the new column according to privilege 
already granted on the table.

4. Allow pg_dump and pg_dumpall to dump in/out column privileges.
5. Add a column named objsubid in pg_shdepend catalog to record ACL 
dependencies between column and roles.

6. modify the grammar of ECPG to support column level privileges.
7. change psql's \z (\dp) command to support listing column privileges 
for tables and views. If \z(\dp) is run with a pattern, column 
privileges are listed after table level privileges.
8. Regression test for column-level privileges. I changed both 
privileges.sql and expected/privileges.out, so regression check is now 
all passed.


  







pg_colpriv_version_0.4-20080421.patch.gz
Description: GNU Zip compressed data

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


Re: [PATCHES] [HACKERS] Text - C string

2008-04-16 Thread Andrew Dunstan



Tom Lane wrote:

Brendan Jurd [EMAIL PROTECTED] writes:
  

If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
have to revert those changes, and I'll have to seriously scale back
the cleanup patch I was about to post.
  


  

Still not sure where we stand on the above.  To cast, or not to cast?



I dunno.  I know there was previously some handwaving about representing
XML values in some more intelligent fashion than a plain text string,
but I have no idea if anyone is likely to do anything about it in the
foreseeable future.


  


It seems very unlikely to me.

cheers

andrew

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


Re: [PATCHES] Suppress compiler warnings on mingw

2008-04-16 Thread Andrew Dunstan



Applied, Thanks.

wiki updated.

cheers

andrew

ITAGAKI Takahiro wrote:

Peter Eisentraut [EMAIL PROTECTED] wrote:

  
Then try using %lu and no casts.  That should get rid of the warnings the 
proper way.



Ok, I rewrote it to use %lu for format strings.


Jeremy Drake [EMAIL PROTECTED] wrote:
  

sizeof(DWORD) is always 4, even on 64-bit windows.  sizeof(long) is also
always 4.



I got it. This change will work on 64-bit windows, because DWORD is
defined as 'unsigned long' there, too. We need to support LLP64
compliers in advance, though.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center


  




  


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


Re: [PATCHES] [HACKERS] MSVC build broken with perl 5.10

2008-04-15 Thread Andrew Dunstan



Zeugswetter Andreas OSB SD wrote:

Magnus Hagander wrote:
  

I just tried the MSVC build on a system with ActiveState Perl 5.10,


and
  

it doesn't work. Some quick debugging before I downgraded to 5.8


showed
  

that this regexp in Project.pm line 262:
my $replace_re = qr{^([^:\n\$]+\.c)\s*:\s*(?:%\s*:


)?\$(\([^\)]+\))\/(.*)\/[^\/]+$};
  

matches things properly using Perl 5.8 in for example
src/bin/initdb/Makefile (matches a total of around 10 Makefiles), but
in 5.10 it simply does not match anything...

Any perl guru out there who can comment on why? ;-)



The answer is actually simple, the \n needs the multiline modifier,
and thus the m needs to be part of the quote-like operator.

The perl doc states:
This operator quotes (and possibly compiles) its STRING
(it seems 5.8 did not compile, but 5.10 does)

I feel that it is rather not a perl bug, and that the modifiers need to
be put
on the qr{}. I do not quite see why this re needs to be multiline in the
first place,
but I have not touched that in the attached patch, that is ready to
apply.
(modification works in perl 5.6, 5.8, 5.10)

  



Thanks, that makes sense. I wonder how it ever worked before. Anyway, 
patch applied back as far as 8.2.


cheers

andrew

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


[PATCHES] libpq object hooks patch

2008-04-15 Thread Andrew Chernow
Here is an updated version of the object hooks patch.  It now supports a 
list of hooks for a PGconn, and PGresult.  This had to re-introduce the 
concept of hook name.  Being that there is now a list, you need a way to 
reference an item of that list.


Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to 
the conn or result hook list.  PQmakeResult must allow the ability to 
pass a list of object hooks in.  So, PQresultObjectHooks was born. 
pqtypes doesn't need (at least at this time) PQobjectHooks but leaving 
it out felt unbalanced.


Note: PQhookData and PQresultHookData can be removed.  There 
functionality can be reproduced by an API user issuing PQobjectHooks or 
PQresultObjectHooks and manually looking for there hook (normaly to get 
at the hook-data).  Basically, an API user would do themselves what 
PQhookData is doing.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 -  1.19
--- exports.txt 15 Apr 2008 17:18:38 -
***
*** 138,143 
--- 138,151 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQmakeResult  142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks  145
+ PQhookData146
+ PQresultHookData  147
+ PQobjectHooks 148
+ PQresultObjectHooks   149
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 -  1.357
--- fe-connect.c15 Apr 2008 17:18:39 -
***
*** 241,253 
 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 char *username);
  static void default_threadlock(int acquire);
  
- 
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
   *Connecting to a Database
--- 241,252 
***
*** 979,990 
--- 978,990 
   * o  If your backend wants to use Kerberos authentication then you 
must
   *supply both a host name and a host address, otherwise this 
function
   *may block on gethostname.
   *
   * 
   */
+ 
  PostgresPollingStatusType
  PQconnectPoll(PGconn *conn)
  {
PGresult   *res;
charsebuf[256];
  
***
*** 1875,1887 
 * the connection structure must be freed).
 */
conn-status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
  }
  
- 
  /*
   * makeEmptyPGconn
   * - create a PGconn data structure with (as yet) no interesting data
   */
  static PGconn *
  makeEmptyPGconn(void)
--- 1875,1886 
***
*** 1970,1981 
--- 1969,1998 
   * release data that is to be held for the life of the PGconn structure.
   * If a value ought to be cleared/freed during PQreset(), do it there not 
here.
   */
  static void
  freePGconn(PGconn *conn)
  {
+   int i;
+ 
+   for(i=0; i  conn-objHooksCount; i++)
+   {
+   if(conn-objHooks[i].connDestroy)
+   {
+   conn-objHooks[i].connDestroy((const PGconn *)conn);
+   free(conn-objHooks[i].name);
+   }
+   }
+ 
+   if(conn-objHooks)
+   {
+   free(conn-objHooks);
+   conn-objHooks = NULL;
+   conn-objHooksCount = conn-objHooksSize = 0;
+   }
+ 
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr);
if (conn-pgport)
free(conn-pgport);
***
*** 2152,2164 
--- 2169,2189 
  {
if (conn)
{
closePGconn(conn);
  
if (connectDBStart(conn))
+   {
+   int i;
+ 
(void) connectDBComplete(conn);
+ 
+   for(i=0; i  conn-objHooksCount; i++)
+   if(conn-objHooks[i].connReset)
+   conn-objHooks[i].connReset((const 
PGconn *)conn);
+   }
}
  }
  
  
  /*
   * PQresetStart:
***
*** 2176,2198 
return connectDBStart(conn);
}
  
return 0

Re: [PATCHES] libpq object hooks patch

2008-04-15 Thread Andrew Chernow

Andrew Chernow wrote:
Here is an updated version of the object hooks patch.  It now supports a 
list of hooks for a PGconn, and PGresult.  This had to re-introduce the 
concept of hook name.  Being that there is now a list, you need a way to 
reference an item of that list.


Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to 
the conn or result hook list.  PQmakeResult must allow the ability to 
pass a list of object hooks in.  So, PQresultObjectHooks was born. 
pqtypes doesn't need (at least at this time) PQobjectHooks but leaving 
it out felt unbalanced.


Note: PQhookData and PQresultHookData can be removed.  There 
functionality can be reproduced by an API user issuing PQobjectHooks or 
PQresultObjectHooks and manually looking for there hook (normaly to get 
at the hook-data).  Basically, an API user would do themselves what 
PQhookData is doing.





Made some changes:

1. Removed the hookName argument to PQaddObjectHooks, since its in the 
supplied PQobjectHooks struct.  I think the argument was lingering from 
a previous patch.


2. Added the ability to install global object hooks: 
PQaddGlobalObjectHooks(PGobjectHooks*).  The header docs for this 
function warns that it should only be used before calling libpq 
functions or creating application threads.  There are no management 
functions for global hooks, like get or remove.  If you add global 
object hooks, your stuck with them until process death.


3. There is a new internal pqInitObjectHooks(PGconn *) that installs the 
global object hooks on new conns in the CONNECTION_OK status.  I call 
this function within PQconnectPoll (3 different locations).  This will 
call PQaddObjectHooks(conn) for each global hook (so global hooks are 
always at the front of a conn's hook list).  pqInitObjectHooks checks to 
see if conn-objHooksCount  0 and if it is, the request is ignored and 
the function returns success.  This only happens during a PQreset and 
PQresetPoll.


// global
PQaddGlobalObjectHooks(libpq_debugger_hook);

// per-conn
PQaddObjectHooks(conn, session_manager_hook);

Since the existing list of object hooks is scanned for duplicate names 
when adding them, you will never run into duplicate object hooks in a 
connection (or in the global list).  If the two examples above were both 
called by an application, the per-conn call would fail.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
? exports.list
? libpq.so.5.2
? object_hooks.patch
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 -  1.19
--- exports.txt 16 Apr 2008 01:14:40 -
***
*** 138,143 
--- 138,152 
  PQsendDescribePortal  136
  lo_truncate   137
  PQconnectionUsedPassword  138
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQmakeResult  142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks  145
+ PQhookData146
+ PQresultHookData  147
+ PQobjectHooks 148
+ PQresultObjectHooks   149
+ PQaddGlobalObjectHooks150
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 -  1.357
--- fe-connect.c16 Apr 2008 01:14:40 -
***
*** 241,253 
 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 char *username);
  static void default_threadlock(int acquire);
  
- 
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
   *Connecting to a Database
--- 241,252 
***
*** 979,990 
--- 978,990 
   * o  If your backend wants to use Kerberos authentication then you 
must
   *supply both a host name and a host address, otherwise this 
function
   *may block on gethostname.
   *
   * 
   */
+ 
  PostgresPollingStatusType
  PQconnectPoll(PGconn *conn)
  {
PGresult   *res;
charsebuf[256];
  
***
*** 998,1009 
--- 998,1010 
 * We really shouldn't have been polled in these two 
cases, but we
 * can handle it.
 */
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+   pqInitObjectHooks

Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation

2008-04-13 Thread Andrew Chernow
Kind of a long post, but if you take the time to read it we think it accurately 
clarifies how we interrupt the current objections and how we see this working. 
NOTE: any references to overhead are in regards to library size, not performance.


would be to insert hooks at library
_init() time, meaning that the mere linking of libpgtypes

Alvaro Herrera wrote:
Maybe there's a way we can have libpqtypes adding calls into some
hypothetical libpq hooks.  So libpqtypes registers its hooks in _init()
or some such, and it gets picked up automatically by any app that links
to it.

the hook name concept
Not needed anymore if we do per-conn hooks.  I was doing library wide hooks, it 
felt natural to allow them to be removed (ability not needed per-conn).  You can 
only remove hooks if you have a means of referencing what you want to remove. 
From that perspective, the names served a purpose - PQremoveObjectHooks(myhook);


you've got it holding a process-wide lock
Okay, easy change to install per-conn. I was trying to avoid having to set these 
hooks on every connection.


There are some dirty details in regards to locking.  Even if you remove the 
locking from libpq hooks, you still incur a lock at every hook point inside 
pqtypes.  pqtypes has to map a conn and result (we'll call this a pqobj) to 
pqtypes typeData. Adding a void* to the hook funcs doesn't help because non-hook 
functions like getf, paramcreate, etc. only have a ptr to a pqobj: PQgetf(res, 
..), PQparamCreate(conn, ..).  Since the private storage of a pqobj is not 
directly accessible, you have to either A) map pqobj addresses to typeData in a 
pqtypes global array that must be locked or B) add two libpq API calls 
PQtypeData(conn), PQresultTypeData(res).


 libpgtypes calls PQinitTypes(PGconn *conn)
As this stands, it wouldn't work.  You need some hook funcptr arguments. Without 
them, there is no way to communicate with pqtypes.


Tom Lane wrote:
hooks that could be used by either libpgtypes or something that would like to 
do something roughly similar


I don't think PQinitTypes, private type data per conn/result or the need for 
PQtypeData(conn), PQresultTypeData(res) (to avoid locking in pqtypes) keeps 
things in line with this requirement (generic hook api).  Has this requirement 
changed?  BTW, Tom was not the only one to suggest generic design.  That's why I 
came up with object hooks - notifications of libpq object states.  Best 
name/concept I can come up with.  PQinitTypes(conn) is really 
PQaddObjectHook(conn, hook) -- addition of the conn argument -- to keep it generic.


In the end, the problem is that the wrong tool hooks is being used for 
pqtypes.  Hooks normally allow a completely unrelated library to receive events. 
 I think we are forcing a hook design on to something that is completely 
related (libpqtypes is an extension of libpq, getvalue and getf are siblings). 
 There is no need for hooks.  If we wanted to add 
PQsetBillingMethod(PQconn*,PQbillingMethod*), then you could make a case for 
hooks (obviously the billing api doesn't fit).  But that is not the case for 
PQgetf, PQputf, PQparamExec, PQparamSend, 


The argument against pqtypes being part of libpq was library size overhead. 
This was verbalized by many people (issues with redhat packaging were also 
brought up). I never heard a complaint about the 10 API calls we wanted to add. 
 Only that those 10 API calls came at a 50K library bloat cost, and there were 
no buyers.


This brings me back to the dlopen idea.  If you want to use pqtypes, 
PQtypesLoad().  The guts of the library are in libpqtypes.so so the only thing 
left in libpq are simple functions like below:


// libpq
PQparamExec(...)
{
  if(libpqtypes-paramExec)//typesLoad issued dlsym calls
// we are in libpq, access to private conn storage granted
return libpqtypes-paramExec(conn, conn-typeData, ...);
  return library not loaded: call PQtypesLoad;
}

// end user
#include libpqtypes.h // pqtypes typedefs, includes libpq-fe.h
PQtypesLoad(); // call before using libpq
res = PQparamExec(conn, param, .);

The library size issue is resolved.  I never heard any complaints about this 
approach.  Andrew Dunstan said Please make sure that any scheme you have along 
these lines will work on Windows DLLs too., which didn't sound like a complaint 
to me.


#ifdef WIN32
# define dlopen(libname, flags) LoadLibraryA(libname)
# define dlsym(handle, sym) GetProcAddress(handle, sym)
#endif

Tom also weighed in but he thought I was confused about his hook idea (as the 
proposed dlopen is completely different):


Tom Wrote:
This is still 100% backwards.  My idea of a libpq hook is something that
could be used by libpgtypes *and other things*.  What you are proposing
is something where the entire API of the supposed add-on is hard-wired
into libpq.

He is 100% correct, the dlopen idea is 100% backwards from a hook concept.  It 
was not an implementation idea for the hooks concept, it was a different 
approach

Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation

2008-04-12 Thread Andrew Chernow

Merlin Moncure wrote:

On Fri, Apr 11, 2008 at 1:47 PM, Andrew Chernow [EMAIL PROTECTED] wrote:

Here are the changes to libpq.  It adds the ability to register an Object
Hook and create a home-grown result.  Patch adds 4 functions.

 We changed the name of PQresultSetFieldValue to PQsetvalue, which better
compliments PQgetvalue.  If this patch is acceptable, we will move on to
making the required changes to pqtypes; some work has already been done.


Whoops! One missing thing here...we forgot to make pqResultAlloc
pubilc...pqResultAlloc - PQresultAlloc (see discussion in -hackers).
Also, we could use pqResultStrdup (or keep it private, in which case
we can re-implement in libpqtypes).

merlin



The connCreate and resultCreate hooks are in the wrong place.  I didn't 
realize this until I started to implement the hook functions in pqtypes.


void (*connCreate)(const char *hookName, const PGconn *conn);

The requirements for the connCreate hook are that the PGconn is ready 
for use.  I am currently hooking in connectDBStart, which is dead wrong. 
 After some poking around, it looks like the correct place to hook 
would be in PQconnectPoll ... does this sound correct?  There are 3 
places PQconnectPoll returns PGRES_POLLING_OK: one is at the top of the 
function and the other two are further down with We are open for 
business! comments.  Would I be correct to hook in at these 3 places in 
PQconnectPoll?


void (*resultCreate)(const char *hookName, const PGconn *conn,
  const PGresult *res);

The requirements for resultCreate are that the result is fully 
constructed.  I am currently hooked in PQmakeEmptyResult, again dead 
wrong.  Does PQgetResult sound correct?


Also, pqtypes is only interested in CONNECTION_OK and successfull 
results.  But, these hooks are available to more than just pqtypes. 
What should the when to call connCreate and resultCreate policy be? 
Should the hook only be called when the conn or result was successfull 
or should the hooks be called for failed connections/commands as well?


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation

2008-04-12 Thread Andrew Chernow


Should the hook only be called when the conn or result was 

 successfull or should the hooks be called for failed

connections/commands as well?



ISTM that the hooks should be called on success and error (doesn't 
include cases where a NULL conn or result is returned).  I think this 
makes things more useful.  If the hooker (pun intended) isn't interested 
in error results or conns, it can easily ignore them.


connCreate: The best solution I found was to hook into PQconnectPoll. 
This required making the current PQconnectPoll a static named 
connectPoll and making PQconnectPoll a wrapper to it.  The wrapper just 
calls connectPoll and checks the status for hook points.


resultCreate: PQgetResult seemed like the best place.  I only notify the 
hooks if the resultStatus is NOT copyin or copyout.


I diff'd fe-connect.c and fe-exec.c against cvs which shows these changes.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 -  1.357
--- fe-connect.c12 Apr 2008 13:22:30 -
***
*** 240,252 
  static int parseServiceInfo(PQconninfoOption *options,
 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 char *username);
  static void default_threadlock(int acquire);
! 
  
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
--- 240,252 
  static int parseServiceInfo(PQconninfoOption *options,
 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 char *username);
  static void default_threadlock(int acquire);
! static void notifyConnCreateHooks(PGconn *conn);
  
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
***
*** 891,903 
--- 891,907 
  connectDBComplete(PGconn *conn)
  {
PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
time_t  finish_time = ((time_t) -1);
  
if (conn == NULL || conn-status == CONNECTION_BAD)
+   {
+   if(conn)
+   notifyConnCreateHooks(conn);
return 0;
+   }
  
/*
 * Set up a time limit, if connect_timeout isn't zero.
 */
if (conn-connect_timeout != NULL)
{
***
*** 979,992 
   * o  If your backend wants to use Kerberos authentication then you 
must
   *supply both a host name and a host address, otherwise this 
function
   *may block on gethostname.
   *
   * 
   */
! PostgresPollingStatusType
! PQconnectPoll(PGconn *conn)
  {
PGresult   *res;
charsebuf[256];
  
if (conn == NULL)
return PGRES_POLLING_FAILED;
--- 983,997 
   * o  If your backend wants to use Kerberos authentication then you 
must
   *supply both a host name and a host address, otherwise this 
function
   *may block on gethostname.
   *
   * 
   */
! 
! static PostgresPollingStatusType
! connectPoll(PGconn *conn)
  {
PGresult   *res;
charsebuf[256];
  
if (conn == NULL)
return PGRES_POLLING_FAILED;
***
*** 1875,1886 
--- 1880,1908 
 * the connection structure must be freed).
 */
conn-status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
  }
  
+ /* See connectPoll.  All this does is wrap the real PQconnectPoll so
+  * we can trap PGRES_POLLING_OK or PGRES_POLLING_FAILED statuses.  This
+  * could be done in the real connectPoll, but that requires littering the
+  * function with notifyConnCreateHooks calls because there are many
+  * places the function returns a status.  This solution is much less
+  * obtrusive and avoids messing with the black magic of connectPoll.
+  */
+ PostgresPollingStatusType
+ PQconnectPoll(PGconn *conn)
+ {
+   PostgresPollingStatusType status = connectPoll(conn);
+ 
+   if(status == PGRES_POLLING_OK || status == PGRES_POLLING_FAILED)
+   notifyConnCreateHooks(conn);
+ 
+   return status;
+ }
  
  /*
   * makeEmptyPGconn
   * - create a PGconn data structure with (as yet) no interesting data
   */
  static PGconn *
***
*** 1970,1981 
--- 1992,2015 
   * release data that is to be held for the life of the PGconn structure

[PATCHES] libpq patch for pqtypes hook api and PGresult creation

2008-04-11 Thread Andrew Chernow
Here are the changes to libpq.  It adds the ability to register an 
Object Hook and create a home-grown result.  Patch adds 4 functions.


We changed the name of PQresultSetFieldValue to PQsetvalue, which better 
compliments PQgetvalue.  If this patch is acceptable, we will move on to 
making the required changes to pqtypes; some work has already been done.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -c -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt19 Mar 2008 00:39:33 -  1.19
--- src/interfaces/libpq/exports.txt11 Apr 2008 17:36:26 -
***
*** 141,143 
--- 141,147 
  pg_valid_server_encoding_id 139
  PQconnectionNeedsPassword 140
  lo_import_with_oid  141
+ PQaddObjectHook   142
+ PQremoveObjectHook143
+ PQmakeResult  144
+ PQsetvalue145
\ No newline at end of file
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -c -r1.357 fe-connect.c
*** src/interfaces/libpq/fe-connect.c   31 Mar 2008 02:43:14 -  1.357
--- src/interfaces/libpq/fe-connect.c   11 Apr 2008 17:36:26 -
***
*** 866,872 
--- 866,887 
 * we are in PGRES_POLLING_WRITING connection state.
 */
if (PQconnectPoll(conn) == PGRES_POLLING_WRITING)
+   {
+   int objHooksCount;
+   const PGobjectHooks *objHooks;
+ 
+   if((objHooksCount = pqGetObjectHooks(objHooks))  0)
+   {
+   int i;
+ 
+   for(i=0; i  objHooksCount; i++)
+   if(objHooks[i].connCreate)
+   
objHooks[i].connCreate(objHooks[i].name, conn);
+   }
+   pqReleaseObjectHooks();
+ 
return 1;
+   }
  
  connect_errReturn:
if (conn-sock = 0)
***
*** 1973,1978 
--- 1988,2006 
  static void
  freePGconn(PGconn *conn)
  {
+   int objHooksCount;
+   const PGobjectHooks *objHooks;
+ 
+   if((objHooksCount = pqGetObjectHooks(objHooks))  0)
+   {
+   int i;
+ 
+   for(i=0; i  objHooksCount; i++)
+   if(objHooks[i].connDestroy)
+   objHooks[i].connDestroy(objHooks[i].name, conn);
+   }
+   pqReleaseObjectHooks();
+ 
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
Index: src/interfaces/libpq/fe-exec.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.194
diff -c -r1.194 fe-exec.c
*** src/interfaces/libpq/fe-exec.c  1 Jan 2008 19:46:00 -   1.194
--- src/interfaces/libpq/fe-exec.c  11 Apr 2008 17:36:27 -
***
*** 63,69 
  static PGresult *PQexecFinish(PGconn *conn);
  static int PQsendDescribe(PGconn *conn, char desc_type,
   const char *desc_target);
! 
  
  /* 
   * Space management for PGresult.
--- 63,69 
  static PGresult *PQexecFinish(PGconn *conn);
  static int PQsendDescribe(PGconn *conn, char desc_type,
   const char *desc_target);
! static int check_field_number(const PGresult *res, int field_num);
  
  /* 
   * Space management for PGresult.
***
*** 139,144 
--- 139,146 
  PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
  {
PGresult   *result;
+   int objHooksCount;
+   const PGobjectHooks *objHooks;
  
result = (PGresult *) malloc(sizeof(PGresult));
if (!result)
***
*** 192,200 
--- 194,341 
result-client_encoding = PG_SQL_ASCII;
}
  
+   if((objHooksCount = pqGetObjectHooks(objHooks))  0)
+   {
+   int i;
+ 
+   for(i=0; i  objHooksCount; i++)
+   if(objHooks[i].resultCreate)
+   objHooks[i].resultCreate(objHooks[i].name, 
conn, result);
+   }
+   pqReleaseObjectHooks();
+ 
return result;
  }
  
+ PGresult *PQmakeResult(
+   PGconn *conn,
+   int numAttributes,
+   PGresAttDesc *attDescs)
+ {
+   int i;
+   PGresult *res;
+ 
+   if(numAttributes = 0 || !attDescs)
+   return NULL;
+ 
+   res = PQmakeEmptyPGresult(conn, PGRES_TUPLES_OK);
+   if(!res)
+   return NULL;
+ 
+   res-attDescs = (PGresAttDesc *)
+   pqResultAlloc(res, numAttributes * sizeof(PGresAttDesc), TRUE);
+ 
+   if(!res-attDescs

[PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow
I noticed several months ago, and came across it again today, that 
libpq's pthread-win32.c implementation is using CreateMutex rather than 
CRITICAL_SECTION.  CreateMutex is like a semaphore in that it is 
designed to be accessible via name system-wide.  Even when you don't 
give it a name, thus bound to process that created it, it still carries 
significant overhead compared to using win32 CRITICAL_SECTIONs.


The attached patch replaces the win32 mutex calls with critical section 
calls.  The change will not affect the behavior of the windows 
pthread_xxx functions.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/port/pthread-win32.h
===
RCS file: /projects/cvsroot/pgsql/src/port/pthread-win32.h,v
retrieving revision 1.2
diff -c -r1.2 pthread-win32.h
*** src/port/pthread-win32.h18 Apr 2007 08:32:40 -  1.2
--- src/port/pthread-win32.h11 Apr 2008 18:35:33 -
***
*** 2,8 
  #define __PTHREAD_H
  
  typedef ULONG pthread_key_t;
! typedef HANDLE pthread_mutex_t;
  typedef int pthread_once_t;
  
  DWORD pthread_self(void);
--- 2,8 
  #define __PTHREAD_H
  
  typedef ULONG pthread_key_t;
! typedef CRITICAL_SECTION *pthread_mutex_t;
  typedef int pthread_once_t;
  
  DWORD pthread_self(void);
Index: src/interfaces/libpq/pthread-win32.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** src/interfaces/libpq/pthread-win32.c1 Jan 2008 19:46:00 -   
1.15
--- src/interfaces/libpq/pthread-win32.c11 Apr 2008 18:35:33 -
***
*** 35,51 
  void
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
!   *mp = CreateMutex(0, 0, 0);
  }
  
  void
  pthread_mutex_lock(pthread_mutex_t *mp)
  {
!   WaitForSingleObject(*mp, INFINITE);
  }
  
  void
  pthread_mutex_unlock(pthread_mutex_t *mp)
  {
!   ReleaseMutex(*mp);
  }
--- 35,69 
  void
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
!   if(mp)
!   {
!   *mp = (CRITICAL_SECTION *)malloc(sizeof(CRITICAL_SECTION));
!   if(*mp)
!   InitializeCriticalSection(*mp);
!   }
  }
  
  void
  pthread_mutex_lock(pthread_mutex_t *mp)
  {
!   if(mp  *mp)
!   EnterCriticalSection(*mp);
  }
  
  void
  pthread_mutex_unlock(pthread_mutex_t *mp)
  {
!   if(mp  *mp)
!   LeaveCriticalSection(*mp);
  }
+ 
+ /* If ever needed
+ pthread_mutex_destroy(pthread_mutex_t *mp)
+ {
+   if(mp  *mp)
+   {
+   DeleteCriticalSection(*mp);
+   *mp = NULL;
+   }
+ }
+ */
\ No newline at end of file

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


Re: [PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow

Magnus Hagander wrote:

It changes the behavior when the pointer passed in is invalid from
crash to silent working, right?

Correct, it a Habit.  I sub-consciously write code that checks pointers. 
 We can remove the pointer checks and let the thing dump core if people 
prefer.



Which brings up the second point - is there any actual reason for
adding the pthread_mutex_destroy call? Since libpq never calls it, and
it's never used from outside libpq (it's not exported outside the
library even), isn't it just going to end up as dead code?

//Magnus



The destroy call is within a comment.  I only put it there in case it is 
ever needeed.  BTW, I just noticed the commented destroy call forgot to 
free(*mp) ... ooppssseee.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow

Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:
The attached patch replaces the win32 mutex calls with critical section 
calls.  The change will not affect the behavior of the windows 
pthread_xxx functions.


Why have you defined the lock/unlock functions as willing to fall
through silently if handed a null pointer?  I think a crash in
such a case is what we *want*.  Silently not locking is surely
not very safe.

regards, tom lane



Yeah, both naughty.

These functions were not implemented to spec.  These pthread functions 
are all supposed to return an int (which is an errno value).  I was 
trying not to change the existing prototypes ... should I?  I can return 
EINVAL if something is NULL and ENOMEM if the malloc fails ... or just 
dump core.


If you like the return value idea, I can make all occurances of 
pthread_xxx check the return value.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow

Tom Lane wrote:

Silently not locking is surely
not very safe.



Here is the dump code version of the patch.  If anyone wants the return 
value idea, let me know.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/port/pthread-win32.h
===
RCS file: /projects/cvsroot/pgsql/src/port/pthread-win32.h,v
retrieving revision 1.2
diff -c -r1.2 pthread-win32.h
*** src/port/pthread-win32.h18 Apr 2007 08:32:40 -  1.2
--- src/port/pthread-win32.h11 Apr 2008 19:22:17 -
***
*** 2,8 
  #define __PTHREAD_H
  
  typedef ULONG pthread_key_t;
! typedef HANDLE pthread_mutex_t;
  typedef int pthread_once_t;
  
  DWORD pthread_self(void);
--- 2,8 
  #define __PTHREAD_H
  
  typedef ULONG pthread_key_t;
! typedef CRITICAL_SECTION *pthread_mutex_t;
  typedef int pthread_once_t;
  
  DWORD pthread_self(void);
Index: src/interfaces/libpq/pthread-win32.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** src/interfaces/libpq/pthread-win32.c1 Jan 2008 19:46:00 -   
1.15
--- src/interfaces/libpq/pthread-win32.c11 Apr 2008 19:22:17 -
***
*** 35,51 
  void
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
!   *mp = CreateMutex(0, 0, 0);
  }
  
  void
  pthread_mutex_lock(pthread_mutex_t *mp)
  {
!   WaitForSingleObject(*mp, INFINITE);
  }
  
  void
  pthread_mutex_unlock(pthread_mutex_t *mp)
  {
!   ReleaseMutex(*mp);
  }
--- 35,61 
  void
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
!   *mp = (CRITICAL_SECTION *)malloc(sizeof(CRITICAL_SECTION));
!   InitializeCriticalSection(*mp);
  }
  
  void
  pthread_mutex_lock(pthread_mutex_t *mp)
  {
!   EnterCriticalSection(*mp);
  }
  
  void
  pthread_mutex_unlock(pthread_mutex_t *mp)
  {
!   LeaveCriticalSection(*mp);
  }
+ 
+ /* If ever needed
+ void pthread_mutex_destroy(pthread_mutex_t *mp)
+ {
+   DeleteCriticalSection(*mp);
+   free(*mp);
+   *mp = NULL;
+ }
+ */
\ No newline at end of file

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


Re: [PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow

Andrew Chernow wrote:

Tom Lane wrote:

Silently not locking is surely
not very safe.



Here is the dump code version of the patch.  If anyone wants the return 
value idea, let me know.








A more graceful solution would be to print something to stderr and then 
exit.  This allows an app's atexit funcs to run.  I don't think libpq 
should core dump an app by choice.  I'd be pretty upset if a library I 
was using core dumped in some cases rather than exiting.


andrew

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


Re: [PATCHES] libpq Win32 Mutex performance patch

2008-04-11 Thread Andrew Chernow

daveg wrote:

On Fri, Apr 11, 2008 at 06:25:53PM -0400, Tom Lane wrote:

Andrew Chernow [EMAIL PROTECTED] writes:
A more graceful solution would be to print something to stderr and then 
exit.

stderr doesn't exist, or point to a useful place, in many environments.
And a forced exit() is no better than a crash for most purposes.


I don't think libpq should core dump an app by choice.

The case that we are talking about is a bug, or would be a bug if it
could happen (the fact that we've gotten along happily with no similar
test in the existing code shows that it can't).  Many forms of bug can
result in core dumps; it's foolish to try to prevent them all.  And
bloating one line of code into five or more lines to defend against
can't-happen cases is a good way to end up with unreadable,
unmaintainable software.

regards, tom lane


+1

-dg


okay.

BTW, my real interest here is the libpq hooks patch requires a 
lock/unlock for every conn-create, conn-destroy, result-create, 
result-destroy.  Currently, it looks like only the ssl stuff uses 
mutexes.  Adding more mutex use is a good case for a more optimized 
approach on windows.


andrew


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


Re: [PATCHES] Fix for win32 stat() problems

2008-04-09 Thread Andrew Dunstan



Magnus Hagander wrote:

Tom Lane wrote:
  

Magnus Hagander [EMAIL PROTECTED] writes:


Trying to prepare a patch that does it the normal way, but so far
I'm failing rather miserably. The *struct* stat is already
redefined on win32, so whenever I try #undef or so it conflicts
with that :-( Since there is no way to #undef only the parametrized
version.
  

I don't follow ... there's no such redefinition in our code AFAICS.
Do you mean that the system header files declare it as a macro?



Yes.


  


How about #defining safe_stat to be pg_win32_safe_stat on Windows and 
simply stat elsewhere? Then use safe_stat at the places you consider 
critical.


cheers

andrew

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Andrew Dunstan



Bruce Momjian wrote:

Tom Lane wrote:
  

Bruce Momjian [EMAIL PROTECTED] writes:


This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
  

Er, why saved?  Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.



It is in the next fest so I will remember to ask if people have done any
work on them --- if not they are either deleted or moved to the next
commit fest.

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

  


My understanding was that all items in a commit-fest have one of these 
three dispositions:


. committed
. rejected
. referred back to author for more work

We're really only interested in the third one here, and so, yes, the 
ball should be in the author's court, not yours. I don't see any reason 
for you to move items from one queue to another like that. It just looks 
like it's making work.


cheers

andrew


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


Re: [PATCHES] Headers dependencies cleanup

2008-04-07 Thread Andrew Dunstan



Zdenek Kotala wrote:


Is it your assumption or do you mean some specific compiler? IIRC, 
inline is defined in C99 and my assumption :-) is that it should be 
supported by all compilers today. I try to look on buildmachine, There 
should be some useful configure output.




My recollection is that we support C89, which might be a bit out of 
date, but then we try not to obsolete platforms if possible.


cheers

andrew

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


Re: [PATCHES] libpq type system 0.9a

2008-04-04 Thread Andrew Chernow

Joe Conway wrote:

Alvaro Herrera wrote:

Merlin Moncure escribió:

Yesterday, we notified -hackers of the latest version of the libpq
type system.  Just to be sure the right people are getting notified,
we are posting the latest patch here as well.  Would love to get some
feedback on this.


I had a look at this patch some days ago, and the first question in my
mind was: why is it explicitely on libpq?  Why not have it as a separate
library (say libpqtypes)?  That way, applications not using it would not
need to link to it.  Applications interested in using it would just need
to add another -l switch to their link line.



+1

Joe




What is gained by having a separate library?  Our changes don't bloat the 
library size so I'm curious what the benefits are to not linking with it?  If 
someone doesn't want to use, they don't have to.  Similar to the backend, there 
is stuff in there I personally don't use (like geo types), but I'm not sure that 
justifies a link option -lgeotypes.


The changes we made are closely tied to libpq's functionality.  Adding PQputf to 
simplify the parameterized API, adding PQgetf to compliment PQgetvalue and added 
the ability to register user-defined type handlers (used by putf and getf). 
PQgetf makes extensive use of PGresult's internal API, especially for arrays and 
composites.  Breaking this into a separate library would require an external 
library to access the private internals of libpq.


Personally, I am not really in favor of this idea because it breaks apart code 
that is very related.  Although, it is doable.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] libpq type system 0.9a

2008-04-04 Thread Andrew Chernow

Andrew Chernow wrote:

Joe Conway wrote:

Alvaro Herrera wrote:

Merlin Moncure escribió:

Yesterday, we notified -hackers of the latest version of the libpq
type system.  Just to be sure the right people are getting notified,
we are posting the latest patch here as well.  Would love to get some
feedback on this.


I had a look at this patch some days ago, and the first question in my
mind was: why is it explicitely on libpq?  Why not have it as a separate
library (say libpqtypes)?  That way, applications not using it would not
need to link to it.  Applications interested in using it would just need
to add another -l switch to their link line.



+1

Joe




What is gained by having a separate library?  Our changes don't bloat 
the library size so I'm curious what the benefits are to not linking 
with it?  If someone doesn't want to use, they don't have to.  Similar 
to the backend, there is stuff in there I personally don't use (like geo 
types), but I'm not sure that justifies a link option -lgeotypes.


The changes we made are closely tied to libpq's functionality.  Adding 
PQputf to simplify the parameterized API, adding PQgetf to compliment 
PQgetvalue and added the ability to register user-defined type handlers 
(used by putf and getf). PQgetf makes extensive use of PGresult's 
internal API, especially for arrays and composites.  Breaking this into 
a separate library would require an external library to access the 
private internals of libpq.


Personally, I am not really in favor of this idea because it breaks 
apart code that is very related.  Although, it is doable.




I poked around to see how this would work.  There are some problems.

1. members were added to PGconn so connection-based type handler information can 
be copied to PGparam and PGresult objects.


2. members were added to PGresult, referenced in #1.  To properly getf values, 
the connection-based type handler information must be available to PGresult. 
Otherwise, PQgetf would require an additional argument, a PGconn, which may have 
been closed already.


3. PQfinish calls pqClearTypeHandler to free type info assigned to the PGconn.

4. PQclear also calls pqClearTypeHandlers

It would also remove some of the simplicity.  Creating a connection would no 
longer initialized type info, which gets copied to PGparam and PGresult.  Type 
info includes a list of built-in handlers and backend config, like 
integer_datetimes, server-version, etc...  That means an additional function 
must be called after PQconnectdb.  But where would the type info be stored?  It 
wouldn't exist in PGconn anymore?  Also, this would require double frees.  You 
have to free the result as well as the type info since they are no longer one 
object.  Same holds true for a pgconn.


There is something elegant about not requiring additional API calls to perform a 
putf or getf.  It'll just work if you want to use it.  You can use PQgetf on a 
result returned by PQexec and you can use PQputf, PQparamExec followed by 
PQgetvalue.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

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


Re: [PATCHES] Expose checkpoint start/finish times into SQL.

2008-04-03 Thread Andrew Dunstan



Joshua D. Drake wrote:

Theo Schlossnagle wrote:


First whack at exposing the start and finish checkpoint times into
SQL.
  

Why is that useful?



For knowing how long checkpoints are taking. If they are taking too
long you may need to adjust your bgwriter settings, and it is a
serious drag to parse postgresql logs for this info.


  


Even if this were true, surely the answer is to improve the logging.

Has this feature been discussed on -hackers? I don't recall it (and my 
memory has plenty of holes in it), but I'm sure that after attending my 
talk last Sunday Theo hasn't sent in a patch for an undiscussed feature ;-)


cheers

andrew

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


Re: [PATCHES] Expose checkpoint start/finish times into SQL.

2008-04-03 Thread Andrew Dunstan



Robert Treat wrote:

On Thursday 03 April 2008 19:08, Andrew Dunstan wrote:
  

Joshua D. Drake wrote:


Theo Schlossnagle wrote:


First whack at exposing the start and finish checkpoint times into
SQL.
  

Why is that useful?


For knowing how long checkpoints are taking. If they are taking too
long you may need to adjust your bgwriter settings, and it is a
serious drag to parse postgresql logs for this info.
  

Even if this were true, surely the answer is to improve the logging.




Exposing everything into the log files isn't always sufficient (says the guy 
who maintains a remote admin tool)
  


It should be now that you can have machine readable logs (says the guy 
who literally spent weeks making that happen) ;-)


cheers

andrew

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


Re: [PATCHES] Expose checkpoint start/finish times into SQL.

2008-04-03 Thread Andrew Dunstan



Joshua D. Drake wrote:

Exposing everything into the log files isn't always sufficient
(says the guy who maintains a remote admin tool)
  
  

It should be now that you can have machine readable logs (says the
guy who literally spent weeks making that happen) ;-)



And how does the person get access to those? And what script do I need
to write to make it happen? Don't get me wrong, the feature you worked
entirely too hard on to get working is valuable but... being able to
say, SELECT * FROM give_me_my_db_info; is much more useful in this
context.
  


How to load the CSV logs is very clearly documented. It's really *very* 
easy, so easy it's mostly CP. See 
http://www.postgresql.org/docs/current/static/runtime-config-logging.html#RUNTIME-CONFIG-LOGGING-CSVLOG


If you are trying to tell me that that's too hard for a DBA, then I have 
to say you need better DBAs.



In short, I should never have to go to log for this class of
information. It should be available in the database.

  


What you haven't explained is why this information needs to be kept in 
the db on a historical basis, as opposed to all the other possible 
diagnostics where history might be useful (and, as Tom has pointed out, 
this patch doesn keep it historically any way).


I think there is quite possibly a good case for keeping some diagnostics 
in a table or tables, on a rolling basis, maybe. But then that's a 
facility that needs to be properly designed.


cheers

andrew



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


Re: [PATCHES] Expose checkpoint start/finish times into SQL.

2008-04-03 Thread Andrew Dunstan



Theo Schlossnagle wrote:





Has this feature been discussed on -hackers? I don't recall it (and 
my memory has plenty of holes in it), but I'm sure that after 
attending my talk last Sunday Theo hasn't sent in a patch for an 
undiscussed feature ;-)



Andrew: I don't think this feature has been discussed on hackers.  The 
patch took about 15 minutes to author, so it sounds like the most 
concise way to start a conversation.  Seems silly to start the 
conversation on hackers with a patch. :-)





Well, not really. I believe -hackers has a much larger readership than 
-patches, so even for small features we generally want them discussed 
there.


cheers

andrew

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


Re: [PATCHES] actualized SQL/PSM patch

2008-04-02 Thread Andrew Dunstan



Jonah H. Harris wrote:

On Tue, Apr 1, 2008 at 5:55 PM, Tom Lane [EMAIL PROTECTED] wrote:
  

 The fundamental problem I've got with this patch is that it adds 400K
 of new code (and that's just the code, not counting documentation or
 regression tests) that we'll have to maintain, to obtain a feature that
 so far as I've heard there is precisely zero demand for.



We have a customer that wants to use it as part of a MySQL-to-Postgres
migration.

  


Using an implementation like this? I suspect anyone wanting to migrate 
their existing SQL/PSM stuff to Postgres will be less than impressed by 
our function body as a string mechanism.


cheers

andrew

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


Re: [PATCHES] Consistent \d commands in psql

2008-04-01 Thread Andrew Dunstan



Tom Lane wrote:

One question: should \df really list *all* nonsystem functions?  Or just
the ones that are visible in your search path?  I'd be inclined to say
the second.


  



+1 (although maybe that discussion belongs on -hackers, or even -general)


cheers

andrew

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


Re: [PATCHES] Fwd: WIP: CASE statement for PL/pgSQL

2008-03-31 Thread Andrew Dunstan



Pavel Stehule wrote:

correct queue

Hello

 I finished this patch.

 Proposal: http://archives.postgresql.org/pgsql-hackers/2008-01/msg00696.php

 It's compatible with PL/SQL (Oracle) and SQL/PSM (ANSI).

 
  


At the very least this patch is missing documentation and regression tests.

cheers

andrew



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


Re: [PATCHES] create language ... if not exists

2008-03-29 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andreas 'ads' Scherbaum wrote:

The attached patch for HEAD extends the CREATE LANGUAGE statement by an
IF NOT EXISTS option which in effect changes the raised error into a
notice.

Before i continue working on this patch i would like to know if this
extension has a chance to go into PG and what other changes i should
apply (beside the missing documentation).


The way we've solved this problem for other CREATE commands is to add 
OR REPLACE option, instead of IF NOT EXISTS. We should do the same 
here.





My recollection is that we only do that where we need to for reasons of 
dependency. Not sure that applies here.


cheers

andrew

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


Re: [PATCHES] [HACKERS] Include Lists for Text Search

2008-03-10 Thread Andrew Dunstan



Simon Riggs wrote:

As Greg mentions on another thread, not all patches are *intended* to be
production quality by their authors. Many patches are shared for the
purpose of eliciting general feedback. You yourself encourage a group
development approach and specifically punish those people dropping
completely finished code into the queue and expecting it to be
committed as-is. 
  


If you post a patch that is not intended to be of production quality, it 
is best to mark it so explicitly. Then nobody can point fingers at you. 
Also, Bruce would then know not to put it in the queue of patches 
waiting for application.


cheers

andrew

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


Re: [PATCHES] [HACKERS] Include Lists for Text Search

2008-03-10 Thread Andrew Dunstan



Simon Riggs wrote:

On Mon, 2008-03-10 at 08:24 -0400, Andrew Dunstan wrote:
  

Simon Riggs wrote:


As Greg mentions on another thread, not all patches are *intended* to be
production quality by their authors. Many patches are shared for the
purpose of eliciting general feedback. You yourself encourage a group
development approach and specifically punish those people dropping
completely finished code into the queue and expecting it to be
committed as-is. 
  


  
If you post a patch that is not intended to be of production quality, it 
is best to mark it so explicitly. Then nobody can point fingers at you. 
Also, Bruce would then know not to put it in the queue of patches 
waiting for application.



So it can be forgotten about entirely? H. 

  


I think if you post something marked Work In Progress, there is an 
implied commitment on your part to post something complete at a later stage.


So if it's forgotten you would be the one doing the forgetting. ;-)

cheers

andrew

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


Re: [PATCHES] CopyReadLineText optimization

2008-03-10 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andrew Dunstan wrote:
Another question that occurred to me - did you try using strpbrk() to 
look for the next interesting character rather than your homegrown 
searcher gadget? If so, how did that perform?


It looks like strpbrk() performs poorly:



Yes, not surprising. I just looked at the implementation in glibc, which 
I assume you are using, and it seemed rather basic. The one in NetBSD's 
libc looks much more efficient.


See

http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/libc/string/strpbrk.c?rev=1.1.2.1content-type=text/plaincvsroot=glibc
and
http://cvsweb.de.netbsd.org/cgi-bin/cvsweb.cgi/src/lib/libc/string/strpbrk.c?rev=1.16;content-type=text%2Fx-cvsweb-markup

Not that what you've done isn't good, if a little obscure (as is the 
NetBSD implementation)


cheers

andrew




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


Re: [PATCHES] CopyReadLineText optimization

2008-03-07 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andrew Dunstan wrote:
I'm still a bit worried about applying it unless it gets some 
adaptive behaviour or something so that we don't cause any serious 
performance regressions in some cases.


I'll try to come up with something. At the most conservative end, we 
could fall back to the current method on the first escape, quote or 
backslash character.


Also, could we perhaps benefit from inlining some calls, or is your 
compiler doing that anyway?


gcc does inline all static functions that are only called from one 
site,  and small functions, using some heuristic. I don't think more 
aggressive inlining would help.




Another question that occurred to me - did you try using strpbrk() to 
look for the next interesting character rather than your homegrown 
searcher gadget? If so, how did that perform?


cheers

andrew

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


Re: [PATCHES] CopyReadAttributesCSV optimization

2008-03-07 Thread Andrew Dunstan



Heikki Linnakangas wrote:
Here's a patch to speed up CopyReadAttributesCSV. On the test case 
I've been playing with, loading the TPC-H partsupp table, about 20% 
CopyReadAttributesCSV (inlined into DoCopy, DoCopy itself is 
insignificant):




[snip]


The trick is to split the loop in CopyReadAttributesCSV into two 
parts, inside quotes, and outside quotes, saving some instructions in 
both parts.


Your mileage may vary, but I'm quite happy with this. I haven't tested 
it much yet, but I wouldn't expect it to be a loss in any interesting 
scenario. The code also doesn't look much worse after the patch, 
perhaps even better.


  


This looks sane enough, and worked for me in testing, so I'm going to 
apply it shortly. I'll probably add a comment or two about how the loops 
interact.


cheers

andrew

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


Re: [PATCHES] CopyReadLineText optimization

2008-03-06 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Heikki Linnakangas wrote:

Heikki Linnakangas wrote:
Attached is a patch that modifies CopyReadLineText so that it uses 
memchr to speed up the scan. The nice thing about memchr is that we 
can take advantage of any clever optimizations that might be in libc 
or compiler.


Here's an updated version of the patch. The principle is the same, 
but the same optimization is now used for CSV input as well, and 
there's more comments.


Another update attached: It occurred to me that the memchr approach is 
only safe for server encodings, where the non-first bytes of a 
multi-byte character always have the hi-bit set.




We currently make the following assumption in the code:

* These four characters, and the CSV escape and quote characters, are
* assumed the same in frontend and backend encodings.
*

The four characters are the carriage return, line feed, backslash and dot.

I think the requirement might well actually be somewhat stronger than 
that: i.e. that none of these will appear as a non-first byte in any 
multi-byte client encoding character. If that's right, then we should be 
able to write CopyReadLineText without bothering about multi-byte chars. 
If it's not right then I suspect we have some cases that can fail now 
anyway. (I believe some client encodings at least use backslash in 
subsequent chars, and that's a nasty one because the \. end sequence 
is hard coded). I believe all the chars up to 0x2f are safe - that 
includes both quote chars and dot)


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] CopyReadLineText optimization

2008-03-06 Thread Andrew Dunstan



Tom Lane wrote:

BTW, I notice that the code allows CSV escape and quote characters that
have the high bit set (in single-byte server encodings that is).  Is
this a good idea?  It seems like such are extremely unlikely to be the
same in two different encodings.  Maybe we should restrict to the ASCII
range?  Especially if the client encoding is multibyte ...


  


In the commonest case these are both either  or '. I would not have any 
objection to requiring them to be ASCII chars.


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] CopyReadLineText optimization

2008-03-06 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andrew Dunstan wrote:

Heikki Linnakangas wrote:

Another update attached: It occurred to me that the memchr approach is
only safe for server encodings, where the non-first bytes of a 
multi-byte character always have the hi-bit set.




We currently make the following assumption in the code:

* These four characters, and the CSV escape and quote characters, 
are

* assumed the same in frontend and backend encodings.
*

The four characters are the carriage return, line feed, backslash and 
dot.


I think the requirement might well actually be somewhat stronger than 
that: i.e. that none of these will appear as a non-first byte in any 
multi-byte client encoding character. If that's right, then we should 
be able to write CopyReadLineText without bothering about multi-byte 
chars. If it's not right then I suspect we have some cases that can 
fail now anyway.


No, we don't require that, and we do handle it correctly. We use 
pg_encoding_mblen to determine the length of each character in 
CopyReadLineText when the encoding is a client-only encoding, and only 
look at the first byte of each character. In CopyReadAttributesText, 
where we have a similar loop, we've already transformed the input to 
server encoding.


Oops. I see that now. Funny how I missed it when I went looking for it :-(

I think I understand the patch now :-)

I'm still a bit worried about applying it unless it gets some adaptive 
behaviour or something so that we don't cause any serious performance 
regressions in some cases. Also, could we perhaps benefit from inlining 
some calls, or is your compiler doing that anyway?


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] CopyReadLineText optimization

2008-03-06 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Andrew Dunstan wrote:
I'm still a bit worried about applying it unless it gets some 
adaptive behaviour or something so that we don't cause any serious 
performance regressions in some cases.


I'll try to come up with something. At the most conservative end, we 
could fall back to the current method on the first escape, quote or 
backslash character.





That's far too conservative, I think. Somewhere a bit short of your 
observed breakeven point seems about right.


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] CopyReadLineText optimization

2008-03-06 Thread Andrew Dunstan



Greg Smith wrote:

On Thu, 6 Mar 2008, Heikki Linnakangas wrote:

At the most conservative end, we could fall back to the current 
method on the first escape, quote or backslash character.


I would just count the number of escaped/quote characters on each 
line, and then at the end of the line switch modes between the current 
code on the new version based on what the previous line looked like.  
That way the only additional overhead is a small bit only when escapes 
show up often, plus a touch more just once per line.  Barely noticable 
in the case where nothing is escaped, very small regression for 
escape-heavy stuff but certainly better than the drop you reported in 
the last rev of this patch.


Rev two of that design would keep a weighted moving average of the 
total number of escaped characters per line (say 
wma=(7*wma+current)/8) and switch modes based on that instead of the 
previous one.  There's enough play in the transition between where the 
two approaches work better at that this should be easy enough to get a 
decent transition between. Based on your data I would put the 
transition at wma4, which should keep the old code in play even if 
only half the lines have the bad regression that shows up with 8 
escapes per line.





I'd be inclined just to look at the first buffer of data we read in, and 
make a one-off decision there, if we can get away with it. Then the cost 
of testing is fixed rather than per line.


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] WIP: guc enums

2008-03-05 Thread Andrew Dunstan



Tom Lane wrote:

Having to have two extra hook functions for every variable
seems like a lot of notational overhead for not much gain.  
  


+1

cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] CopyReadLineText optimization

2008-03-05 Thread Andrew Dunstan



Heikki Linnakangas wrote:



So the overhead of using memchr slows us down if there's a lot of 
escape or quote characters. The breakeven point seems to be about 1 in 
8 characters. I'm not sure if that's a good tradeoff or not...





How about we test the first buffer read in from the file and change 
strategy accordingly?


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] NetBSD/MIPS supports dlopen

2008-03-05 Thread Andrew Dunstan



Alvaro Herrera wrote:

Both done -- I backpatched all the way down to 7.4 (and later I noticed
that the only 7.3 BF members are NetBSD).
  
  


Haven't we declared 7.3 at EOL anyway?

cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] NetBSD/MIPS supports dlopen

2008-03-05 Thread Andrew Dunstan



Alvaro Herrera wrote:

Andrew Dunstan wrote:
  

Alvaro Herrera wrote:


Both done -- I backpatched all the way down to 7.4 (and later I noticed
that the only 7.3 BF members are NetBSD).
  

Haven't we declared 7.3 at EOL anyway?



That's why I didn't backpatch it there.  But if that's the case, why are
we still reporting 7.3 in the buildfarm status page?

  


Because until a couple of weeks ago those two machines were still 
reporting that branch. When they are 30 days old the reports will drop 
off the page. (It looks like salamander has stopped altogether, which 
Tom mentioned the other day would distress him some.)


cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] libpq type system 0.9a

2008-03-05 Thread Andrew Chernow

Merlin Moncure wrote:

On Wed, Mar 5, 2008 at 5:47 PM, Florian G. Pflug [EMAIL PROTECTED] wrote:

Merlin Moncure wrote:
  Yesterday, we notified -hackers of the latest version of the libpq
  type system.  Just to be sure the right people are getting notified,
  we are posting the latest patch here as well.  Would love to get some
  feedback on this.
 Sorry if this has been discussed before, but why is it necessary
 to specify the type when calling PQgetf on a result? It seems that this
 formatting string *always* has to match the type list of your select
 statement, no?


yes...it always has to match.  the format string requirements could in
theory be relaxed (for 'get') but this would break symmetry with 'put'
and you would lose a sanity check...getf like scanf writes directly
into application memory so the double-specifying (directly in the
format string and indirectly in the query) isn't necessarily a bad
thing.  imagine if your application was 'select * from table' and one
of the field types changed...disaster.

merlin




A few other reasons

why is it necessary to specify the type when calling PQgetf on a result

Unlike PQgetvalue, all values returned by PQgetf are either native C types or 
structures ... not C strings.  When you call getf you must tell it what types to 
read out of the result object.  Like scanf, they must be the correctly sized 
data types.


PGdate date;
int i4;
PQgetf(result, tup_num, %date %int4, 0, date, 1, i4);

Specifying anything other than a %date or %int4 in the above example is a 
programming error.  You would be asking to fetch a value of the wrong type. 
Without the formatting string, libpq would have to va_arg(ASSUME_T) your value.


// no specifier
int i;
PQgetf(result, tup, field, i);

In the above, libpq would have to use PQftype to determine what the native C 
type is of your variable argument.  If PQftype returned INT8OID, you begin to 
clobber your application's memory space ... va_arg(ap, long long) on a 32-bit 
value.  This problem is solved by telling libpq what data type you want from a 
field.


Also, the libpq type system enforces strict type checking when performing getf 
calls.  This protects from mis-matches programming errors on types:


For example:

-- create table t (a int8);
PQresult *result = PQexec(conn, SELECT a FROM t);
char *val = PQgetvalue(result, ...);
int a = atoi(val); // assumed its an int4

In the above example, the libpq user thinks the 'a' column of the 't' table is 
an int4 when in fact its an int8.  The above may work most of the time but will 
eventually truncate the value and nip you in the butt.  With PQgetf, you would 
get an error saying the server returned an int8 and you are asking for an int4. 
 Thus, the programming bug would be squashed immediately.


Also, user-defined types are not known to libpq so PQftype would not really 
work.  They could if the libpq type system referenced data types by OID, but 
this is not portable to other servers.  It is more portable to use the type 
name.  For example, a company with 15 postgresql servers that use the same 
collection of company-specific user-defined data types.  The type names would be 
the same across the 15 servers but there is no guarentee the OIDs would be.


Composites and arrays caused a few issues as well.

We also tried to provide as much protection as possible ... in the spirit of the 
backend.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] [BUGS] BUG #4007: chr(0) doesn't work anymore

2008-03-03 Thread Andrew Dunstan



Bruce Momjian wrote:

Tom Lane wrote:
  

Steve Clark [EMAIL PROTECTED] writes:

I'm not sure I understand what you mean about TEXT being null-safe. 
What are the issues, and why was

it supported for years and now abruptly changed.
  

It never was supported, we are simply plugging a hole that let you
create a text value that would be likely to malfunction in subsequent
use.



Seems we never documented that chr(0) is not supported.  I have applied
the following doc patch to CVS HEAD and 8.3.X.

  
  The NULL (0) character is not

  allowed because text data types cannot reliably store such bytes.






Reliably is arguably misleading here.

cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your Subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


  1   2   3   4   5   6   7   8   9   >