Re: [PATCHES] psql: add volatility to \df+

2007-06-27 Thread Neil Conway
On Wed, 2007-27-06 at 00:16 -0400, Alvaro Herrera wrote:
 +1 as well but I'm wondering whether the output is too wide.

Well, the output of \df+ is already likely to be wider than 72
characters, so I'm not sure the patch would really make the situation
materially worse (\df+ nextval is 118 characters wide without the
patch, for example). You could also argue that if the user specifies
+, presumably they are less worried about a concise output format.

 Is it possible to find some shorter string to convey the same meaning?

Nothing obvious comes to mind, but I'm not opposed in principle. Any
suggestions?

-Neil



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

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


Re: [PATCHES] Cancel autovacuum conflicting with DROP TABLE

2007-06-27 Thread ITAGAKI Takahiro

Alvaro Herrera [EMAIL PROTECTED] wrote:

 1. changing SIGINT so that it cancels the current table instead of
 shutting down the entire worker.
 
 2. changing DROP TABLE and TRUNCATE so that they cancel an autovac
 worker by sending SIGINT.

Quite so.

 3. change the interrupt code so that autovac workers are terminated with
 ERROR instead of FATAL, knowing that the final outcome is the same.  If
 I'm not mistaken the only point of the change is to be able to change
 the error message, is that correct?

Yes, I used ERROR instead of FATAL just for changing the message.
In addition, the actual message level will be LOG, instead of FATAL.

 I don't feel very much comfortable with the patch (3).  I would prefer
 that we keep errcode FATAL and select a different error message in
 ProcessInterrupts instead.  I don't see the point in complicating the
 sigjmp target without need.

My first patch did so and I changed it in the second patch because
I feel the FATAL entry in syslog is too obtrusive; Internal termination
of autovacuum workers in this way is ignorable for normal users.
However, I'm sure that my complaint is not so important. Please don't
apply thie part of the patch if you don't like it.


 I agree with the (2) change.
 
 The change in (1) I don't feel comfortable with commenting.  It seems
 strange to me, and although it seems like it would be safe, I cannot
 help having a strange feeling about it.  I'll try to digest it a bit
 more.

Thanks. I forgot to adjust comments in the code, sorry.

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



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


[PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Jacob Rief
This is a request I posted in February. The thread was named Writing
triggers in C++. However I did not supply a patch then, and some people
misunderstood my problem. I will try to explain it again:

My problem is, I wrote some triggers in C using the SPI-API. Those
triggers call some functions defined in an external C++ library. In
order to use name-mangled functions, namespaces and C++-header-files,
provided by this library, I have to use a C++ compiler to compile my
trigger-functions. But the C++-compiler rejects to compile legal C code,
because some of the included Postgres-headers, ie. postgres.h,
executor/spi.h, commands/trigger.h, fmgr.h use a few C++ keywords to
defined a some struct members and function arguments. The incriminating
C++-keywords used in the Postgres-headers are: 'typeid', 'typename' and
'using'.

It would do no harm to the Postgres-sources if these keywords would be
replaced with a similar identifier. I wrote a patch which applies cleanly
onto version 8.2.4 (and 8.2.3) and keeps the Postgres binary compatible to
an unpatched version.

I would appreciate to see this patch applied onto the Postgres-sources.
Other authors using the SPI-API together with a C++-compiler would also
benefit from this patch.

Regards, Jacob
diff -ur postgresql-8.2.4/src/backend/access/common/tupdesc.c postgresql-8.2.4-c++-safe/src/backend/access/common/tupdesc.c
--- postgresql-8.2.4/src/backend/access/common/tupdesc.c	2006-07-14 16:52:16.0 +0200
+++ postgresql-8.2.4-c++-safe/src/backend/access/common/tupdesc.c	2007-06-27 23:28:21.0 +0200
@@ -533,17 +533,17 @@
 		attnum++;
 
 		attname = entry-colname;
-		atttypmod = entry-typename-typmod;
-		attdim = list_length(entry-typename-arrayBounds);
+		atttypmod = entry-type_name-typmod;
+		attdim = list_length(entry-type_name-arrayBounds);
 
-		if (entry-typename-setof)
+		if (entry-type_name-setof)
 			ereport(ERROR,
 	(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
 	 errmsg(column \%s\ cannot be declared SETOF,
 			attname)));
 
 		TupleDescInitEntry(desc, attnum, attname,
-		   typenameTypeId(NULL, entry-typename),
+		   typenameTypeId(NULL, entry-type_name),
 		   atttypmod, attdim);
 
 		/* Fill in additional stuff not handled by TupleDescInitEntry */
diff -ur postgresql-8.2.4/src/backend/commands/sequence.c postgresql-8.2.4-c++-safe/src/backend/commands/sequence.c
--- postgresql-8.2.4/src/backend/commands/sequence.c	2006-10-06 19:13:58.0 +0200
+++ postgresql-8.2.4-c++-safe/src/backend/commands/sequence.c	2007-06-27 23:28:21.0 +0200
@@ -135,48 +135,48 @@
 		switch (i)
 		{
 			case SEQ_COL_NAME:
-coldef-typename = makeTypeNameFromOid(NAMEOID, -1);
+coldef-type_name = makeTypeNameFromOid(NAMEOID, -1);
 coldef-colname = sequence_name;
 namestrcpy(name, seq-sequence-relname);
 value[i - 1] = NameGetDatum(name);
 break;
 			case SEQ_COL_LASTVAL:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = last_value;
 value[i - 1] = Int64GetDatumFast(new.last_value);
 break;
 			case SEQ_COL_INCBY:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = increment_by;
 value[i - 1] = Int64GetDatumFast(new.increment_by);
 break;
 			case SEQ_COL_MAXVALUE:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = max_value;
 value[i - 1] = Int64GetDatumFast(new.max_value);
 break;
 			case SEQ_COL_MINVALUE:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = min_value;
 value[i - 1] = Int64GetDatumFast(new.min_value);
 break;
 			case SEQ_COL_CACHE:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = cache_value;
 value[i - 1] = Int64GetDatumFast(new.cache_value);
 break;
 			case SEQ_COL_LOG:
-coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+coldef-type_name = makeTypeNameFromOid(INT8OID, -1);
 coldef-colname = log_cnt;
 value[i - 1] = Int64GetDatum((int64) 1);
 break;
 			case SEQ_COL_CYCLE:
-coldef-typename = makeTypeNameFromOid(BOOLOID, -1);
+coldef-type_name = makeTypeNameFromOid(BOOLOID, -1);
 coldef-colname = is_cycled;
 value[i - 1] = BoolGetDatum(new.is_cycled);
 break;
 			case SEQ_COL_CALLED:
-coldef-typename = makeTypeNameFromOid(BOOLOID, -1);
+coldef-type_name = makeTypeNameFromOid(BOOLOID, -1);
 coldef-colname = is_called;
 value[i - 1] = BoolGetDatum(false);
 break;
diff -ur postgresql-8.2.4/src/backend/commands/tablecmds.c postgresql-8.2.4-c++-safe/src/backend/commands/tablecmds.c
--- postgresql-8.2.4/src/backend/commands/tablecmds.c	2007-02-02 01:07:27.0 +0100
+++ 

Re: [PATCHES] Load Distributed Checkpoints, final patch

2007-06-27 Thread Tom Lane
Heikki Linnakangas [EMAIL PROTECTED] writes:
 Here's latest revision of Itagaki-sans Load Distributed Checkpoints patch:

Applied with some minor revisions to make some of the internal APIs a
bit cleaner; mostly, it seemed like a good idea to replace all those
bool parameters with a flag-bits approach, so that you could have
something like CHECKPOINT_FORCE | CHECKPOINT_WAIT instead of
false, true, true, false ...

For the moment I removed all the debugging elog's in the patch.
We still have Greg Smith's checkpoint logging patch to look at
(which I suppose needs adjustment now), and that seems like the
appropriate venue to consider what to put in.

Also, the question of redesigning the bgwriter's LRU scan is
still open.  I believe that's on Greg's plate, too.

One other closely connected item that might be worth looking at is the
code for creating new future xlog segments (PreallocXlogFiles).  Greg
was griping upthread about xlog segment creation being a real
performance drag.  I realized that as we currently have it set up, the
checkpoint code is next to useless for high-WAL-volume installations,
because it only considers making *one* future XLOG segment.  Once you've
built up enough XLOG segments, the system isn't too bad about recycling
them, but there will be a nasty startup transient where foreground
processes have to stop and make the things.  I wonder whether it would
help if we (a) have the bgwriter call PreallocXlogFiles during its
normal loop, and (b) back the slop in PreallocXlogFiles way off, so that
it will make a future segment as soon as we start using the last
existing segment, instead of only when we're nearly done.  This would at
least make it more likely that the bgwriter does the work instead of a
foreground process.  I'm hesitant to go much further than that, because
I don't want to bloat the minimum disk footprint for low-volume
installations, but the minimum footprint is really 2 xlog files anyway...

regards, tom lane

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

   http://archives.postgresql.org


Re: [PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Neil Conway
On Thu, 2007-06-28 at 00:43 +0200, Jacob Rief wrote:
 But the C++-compiler rejects to compile legal C code,
 because some of the included Postgres-headers, ie. postgres.h,
 executor/spi.h, commands/trigger.h, fmgr.h use a few C++ keywords to
 defined a some struct members and function arguments. The incriminating
 C++-keywords used in the Postgres-headers are: 'typeid', 'typename' and
 'using'.
 
 It would do no harm to the Postgres-sources if these keywords would be
 replaced with a similar identifier.

No objection, although it would be nice if we could find something nicer
to rename using to than using_. What about using_clause or
using_list?

You also changed namespace = name_space in builtins.h; is that
necessary?

 I wrote a patch which applies cleanly onto version 8.2.4

Patches should be submitted against the CVS HEAD code: we wouldn't want
to apply a patch like this to the REL8_2_STABLE branch, in any case.

BTW, I notice the patch also adds 'extern C { ... }' statements to a
few random header files. Can't client programs do this before including
the headers, if necessary?

-Neil



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

   http://archives.postgresql.org


Re: [PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Andrew Dunstan



Neil Conway wrote:


BTW, I notice the patch also adds 'extern C { ... }' statements to a
few random header files. Can't client programs do this before including
the headers, if necessary?


  


He's used the usual #ifdef __cplusplus wrapper - isn't that good enough?

cheers

andrew

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

  http://archives.postgresql.org


Re: [PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Neil Conway
On Wed, 2007-06-27 at 20:51 -0400, Andrew Dunstan wrote:
 He's used the usual #ifdef __cplusplus wrapper - isn't that good enough?

Well, there's nothing wrong with it per se, I'm just wondering (1) what
the proper set of headers to add it to is -- the patch just does a
handful (2) whether we need to bother doing it at all -- can't clients
do the extern C trick before including any Postgres headers?

-Neil



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


Re: [PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Tom Lane
Neil Conway [EMAIL PROTECTED] writes:
 On Wed, 2007-06-27 at 20:51 -0400, Andrew Dunstan wrote:
 He's used the usual #ifdef __cplusplus wrapper - isn't that good enough?

 Well, there's nothing wrong with it per se, I'm just wondering (1) what
 the proper set of headers to add it to is -- the patch just does a
 handful (2) whether we need to bother doing it at all -- can't clients
 do the extern C trick before including any Postgres headers?

The whole thing has been proposed before and rejected before.

The patch as given merely renames some random identifiers that happen to
be keywords in some non-C language (thereby breaking not only a lot of
core backend code, which we can fix, but who knows what other
user-written extensions that we can't fix so easily).  The *real*
problem here is that to make this useful, we have to buy into the
assumption that C++ should work in the backend.  The error handling
assumptions are completely incompatible (setjmp and throw do not usually
interoperate), and there are a lot of other bits of infrastructure that
C++ expects that are not likely to be in a C-based executable either.

I might be willing to accept this patch if I thought it were the end of
the story, but I know that it is only the camel's nose poking under the
tent.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [PATCHES] SPI-header-files safe for C++-compiler

2007-06-27 Thread Neil Conway
On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote:
 The patch as given merely renames some random identifiers that happen to
 be keywords in some non-C language (thereby breaking not only a lot of
 core backend code, which we can fix, but who knows what other
 user-written extensions that we can't fix so easily).

The fact is, any user-written extensions that depend on types defined in
parsenodes.h and primnodes.h are going to get broken all the time
*anyway*, so I don't see this as a major disadvantage. Doing
s/typename/type_name/g is likely to be the least of your concerns if
your extension integrates with Postgres that closely.

It would be one thing if we made a significant effort to preserve
internal API stability -- but we plainly do not. (See the varlena API
changes made in 8.3 for an example of an API change that will break far
more user-written extensions.)

 The *real* problem here is that to make this useful, we have to buy into
 the assumption that C++ should work in the backend.

I agree that C++ in the backend is always going to be a little hokey,
but (a) I don't agree that it is completely impossible (b) if we can
make people's lives a bit easier, I don't see a good reason not to. Like
it or not, people have called into C++ libraries from C UDFs in the
past, and likely will do so in the future.

 The error handling assumptions are completely incompatible (setjmp and
 throw do not usually interoperate)

AFAIK this is resolvable with some degree of pain: before entering C++
land, wrap the call site in a C++ exception handler, and before calling
back into Postgres, use a PG_TRY() block to rethrow elog(ERROR) as a C++
exception (and then rethrow as an elog(ERROR) once you've unwound the
C++ portion of the stack) ... hey, I didn't say it was clean ;-)

It's also worth noting that some people use C++ as C with classes, and
disallow the use of exceptions, RTTI, and that sort of stuff. Calling
into such code from the backend is marginally more sane.

-Neil



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