Re: [PATCHES] Preliminary patch for FRONTEND
> > The following patch addresses this issue by making libpgport usable > > unchanged by client applications, and makes a special > server version > > for the backend. > > This raises some alarm bells for me. Why does a "port > support" library need to distinguish whether it is running in > frontend or backend? Just from the problems I've seen with several modules - ereport(). Several functions use ereport() if !FRONTEND and something else if FRONTEND. I've seen this problem several times when trying to compile things "out of sync". The main issue is that the port stuff behave differently, certainly. I originally thought the deal was that anything that relied on backend stuff would go in backend/port, but there are (and has been since before I started looking at it) several files in /port/ taht rely heavily on functions and variables in the backend. //Magnus ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Preliminary patch for FRONTEND
Magnus Hagander wrote: > > > The following patch addresses this issue by making libpgport usable > > > unchanged by client applications, and makes a special > > server version > > > for the backend. > > > > This raises some alarm bells for me. Why does a "port > > support" library need to distinguish whether it is running in > > frontend or backend? > > Just from the problems I've seen with several modules - ereport(). > Several functions use ereport() if !FRONTEND and something else if > FRONTEND. > > I've seen this problem several times when trying to compile things "out > of sync". The main issue is that the port stuff behave differently, > certainly. I originally thought the deal was that anything that relied > on backend stuff would go in backend/port, but there are (and has been > since before I started looking at it) several files in /port/ taht rely > heavily on functions and variables in the backend. The basic issue is ereport and memory allocation for dirmod, ereport for exec.c, thread-safety for getaddrinfo and thread.c. I can't think of how to cleanly abstract them so the backend and libpq have compatibile versions to call so I can use the same object file, so I made new libraries. And as Magnus has pointed out, they do get out of sync too easily without this cleanup. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Preliminary patch for FRONTEND
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > The following patch addresses this issue by making libpgport usable > > unchanged by client applications, and makes a special server version for > > the backend. > > This raises some alarm bells for me. Why does a "port support" library > need to distinguish whether it is running in frontend or backend? > Certainly the standard Unix library calls that it is purporting to > replace do not care. Already described --- ereport, memory allocation, and thread safety. > Also, the patch does both more and less than I'd expect from your > description. Why are there no changes at all to C code? And how can > you just remove, eg, dirmod.c and exec.c from pg_resetxlog? Basically libpgport now is compiled for the frontend so it just gets it from the link line rather than build its own. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Preliminary patch for FRONTEND
Neil Conway wrote: > Pasto: > > *** src/bin/pg_config/Makefile 1 Aug 2004 06:56:38 - 1.8 > --- src/bin/pg_config/Makefile 1 Oct 2004 04:04:06 - > *** > *** 1,18 > [...] > --- 1,23 > ! > #- > ! # > ! # Makefile for src/bin/pg_controldata > ! # > ! # Copyright (c) 1998-2002, PostgreSQL Global Development Group > ! # > ! # $PostgreSQL: pgsql-server/src/bin/pg_controldata/Makefile,v 1.9 > 2004/05/26 17:24:01 tgl Exp $ > ! # > ! > #- > > s/pg_controldata/pg_config/ (and maybe fix the copyright date range?) > > *** src/port/Makefile 28 Aug 2004 22:55:06 - 1.18 > --- src/port/Makefile 1 Oct 2004 04:04:11 - > *** > *** 48,51 > echo "#define LOCALEDIR \"$(localedir)\"" >>$@ > > clean distclean maintainer-clean: > ! rm -f libpgport.a $(LIBOBJS) pg_config_paths.h > --- 68,71 > echo "#define LOCALEDIR \"$(localedir)\"" >>$@ > > clean distclean maintainer-clean: > ! rm -f libpgport.a $(LIBOBJS) $(LIBOBJS_SRV) pg_config_paths.h > > "clean" should remove libpgport_srv.a as well, shouldn't it? Fixed, thanks. > *** src/port/getaddrinfo.c 28 Sep 2004 00:07:01 - 1.15 > --- src/port/getaddrinfo.c 1 Oct 2004 04:04:12 - > *** > *** 26,31 > --- 26,32 > #include > #include > #include > + #include > #endif > > #include "getaddrinfo.h" > > What is this change for? My OS couldn't compile getaddrinfo when I tried, though my OS doesn't need getaddrinfo so maybe we shouldn't make that change. Comments? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
[PATCHES] Int_aggregate/pgstattuple minor fix
The patch below removes attempts to drop objects before creating them from the int_aggregate and pgstattuple SQL scripts. This causes the script to fail on databases where the affected objects don't already exist when the script is executed by the Win32 installer, pgAdmin or any other interface that executes scripts 'as-is'. None of the other contrib scripts appear do this. Please apply. Regards, Dave Index: int_aggregate.sql.in === RCS file: /projects/cvsroot/pgsql-server/contrib/intagg/int_aggregate.sql.in,v retrieving revision 1.5 diff -u -r1.5 int_aggregate.sql.in --- int_aggregate.sql.in14 May 2003 03:25:56 - 1.5 +++ int_aggregate.sql.in1 Oct 2004 13:23:25 - @@ -17,7 +17,7 @@ -- The aggration funcion. -- uses the above functions to create an array of integers from an aggregation. -DROP AGGREGATE int_array_aggregate(int4); +-- DROP AGGREGATE int_array_aggregate(int4); CREATE AGGREGATE int_array_aggregate ( BASETYPE = int4, SFUNC = int_agg_state, Index: pgstattuple.sql.in === RCS file: /projects/cvsroot/pgsql-server/contrib/pgstattuple/pgstattuple.sql.in,v retrieving revision 1.8 diff -u -r1.8 pgstattuple.sql.in --- pgstattuple.sql.in 12 Jun 2003 08:02:53 - 1.8 +++ pgstattuple.sql.in 1 Oct 2004 13:30:27 - @@ -1,7 +1,7 @@ -- Adjust this setting to control where the objects get created. SET search_path = public; -DROP TYPE pgstattuple_type CASCADE; +-- DROP TYPE pgstattuple_type CASCADE; CREATE TYPE pgstattuple_type AS ( table_len BIGINT, -- physical table length in bytes tuple_count BIGINT, -- number of live tuples ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plperl features
any comments on my last patch? -- g.gRay: PL/perl, PL/PHP ;) ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] plperl features
Sergej Sergeev wrote: any comments on my last patch? Speaking for myself, I am not really thinking much about new plperl stuff until we get closer to branching the code, which as I understand the current processes would be at the time we release an RC - that seems a little way off yet. cheers andrew ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Preliminary patch for FRONTEND
Bruce Momjian <[EMAIL PROTECTED]> writes: >> + #include >> >> What is this change for? > My OS couldn't compile getaddrinfo when I tried, though my OS doesn't > need getaddrinfo so maybe we shouldn't make that change. Comments? Don't put it in. That looks like the sort of file that isn't even going to exist on a lot of platforms. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Int_aggregate/pgstattuple minor fix
"Dave Page" <[EMAIL PROTECTED]> writes: > The patch below removes attempts to drop objects before creating them > from the int_aggregate and pgstattuple SQL scripts. This causes the > script to fail on databases where the affected objects don't already > exist when the script is executed by the Win32 installer, pgAdmin or any > other interface that executes scripts 'as-is'. None of the other contrib > scripts appear do this. Seems reasonable. You could also make the argument that the DROPs actually risk dropping the wrong object, since they could find a matching name in the schema search path that is not in the current schema (and thus not really a conflict). regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] LDFLAGS overriding
Reini Urban wrote: > I think that LDFLAGS overriding is in some situations bad for newer > libtool, as it is used with some postgresql contrib makefiles and > interfaces. We're not using libtool. > LIBS are added to LDFLAGS where they really should be added to > LIBS, not LDFLAGS. > This causes incorrect order in the linker command-line, which fails > in building shared libs. A similar problem as with building php. Please point out where that happens so it can be corrected. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] SSL on win32
>> Hello! >> >> Here is a patch to fix win32 ssl builds. Summary of changes: >> > >> >> I'd appreciate it if one of the win32 guys can confirm that >> this patch fixes the build for them as well. > >Yup, looks good to me. Compiles OK and SSL appears to work >just fine :-) Okay. Then please commit, assuming it's ok in other aspects. Would love to get this in the next beta of the installer :-) //Magnus ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
[PATCHES] [PATCH] 5 plperl patches
I have attached 5 patches (split up for ease of review) to plperl.c.
1. Two minor cleanups:
- We don't need to call hv_exists+hv_fetch; we should just check the
return value of hv_fetch.
- newSVpv("undef",0) is the string "undef", not a real undef.
2. This should fix the bug Andrew Dunstan described in a recent -hackers
post. It replaces three bogus "eval_pv(key, 0)" calls with newSVpv,
and eliminates another redundant hv_exists+hv_fetch pair.
3. plperl_build_tuple_argument builds up a string of Perl code to create
a hash representing the tuple. This patch creates the hash directly.
4. Another minor cleanup: replace a couple of av_store()s with av_push.
5. Analogous to #3 for plperl_trigger_build_args. This patch removes the
static sv_add_tuple_value function, which does much the same as two
other utility functions defined later, and merges the functionality
into plperl_hash_from_tuple.
I have tested the patches to the best of my limited ability, but I would
appreciate it very much if someone else could review and test them too.
(Thanks to Andrew and David Fetter for their help with some testing.)
-- ams
--- plperl.c~ 2004-10-02 04:12:05.189765562 +0530
+++ plperl.c2004-10-02 04:12:28.017002164 +0530
@@ -1270,6 +1270,7 @@ compile_plperl_function(Oid fn_oid, bool
int proname_len;
plperl_proc_desc *prodesc = NULL;
int i;
+ SV **svp;
/* We'll need the pg_proc tuple in any case... */
procTup = SearchSysCache(PROCOID,
@@ -1292,12 +1293,12 @@ compile_plperl_function(Oid fn_oid, bool
/
* Lookup the internal proc name in the hashtable
/
- if (hv_exists(plperl_proc_hash, internal_proname, proname_len))
+ svp = hv_fetch(plperl_proc_hash, internal_proname, proname_len, FALSE);
+ if (svp)
{
booluptodate;
- prodesc = (plperl_proc_desc *) SvIV(*hv_fetch(plperl_proc_hash,
-
internal_proname, proname_len, 0));
+ prodesc = (plperl_proc_desc *) SvIV(*svp);
/
* If it's present, must check whether it's still up to date.
@@ -1625,7 +1626,7 @@ plperl_hash_from_tuple(HeapTuple tuple,
if (attdata)
hv_store(array, attname, strlen(attname), newSVpv(attdata, 0),
0);
else
- hv_store(array, attname, strlen(attname), newSVpv("undef", 0),
0);
+ hv_store(array, attname, strlen(attname), newSV(0), 0);
}
return array;
}
--- plperl.c~ 2004-10-02 04:14:43.768721344 +0530
+++ plperl.c2004-10-02 04:14:53.325733327 +0530
@@ -451,7 +451,7 @@ plperl_get_keys(HV *hv)
hv_iterinit(hv);
while ((val = hv_iternextsv(hv, (char **) &key, &klen)))
{
- av_store(ret, key_count, eval_pv(key, TRUE));
+ av_store(ret, key_count, newSVpv(key, 0));
key_count++;
}
hv_iterinit(hv);
@@ -484,11 +484,8 @@ plperl_get_key(AV *keys, int index)
static char *
plperl_get_elem(HV *hash, char *key)
{
- SV**svp;
-
- if (hv_exists_ent(hash, eval_pv(key, TRUE), FALSE))
- svp = hv_fetch(hash, key, strlen(key), FALSE);
- else
+ SV **svp = hv_fetch(hash, key, strlen(key), FALSE);
+ if (!svp)
{
elog(ERROR, "plperl: key '%s' not found", key);
return NULL;
@@ -998,7 +995,8 @@ plperl_func_handler(PG_FUNCTION_ARGS)
g_attr_num = tupdesc->natts;
for (i = 0; i < tupdesc->natts; i++)
- av_store(g_column_keys, i + 1,
eval_pv(SPI_fname(tupdesc, i + 1), TRUE));
+ av_store(g_column_keys, i + 1,
+newSVpv(SPI_fname(tupdesc, i+1), 0));
slot = TupleDescGetSlot(tupdesc);
funcctx->slot = slot;
--- plperl.c~ 2004-10-02 04:15:16.737864847 +0530
+++ plperl.c2004-10-02 04:16:36.108850361 +0530
@@ -1519,7 +1519,7 @@ static SV *
plperl_build_tuple_argument(HeapTuple tuple, TupleDesc tupdesc)
{
int i;
- SV *output;
+ HV *hv;
Datum attr;
boolisnull;
char *attname;
@@ -1527,31 +1527,22 @@ plperl_build_tuple_argument(HeapTuple tu
HeapTuple typeTup;
Oid typoutput;
Oid typioparam;
+ int namelen;
- output = sv_2mortal(newSVpv
Re: [PATCHES] 8.0.0beta3 release documentation fixes for Tcl unbundling
On Fri, Oct 01, 2004 at 11:59:56AM +1000, Neil Conway wrote: > On Thu, 2004-09-30 at 10:30, ljb wrote: > > These corrections against 8.0.0beta3 are for removal of the Tcl interface > > from core. > > Patch applied to HEAD, with some editorializing. autoconf (2.53) re-run. > > For future reference, patches should be made with a consistent directory > prefix (i.e. if you're going to make some hunks of the patch relevant to > the top-level of the PG source tree, please do so for all hunks). Sorry. Yes I should know better. > > In addition, please delete doc/man1/pgtclsh.1 and doc/man1/pgtksh.1 from > > the distribution, since these commands are no longer included. > > I couldn't see these files in the beta3 distribution. Sorry again; that should be man/man1 not doc/man1, and that's where they get installed (which isn't what you want). They arrive in the distribution inside the man.tar.gz file: $ tar -tvzf doc/man.tar.gz | grep pgt -rw-r--r-- petere/wheel852 2003-11-02 06:39:09 man1/pgtclsh.1 -rw-r--r-- petere/wheel963 2003-11-02 06:39:08 man1/pgtksh.1 And they get built from this source file: doc/src/sgml/ref/pgtclsh.sgml I don't think it is explicitly referenced from any Makefile or other sgml file. ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Cosmetic changes
Tom Lane wrote: > Euler Taveira de Oliveira <[EMAIL PROTECTED]> writes: > > This small patch correct a hint style and change from multiple to > > single line comment because gettext seems not to like multiple line > > comments. Then we could see the "translator: ..." in po files too. > > I believe most of these changes are fixing comments that were written as > single lines to begin with, and were changed to multiline format by > pgindent which has extremely narrow (ahem) ideas about the appropriate > right margin for code and comments. Bruce, could we *please* fix > pgindent to not enforce a 72-column limit? 72 hasn't been interesting > since people stopped punching FORTRAN code into cards. 79 would be a > reasonable line length limit. It was set to 75. Updated to 79. I assume you don't want a rerun now. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Preliminary patch for FRONTEND
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > >> + #include > >> > >> What is this change for? > > > My OS couldn't compile getaddrinfo when I tried, though my OS doesn't > > need getaddrinfo so maybe we shouldn't make that change. Comments? > > Don't put it in. That looks like the sort of file that isn't even going > to exist on a lot of platforms. OK, removed. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Preliminary patch for FRONTEND
Bruce Momjian wrote: > As many know, the FRONTEND usage of /src/port is very fragile. It > requires every binary that uses certain libpgport object files to create > its own version, which is very fragile, and could easily break if a > function call is added in a subrelease, especially on certain ports. > > The following patch addresses this issue by making libpgport usable > unchanged by client applications, and makes a special server version for > the backend. This is much less fragile than trying to make sure you > hit every single binary that uses libpgport. > > I originally tried abstracting out the #ifdef FRONTEND defines in pgport > but found that that wasn't easily done so making separate libraries was > cleaner. Here is an updated version of the patch: ftp://candle.pha.pa.us/pub/postgresql/mypatches I made the suggested modifications and added more comments. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
