Re: [PATCHES] TupleDesc refcounting
On Tue, 2006-01-17 at 09:36 -0500, Tom Lane wrote:
I suspect you'll find that it's convenient to treat typcache's own
link to a tupdesc as a reference count too, so it's not strictly
external references that should be counted.
The problem with this is that incrementing a TupleDesc's refcount
informs the CurrentResourceOwner. That causes the refcount to be
decremented when the resource owner is released, which is wrong when the
TupleDesc should live beyond the current query (as it should in the
typcache). We could avoid that by creating a CacheResourceOwner that is
never reset, as discussed earlier, but at present I'm not sure if
there's any point, so I've just used a dead field.
Attached is a revised patch. Reference counting is only used for
lookup_rowtype_tupdesc(): as discussed there might be other places that
would benefit from this, but I haven't looked at that yet.
There was some call-sites of lookup_rowtype_tupdesc() where it doesn't
seem to be easy to use reference counting. For example, consider the
implementation of get_expr_result_type(): some code paths within that
function call lookup_rowtype_tupdesc() to produce the returned
TupleDesc, and some do not. The easiest fix seemed to be just making a
copy of the TupleDesc for the lookup_rowtype_tupdesc() cases.
The patch is WIP: a few regression tests fail due to TupleDesc leaks I
haven't fixed yet but that should be easily fixable, and there are a few
other minor issues to address. I'm just posting the patch now to get any
feedback.
(Apologies for not getting this done earlier, I had a touch of the flu
yesterday...)
-Neil
*** src/backend/access/common/tupdesc.c 83ca807d4fdd572c409bc9214922b6ba9da7ce18
--- src/backend/access/common/tupdesc.c c423df65341ed283d20ff03c4437ad67876ef0ba
***
*** 23,28
--- 23,29
#include catalog/pg_type.h
#include parser/parse_type.h
#include utils/builtins.h
+ #include utils/resowner.h
#include utils/syscache.h
***
*** 84,89
--- 85,92
desc-tdtypeid = RECORDOID;
desc-tdtypmod = -1;
desc-tdhasoid = hasoid;
+ desc-refcount = 0;
+ desc-dead = false;
return desc;
}
***
*** 116,121
--- 119,126
desc-tdtypeid = RECORDOID;
desc-tdtypmod = -1;
desc-tdhasoid = hasoid;
+ desc-refcount = 0;
+ desc-dead = false;
return desc;
}
***
*** 214,219
--- 219,226
{
int i;
+ Assert(tupdesc-refcount == 0);
+
if (tupdesc-constr)
{
if (tupdesc-constr-num_defval 0)
***
*** 246,251
--- 253,279
pfree(tupdesc);
}
+ void
+ IncrTupleDescRefCount(TupleDesc tupdesc)
+ {
+ Assert(tupdesc-refcount = 0);
+
+ ResourceOwnerEnlargeTupleDescs(CurrentResourceOwner);
+ tupdesc-refcount++;
+ ResourceOwnerRememberTupleDesc(CurrentResourceOwner, tupdesc);
+ }
+
+ void
+ DecrTupleDescRefCount(TupleDesc tupdesc)
+ {
+ Assert(tupdesc-refcount 0);
+
+ tupdesc-refcount--;
+ ResourceOwnerForgetTupleDesc(CurrentResourceOwner, tupdesc);
+ if (tupdesc-refcount == 0 tupdesc-dead)
+ FreeTupleDesc(tupdesc);
+ }
+
/*
* Compare two TupleDesc structures for logical equality
*
*** src/backend/access/heap/tuptoaster.c f3ec822c0e6b6328bc0026716fcf4b133f89b092
--- src/backend/access/heap/tuptoaster.c bbdea6bdd98d250326fad68899d07a8b153fb263
***
*** 892,898
--- 892,901
* If nothing to untoast, just return the original tuple.
*/
if (!need_change)
+ {
+ DecrTupleDescRefCount(tupleDesc);
return value;
+ }
/*
* Calculate the new size of the tuple. Header size should not change,
***
*** 930,935
--- 933,939
if (toast_free[i])
pfree(DatumGetPointer(toast_values[i]));
+ DecrTupleDescRefCount(tupleDesc);
return PointerGetDatum(new_data);
}
*** src/backend/executor/execQual.c eb1daef85341439578a3f6bb73549c4420292bc3
--- src/backend/executor/execQual.c 73a3d813603ce497b0a2c45a6f21fc001445d1b8
***
*** 707,712
--- 707,714
attrno,
tupDesc,
isNull);
+
+ DecrTupleDescRefCount(tupDesc);
return result;
}
***
*** 765,770
--- 767,774
attrno,
tupDesc,
isNull);
+
+ DecrTupleDescRefCount(tupDesc);
return result;
}
***
*** 1149,1157
/*
* ExecMakeTableFunctionResult
*
! * Evaluate a table function, producing a materialized result in a Tuplestore
! * object. *returnDesc is set to the tupledesc actually returned by the
! * function, or NULL if it didn't provide one.
*/
Tuplestorestate *
ExecMakeTableFunctionResult(ExprState *funcexpr,
--- 1153,1163
/*
* ExecMakeTableFunctionResult
*
! * Evaluate a table function, producing a materialized result
Re: [PATCHES] Uninstall scripts for contrib
Am Montag, 16. Januar 2006 06:55 schrieb David Fetter: Oops. My FM R'ing skills need some work. This patch includes the files. This patch is rather useless because all the uninstall.sql files install on top of each other. I suggest naming them cube_uninstall.sql dblink_uninstall.sql etc. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Uninstall scripts for contrib
On Thu, Jan 19, 2006 at 02:22:17PM +0100, Peter Eisentraut wrote: Am Montag, 16. Januar 2006 06:55 schrieb David Fetter: Oops. My FM R'ing skills need some work. This patch includes the files. This patch is rather useless because all the uninstall.sql files install on top of each other. I suggest naming them cube_uninstall.sql dblink_uninstall.sql etc. It's paper bag time for me. How about a new patch which gives each contrib project its own directory and places them there? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778 Remember to vote! ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] TupleDesc refcounting
Neil Conway [EMAIL PROTECTED] writes: On Tue, 2006-01-17 at 09:36 -0500, Tom Lane wrote: I suspect you'll find that it's convenient to treat typcache's own link to a tupdesc as a reference count too, so it's not strictly external references that should be counted. The problem with this is that incrementing a TupleDesc's refcount informs the CurrentResourceOwner. Not if it's just tupdesc-refcount++; ... ;-) Seriously, I was imagining a two-level structure where tupdesc itself just has operations on the order of refcount++; if (--refcount = 0) FreeTupleDesc(); and then a separate layer on top of that adds ResourceOwner management. This is precisely because the cache code requires non-resource-owner- managed links (or at least, has no particular use for the services of a ResourceOwner). There was some call-sites of lookup_rowtype_tupdesc() where it doesn't seem to be easy to use reference counting. For example, consider the implementation of get_expr_result_type(): some code paths within that function call lookup_rowtype_tupdesc() to produce the returned TupleDesc, and some do not. The easiest fix seemed to be just making a copy of the TupleDesc for the lookup_rowtype_tupdesc() cases. Yeah, I noticed this while making the back-branch patch. I agree that any given routine had better have a consistent return convention: if the tupdesc needs to be refcount-decremented when done, that must be the case in all code paths, since the caller can't be expected to know which case applies. My recollection though is that the non-lookup cases return freshly-built-in-local-storage tupdescs, which could certainly be set up as having refcount 1 so that the decrement would work correctly. It may or may not be worth the trouble compared to just copying, though. You should think about whether a routine is a performance hotspot before going out of your way to avoid a copy step. (Apologies for not getting this done earlier, I had a touch of the flu yesterday...) There's no hurry about it. Hope you're feeling better. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Uninstall scripts for contrib
David Fetter wrote: It's paper bag time for me. How about a new patch which gives each contrib project its own directory and places them there? This would presumably imply that the installation scripts would be renamed to install.sql. On a green field this might make sense but as it is maybe it would break too much without much benefit? -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Uninstall scripts for contrib
On Thu, Jan 19, 2006 at 07:08:28PM +0100, Peter Eisentraut wrote: David Fetter wrote: It's paper bag time for me. How about a new patch which gives each contrib project its own directory and places them there? This would presumably imply that the installation scripts would be renamed to install.sql. On a green field this might make sense but as it is maybe it would break too much without much benefit? You're right. I'll send a patch this evening my time that renames each to [module_name]_uninstall.sql. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778 Remember to vote! ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] pgxs/windows
OK, I found the cause. Using tests from Magnus and Andrew, I could reproduce Magnus's success and Andrew's failure in an 8.1.2 pginstaller install using MinGW. The difference between Magnus's and Andrew's is that Magnus used MODULE_big (which means create a shared library), while Andrew used MODULES. So, Magnus's only worked because he was creating a DLL and that brought in the -L from MODULE_big. I have applied the following patch to change /bin to /lib for Cygwin and Win32. Looking at Darwin and AIX, both those are pointing to the postgres binary, so they should be using /bin, so we are OK on those. I patched CVS HEAD, and 8.1.X and 8.0.X. Earlier releases didn't support pgxs. --- Andrew Dunstan wrote: Bruce Momjian wrote: OK, thanks. Next question --- are the installed file locations the same for a MinGW install and a pginstaller install? I don't think pginstaller does a MinGW install because it doesn't have the build environment in the tarball. However, the big difference seems to be that Magnus has -Llib and -Lbin, while you have only the -Lbin. I have MinGW and pginstaller installed here. How can I set things up to test this? Now looking at the Makefile.global in the 8.1.2 pginstaller install, in Makefile.global, $libdir is set in a pgxs-specific block: libdir := $(shell pg_config --libdir) and that seems to work: C:\Program Files\PostgreSQL\8.1\binpg_config --libdir C:/PROGRA~1/POSTGR~1/8.1/lib and that is set to LDFLAGS, which is later propogated to SHLIB_LINK, though SHLIB_LINK moves all the -L flags to the front, so what you see on the link line is not the ordering used to create the value. Andrew, can you try echoing $libdir and $SHLIB_LINK in the Makefile to find those values? here is a test case log (including a test makefile). I ran the tests as you can see with both installer 8.1 and mingw installed CVS tip, with the same results. cheers andrew $ touch foo.c $ cat Makefile MODULES = foo SRCS += foo.c OBJS = $(SRCS:.c=.o) PGXS := $(shell pg_config --pgxs) include $(PGXS) override CFLAGS := $(filter-out -Wendif-labels -Wdeclaration-after-statement, $(shell pg_config --cflags)) showme: @echo libdir = $(libdir) @echo SHLIB_LINK = $(SHLIB_LINK) $ which pg_config /c/Program Files/PostgreSQL/8.1/bin/pg_config $ rm -f foo.dll $ make gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -fno-strict-aliasing -I. -Ic:/PROGRA~1/POSTGR~1/8.1/include/server -Ic:/PROGRA~1/POSTGR~1/8.1/include/internal -I./src/include/port/win32 -DEXEC_BACKEND -I/mingw/include/krb5 -Ic:/PROGRA~1/POSTGR~1/8.1/lib/pgxs/src/MAKEFI~1/../../src/include/port/win32 -Ic:/PROGRA~1/POSTGR~1/8.1/include/server/port/win32 -c -o foo.o foo.c dlltool --export-all --output-def foo.def foo.o dllwrap -o foo.dll --def foo.def foo.o c:/PROGRA~1/POSTGR~1/8.1/lib/pgxs/src/MAKEFI~1/../../src/utils/dllinit.o -Lc:/PROGRA~1/POSTGR~1/8.1/bin -lpostgres c:\mingw\bin\..\lib\gcc-lib\mingw32\3.2.3\..\..\..\..\mingw32\bin\ld.exe: cannot find -lpostgres c:\mingw\bin\dllwrap.exe: c:\mingw\bin\gcc exited with status 1 make: *** [foo.dll] Error 1 rm foo.o $ make showme libdir = c:/PROGRA~1/POSTGR~1/8.1/lib SHLIB_LINK = -Lc:/PROGRA~1/POSTGR~1/8.1/bin -lpostgres $ export PATH=/usr/local/pgsql/bin:$PATH $ which pg_config /usr/local/pgsql/bin/pg_config $ rm -f foo.dll $ make gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -fno-strict-aliasing -I. -IC:/msys/1.0/local/pgsql/include/server -IC:/msys/1.0/local/pgsql/include/internal -I./src/include/port/win32 -DEXEC_BACKEND -IC:/msys/1.0/local/pgsql/lib/pgxs/src/MAKEFI~1/../../src/include/port/win32 -IC:/msys/1.0/local/pgsql/include/server/port/win32 -c -o foo.o foo.c dlltool --export-all --output-def foo.def foo.o dllwrap -o foo.dll --def foo.def foo.o C:/msys/1.0/local/pgsql/lib/pgxs/src/MAKEFI~1/../../src/utils/dllinit.o -LC:/msys/1.0/local/pgsql/bin -lpostgres c:\mingw\bin\..\lib\gcc-lib\mingw32\3.2.3\..\..\..\..\mingw32\bin\ld.exe: cannot find -lpostgres c:\mingw\bin\dllwrap.exe: c:\mingw\bin\gcc exited with status 1 make: *** [foo.dll] Error 1 rm foo.o $ make showme libdir = C:/msys/1.0/local/pgsql/lib SHLIB_LINK = -LC:/msys/1.0/local/pgsql/bin -lpostgres ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/makefiles/Makefile.cygwin
Re: [PATCHES] [HACKERS] pgxs/windows
Bruce Momjian pgman@candle.pha.pa.us writes: The difference between Magnus's and Andrew's is that Magnus used MODULE_big (which means create a shared library), while Andrew used MODULES. So, Magnus's only worked because he was creating a DLL and that brought in the -L from MODULE_big. Ah, good catch. I have applied the following patch to change /bin to /lib for Cygwin and Win32. What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] pgxs/windows
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: The difference between Magnus's and Andrew's is that Magnus used MODULE_big (which means create a shared library), while Andrew used MODULES. So, Magnus's only worked because he was creating a DLL and that brought in the -L from MODULE_big. Ah, good catch. I have applied the following patch to change /bin to /lib for Cygwin and Win32. What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. I considered that a separate issue and didn't explore it, but I think you are right that $(DESTDIR) makes no sense so I will remove it. Patch attached. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/makefiles/Makefile.cygwin === RCS file: /cvsroot/pgsql/src/makefiles/Makefile.cygwin,v retrieving revision 1.7 diff -c -c -r1.7 Makefile.cygwin *** src/makefiles/Makefile.cygwin 19 Jan 2006 20:00:54 - 1.7 --- src/makefiles/Makefile.cygwin 19 Jan 2006 20:42:50 - *** *** 2,8 DLLTOOL= dlltool DLLWRAP= dllwrap ifdef PGXS ! BE_DLLLIBS= -L$(DESTDIR)$(libdir) -lpostgres else BE_DLLLIBS= -L$(top_builddir)/src/backend -lpostgres endif --- 2,8 DLLTOOL= dlltool DLLWRAP= dllwrap ifdef PGXS ! BE_DLLLIBS= -L$(libdir) -lpostgres else BE_DLLLIBS= -L$(top_builddir)/src/backend -lpostgres endif Index: src/makefiles/Makefile.win32 === RCS file: /cvsroot/pgsql/src/makefiles/Makefile.win32,v retrieving revision 1.8 diff -c -c -r1.8 Makefile.win32 *** src/makefiles/Makefile.win3219 Jan 2006 20:00:54 - 1.8 --- src/makefiles/Makefile.win3219 Jan 2006 20:42:50 - *** *** 6,12 DLLTOOL= dlltool DLLWRAP= dllwrap ifdef PGXS ! BE_DLLLIBS= -L$(DESTDIR)$(libdir) -lpostgres else BE_DLLLIBS= -L$(top_builddir)/src/backend -lpostgres endif --- 6,12 DLLTOOL= dlltool DLLWRAP= dllwrap ifdef PGXS ! BE_DLLLIBS= -L$(libdir) -lpostgres else BE_DLLLIBS= -L$(top_builddir)/src/backend -lpostgres endif ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Uninstall scripts for contrib
Peter Eisentraut [EMAIL PROTECTED] writes: David Fetter wrote: It's paper bag time for me. How about a new patch which gives each contrib project its own directory and places them there? This would presumably imply that the installation scripts would be renamed to install.sql. On a green field this might make sense but as it is maybe it would break too much without much benefit? I agree, the current setup is not broken and doesn't need to be fixed. What *is* broken is the contrib documentation install layout, though, because of the modules that have additional documentation in sub-directories. Any thoughts what to do about that? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] pgxs/windows
Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. I considered that a separate issue and didn't explore it, but I think you are right that $(DESTDIR) makes no sense so I will remove it. Patch attached. If these are bogus then so are the uses in Makefile.aix and Makefile.darwin. I'm a bit bothered by the ones in pgxs.mk, too, although I suspect we have to leave those there for the benefit of contrib? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Uninstall scripts for contrib
Removed at submitter request. --- David Fetter wrote: On Mon, Jan 16, 2006 at 12:13:11AM -0500, Neil Conway wrote: On Sun, 2006-01-15 at 20:08 -0800, David Fetter wrote: ifdef USE_PGXS The change to $PostgreSQL$ is bogus (perhaps due to the way you setup cvsup?), as are all the other $PostgreSQL$ changes in the patch. Also, the patch doesn't actually add any files called uninstall.sql. Oops. My FM R'ing skills need some work. This patch includes the files. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778 Remember to vote! [ Attachment, skipping... ] ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] pgxs/windows
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. I considered that a separate issue and didn't explore it, but I think you are right that $(DESTDIR) makes no sense so I will remove it. Patch attached. If these are bogus then so are the uses in Makefile.aix and Makefile.darwin. Done, and backpatched. I'm a bit bothered by the ones in pgxs.mk, too, although I suspect we have to leave those there for the benefit of contrib? No idea, sorry. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/makefiles/Makefile.aix === RCS file: /cvsroot/pgsql/src/makefiles/Makefile.aix,v retrieving revision 1.23 diff -c -c -r1.23 Makefile.aix *** src/makefiles/Makefile.aix 28 Oct 2005 17:32:22 - 1.23 --- src/makefiles/Makefile.aix 19 Jan 2006 21:16:36 - *** *** 22,28 POSTGRES_IMP= postgres$(IMPSUFF) ifdef PGXS ! BE_DLLLIBS= -Wl,-bI:$(DESTDIR)$(bindir)/postgres/$(POSTGRES_IMP) else BE_DLLLIBS= -Wl,-bI:$(top_builddir)/src/backend/$(POSTGRES_IMP) endif --- 22,28 POSTGRES_IMP= postgres$(IMPSUFF) ifdef PGXS ! BE_DLLLIBS= -Wl,-bI:$(bindir)/postgres/$(POSTGRES_IMP) else BE_DLLLIBS= -Wl,-bI:$(top_builddir)/src/backend/$(POSTGRES_IMP) endif Index: src/makefiles/Makefile.darwin === RCS file: /cvsroot/pgsql/src/makefiles/Makefile.darwin,v retrieving revision 1.8 diff -c -c -r1.8 Makefile.darwin *** src/makefiles/Makefile.darwin 17 Dec 2004 03:52:48 - 1.8 --- src/makefiles/Makefile.darwin 19 Jan 2006 21:16:36 - *** *** 5,11 CFLAGS_SL = ifdef PGXS ! BE_DLLLIBS= -bundle_loader $(DESTDIR)$(bindir)/postgres else BE_DLLLIBS= -bundle_loader $(top_builddir)/src/backend/postgres endif --- 5,11 CFLAGS_SL = ifdef PGXS ! BE_DLLLIBS= -bundle_loader $(bindir)/postgres else BE_DLLLIBS= -bundle_loader $(top_builddir)/src/backend/postgres endif ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] pgxs/windows
If I understand correctly, atleast for the AIX case, the reason that Makefile.aix is referencing DESTDIR is for the PGXS case. I thought that PGXS was supposed to be able to be run after things were installed, and you might not have the compile tree anymore. Basically, if you wanted to build some extensions on top of a precompiled set of binaries. If we take the DESTDIR out of here, where should we get the POSTGRES_IMP file from? -rocco -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Tom Lane Sent: Thursday, January 19, 2006 3:54 PM To: Bruce Momjian Cc: Andrew Dunstan; Magnus Hagander; PostgreSQL-patches Subject: Re: [PATCHES] [HACKERS] pgxs/windows Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. I considered that a separate issue and didn't explore it, but I think you are right that $(DESTDIR) makes no sense so I will remove it. Patch attached. If these are bogus then so are the uses in Makefile.aix and Makefile.darwin. I'm a bit bothered by the ones in pgxs.mk, too, although I suspect we have to leave those there for the benefit of contrib? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] pgxs/windows
On Thu, 2006-01-19 at 15:33 -0500, Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: The difference between Magnus's and Andrew's is that Magnus used MODULE_big (which means create a shared library), while Andrew used MODULES. So, Magnus's only worked because he was creating a DLL and that brought in the -L from MODULE_big. Ah, good catch. As I understand the docs, one can use either to produce a dll. The difference is how it processes the name. I have applied the following patch to change /bin to /lib for Cygwin and Win32. What about the question of whether $(DESTDIR) belongs there or not? I think we had concluded that PGXS shouldn't ever use $(DESTDIR), because that's only for install-time stuff. Looks that way to me - IIRC it's only used by the regression suite. cheers andrew ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Uninstall scripts for contrib
On Thu, Jan 19, 2006 at 11:48:54AM -0800, David Fetter wrote: On Thu, Jan 19, 2006 at 07:08:28PM +0100, Peter Eisentraut wrote: David Fetter wrote: It's paper bag time for me. How about a new patch which gives each contrib project its own directory and places them there? This would presumably imply that the installation scripts would be renamed to install.sql. On a green field this might make sense but as it is maybe it would break too much without much benefit? You're right. I'll send a patch this evening my time that renames each to [module_name]_uninstall.sql. ISTM there's a pretty good usecase for going per-directory down the road. IIRC we already decided to do that for doc directories, and now that we'd have 2 scripts (_install and _uninstall) it seems a good idea for that as well. For backwards compatability maybe an install option that also installed the old-style _install.sql script would be adequate? Perhaps this could default to on for 8.2, off for 8.3 and gone for 8.4... -- Jim C. Nasby, Sr. Engineering Consultant [EMAIL PROTECTED] Pervasive Software http://pervasive.comwork: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Example for UPDATE FROM with correllation
Patch applied, with minor space adjustments to HEAD and 8.1.X. I also noticed a few earlier examples were missing paragraph formatting. --- David Fetter wrote: Folks, Please find enclosed a doc patch that adds an example of a correllated UPDATE. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778 Remember to vote! [ Attachment, skipping... ] ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/update.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/update.sgml,v retrieving revision 1.33 diff -c -c -r1.33 update.sgml *** doc/src/sgml/ref/update.sgml12 Oct 2005 23:19:22 - 1.33 --- doc/src/sgml/ref/update.sgml19 Jan 2006 23:08:08 - *** *** 205,218 --- 205,236 WHERE accounts.name = 'Acme Corporation' AND employees.id = accounts.sales_person; /programlisting + /para + para Perform the same operation, using a sub-select in the literalWHERE/literal clause: programlisting UPDATE employees SET sales_count = sales_count + 1 WHERE id = (SELECT sales_person FROM accounts WHERE name = 'Acme Corporation'); /programlisting + /para + para +Now that all the papers are signed, update the most recently closed +deal of the travelling salesperson who closed the Rocket Powered +Skates deal with the Acme Corporation. + programlisting + UPDATE employees SET last_closed_deal = deal.id + FROM accounts JOIN deals ON (account.id = deal.account_id) + WHERE deal.employee_id = employees.id + AND deal.name = 'Rocket Powered Skates' + AND accounts.name = 'Acme Corporation' + ORDER BY deal.signed_date DESC LIMIT 1; + /programlisting + /para + + para Attempt to insert a new stock item along with the quantity of stock. If the item already exists, instead update the stock count of the existing item. To do this without failing the entire transaction, use savepoints. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] pgxs/windows
Rocco Altier [EMAIL PROTECTED] writes: If we take the DESTDIR out of here, where should we get the POSTGRES_IMP file from? In the PGXS case, $(bindir) has been gotten from pg_config, and it's correct as-is by definition. Adding $(DESTDIR) to it cannot be right. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Fix overflow of bgwriter's request queue
ITAGAKI Takahiro [EMAIL PROTECTED] writes: Attached is a revised patch. It became very simple, but I worry that one magic number (BUFFERS_PER_ABSORB) is still left. Have you checked that this version of the patch fixes the problem you saw originally? Does the problem come back if you change BUFFERS_PER_ABSORB to too large a value? If you can identify a threshold where the problem reappears in your test case, that would help us choose the right value to use. I suspect it'd probably be sufficient to absorb requests every few times through the fsync loop, too, if you want to experiment with that. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
