Re: [HACKERS] blatantly a bug in the documentation
On Tue, Nov 25, 2008 at 1:42 AM, Robert Treat [EMAIL PROTECTED] wrote: We actually have such a database on pgfoundry already (http://pgfoundry.org/frs/download.php/1719/pagila-0.10.1.zip), which i think devrim may have packaged into an rpm; it wouldn't hurt to add it to the win32 installer, but would you feel better if it were a contrib module or something? I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install - however, there is only a point to that if all the documentation examples are based on that database to allow copy-paste-play. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
Hiroshi Saito [EMAIL PROTECTED] wrote: Umm, format operand seems to be a wide character sequence. Here is a patch to work around the wide character format string. The hack is the following line: +#define strftime(a,b,c,d) strftime_win32(a,b,L##c,d) We use only literals in the format strings, the macro adds wide character prefix (L...) to them. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center pg_locale-1125.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments to Synchronous replication patch v3
On Tue, Nov 25, 2008 at 11:42 AM, ITAGAKI Takahiro
[EMAIL PROTECTED] wrote:
Fujii Masao [EMAIL PROTECTED] wrote:
On Fri, Nov 14, 2008 at 7:15 PM, Fujii Masao [EMAIL PROTECTED] wrote:
Attached is the latest version of Synch Rep patch. Sorry for my late
posting.
I fixed some bugs in v1patch and integrated walreceiver into core.
I have some comments about v3 patch.
Hi, Itagaki-san. Thanks for taking time to review the patch.
[1] Should walsender database be real or not?
[2] User-configurable replication_timeout is dangerous
[3] How to configure wal_buffers and wal_sender_delay?
[4] XLogArchivingActive on replication mode
[5] Usage of XLog Send-Flush-Wait
[6] Bit-OR or Logical-OR
[1] Should walsender database be real or not?
Index: bin/initdb/initdb.c
+ CREATE DATABASE walsender;\n,
You create walsender as a *real* database, but is it really needed?
Can we make it a virtual database only for walreceiver?
I think that the real database is better, because it's simpler and the
user can re-create it easily even if it is lost accidentally. Of course,
if we introduce the new API to handle a virtual database, we can
virtualize it. Is it worth introducing it?
And backend process crashes when I login the walsender database:
$ psql walsender
psql: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Obviously undesirable behavior :(
I will fix it.
Even if we need to have the database in real, I'd like to use another
name for it. The name 'walsender' seems to be an internal module name
but it should be a feature name (ex. 'replication').
Agreed. The name 'replication' is more suitable, I also think.
Any other ideas?
[2] User-configurable replication_timeout is dangerous
Index: backend/utils/misc/guc.c
+ {replication_timeout, PGC_USERSET, WAL_REPLICATION,
You export replication_timeout as a PGC_USERSET variable, but it is
dangerous. It allows non-superusers to kill servers easily by setting it
too low value. Walsender dies with FATAL on timeout.
BTW, how about adding an option to choose FATAL or ERROR on timeout?
I think FATAL is too strong for some cases.
OK, I will add the new GUC option for specifying the behavior when the
timeout occurs. I think the valid values are ERROR, FATAL and PANIC.
* ERROR only cancels that the backend waits for replication. The backends
(including the canceled one) and walsender continue processing. But,
synchronous replication might be broken.
* FATAL terminates walsender. The backends continue processing without
replication.
* PANIC terminates all processes. In previous discussion, someone wanted
this behavior.
Or, should I define replication_timeout as PGC_SUSET?
[3] How to configure wal_buffers and wal_sender_delay?
The parameter wal_buffers might be more important in synch rep,
but we don't have a mean to know whether wal_buffers is enough or not.
Should we have a new system view 'pg_stat_xlog' to find shortage
of wal_buffers and wal_sender_delay?
Couting the number of times which the buffer page is replaced in
AdvanceXLInsertBuffer might be help to configure them. Of course,
this kind of feature is very useful. But, is it essential for Synch Rep?
If not, I'd like to postpone it to 8.5.
[4] XLogArchivingActive on replication mode
XLogArchivingActive is not modified in the patch, but is it safe?
For example, we might need to disable COPY-without-wal optimization
when replication is enabled.
You are right! I will fix it.
[5] Usage of XLog Send-Flush-Wait
There are many following sequences:
RequestXLogSend(WriteRqstPtr, true);
XLogFlush(WriteRqstPtr);
WaitXLogSend(WriteRqstPtr, false, true);
It introduces additional complexities to use XLogFlush.
Is it possible to push RequestXLogSend and WaitXLogSend into XLogFlush?
It might require a new flag argument in XLogFlush.
OK, I will push them into XLogFlush.
[6] Bit-OR or Logical-OR
WaitXLogSend(XactLastRecEnd, false, forceSyncCommit | haveNonTemp);
should be
WaitXLogSend(XactLastRecEnd, false, forceSyncCommit || haveNonTemp);
| is bit OR and || is logical OR.
Oops! I will fix it.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
Dave Page wrote: On Tue, Nov 25, 2008 at 1:42 AM, Robert Treat [EMAIL PROTECTED] wrote: We actually have such a database on pgfoundry already (http://pgfoundry.org/frs/download.php/1719/pagila-0.10.1.zip), which i think devrim may have packaged into an rpm; it wouldn't hurt to add it to the win32 installer, but would you feel better if it were a contrib module or something? I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install - however, there is +1 It's reasonably easy to create a sample database during the initdb phase automatically. It's rather easy for an experienced DBA to dropdb sampledb to get rid of it, and it would be orders of magnitude more difficult for a novice to create the sample database from contrib or anywhere else. Besides the waste of space for the sample database should be negligible for almost any real production environment. only a point to that if all the documentation examples are based on that database to allow copy-paste-play. True. Though this is a chicken and egg problem, by simply creating the sample DB first, it creates the opportunity to gradually convert all examples in the manual to use the sample DB (and patch the sample DB in the process to be populated with the proper tables/functions to actually be useful in the required examples). -- Sincerely, Stephen R. van den Berg. Give a man a fire and he's warm for a day, but set fire to him and he's warm for the rest of his life. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Hot standby
The following sequence of commands causes server crash. postgres=# BEGIN TRANSACTION READ WRITE ; BEGIN postgres=# SELECT * FROM pg_class FOR UPDATE; FATAL: cannot assign TransactionIds during recovery STATEMENT: SELECT * FROM pg_class FOR UPDATE; FATAL: cannot assign TransactionIds during recovery server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Succeeded. I think we should disallow starting a read-write transaction during recovery. The patch attempts to do that by setting transaction mode to read-only, but not enough to prevent somebody to explicitly mark the transaction as read-write. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
[HACKERS] Erroring out on parser conflicts
While hacking on parser/gram.y just now I noticed in passing that the automatically generated ecpg parser had 402 shift/reduce conflicts. (Don't panic, the parser in CVS is fine.) If you don't pay very close attention, it is easy to miss this. Considering also that we frequently have to educate contributors that parser conflicts are not acceptable, should we try to error out if we see conflicts? Something like this could work: $(srcdir)/preproc.c: $(srcdir)/preproc.y $(BISON) -d $(BISONFLAGS) -o $@ $ 2preproc.stderr cat preproc.stderr [ ! -s preproc.stderr ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL/MED compatible connection manager
Peter Eisentraut wrote:
Looks very good, please continue.
Thanks, will do :)
On your wiki page, you have this example:
CREATE SERVER userdb TYPE 'plproxy_cluster'
FOREIGN DATA WRAPPER plproxy
OPTIONS (
server='dbname=userdb_p0 host=127.0.0.1 port=6000',
server='dbname=userdb_p1 host=127.0.0.1 port=6000',
[snip]
If I read this right, SQL/MED requires option names to be unique for a
server. To this needs to be rethought.
Indeed, seems that I somehow managed to miss that. Additionally, according
to the standard the options should be specified as name value, instead
of name = value. Plus the possibility to alter individual options.
I'll look into that.
Updated the wiki to use unique option names.
Do we really need the function pg_get_remote_connection_info()? This
could be done directly with the information schema. E.g., your example
SELECT * FROM pg_get_remote_connection_info('userdb');
The purpouse of the connection lookup function is to compose the the actual
connection string from generic options (adds user mapping if needed). This
aims to make it easier for the clients to perform connection lookups. The
idea is that the connection string should be composed by the foreign data
wrapper, instead of letting each client have it's own interpretation of the
options. Though, it is still possible to query the options directly.
And similarly, pg_get_user_mapping_options() is equivalent to
information_schema.user_mapping_options.
Hmm, the options are stored as text[], these need to be transformed to be
usable in the views. Seems that additional functions for foreign data wrapper
and server options are also needed. Will add those, along with the
information_schema views.
regards,
Martin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Distinct types
Peter Eisentraut wrote:
Here is an implementation of distinct types,
I'm withdrawing this patch from the current commit fest for further
work. For the record, I have attached the current patch, including the
documentation work by Jeff Davis.
Index: doc/src/sgml/ref/create_domain.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/create_domain.sgml,v
retrieving revision 1.32
diff -u -3 -p -r1.32 create_domain.sgml
--- doc/src/sgml/ref/create_domain.sgml 14 Nov 2008 10:22:46 - 1.32
+++ doc/src/sgml/ref/create_domain.sgml 25 Nov 2008 10:13:18 -
@@ -58,6 +58,12 @@ where replaceable class=PARAMETERcon
Define a domain rather than setting up each table's constraint
individually.
/para
+
+ para
+ Domains are similar to distinct types, described in xref
+ linkend=sql-createtype, except that you can specify defaults or
+ constraints, and a domain can be implicitly cast to its base type.
+ /para
/refsect1
refsect1
Index: doc/src/sgml/ref/create_type.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/create_type.sgml,v
retrieving revision 1.78
diff -u -3 -p -r1.78 create_type.sgml
--- doc/src/sgml/ref/create_type.sgml 14 Nov 2008 10:22:46 - 1.78
+++ doc/src/sgml/ref/create_type.sgml 25 Nov 2008 10:13:18 -
@@ -27,6 +27,8 @@ CREATE TYPE replaceable class=paramete
CREATE TYPE replaceable class=parametername/replaceable AS ENUM
( 'replaceable class=parameterlabel/replaceable' [, ... ] )
+CREATE TYPE replaceable class=parametername/replaceable AS replaceable
class=parameterdata_type/replaceable
+
CREATE TYPE replaceable class=parametername/replaceable (
INPUT = replaceable class=parameterinput_function/replaceable,
OUTPUT = replaceable class=parameteroutput_function/replaceable
@@ -96,10 +98,25 @@ CREATE TYPE replaceable class=paramete
/refsect2
refsect2
+ titleDistinct Types/title
+
+ para
+The third form of commandCREATE TYPE/command creates a
+distinct type. Distinct types are similar to domains, as described
+in xref linkend=sql-createdomain, except that they do not have
+defaults or constraints, and they cannot be implicitly cast to
+their base type. Distinct types are useful to avoid making
+mistakes by comparing two unrelated values that happen to be the
+same type, such as two integers representing supplier number and
+part number.
+ /para
+ /refsect2
+
+ refsect2
titleBase Types/title
para
- The third form of commandCREATE TYPE/command creates a new base type
+ The fourth form of commandCREATE TYPE/command creates a new base type
(scalar type). To create a new base type, you must be a superuser.
(This restriction is made because an erroneous type definition could
confuse or even crash the server.)
Index: src/backend/commands/typecmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.126
diff -u -3 -p -r1.126 typecmds.c
--- src/backend/commands/typecmds.c 2 Nov 2008 01:45:28 - 1.126
+++ src/backend/commands/typecmds.c 25 Nov 2008 10:13:18 -
@@ -648,7 +648,7 @@ RemoveTypeById(Oid typeOid)
/*
* DefineDomain
- * Registers a new domain.
+ * Registers a new domain or distinct type.
*/
void
DefineDomain(CreateDomainStmt *stmt)
@@ -721,18 +721,27 @@ DefineDomain(CreateDomainStmt *stmt)
basetypeoid = HeapTupleGetOid(typeTup);
/*
-* Base type must be a plain base type, another domain or an enum.
Domains
+* Base type must be a plain base type, another domain, distinct type,
or an enum. Domains
* over pseudotypes would create a security hole. Domains over
composite
* types might be made to work in the future, but not today.
*/
typtype = baseType-typtype;
if (typtype != TYPTYPE_BASE
typtype != TYPTYPE_DOMAIN
+ typtype != TYPTYPE_DISTINCT
typtype != TYPTYPE_ENUM)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
-errmsg(\%s\ is not a valid base type for a
domain,
-
TypeNameToString(stmt-typename;
+ {
+ if (stmt-distinct_type)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+errmsg(\%s\ is not a valid base
type for a distinct type,
+
TypeNameToString(stmt-typename;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+errmsg(\%s\
Re: [HACKERS] blatantly a bug in the documentation
On Tue, Nov 25, 2008 at 7:31 PM, Dave Page [EMAIL PROTECTED] wrote: I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install ... Could we perhaps punt on the issue of whether to install the sampledb by default. It could be controlled by a flag to initdb, say $ initdb --sample data Whereas if you omit the flag, then initdb just does a vanilla install. Then we can leave it up to the distributions whether they want to install it by default, or give the user options in their package management systems to control this. In Apt it could be a debconf question, in Portage it could be a USE flag, etc. I think it's a pretty safe bet that the newbies we're talking about in this thread are going to be installing postgres via a package management system. If they are installing from source, they are already reading the documentation and will have to learn about running initdb on the command line anyway. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments to Synchronous replication patch v3
Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. []s -- Dickson S. Guedes Administrador de Banco de Dados Confesol - Projeto Colmeia Florianopolis, SC, Brasil (48) 3322-1185, ramal: 26 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Exporting PGINTERVALSTYLE prevents access to older server versions
Exporting PGINTERVALSTYLE causes errors of the following kind when connecting to an older server version: psql: FATAL: unrecognized configuration parameter intervalstyle Should the processing of this variable be made conditional upon the server version? (What is the use case for this variable anyway? Is it there just because PGDATESTYLE was there previously?) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.
Magnus Hagander wrote: Heikki Linnakangas wrote: Magnus Hagander wrote: Log Message: --- Silence compiler warning about ignored return value. Our comment already clearly stated that we are aware that we're ignoring it. I think the usual way is to call the function like: (void) function_with_return_value() I tried that first, of course. gcc is too smart about that - it still throws the warning in this case. I have equipped this part with a proper error message now. At the time I was thinking this is a can't happen error, but it has in fact already helped clarifying some other regression test run failures for me: ./pg_regress --inputdir=. --dlpath=. --multibyte=SQL_ASCII --load-language=plpgsql --temp-install=./tmp_check --top-builddir=../../.. --temp-port=565432 --schedule=./parallel_schedule == removing existing temp installation== == creating temporary installation== == initializing database system == == starting postmaster== running on port 65432 with pid 44940 == creating database regression == ERROR: database regression already exists command failed: /Users/peter/devel/postgresql/cvs/pg84/pgsql/src/test/regress/./tmp_check/install//Users/peter/devel/postgresql/cvs/pg84/pg-install/bin/psql -X -c CREATE DATABASE \regression\ TEMPLATE=template0 ENCODING='SQL_ASCII' postgres pg_ctl: server does not shut down pg_regress: could not stop postmaster: exit code was 256NEW make: *** [check] Error 2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
Hi ITAGAKI-san. Um, It was not supported. http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/ITAGAKI_PATCH.txt The test of my various past is reproduced. Then, I proposed the management regarded as best in them. and, to Magnus-san. Does the reason for persisting in wsftime exceed a safe reason? However, I may have missed something... Regards, Hiroshi Saito - Original Message - From: ITAGAKI Takahiro [EMAIL PROTECTED] Hiroshi Saito [EMAIL PROTECTED] wrote: Umm, format operand seems to be a wide character sequence. Here is a patch to work around the wide character format string. The hack is the following line: +#define strftime(a,b,c,d) strftime_win32(a,b,L##c,d) We use only literals in the format strings, the macro adds wide character prefix (L...) to them. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Erroring out on parser conflicts
Peter Eisentraut [EMAIL PROTECTED] writes: While hacking on parser/gram.y just now I noticed in passing that the automatically generated ecpg parser had 402 shift/reduce conflicts. (Don't panic, the parser in CVS is fine.) If you don't pay very close attention, it is easy to miss this. Considering also that we frequently have to educate contributors that parser conflicts are not acceptable, should we try to error out if we see conflicts? Would %expect 0 produce the same result in a less klugy way? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Hot standby
Pavan Deolasee [EMAIL PROTECTED] writes: I think we should disallow starting a read-write transaction during recovery. The patch attempts to do that by setting transaction mode to read-only, but not enough to prevent somebody to explicitly mark the transaction as read-write. Huh? The read only transaction mode is not hard read-only anyway, so if that's the only step being taken, it's entirely useless. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
Dave Page [EMAIL PROTECTED] writes: I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install - however, there is only a point to that if all the documentation examples are based on that database to allow copy-paste-play. You would also have to assume that all the examples are non-destructive, and that no one ever extends an example in a destructive way. This seems like a non-starter. Better to provide the sample database in a form in which it can be easily dropped/reloaded. I'm envisioning that there's a source file in $sharedir and we tell people createdb example psql -f $sharedir/example.sql example regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exporting PGINTERVALSTYLE prevents access to older server versions
Peter Eisentraut [EMAIL PROTECTED] writes: Exporting PGINTERVALSTYLE causes errors of the following kind when connecting to an older server version: psql: FATAL: unrecognized configuration parameter intervalstyle Ooops. (What is the use case for this variable anyway? Is it there just because PGDATESTYLE was there previously?) Pretty much. I'd be fine with taking it out entirely. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
On Tue, Nov 25, 2008 at 1:23 PM, Tom Lane [EMAIL PROTECTED] wrote: Dave Page [EMAIL PROTECTED] writes: I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install - however, there is only a point to that if all the documentation examples are based on that database to allow copy-paste-play. You would also have to assume that all the examples are non-destructive, and that no one ever extends an example in a destructive way. That's a good point. This seems like a non-starter. Better to provide the sample database in a form in which it can be easily dropped/reloaded. I'm envisioning that there's a source file in $sharedir and we tell people createdb example psql -f $sharedir/example.sql example I was assuming that it would be in such a form anyway - similar to system_views.sql for example. I'd be happy with that setup - it's far better than referring people to pgFoundry, and should be simple for anyone to use. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Hot standby
On Tue, Nov 25, 2008 at 6:55 PM, Tom Lane [EMAIL PROTECTED] wrote: Huh? The read only transaction mode is not hard read-only anyway, so if that's the only step being taken, it's entirely useless. I think there are explicit checks for some utility statements (like VACUUM), but I haven't checked if all necessary code paths are covered or not. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Brittleness in regression test setup
Peter Eisentraut wrote:
== removing existing temp installation==
== creating temporary installation==
== initializing database system ==
== starting postmaster==
running on port 65432 with pid 94178
== creating database regression ==
ERROR: database regression already exists
It evidently failed to realize that there is another postmaster already
running at that port and just ran its test setup routines against that
other instance.
On this matter, I noticed that pg_regress doesn't do anything to clean
up its child processes. I see zombies lying around on Linux and Mac OS
when the postmaster dies. (And the zombie is exactly the pid 94178 it
announced in the example above.)
I played around a little with signal handling to collect the dying
postmaster and report and error; see attached rough patch. I'm not
exactly understanding how this works though. I would expect lots of
psql zombies for example, because those go through the same
spawn_process() call, but I'm not seeing any. Also, the sleep() call in
my patch is necessary to get some effect. Anyone else go a clue on what
to do here?
Index: src/test/regress/GNUmakefile
===
RCS file: /cvsroot/pgsql/src/test/regress/GNUmakefile,v
retrieving revision 1.75
diff -u -3 -p -r1.75 GNUmakefile
--- src/test/regress/GNUmakefile1 Oct 2008 22:38:57 - 1.75
+++ src/test/regress/GNUmakefile25 Nov 2008 13:44:05 -
@@ -47,6 +47,8 @@ EXTRADEFS = '-DHOST_TUPLE=$(host_tuple)
'-DSHELLPROG=$(SHELL)' \
'-DDLSUFFIX=$(DLSUFFIX)'
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
##
## Prepare for tests
##
@@ -55,7 +57,7 @@ EXTRADEFS = '-DHOST_TUPLE=$(host_tuple)
all: submake-libpgport pg_regress$(X)
-pg_regress$(X): pg_regress.o pg_regress_main.o
+pg_regress$(X): pg_regress.o pg_regress_main.o pqsignal.o
$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LIBS) -o $@
# dependencies ensure that path changes propagate
@@ -65,6 +67,10 @@ pg_regress.o: pg_regress.c $(top_builddi
$(top_builddir)/src/port/pg_config_paths.h: $(top_builddir)/src/Makefile.global
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
+pqsignal.c: % : $(top_srcdir)/src/interfaces/libpq/%
+ rm -f $@ $(LN_S) $ .
+
+
install: all installdirs
$(INSTALL_PROGRAM) pg_regress$(X)
'$(DESTDIR)$(pgxsdir)/$(subdir)/pg_regress$(X)'
@@ -172,7 +178,7 @@ bigcheck: all
clean distclean maintainer-clean: clean-lib
# things built by `all' target
- rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) pg_regress_main.o
pg_regress.o pg_regress$(X)
+ rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) pg_regress_main.o
pg_regress.o pg_regress$(X) pqsignal.c
# things created by various check targets
rm -f $(output_files) $(input_files)
rm -rf testtablespace
Index: src/test/regress/pg_regress.c
===
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.51
diff -u -3 -p -r1.51 pg_regress.c
--- src/test/regress/pg_regress.c 25 Nov 2008 11:49:35 - 1.51
+++ src/test/regress/pg_regress.c 25 Nov 2008 13:44:05 -
@@ -31,6 +31,7 @@
#include getopt_long.h
#include pg_config_paths.h
+#include libpq/pqsignal.h
/* for resultmap we need a list of pairs of strings */
typedef struct _resultmap
@@ -277,6 +278,8 @@ stop_postmaster(void)
fflush(stdout);
fflush(stderr);
+ postmaster_pid = INVALID_PID;
+
snprintf(buf, sizeof(buf),
SYSTEMQUOTE \%s/pg_ctl\ stop -D \%s/data\
-s -m fast SYSTEMQUOTE,
bindir, temp_install);
@@ -1803,6 +1806,29 @@ make_absolute_path(const char *in)
}
static void
+sigchld_handler(int signum)
+{
+ pid_t pid;
+ int status;
+ int save_errno;
+
+ save_errno = errno;
+ while (1)
+ {
+ pid = waitpid(postmaster_pid, status, WNOHANG);
+ if (pid = 0)
+ break;
+
+ if (pid == postmaster_pid)
+ {
+ fprintf(stderr, postmaster died\n);
+ exit(2);
+ }
+ }
+ errno = save_errno;
+}
+
+static void
help(void)
{
printf(_(PostgreSQL regression test driver\n));
@@ -2100,6 +2126,7 @@ regression_main(int argc, char *argv[],
debug ? -d 5 : ,
hostname ? hostname : ,
outputdir);
+ pqsignal(SIGCHLD, sigchld_handler);
postmaster_pid = spawn_process(buf);
if (postmaster_pid == INVALID_PID)
{
@@
Re: [HACKERS] Comments to Synchronous replication patch v3
Dickson S. Guedes escribió: Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. Hmm, what is this database there for? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Brittleness in regression test setup
Peter Eisentraut [EMAIL PROTECTED] writes: I played around a little with signal handling to collect the dying postmaster and report and error; see attached rough patch. I'm not exactly understanding how this works though. I would expect lots of psql zombies for example, because those go through the same spawn_process() call, but I'm not seeing any. That's because wait_for_tests wait()s for them. AFAICS the only way you'd end up with a zombie postmaster is if pg_ctl stop fails, but I'm failing to understand why that's likely to happen. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
On Tue, Nov 25, 2008 at 8:23 AM, Tom Lane [EMAIL PROTECTED] wrote: Dave Page [EMAIL PROTECTED] writes: I'm in favour of including it by default (at initdb), so it's there for new users to play with on any fresh install - however, there is only a point to that if all the documentation examples are based on that database to allow copy-paste-play. You would also have to assume that all the examples are non-destructive, and that no one ever extends an example in a destructive way. This seems like a non-starter. Better to provide the sample database in a form in which it can be easily dropped/reloaded. I'm envisioning that there's a source file in $sharedir and we tell people createdb example psql -f $sharedir/example.sql example There's good precedent for that...SQL Server for example does something similar: http://msdn.microsoft.com/en-us/library/aa276825(SQL.80).aspx It's a good learning tool and would really raise the value of the documentation. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
Tom Lane wrote: Better to provide the sample database in a form in which it can be easily dropped/reloaded. I'm envisioning that there's a source file in $sharedir and we tell people createdb example psql -f $sharedir/example.sql example This is a much better idea than doing it at initdb time, IMNSHO. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exporting PGINTERVALSTYLE prevents access to older server versions
I wrote:
Peter Eisentraut [EMAIL PROTECTED] writes:
(What is the use case for this variable anyway? Is it there just
because PGDATESTYLE was there previously?)
Pretty much. I'd be fine with taking it out entirely.
Actually ... I started to take this out and replace
*** pgsql/src/test/regress/pg_regress.c.orig Tue Nov 25 09:18:16 2008
--- pgsql/src/test/regress/pg_regress.c Tue Nov 25 09:29:46 2008
***
*** 716,722
*/
putenv(PGTZ=PST8PDT);
putenv(PGDATESTYLE=Postgres, MDY);
! putenv(PGINTERVALSTYLE=postgres_verbose);
if (temp_install)
{
--- 716,722
*/
putenv(PGTZ=PST8PDT);
putenv(PGDATESTYLE=Postgres, MDY);
! putenv(PGOPTIONS=--intervalstyle=postgres_verbose);
if (temp_install)
{
when it struck me that that's going to still cause pg_regress to fail to
connect to older servers, which I suppose is the case that prompted you
to complain originally. So I guess the real question is what is the
use case for having pg_regress talk to older servers?
I looked at whether we could send the value conditionally, but this
doesn't really work because libpq sends this stuff in the startup
packet; it doesn't know the exact server version at that point.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] New trigger file in pg_standby to promote the standby to the primary
Hi, In current pg_standby, the presence of the trigger file causes recovery to end whether or not the next WAL file is available. Thereby, some transactions in the available WAL files will be lost. So, we cannot use this trigger file to promote the standby to the primary. I'd like to add new trigger file option into pg_standby, and end recovery after redoing all the available WAL files. Concretely speaking, I would always make pg_standby look for the trigger file after trying to restore the WAL file once. If the trigger file is present, pg_standby exits immediately without deleting the trigger file. Subsequent pg_standby uses the existing trigger file. This option is also useful for warm-standby. Any comments welcome! Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Brittleness in regression test setup
Tom Lane wrote:
One thing we should do is have pg_regress.c, not the Makefile,
select the default port to use. The concatenate-5 behavior is
just not intelligent enough.
How about something like this, constructing a port number from the
version and a timestamp? We could also take 2 more bits from the
version and give it to the timestamp, which would make this a bit safer,
I think.
Index: src/test/regress/GNUmakefile
===
RCS file: /cvsroot/pgsql/src/test/regress/GNUmakefile,v
retrieving revision 1.75
diff -u -3 -p -r1.75 GNUmakefile
--- src/test/regress/GNUmakefile1 Oct 2008 22:38:57 - 1.75
+++ src/test/regress/GNUmakefile25 Nov 2008 15:14:19 -
@@ -14,9 +14,6 @@ subdir = src/test/regress
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-# port number for temp-installation test postmaster
-TEMP_PORT = 5$(DEF_PGPORT)
-
# file with extra config for temp build
TEMP_CONF =
ifdef TEMP_CONFIG
@@ -144,7 +141,7 @@ tablespace-setup:
pg_regress_call = ./pg_regress --inputdir=$(srcdir) --dlpath=.
--multibyte=$(MULTIBYTE) --load-language=plpgsql $(NOLOCALE)
check: all
- $(pg_regress_call) --temp-install=./tmp_check
--top-builddir=$(top_builddir) --temp-port=$(TEMP_PORT)
--schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(TEMP_CONF)
+ $(pg_regress_call) --temp-install=./tmp_check
--top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
$(MAXCONNOPT) $(TEMP_CONF)
installcheck: all
$(pg_regress_call) --psqldir=$(PSQLDIR)
--schedule=$(srcdir)/serial_schedule
@@ -163,7 +160,7 @@ bigtest: all
$(pg_regress_call) --psqldir=$(PSQLDIR)
--schedule=$(srcdir)/serial_schedule numeric_big
bigcheck: all
- $(pg_regress_call) --temp-install=./tmp_check
--top-builddir=$(top_builddir) --temp-port=$(TEMP_PORT)
--schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
+ $(pg_regress_call) --temp-install=./tmp_check
--top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
$(MAXCONNOPT) numeric_big
##
Index: src/test/regress/pg_regress.c
===
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.50
diff -u -3 -p -r1.50 pg_regress.c
--- src/test/regress/pg_regress.c 20 Nov 2008 15:03:39 - 1.50
+++ src/test/regress/pg_regress.c 25 Nov 2008 15:14:20 -
@@ -83,10 +83,9 @@ static _stringlist *extra_tests = NULL;
static char *temp_install = NULL;
static char *temp_config = NULL;
static char *top_builddir = NULL;
-static int temp_port = 65432;
static bool nolocale = false;
static char *hostname = NULL;
-static int port = -1;
+static int port = 0;
static char *dlpath = PKGLIBDIR;
static char *user = NULL;
static _stringlist *extraroles = NULL;
@@ -733,7 +732,7 @@ initialize_environment(void)
else
unsetenv(PGHOST);
unsetenv(PGHOSTADDR);
- if (port != -1)
+ if (port)
{
chars[16];
@@ -789,7 +788,7 @@ initialize_environment(void)
doputenv(PGHOST, hostname);
unsetenv(PGHOSTADDR);
}
- if (port != -1)
+ if (port)
{
chars[16];
@@ -1821,7 +1820,6 @@ help(void)
printf(_(Options for \temp-install\ mode:\n));
printf(_( --no-locale use C locale\n));
printf(_( --top-builddir=DIR(relative) path to top level
build directory\n));
- printf(_( --temp-port=PORT port number to start temp
postmaster on\n));
printf(_( --temp-config=PATHappend contents of PATH to
temporary config\n));
printf(_(\n));
printf(_(Options for using an existing installation:\n));
@@ -1859,7 +1857,6 @@ regression_main(int argc, char *argv[],
{temp-install, required_argument, NULL, 9},
{no-locale, no_argument, NULL, 10},
{top-builddir, required_argument, NULL, 11},
- {temp-port, required_argument, NULL, 12},
{host, required_argument, NULL, 13},
{port, required_argument, NULL, 14},
{user, required_argument, NULL, 15},
@@ -1933,15 +1930,6 @@ regression_main(int argc, char *argv[],
case 11:
top_builddir = strdup(optarg);
break;
- case 12:
- {
- int p =
atoi(optarg);
-
- /* Since Makefile isn't very bright,
check port range */
- if (p = 1024 p = 65535)
-
Re: [HACKERS] Comments to Synchronous replication patch v3
On Tue, Nov 25, 2008 at 10:57 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Dickson S. Guedes escribió: Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. Hmm, what is this database there for? It's for authentication for replication. This was discussed before. Please see the following thread and feel free to comment. http://archives.postgresql.org/pgsql-hackers/2008-11/msg00187.php Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments to Synchronous replication patch v3
Fujii Masao escribió: On Tue, Nov 25, 2008 at 10:57 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Dickson S. Guedes escribió: Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. Hmm, what is this database there for? It's for authentication for replication. This was discussed before. Please see the following thread and feel free to comment. http://archives.postgresql.org/pgsql-hackers/2008-11/msg00187.php Hmm ... I think this means that the suggestion by Dickson does not make much sense, right? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Brittleness in regression test setup
Peter Eisentraut wrote: Tom Lane wrote: One thing we should do is have pg_regress.c, not the Makefile, select the default port to use. The concatenate-5 behavior is just not intelligent enough. How about something like this, constructing a port number from the version and a timestamp? We could also take 2 more bits from the version and give it to the timestamp, which would make this a bit safer, I think. Is it possible to make it retry in case the chosen port is busy? I guess a simple check should suffice, ignoring the obvious race condition that someone uses the port after you checked it was OK. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Proposal for better PQExpBuffer out-of-memory behavior
I've been chewing on the problem described here: http://archives.postgresql.org/pgsql-general/2008-11/msg01220.php It's not particularly easy to fix without making annoyingly large changes to the API for PQExpBuffer. The best idea I have come up with so far goes like this: * Upon failure to malloc or realloc the buffer for a PQExpBuffer, the pqexpbuffer.c code should release whatever buffer it might have had and set data = pointer to empty, statically allocated string len = 0 maxlen = 0 This is distinguishable from the normal non-error case because maxlen can never be zero in non-error cases. * All subsequent operations except resetPQExpBuffer will do nothing to such a PQExpBuffer. resetPQExpBuffer will attempt to restore the string to normal empty status by allocating a new default-size buffer. The result of this would be that in cases such as the one exhibited by Sam Mason, we'd end up with a guaranteed-empty string rather than one that had had subsections unexpectedly removed. Also, we could add out-of-memory checks to callers where it seems important to do so. The main advantage of this approach is that it avoids making ABI breaks (such as would occur if we added an error flag field to PQExpBufferData). The main disadvantage is the need to add explicit error checks to callers anyplace we're not satisfied with just letting the string become empty. The only alternative that I can think of that avoids the latter disadvantage is to allow the pqexpbuffer routines to abort on out-of-memory (ie, printf(stderr) and exit(1)). This seems pretty unpleasant though for functions that are part of libpq's infrastructure. In particular, although we could allow the calling application to override such behavior via some sort of callback hook function, it's far from clear what it could do instead without risking bizarre misbehavior by libpq. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Brittleness in regression test setup
Peter Eisentraut [EMAIL PROTECTED] writes: Tom Lane wrote: One thing we should do is have pg_regress.c, not the Makefile, select the default port to use. The concatenate-5 behavior is just not intelligent enough. How about something like this, constructing a port number from the version and a timestamp? We could also take 2 more bits from the version and give it to the timestamp, which would make this a bit safer, I think. I'd vote for keeping the --temp-port option but not having the Makefile use it. Seems like it'd still be potentially useful for hand use of pg_regress. Also, like Alvaro I'm thinking that a retry is really needed. As this patch stands you'd be vulnerable to random, unrepeatable failures anytime you picked a port that happened to be in use for something else. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments to Synchronous replication patch v3
On Wed, Nov 26, 2008 at 12:23 AM, Alvaro Herrera [EMAIL PROTECTED] wrote: Fujii Masao escribió: On Tue, Nov 25, 2008 at 10:57 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Dickson S. Guedes escribió: Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. Hmm, what is this database there for? It's for authentication for replication. This was discussed before. Please see the following thread and feel free to comment. http://archives.postgresql.org/pgsql-hackers/2008-11/msg00187.php Hmm ... I think this means that the suggestion by Dickson does not make much sense, right? Oh, I'm sorry! -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [Fwd: error compiling postgresql with fedora mingw]
(I'm still downloading Fedora-10) Even though Tom is subscribed to the list below, I thought I should forward this e-mail to this list: Forwarded Message From: Itamar - IspBrasil [EMAIL PROTECTED] Reply-To: Development discussions related to Fedora [EMAIL PROTECTED] To: Development discussions related to Fedora [EMAIL PROTECTED] Subject: error compiling postgresql with fedora mingw Date: Tue, 25 Nov 2008 08:43:12 -0200 any help ? make[1]: Entering directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src' make -C port all make[2]: Entering directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/port' make[2]: Nothing to be done for `all'. make[2]: Leaving directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/port' make -C timezone all make[2]: Entering directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/timezone' make -C ../../src/port all make[3]: Entering directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/port' make[3]: Nothing to be done for `all'. make[3]: Leaving directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/port' i686-pc-mingw32-gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions --param=ssp-buffer-size=4 -mms-bitfields -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv zic.o ialloc.o scheck.o localtime.o -L../../src/port -Wl,--allow-multiple-definition -lpgport -lz -lm -lws2_32 -lshfolder -o zic.exe ../../src/port/libpgport.a: could not read symbols: Archive has no index; run ranlib to add one collect2: ld returned 1 exit status make[2]: *** [zic] Error 1 make[2]: Leaving directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src/timezone' make[1]: *** [all] Error 2 make[1]: Leaving directory `/home/itamar/fedora/mingw-postgresql/postgresql-8.3.5/src' make: *** [all] Error 2 -- Itamar Reis Peixoto e-mail/msn: [EMAIL PROTECTED] sip: [EMAIL PROTECTED] skype: itamarjp icq: 81053601 +55 11 4063 5033 +55 34 3221 8599 -- Devrim GÜNDÜZ, RHCE devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org signature.asc Description: This is a digitally signed message part
Re: [HACKERS] TODO item: adding VERBOSE option to CLUSTER [with patch]
Peter Eisentraut [EMAIL PROTECTED] wrote: Additional processing information as discussed later in the thread can now be added easily, but I think there was no consensus on what exactly to print. Since I use CLUSTER almost exclusively to eliminate bloat, I'd like to see the before and after value for pg_total_relation_size for each table. Some didn't seem terribly interested in that, as they use the command primarily to sequence the heap, so some measure(s) of how out-of-order the heap was would make sense. We are talking about an option named VERBOSE, so there's no real reason not to throw in all useful information. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Snapshot warning
Tom Lane escribió:
Alvaro Herrera [EMAIL PROTECTED] writes:
Tom Lane escribió:
I think the fundamental bug here is that you tried to skip using the
ResourceOwner mechanism for snapshot references. That's basically
not gonna work.
Right :-( I'll see how to go about this.
It strikes me that you might be able to remove the registered-snapshot
infrastructure in snapmgr.c, and end up with about the same amount of
code overall, just with the responsibility in resowner.c.
Seems to work, and fixes Pavan test case as well.
$ runpg 00head showdiff | diffstat
backend/utils/resowner/resowner.c | 99
backend/utils/time/snapmgr.c | 180 ++-!
include/utils/resowner.h |8 +
include/utils/snapmgr.h |3
4 files changed, 143 insertions(+), 59 deletions(-), 88 modifications(!)
I need to fix some comments before publishing the patch.
The only thing I'm now missing is SnapshotResetXmin(). It currently
looks like this:
static void
SnapshotResetXmin(void)
{
if (RegisteredSnapshotList == NULL ActiveSnapshot == NULL)
MyProc-xmin = InvalidTransactionId;
}
After the patch we don't have any way to detect whether resowner.c has
any snapshot still linked to. I assume there's no objection to adding a
new entry point in resowner.c for this.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Snapshot warning
Alvaro Herrera escribió: The only thing I'm now missing is SnapshotResetXmin(). It currently looks like this: After the patch we don't have any way to detect whether resowner.c has any snapshot still linked to. I assume there's no objection to adding a new entry point in resowner.c for this. Hmm, that doesn't readily work because there's no way to track transaction boundaries in resource owners. I think I'll have to add a separate static counter in snapmgr.c that's maintained by the calls from resowner.c. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Snapshot warning
Alvaro Herrera [EMAIL PROTECTED] writes: After the patch we don't have any way to detect whether resowner.c has any snapshot still linked to. I assume there's no objection to adding a new entry point in resowner.c for this. Hmm, that's a bit problematic because resowner.c doesn't have any global notion of what resource owners exist. I think you still need to have snapmgr.c maintain a list of all known snapshots. resowner.c can only help you with tracking reference counts for particular snapshots. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments to Synchronous replication patch v3
Alvaro Herrera wrote: Fujii Masao escribió: On Tue, Nov 25, 2008 at 10:57 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Dickson S. Guedes escribió: Fujii Masao escreveu: (...) Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. Hmm, what is this database there for? It's for authentication for replication. This was discussed before. Please see the following thread and feel free to comment. http://archives.postgresql.org/pgsql-hackers/2008-11/msg00187.php Hmm ... I think this means that the suggestion by Dickson does not make much sense, right? Right. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Hot standby
On Tue, 2008-11-25 at 19:02 +0530, Pavan Deolasee wrote: On Tue, Nov 25, 2008 at 6:55 PM, Tom Lane [EMAIL PROTECTED] wrote: Huh? The read only transaction mode is not hard read-only anyway, so if that's the only step being taken, it's entirely useless. I think there are explicit checks for some utility statements (like VACUUM), but I haven't checked if all necessary code paths are covered or not. The commands that need protecting have been explicitly identified in the notes and there are 7 files changed that were specifically identified with protective changes. You've identified a way of breaking part the first line of defence, but the command was caught by the second line of defence in the patch. Problem, yes. Major issue, no. Will fix. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bugfix] DISCARD ALL does not release advisory locks
On Mon, Nov 24, 2008 at 10:25 AM, Marko Kreen [EMAIL PROTECTED] wrote: On 11/24/08, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] writes: It was brought to my attention that DISCARD ALL does not release advisory locks: What is the argument that it should? DISCARD ALL is supposed to be used by poolers to reset connection back to startup state to reuse server connection after client disconnect. New client should see no difference between fresh backend and old backend where DISCARD ALL was issued. IOW, DISCARD ALL should be functionally equivalent to backend exit. If user want more explicit control over what resources are released, he should avoid use of DISCARD ALL, instead he should manually pick individual components from the command sequence DISCARD ALL is equivalent to. Eg. when user wants to keep old plans or advisory locks around, he should manually construct command list that resets everything except those. But DISCARD ALL should release everything possible, never should additional commands be needed in addition to it to do full reset. Having done a lot of work with advisory locks, I support this change. Advisory locks are essentially session scoped objects like prepared statements or notifies. It's only natural to clean them up in the same way. That said, I don't think this should be backpatched to 8.3. I'm aware of at least one project that makes heavy use of advisory locks (openads). Since this project and possibly others are probably used in bouncing web environments, you have to be careful with behavior changes like that. People need time...unexpected advisory lock issues can get nasty. If you need the behavior now, just install the patch yourself :-) merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Erroring out on parser conflicts
On Tuesday 25 November 2008 15:09:37 Tom Lane wrote: Peter Eisentraut [EMAIL PROTECTED] writes: While hacking on parser/gram.y just now I noticed in passing that the automatically generated ecpg parser had 402 shift/reduce conflicts. (Don't panic, the parser in CVS is fine.) If you don't pay very close attention, it is easy to miss this. Considering also that we frequently have to educate contributors that parser conflicts are not acceptable, should we try to error out if we see conflicts? Would %expect 0 produce the same result in a less klugy way? Great, that works. I'll see about adding this to our parser files. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exporting PGINTERVALSTYLE prevents access to older server versions
On Tuesday 25 November 2008 16:42:57 Tom Lane wrote:
--- 716,722
*/
putenv(PGTZ=PST8PDT);
putenv(PGDATESTYLE=Postgres, MDY);
! putenv(PGOPTIONS=--intervalstyle=postgres_verbose);
if (temp_install)
{
when it struck me that that's going to still cause pg_regress to fail to
connect to older servers, which I suppose is the case that prompted you
to complain originally.
Yeah, I was trying to reproduce the misbehavior I experienced the other day.
In fact now pg_regress just hung somehow, and I found these errors about
intervalstyle in the server log. This is probably the psql try-to-connect
loop in pg_regress.
So I guess the real question is what is the
use case for having pg_regress talk to older servers?
There is no use. I was just thinking, why create a new environment variable
when actually setting that variable would create all kinds of havoc for
users.
The change above looks appropriate to me.
(Better yet IMO would be to put SET statements into the SQL files where
necessary. But that is different matter.)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
Stephen R. van den Berg wrote: ... it would be orders of magnitude more difficult for a novice to create the sample database from contrib or anywhere else. It seems to me that *this* is the more serious problem that we should fix instead. If, from the psql command prompt I could type: psql=# install module sampledb; Downloading sampledb from pgfoundry... Installing sampledb Connecting to sampledb sampledb=# it'd remove the need for pre-installing a rarely-needed ad-on, as well as being useful for other projects. For exmaple: psql=# install module US_Census_Tiger_Maps; Installing dependency Postgis... Installing module US_Census_Tiger_Maps to install a GIS system with all the road networks. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Snapshot warning
Tom Lane escribió:
Alvaro Herrera [EMAIL PROTECTED] writes:
After the patch we don't have any way to detect whether resowner.c has
any snapshot still linked to. I assume there's no objection to adding a
new entry point in resowner.c for this.
Hmm, that's a bit problematic because resowner.c doesn't have any global
notion of what resource owners exist. I think you still need to have
snapmgr.c maintain a list of all known snapshots. resowner.c can only
help you with tracking reference counts for particular snapshots.
A counter seems to suffice. Patch attached.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/utils/resowner/resowner.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/resowner/resowner.c,v
retrieving revision 1.29
diff -c -p -r1.29 resowner.c
*** src/backend/utils/resowner/resowner.c 19 Jun 2008 00:46:05 - 1.29
--- src/backend/utils/resowner/resowner.c 25 Nov 2008 17:10:22 -
***
*** 26,31
--- 26,32
#include utils/memutils.h
#include utils/rel.h
#include utils/resowner.h
+ #include utils/snapmgr.h
/*
*** typedef struct ResourceOwnerData
*** 66,71
--- 67,77
int ntupdescs; /* number of owned tupdesc references */
TupleDesc *tupdescs; /* dynamically allocated array */
int maxtupdescs; /* currently allocated array size */
+
+ /* We have built-in support for remembering snapshot references */
+ int nsnapshots; /* number of owned snapshot references */
+ Snapshot *snapshots; /* dynamically allocated array */
+ int maxsnapshots; /* currently allocated array size */
} ResourceOwnerData;
*** static void ResourceOwnerReleaseInternal
*** 98,103
--- 104,110
static void PrintRelCacheLeakWarning(Relation rel);
static void PrintPlanCacheLeakWarning(CachedPlan *plan);
static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
+ static void PrintSnapshotLeakWarning(Snapshot snapshot);
/*
*** ResourceOwnerReleaseInternal(ResourceOwn
*** 301,306
--- 308,320
PrintTupleDescLeakWarning(owner-tupdescs[owner-ntupdescs - 1]);
DecrTupleDescRefCount(owner-tupdescs[owner-ntupdescs - 1]);
}
+ /* Ditto for snapshot references */
+ while (owner-nsnapshots 0)
+ {
+ if (isCommit)
+ PrintSnapshotLeakWarning(owner-snapshots[owner-nsnapshots -1]);
+ UnregisterSnapshot(owner-snapshots[owner-nsnapshots -1]);
+ }
/* Clean up index scans too */
ReleaseResources_hash();
*** ResourceOwnerDelete(ResourceOwner owner)
*** 332,337
--- 346,352
Assert(owner-nrelrefs == 0);
Assert(owner-nplanrefs == 0);
Assert(owner-ntupdescs == 0);
+ Assert(owner-nsnapshots == 0);
/*
* Delete children. The recursive call will delink the child from me, so
*** ResourceOwnerDelete(ResourceOwner owner)
*** 360,365
--- 375,382
pfree(owner-planrefs);
if (owner-tupdescs)
pfree(owner-tupdescs);
+ if (owner-snapshots)
+ pfree(owner-snapshots);
pfree(owner);
}
*** PrintTupleDescLeakWarning(TupleDesc tupd
*** 936,938
--- 953,1037
TupleDesc reference leak: TupleDesc %p (%u,%d) still referenced,
tupdesc, tupdesc-tdtypeid, tupdesc-tdtypmod);
}
+
+ /*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * snapshot reference array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+ void
+ ResourceOwnerEnlargeSnapshots(ResourceOwner owner)
+ {
+ int newmax;
+
+ if (owner-nsnapshots owner-maxsnapshots)
+ return; /* nothing to do */
+
+ if (owner-snapshots == NULL)
+ {
+ newmax = 16;
+ owner-snapshots = (Snapshot *)
+ MemoryContextAlloc(TopMemoryContext, newmax * sizeof(Snapshot));
+ owner-maxsnapshots = newmax;
+ }
+ else
+ {
+ newmax = owner-maxsnapshots * 2;
+ owner-snapshots = (Snapshot *)
+ repalloc(owner-snapshots, newmax * sizeof(Snapshot));
+ owner-maxsnapshots = newmax;
+ }
+ }
+
+ /*
+ * Remember that a snapshot reference is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeSnapshots()
+ */
+ void
+ ResourceOwnerRememberSnapshot(ResourceOwner owner, Snapshot snapshot)
+ {
+ Assert(owner-nsnapshots owner-maxsnapshots);
+ owner-snapshots[owner-nsnapshots] = snapshot;
+ owner-nsnapshots++;
+ }
+
+ /*
+ * Forget that a snapshot reference is owned by a ResourceOwner
+ */
+ void
+ ResourceOwnerForgetSnapshot(ResourceOwner owner, Snapshot snapshot)
+ {
+ Snapshot *snapshots = owner-snapshots;
+ int ns1 = owner-nsnapshots -1;
+ int i;
+
+ for (i = ns1; i = 0;
Re: [HACKERS] Snapshot warning
Alvaro Herrera [EMAIL PROTECTED] writes: Tom Lane escribió: Hmm, that's a bit problematic because resowner.c doesn't have any global notion of what resource owners exist. I think you still need to have snapmgr.c maintain a list of all known snapshots. resowner.c can only help you with tracking reference counts for particular snapshots. A counter seems to suffice. Patch attached. Looks sane to me. The list solution might be needed later, if we wanted to get smarter about advancing our xmin after deleting only some of our snapshots ... but this'll do for now. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exporting PGINTERVALSTYLE prevents access to older server versions
Peter Eisentraut [EMAIL PROTECTED] writes: On Tuesday 25 November 2008 16:42:57 Tom Lane wrote: ! putenv(PGOPTIONS=--intervalstyle=postgres_verbose); So I guess the real question is what is the use case for having pg_regress talk to older servers? There is no use. I was just thinking, why create a new environment variable when actually setting that variable would create all kinds of havoc for users. The change above looks appropriate to me. Actually, in view of regressplans.sh, we have to work a bit harder than that in pg_regress.c ... but I still agree that there's no real value in adding a variable to libpq itself. Change committed. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg metadata and doc bug
Dear PostgreSQL developers, 1 I stumbled upon an obscure bug (or undesirable feature:-) in the schema metadata accessible through the information_schema, and possibly pg_catalog as well. As it was mixed in a bug in some in my code, it was hard for me to identify it. The issue is that when one does (in pg 8.3.5) ALTER TABLE foo ADD CONSTRAINT xxx UNIQUE ON (...); this results in a constraint *and* an index, but when one does only the corresponding: CREATE UNIQUE INDEX foo(...); then the index is created but there is no constraint. So what? The consequence arises downhill when one declares a foreign key which uses this index as a target. The FK constraint is accepted, but as the metadata contents does not include the constraint, you cannot find the relevant informations by joining the various information_schema relations. I was just looking for this information, how unlucky of me:-) See the attached file for an example. Comment out the index creation and uncomment the unique constraint to see the difference in the metadata (information_schema, and possibly underlying pg_catalog). ITSM that the fix is that a 'CREATE UNIQUE INDEX...' shoud also add the corresponding constraint. 2 Also, there is a minor bug in the documentation, which was the another source of my troubles: information_schema.KEY_COLUMN_USAGE.position_in_unique_constraint is tagged as NOT IMPLEMENTED, but it looks like it is implemented. -- Fabien.DROP TABLE bla CASCADE; DROP TABLE foo CASCADE; CREATE TABLE foo ( fid SERIAL PRIMARY KEY, stuff TEXT DEFAULT ''::text NOT NULL ); CREATE TABLE bla ( bid SERIAL PRIMARY KEY, stuff TEXT NOT NULL ); -- this should amount to a constraint... CREATE UNIQUE INDEX foo_stuff_uniq ON foo USING btree (stuff); -- BUT: -- 1. it does not appear anywhere --in information_schema.table_constraints --(nor in pg_catalog.pg_constraint, but only in pg_catalog.pg_index(es)) -- 2. foo_bla unique_constraint_name (next) is empty --in information_schema.referential_constraints -- however after this one: (uncomment to test) -- ALTER TABLE foo ADD CONSTRAINT foo_stuff_uniq2 UNIQUE(stuff); -- it appears both as a constraint AND an index, and -- the unique_constraint_name is not empty. ALTER TABLE ONLY bla ADD CONSTRAINT bla_foo FOREIGN KEY (stuff) REFERENCES foo(stuff); SELECT * FROM information_schema.table_constraints WHERE constraint_schema = 'public'; SELECT * FROM information_schema.referential_constraints; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg metadata and doc bug
Fabien COELHO [EMAIL PROTECTED] writes: The issue is that when one does (in pg 8.3.5) ALTER TABLE foo ADD CONSTRAINT xxx UNIQUE ON (...); this results in a constraint *and* an index, but when one does only the corresponding: CREATE UNIQUE INDEX foo(...); then the index is created but there is no constraint. This is intentional. You didn't create a constraint in the sense of the SQL standard, and furthermore it may very well be impossible to represent the index as a constraint in information_schema. (For instance, the index might be functional or partial --- in fact, it most likely is special in some way, or you'd not have bothered to use the nonstandard syntax to make it.) Also, there is a minor bug in the documentation, which was the another source of my troubles: information_schema.KEY_COLUMN_USAGE.position_in_unique_constraint is tagged as NOT IMPLEMENTED, but it looks like it is implemented. Yeah, looks like the documentation is out of date there. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg metadata and doc bug
Dear Tom, The issue is that when one does (in pg 8.3.5) ALTER TABLE foo ADD CONSTRAINT xxx UNIQUE ON (...); this results in a constraint *and* an index, but when one does only the corresponding: CREATE UNIQUE INDEX foo(...); then the index is created but there is no constraint. This is intentional. You didn't create a constraint in the sense of the SQL standard, and furthermore it may very well be impossible to represent the index as a constraint in information_schema. (For instance, the index might be functional or partial --- in fact, it most likely is special in some way, or you'd not have bothered to use the nonstandard syntax to make it.) Ok. I can understand that. ISTM that I still have a bug: I have a query on the information_schema which returns stupid results because there is no matching constraint. The other way to fix is that the foreign key declaration should be rejected because there is no unique constraint on the target attribute. I guess that the FK checks that there is an index while it should (logically) check that there is a unique constraint, which implies the index. Thanks for your answer, -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Snapshot warning
Pavan Deolasee escribió: Following test case gives a warning of snapshot not destroyed at commit time. CREATE TABLE test (a int); INSERT INTO test VALUES (1); BEGIN; DECLARE c CURSOR FOR SELECT * FROM test FOR update; SAVEPOINT A; FETCH -2 FROM c; ROLLBACK TO SAVEPOINT A; COMMIT; Fixed, thanks for the test case. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Enhancement to pg_dump
Hi, I'm very new to hacking postgresql but am using on a very big site ( http://ojp.nationalrail.co.uk). One of the issues that we have is moving data from a live database to a reports one. I've hacked an extra option to pg_dump to delete from tables rather than dropping them. Once I'm happy with it (I'm a bit rusty at C!), do I post the patch here? Thanks Rob
Re: [HACKERS] Enhancement to pg_dump
On Tue, Nov 25, 2008 at 8:39 PM, Rob Kirkbride [EMAIL PROTECTED] wrote: Hi, I'm very new to hacking postgresql but am using on a very big site (http://ojp.nationalrail.co.uk). One of the issues that we have is moving data from a live database to a reports one. I've hacked an extra option to pg_dump to delete from tables rather than dropping them. National Rail use Postgres for their journey planner? Cool :-) Once I'm happy with it (I'm a bit rusty at C!), do I post the patch here? Yes (and please add details to http://wiki.postgresql.org/wiki/CommitFestOpen so it doesn't get lost), but please note that we're in the middle of the final phase of the development cycle at the moment, so new patches are unlikely to be looked at for at least a couple of months. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Enhancement to pg_dump
Dave, Ok thanks. Yes, we've got over 1/2 billion rows in one of our tables which is interesting! Will post back soon. Rob 2008/11/25 Dave Page [EMAIL PROTECTED] On Tue, Nov 25, 2008 at 8:39 PM, Rob Kirkbride [EMAIL PROTECTED] wrote: Hi, I'm very new to hacking postgresql but am using on a very big site (http://ojp.nationalrail.co.uk). One of the issues that we have is moving data from a live database to a reports one. I've hacked an extra option to pg_dump to delete from tables rather than dropping them. National Rail use Postgres for their journey planner? Cool :-) Once I'm happy with it (I'm a bit rusty at C!), do I post the patch here? Yes (and please add details to http://wiki.postgresql.org/wiki/CommitFestOpen so it doesn't get lost), but please note that we're in the middle of the final phase of the development cycle at the moment, so new patches are unlikely to be looked at for at least a couple of months. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Re: [HACKERS] blatantly a bug in the documentation
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, Le 25 nov. 08 à 20:29, Ron Mayer a écrit : psql=# install module sampledb; Downloading sampledb from pgfoundry... Installing sampledb Connecting to sampledb sampledb=# This could be part of an installer for PostgreSQL extensions. See following email for a proposal on how to deal with extension packaging: http://archives.postgresql.org/pgsql-hackers/2008-07/msg01098.php The proposal purposefully let fetching and building steps out of the database manager itself, and makes it so that building is to be cared about by distributors. So it would be something like: create sampledb pg_pkg fetch pgsampledb and either pg_pkg install pgsampledb sampledb psql sampledb or psql sampledb sampledb=# install package pgsampledb; it'd remove the need for pre-installing a rarely-needed ad-on, as well as being useful for other projects. For exmaple: psql=# install module US_Census_Tiger_Maps; Installing dependency Postgis... Installing module US_Census_Tiger_Maps to install a GIS system with all the road networks. The dependancy system is yet to be though about, but definitely in the scope of the extension manager. While at it, calling all those things extensions rather than package would probably help not confusing people with Oracle compatibility etc. Last time I checked I didn't find mention of package into the standard, but still. So PosgtreSQL could have an extension manager at SQL level (create or replace extension, install extension, or maybe load extension (which would LOAD the modules associated), etc) with a system command line tool to fetch build, etc (pg_ext ?) that source level users and packagers (distributors) would use? What do you dear readers think about the extension vocabulary? Regards, - -- dim -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Darwin) iEYEARECAAYFAkksZooACgkQlBXRlnbh1bmyvgCaAobd8kWhtkO+DxmDjbnqAWCz 5pQAoMauBWbyuvYxg6bDndYpb9CYiYZc =Reeq -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Enhancement to pg_dump
Rob Kirkbride [EMAIL PROTECTED] writes: Once I'm happy with it (I'm a bit rusty at C!), do I post the patch here? I would say you should post *before* you have a patch you're happy with. As soon as you have a specific plan of what you want to do it's best to post an outline of it. That way you at least have a chance of avoiding wasting work in the wrong direction. Sometimes things don't really work out that way -- sometimes the plan sounds good and it only becomes apparent there's a better way later -- but you're best off getting the best chance you can. Incidentally, I don't know exactly what the use case you're trying to cover here is but you should consider looking at TRUNCATE instead of DELETE if you're really deleting all the records in the table and can accept locking the table. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Column-level Privileges
Alvaro, * Alvaro Herrera ([EMAIL PROTECTED]) wrote: I had a look at aclchk.c and didn't like your change to objectNamesToOids; seems rather baroque. I changed it per the attached patch. I've incorporated this change. Moreover I didn't very much like the way aclcheck_error_col is dealing with two or one % escapes. I think you should have a separate routine for the column case, and prepend a dummy string to no_priv_msg. I can do this, not really a big deal. Why is there a InternalGrantStmt.rel_level? Doesn't it suffice to check whether col_privs is NIL? No, a single statement can include both relation-level and column-level permission changes. The rel_level flag is there to indicate if there are any relation-level changes. Nothing else indicates that. Is there enough common code in ExecGrant_Relation to justify the way you have it? Can the common be refactored in a better way that separates the two cases more clearly? I've looked at this a couple of times and I've not been able to see a good way to do that. I agree that there's alot of common code and it seems like there should be a way to factor it out, but there are a number of differences that make it difficult. If you see something I'm missing, please let me know. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Enhancement to pg_dump
OK thanks for the advice. What I'm trying to overcome is where we've got a long report running and the process that is taking data from the main database cannot complete because of the drop table. I believe a DELETE (and possibly TRUNCATE?) doesn't need an exclusive lock on the table and therefore can continue. I've introduced a --delete-not-drop option which simply does a DELETE FROM % rather than 'DROP and then CREATE'. I hope this sounds sensible and I haven't missed something - I'm still learning! Rob 2008/11/25 Gregory Stark [EMAIL PROTECTED] Rob Kirkbride [EMAIL PROTECTED] writes: Once I'm happy with it (I'm a bit rusty at C!), do I post the patch here? I would say you should post *before* you have a patch you're happy with. As soon as you have a specific plan of what you want to do it's best to post an outline of it. That way you at least have a chance of avoiding wasting work in the wrong direction. Sometimes things don't really work out that way -- sometimes the plan sounds good and it only becomes apparent there's a better way later -- but you're best off getting the best chance you can. Incidentally, I don't know exactly what the use case you're trying to cover here is but you should consider looking at TRUNCATE instead of DELETE if you're really deleting all the records in the table and can accept locking the table. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Re: [HACKERS] Column reordering in pg_dump
On Nov 14, 2008, at 12:12 PM, Tom Lane wrote: hernan gonzalez [EMAIL PROTECTED] writes: I've added an option to pg_dump to reorder columns in the ouput CREATE TABLE dump. This doesn't seem like a particularly good idea to me. In the first place, pg_dump is a tool for reproducing your database, not altering it, so it seems like basically the wrong place to be inserting this type of feature. (There's been some talk of a Postgres ETL tool, which would be the right place, but so far it's only talk :-(.) In the second place, column order is actually a pretty delicate affair when you start to think about table inheritance situations and tables that have been altered via ADD/DROP COLUMN. We had bugs in pg_dump in the past with its ability to deal with column order in such cases. So I'm not nearly as optimistic as you are that such a feature is incapable of causing problems. IIRC the community did come to a consensus on allowing for a different logical ordering from physical ordering, it was an issue of actually doing the work. If this is an itch you want to scratch, you might look into fixing that problem instead. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] blatantly a bug in the documentation
Dimitri Fontaine [EMAIL PROTECTED] writes: What do you dear readers think about the extension vocabulary? +1 ... we should stay away from package unless we are going to implement an Oracle-compatible facility. Which I don't particularly wish to do, but we should leave it open for the future. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Fwd: error compiling postgresql with fedora mingw]
On Tue, Nov 25, 2008 at 11:46 AM, Devrim GÜNDÜZ [EMAIL PROTECTED] wrote: (I'm still downloading Fedora-10) Even though Tom is subscribed to the list below, I thought I should forward this e-mail to this list: Why? Unless I'm missing something, there's far too little information here to draw any meaningful conclusions about what the problem might be. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column reordering in pg_dump
On Tue, Nov 25, 2008 at 03:10:30PM -0600, Decibel! wrote:
IIRC the community did come to a consensus on allowing for a
different logical ordering from physical ordering, it was an issue of
actually doing the work. If this is an itch you want to scratch, you
might look into fixing that problem instead.
Err, as I recall it was decided that the chance for confusion was too
high.
http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg85548.html
However, it seems to me we could have reasonably bulletproof or
machine-checkable way to keep the two kinds of column numbers
distinct, like so:
typedef struct { short log; } logical_pos;
typedef struct { short phys; } physical_pos;
This doesn't change the size of the objects, but the compiler will
prevent them from being assigned interchangably.
It does mean you need to use macros to access them, even if it's in a
loop. Fortunatly, we don't need to do too much arithmetic on them.
If the size of the object doesn't matter, you can do thing like typedef
a pointer to a one byte struct. Then most standard arithmetic
operations will still work (IIRC the Linux kernel uses this trick a
lot).
Have a nice day,
--
Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
signature.asc
Description: Digital signature
[HACKERS] mingw doesn't like PGOPTIONS?
It appears that the mingw faction of the buildfarm doesn't like this patch: http://archives.postgresql.org/pgsql-committers/2008-11/msg00289.php The failures look like the regression tests are being run with intervalstyle = iso (the default) rather than postgres_verbose as they ought to be. Which means that there's something wrong with the new code in pg_regress.c that tries to set that via PGOPTIONS instead of via PGINTERVALSTYLE. But what, and why is it only happening on those machines? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Windowing Function Patch Review - Standard Conformance
Hitoshi Harada wrote: 2008/11/20 David Rowley [EMAIL PROTECTED]: -- The following query gives incorrect results on the -- maxhighbid column SELECT auctionid, category, description, highestbid, reserve, MAX(highestbid) OVER (ORDER BY auctionid) AS maxhighbid, MAX(reserve) OVER() AS highest_reserve FROM auction_items; If you remove the highest_reserve column you get the correct results. The bug also affects MIN, AVG, COUNT, STDDEV but not SUM. Good report! I fixed this bug, which was by a trival missuse of list_concat() in the planner. This was occurred when the first aggregate trans func is strict and the second aggregate argument may be null. Yep, the argument of the second was implicitly passed to the first wrongly. That's why it didn't occur if you delete the second MAX(). I added a test case with the existing data emulating yours (named strict aggs) but if it is wrong, let me know. It's not quite right yet. I'm also getting regression tests failing on window. Let me know if you want the diffs. I've created a query that uses the table in your regression test. max_salary1 gives incorrect results. If you remove the max_salary2 column it gives the correct results. Please excuse the lack of sanity with the query. I had to do it this way to get 2 columns with NULLs. SELECT depname, empno, salary, salary1, salary2, MAX(salary1) OVER (ORDER BY empno) AS max_salary1, MAX(salary2) OVER() AS max_salary2 FROM (SELECT depname, empno, salary, (CASE WHEN salary 5000 THEN NULL ELSE salary END) AS salary1, (CASE WHEN salary = 5000 THEN NULL ELSE salary END) AS salary2 FROM empsalary ) empsalary; Actual results: depname | empno | salary | salary1 | salary2 | max_salary1 | max_salary2 ---+---++-+-+-+- sales | 1 | 5000 |5000 | | |4800 personnel | 2 | 3900 | |3900 | |4800 sales | 3 | 4800 | |4800 | |4800 sales | 4 | 4800 | |4800 | |4800 personnel | 5 | 3500 | |3500 | |4800 develop | 7 | 4200 | |4200 | |4800 develop | 8 | 6000 |6000 | | |4800 develop | 9 | 4500 | |4500 | |4800 develop |10 | 5200 |5200 | | |4800 develop |11 | 5200 |5200 | | |4800 Correct results: depname | empno | salary | salary1 | salary2 | max_salary1 | max_salary2 ---+---++-+-+-+- sales | 1 | 5000 |5000 | |5000 |4800 personnel | 2 | 3900 | |3900 |5000 |4800 sales | 3 | 4800 | |4800 |5000 |4800 sales | 4 | 4800 | |4800 |5000 |4800 personnel | 5 | 3500 | |3500 |5000 |4800 develop | 7 | 4200 | |4200 |5000 |4800 develop | 8 | 6000 |6000 | |6000 |4800 develop | 9 | 4500 | |4500 |6000 |4800 develop |10 | 5200 |5200 | |6000 |4800 develop |11 | 5200 |5200 | |6000 |4800 This might be a good regression test once it's fixed. I'm at a bit of a loss to what to do now. Should I wait for your work Heikki? Or continue validating this patch? The best thing I can think to do right now is continue and any problems I find you can add regression tests for, then if we keep your regression tests for Heikki's changes then we can validate those changes more quickly. Any thoughts? Better ideas? David. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
On Nov 19, 2008, at 11:51 PM, Tom Lane wrote: Dann Corbit [EMAIL PROTECTED] writes: I think the idea that there IS a magic number is the problem. No amount of testing is ever going to refute the argument that, under some other workload, a different value might better. But that doesn't amount to a reason to leave it the way it is. Perhaps a table of experimental data could serve as a rough guideline. The problem is not that anyone wants to leave it the way it is. The problem is that no one has done even a lick of work to identify a specific number that is demonstrably better than others -- on *any* scale. How about fewer complaints and more effort? Is there even a good way to find out what planning time was? Is there a way to gather that stat for every query a session runs? The thought occurs to me that we're looking at this from the wrong side of the coin. I've never, ever seen query plan time pose a problem with Postgres, even without using prepared statements. Anyone who actually cares that much about plan time is certainly going to use prepared statements, which makes the whole plan time argument moot (plan time, not parse time, but of course stats_target doesn't impact parsing at all). What I *have* seen, on many different databases, was problems with bad plans due to default_stats_target being too low. Most of the time this was solved by simply setting them to 1000. The only case where I backed down from that and went with like 100 was a database that had 150k tables. We've been talking about changing default_stats_target for at least 2 or 3 years now. We know that the current value is causing problems. Can we at least start increasing it? 30 is pretty much guaranteed to be better than 10, even if it's nowhere close to an ideal value. If we start slowly increasing it then at least we can start seeing where people start having issues with query plan time. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Visibility map, partial vacuums
On Nov 23, 2008, at 3:18 PM, Tom Lane wrote: So it seems like we do indeed want to rejigger autovac's rules a bit to account for the possibility of wanting to apply vacuum to get visibility bits set. That makes the idea of not writing out hint bit updates unless the page is already dirty a lot easier to swallow, because now we'd have a mechanism in place to ensure that they were set in a reasonable timeframe by autovacuum. That actually wouldn't incur much extra overhead at all, except in the case of a table that's effectively write-only. Actually, that's not even true; you still have to eventually freeze a write-mostly table. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)
On Mon, 2008-11-24 at 22:09 +0900, KaiGai Kohei wrote: I removed the two hooks at the r1244 patch set. As you said, it is fundamentally danger to load uncertain binary modules. Thus, what we should do is checks on module loading. The default security policy requires loadable modules to be labeled as 'lib_t' type which means shared library files installed correctly. We definitely want to include add-in modules with high security systems, e.g. GIS and oracle compatibility functions. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column reordering in pg_dump
Martijn van Oosterhout [EMAIL PROTECTED] writes: On Tue, Nov 25, 2008 at 03:10:30PM -0600, Decibel! wrote: IIRC the community did come to a consensus on allowing for a different logical ordering from physical ordering, it was an issue of actually doing the work. If this is an itch you want to scratch, you might look into fixing that problem instead. Err, as I recall it was decided that the chance for confusion was too high. http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg85548.html That message was about an approach that didn't have consensus ;-) The ultimate conclusion was that a three-way split (identity, logical position, physical position) could work because most of the code only cares about column identity; the places where logical or physical positions are important are pretty narrowly circumscribed, or could be made so. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
Decibel! [EMAIL PROTECTED] writes: The thought occurs to me that we're looking at this from the wrong side of the coin. I've never, ever seen query plan time pose a problem with Postgres, even without using prepared statements. That tells more about the type of queries you tend to run than about whether there's an issue in general. Anyone who actually cares that much about plan time is certainly going to use prepared statements, This is simply false. There's a significant performance hit caused by using prepared statements in many cases where the planner needs to know the parameter values in order to make good decisions. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)
Simon Riggs wrote: On Mon, 2008-11-24 at 22:09 +0900, KaiGai Kohei wrote: I removed the two hooks at the r1244 patch set. As you said, it is fundamentally danger to load uncertain binary modules. Thus, what we should do is checks on module loading. The default security policy requires loadable modules to be labeled as 'lib_t' type which means shared library files installed correctly. We definitely want to include add-in modules with high security systems, e.g. GIS and oracle compatibility functions. Yes, it is possible. SELinux assigns 'lib_t' type for modules stored in '/usr/lib/pgsql/' in default. like: [EMAIL PROTECTED] ~]$ ls -Z /usr/lib/pgsql -rwxr-xr-x root root system_u:object_r:lib_t ascii_and_mic.so -rwxr-xr-x root root system_u:object_r:lib_t cyrillic_and_mic.so -rwxr-xr-x root root system_u:object_r:lib_t dict_snowball.so -rwxr-xr-x root root system_u:object_r:lib_t euc_cn_and_mic.so -rwxr-xr-x root root system_u:object_r:lib_t euc_jis_2004_and_shift_jis_2004.so -rwxr-xr-x root root system_u:object_r:lib_t euc_jp_and_sjis.so -rwxr-xr-x root root system_u:object_r:lib_t euc_kr_and_mic.so - snip - (*) -Z option enables to show the security context of files. SE-PostgreSQL does not prevent to load them. It means we want to allow to load library files stored by database administrators properly, not a uncertain files. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei [EMAIL PROTECTED] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
Decibel! [EMAIL PROTECTED] writes: Is there even a good way to find out what planning time was? Is there a way to gather that stat for every query a session runs? \timing explain select ... The thought occurs to me that we're looking at this from the wrong side of the coin. I've never, ever seen query plan time pose a problem with Postgres, even without using prepared statements. I certainly have seen plan times be a problem. I wonder if you have too and just didn't realize it. With a default_stats_target of 1000 you'll have hundreds of kilobytes of data to slog through to plan a moderately complex query with a few text columns. Forget about prepared queries, I've seen plan times be unusable for ad-hoc interactive queries before. We've been talking about changing default_stats_target for at least 2 or 3 years now. We know that the current value is causing problems. Can we at least start increasing it? 30 is pretty much guaranteed to be better than 10, even if it's nowhere close to an ideal value. If we start slowly increasing it then at least we can start seeing where people start having issues with query plan time. How would you see anything from doing that? We only hear from people who have problems so we only see half the picture. You would have no way of knowing whether your change has helped or hurt anyone. In any case I don't see we know that the current value is causing problems as a reasonable statement. It's the *default* stats target. There's a reason there's a facility to raise the stats target for individual columns. As Dann said, the idea that there IS a magic number is the problem. *Any* value of default_stats_target will cause problems. Some columns will always have skewed data sets which require unusually large samples, but most won't and the system will run faster with a normal sample size for that majority. The question is what value represents a good trade-off between the costs of having large stats targets -- longer analyze, more data stored in pg_statistics, more vacuuming of pg_statistics needed, longer plan times -- and the benefits of having larger stats targets -- fewer columns which need raised stats targets. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
-Original Message-
From: Greg Stark [mailto:[EMAIL PROTECTED] On Behalf Of
Gregory Stark
Sent: Tuesday, November 25, 2008 5:06 PM
To: Decibel!
Cc: Tom Lane; Dann Corbit; Robert Haas; Bruce Momjian; Mark Wong;
Heikki Linnakangas; Josh Berkus; Greg Smith; pgsql-
[EMAIL PROTECTED]
Subject: Re: [HACKERS] Simple postgresql.conf wizard
[snip]
As Dann said, the idea that there IS a magic number is the problem.
*Any*
value of default_stats_target will cause problems. Some columns will
always
have skewed data sets which require unusually large samples, but most
won't
and the system will run faster with a normal sample size for that
majority.
No, it was somebody smarter than me who said that.
My idea was to create some kind of table which shows curves for
different values and then users will have some sort of basis for
choosing.
Of course, the guy who has 40 tables in his join with an average of 7
indexes on each table (each table containing millions of rows) and a
convoluted WHERE clause will have different needs than someone who has
simple queries and small data loads. The quality of the current
statistical measures stored will also affect the intelligence of the
query preparation process, I am sure. I do have a guess that larger and
more expensive queries can probably benefit more from larger samples
(this principle is used in sorting, for instance, where the sample I
collect to estimate the median might grow as {for instance} the log of
the data set size).
P.S.
I also do not believe that there is any value that will be the right
answer. But a table of data might be useful both for people who want to
toy with altering the values and also for those who want to set the
defaults. I guess that at one time such a table was generated to
produce the initial estimates for default values.
[snip]
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bugfix] DISCARD ALL does not release advisory locks
Merlin Moncure [EMAIL PROTECTED] writes: On Mon, Nov 24, 2008 at 10:25 AM, Marko Kreen [EMAIL PROTECTED] wrote: IOW, DISCARD ALL should be functionally equivalent to backend exit. Having done a lot of work with advisory locks, I support this change. Advisory locks are essentially session scoped objects like prepared statements or notifies. It's only natural to clean them up in the same way. That said, I don't think this should be backpatched to 8.3. Done but not back-patched. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
Dann Corbit [EMAIL PROTECTED] writes: I also do not believe that there is any value that will be the right answer. But a table of data might be useful both for people who want to toy with altering the values and also for those who want to set the defaults. I guess that at one time such a table was generated to produce the initial estimates for default values. Sir, you credit us too much :-(. The actual story is that the current default of 10 was put in when we first implemented stats histograms, replacing code that kept track of only a *single* most common value (and not very well, at that). So it was already a factor of 10 more stats than we had experience with keeping, and accordingly conservatism suggested not boosting the default much past that. So we really don't have any methodically-gathered evidence about the effects of different stats settings. It wouldn't take a lot to convince us to switch to a different default, I think, but it would be nice to have more than none. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
On Tue, 2008-11-25 at 20:33 -0500, Tom Lane wrote: Dann Corbit [EMAIL PROTECTED] writes: I also do not believe that there is any value that will be the right answer. But a table of data might be useful both for people who want to toy with altering the values and also for those who want to set the defaults. I guess that at one time such a table was generated to produce the initial estimates for default values. Sir, you credit us too much :-(. Better than not enough :) So we really don't have any methodically-gathered evidence about the effects of different stats settings. It wouldn't take a lot to convince us to switch to a different default, I think, but it would be nice to have more than none. I don't this is not empirical but really, 150 is very reasonable. Let's just set it to that by default and be done with it. It won't hurt anything and if they need more than that, they are already investigating either via the lists or via a vendor anyway. Joshua D. Drake regards, tom lane -- PostgreSQL Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard
On Tue, Nov 25, 2008 at 8:38 PM, Joshua D. Drake [EMAIL PROTECTED] wrote: I don't this is not empirical but really, 150 is very reasonable. Let's just set it to that by default and be done with it. It won't hurt anything and if they need more than that, they are already investigating either via the lists or via a vendor anyway. Agreed. -- Jonah H. Harris, Senior DBA myYearbook.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column reordering in pg_dump
The ultimate conclusion was that a three-way split (identity, logical position, physical position) could work because most of the code only cares about column identity; the places where logical or physical positions are important are pretty narrowly circumscribed, or could be made so. I started to take a look at this at one point and quickly got intimidated. Do you have any sense of what sort of refactoring would be required to make this viable? I believe that the original discussion[1] may have somewhat underestimated the number of places where logical position is relevant. The list includes at least: SELECT * FROM foo; TABLE foo; INSERT INTO foo VALUES (...) (or SELECT, but without column list COPY foo FROM 'foo'; COPY foo TO 'foo'; There are also some problems with this syntax: alias (column_alias, column_alias, column_alias) Imagine for example: CREATE TABLE foo (c1 integer, c2 text, c3 boolean, c4 date, c5 timestamp, c6 numeric, c7 varchar); CREATE OR REPLACE VIEW tricky AS SELECT * FROM foo AS bar (a, b, c); ALTER TABLE foo ALTER COLUMN c2 POSITION LAST; After some thought, it seems pretty clear, at least to me, that the third (hypothetical) command should not change the result of SELECT * FROM tricky (the contrary conclusion gives rise to a lot of problems, especially if there are other views depending on it). But what will pg_dump -t tricky output at this point? I suspect it will be necessary to introduce some new syntax here. ...Robert [1] http://archives.postgresql.org/pgsql-hackers/2006-12/msg00977.php -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column reordering in pg_dump
Robert Haas escribió: After some thought, it seems pretty clear, at least to me, that the third (hypothetical) command should not change the result of SELECT * FROM tricky (the contrary conclusion gives rise to a lot of problems, especially if there are other views depending on it). But what will pg_dump -t tricky output at this point? I suspect it will be necessary to introduce some new syntax here. Everything that's user-visible needs to use logical positioning. That includes pg_dump. Changing physical positioning is purely an internal matter. A first-cut implementation should probably just make it identical to logical positioning, until the latter is changed by the user (after which, physical positioning continues to reflect the original ordering). Only after this work has been done and gotten battle-tested, we can get into niceties like having the server automatically rearrange physical positioning to improve performance. Column identity is, of course, set in stone as soon as decided for the first time. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Simple postgresql.conf wizard -- Statistics idea...
-Original Message-
From: Tom Lane [mailto:[EMAIL PROTECTED]
Sent: Tuesday, November 25, 2008 5:33 PM
To: Dann Corbit
Cc: Gregory Stark; Decibel!; Robert Haas; Bruce Momjian; Mark Wong;
Heikki Linnakangas; Josh Berkus; Greg Smith; pgsql-
[EMAIL PROTECTED]
Subject: Re: [HACKERS] Simple postgresql.conf wizard
Dann Corbit [EMAIL PROTECTED] writes:
I also do not believe that there is any value that will be the right
answer. But a table of data might be useful both for people who
want
to
toy with altering the values and also for those who want to set the
defaults. I guess that at one time such a table was generated to
produce the initial estimates for default values.
Sir, you credit us too much :-(. The actual story is that the current
default of 10 was put in when we first implemented stats histograms,
replacing code that kept track of only a *single* most common value
(and not very well, at that). So it was already a factor of 10 more
stats than we had experience with keeping, and accordingly
conservatism
suggested not boosting the default much past that.
So we really don't have any methodically-gathered evidence about the
effects of different stats settings. It wouldn't take a lot to
convince
us to switch to a different default, I think, but it would be nice to
have more than none.
I do have a statistics idea/suggestion (possibly useful with some future
PostgreSQL 9.x or something):
It is a simple matter to calculate lots of interesting univarate summary
statistics with a single pass over the data (perhaps during a vacuum
full).
For instance with numerical columns, you can calculate mean, min, max,
standard deviation, skew, kurtosis and things like that with a single
pass over the data.
Here is a C++ template I wrote to do that:
http://cap.connx.com/tournament_software/STATS.HPP
It also uses this template:
http://cap.connx.com/tournament_software/Kahan.Hpp
which is a high-accuracy adder. These things could easily be rewritten
in C instead of C++.
Now, if you store a few numbers calculated in this way, it can be used
to augment your histogram data when you want to estimate the volume of a
request. So (for instance) if someone asks for a scalar that is
value you can look to see what percentage of the tail will hang out in
that neck of the woods using standard deviation and the mean.
I have another, similar idea (possibly useful someday far off in the
future) that I think may have some merit. The idea is to create a
statistical index. This index is updated whenever data values are
modified in any way. For scalar/ordinal values such as float, integer,
numeric it would simply store and update a statistics accumulator (a
small vector of a few items holding statistical moments, counts and
sums) for the column of interest. These indexes would be very small
and inexpensive to {for instance} memory map.
For categorical values (things like 'color' or 'department') we might
store the count for the number of items that correspond to a hash in our
statistical index. It would give you a crude count distinct for any
item -- the only caveat being that more than one item could possibly
have the same hash code (we would also keep a count of null items). If
the count needed to be exact, we could generate a perfect hash for the
data or store each distinct column value in the categorical index with
its hash. The size of such an index would depend on the data so that
bit or char='m'/'f' for male/female or 'y'/'n' for yes/no indexes would
contain just two counts and a column that is unique would have one hash
paired with the number one for each row of the table (a regular unique
index would clearly be better in such a case, but such distributions
could easily arise). The value of an index like this is that it is good
where the normal index types are bad (e.g. it is useless to create a
btree index on a bit column or male/female, yes/no -- things of that
nature but a statistics counter would work nicely -- to give you whole
table measures only of course). The thing that is odd about these
statistics indexes is that they do not even bother to point back to the
data that they represent -- they are just abstract measures of the
general contents.
It seems to me that this kind of information could be used to improve
the query plans for both categorical values and scalars and also be used
to generate instant answers for some kinds of statistical queries. If
used only for query planning, the values would not need to be exact, and
could be updated only at vacuum full time or some other convenient time.
The notion behind creation of a stats index would be that we may not
need to maintain this detailed information for every column, but we can
maintain such data for columns that we often filter with in our queries
to get an idea of cardinality for a subset.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
Hiroshi Saito [EMAIL PROTECTED] wrote: Um, It was not supported. http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/ITAGAKI_PATCH.txt Hmm... the implementation of wcsftime() in msvcrt seems to be completely broken. I ran the attached test (localetest.c) and got the following results. The point is that wcsftime() returns the same character codes as strftime() i.e, the result is not an unicode string :-( The bug might be fixed in recently msvcrts in VC2005 or VC2008, but at least mingw uses the old broken C runtime. We'd better to use strftime() and multiple conversions to avoid the Microsoft's bug. locale: C [Wednesday] C:str = 57 65 64 6e 65 73 64 61 79 C:wcs = 57 65 64 6e 65 73 64 61 79 locale: Japanese_Japan.932 SJIS:str = 90 85 97 6a 93 fa SJIS:wcs = 90 85 97 6a 93 fa locale: Japanese_Japan.20932 EUCJP:str = 90 85 97 6a 93 fa EUCJP:wcs = 90 85 97 6a 93 fa NOTE: There is another problem that specified encoding is ignored by the functions. The result encoding is always platform default one. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center localetest.c Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column reordering in pg_dump
On Tue, Nov 25, 2008 at 9:18 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: After some thought, it seems pretty clear, at least to me, that the third (hypothetical) command should not change the result of SELECT * FROM tricky (the contrary conclusion gives rise to a lot of problems, especially if there are other views depending on it). But what will pg_dump -t tricky output at this point? I suspect it will be necessary to introduce some new syntax here. Everything that's user-visible needs to use logical positioning. That includes pg_dump. Obviously. The point is that the alias (column_alias, column_alias, column_alias) syntax only allows you to alias the columns that are the first N logical positions. If you have a view which is aliasing columns 1..3 of some table, and column 2 of the table gets moved to position 7, the view definition now needs to alias columns 1, 2, and 7, which isn't possible with the present syntax unless you also alias 3, 4, 5, and 6. Changing physical positioning is purely an internal matter. A first-cut implementation should probably just make it identical to logical positioning, until the latter is changed by the user (after which, physical positioning continues to reflect the original ordering). Only after this work has been done and gotten battle-tested, we can get into niceties like having the server automatically rearrange physical positioning to improve performance. Yeah. The problem with that is that, as Tom pointed out in a previous iteration of this discussion, you will likely have lurking bugs. The bugs are going to come from confusing physical vs. logical vs. column identity, and if some of those are always-equal, it's gonna be pretty hard to know if you have bugs that confuse the two. Now, if you could run the regression tests with a special option that would randomly permute the two orderings with respect to one another, that would give you at least some degree of confidence... Column identity is, of course, set in stone as soon as decided for the first time. Agreed... but I'd still like to hear some thoughts on where to put the abstraction boundaries. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Windowing Function Patch Review - Standard Conformance
2008/11/26 David Rowley [EMAIL PROTECTED]: Hitoshi Harada wrote: 2008/11/20 David Rowley [EMAIL PROTECTED]: -- The following query gives incorrect results on the -- maxhighbid column SELECT auctionid, category, description, highestbid, reserve, MAX(highestbid) OVER (ORDER BY auctionid) AS maxhighbid, MAX(reserve) OVER() AS highest_reserve FROM auction_items; If you remove the highest_reserve column you get the correct results. The bug also affects MIN, AVG, COUNT, STDDEV but not SUM. Good report! I fixed this bug, which was by a trival missuse of list_concat() in the planner. This was occurred when the first aggregate trans func is strict and the second aggregate argument may be null. Yep, the argument of the second was implicitly passed to the first wrongly. That's why it didn't occur if you delete the second MAX(). I added a test case with the existing data emulating yours (named strict aggs) but if it is wrong, let me know. It's not quite right yet. I'm also getting regression tests failing on window. Let me know if you want the diffs. I've created a query that uses the table in your regression test. max_salary1 gives incorrect results. If you remove the max_salary2 column it gives the correct results. Please excuse the lack of sanity with the query. I had to do it this way to get 2 columns with NULLs. SELECT depname, empno, salary, salary1, salary2, MAX(salary1) OVER (ORDER BY empno) AS max_salary1, MAX(salary2) OVER() AS max_salary2 FROM (SELECT depname, empno, salary, (CASE WHEN salary 5000 THEN NULL ELSE salary END) AS salary1, (CASE WHEN salary = 5000 THEN NULL ELSE salary END) AS salary2 FROM empsalary ) empsalary; Actual results: depname | empno | salary | salary1 | salary2 | max_salary1 | max_salary2 ---+---++-+-+-+- sales | 1 | 5000 |5000 | | |4800 personnel | 2 | 3900 | |3900 | |4800 sales | 3 | 4800 | |4800 | |4800 sales | 4 | 4800 | |4800 | |4800 personnel | 5 | 3500 | |3500 | |4800 develop | 7 | 4200 | |4200 | |4800 develop | 8 | 6000 |6000 | | |4800 develop | 9 | 4500 | |4500 | |4800 develop |10 | 5200 |5200 | | |4800 develop |11 | 5200 |5200 | | |4800 Correct results: depname | empno | salary | salary1 | salary2 | max_salary1 | max_salary2 ---+---++-+-+-+- sales | 1 | 5000 |5000 | |5000 |4800 personnel | 2 | 3900 | |3900 |5000 |4800 sales | 3 | 4800 | |4800 |5000 |4800 sales | 4 | 4800 | |4800 |5000 |4800 personnel | 5 | 3500 | |3500 |5000 |4800 develop | 7 | 4200 | |4200 |5000 |4800 develop | 8 | 6000 |6000 | |6000 |4800 develop | 9 | 4500 | |4500 |6000 |4800 develop |10 | 5200 |5200 | |6000 |4800 develop |11 | 5200 |5200 | |6000 |4800 This might be a good regression test once it's fixed. Hmm, did you apply the latest patch correctly? My build can produce right results, so I don't see why it isn't fixed. Make sure the lines around 2420-2430 in planner.c like: /* * must copyObject() to avoid args concatenating with each other. */ pulled_exprs = list_concat(pulled_exprs, copyObject(wfunc-args)); where copyObject() is added. I'm not sure if this is related, another bug is found: *** a/src/backend/nodes/equalfuncs.c --- b/src/backend/nodes/equalfuncs.c *** *** 2246,2251 equal(void *a, void *b) --- 2246,2252 break; case T_Aggref: retval = _equalAggref(a, b); +break; case T_WFunc: retval = _equalWFunc(a, b); break; don't laugh at... could you try it and test again? If whole the new patch is needed, tell me then will send it. I'm at a bit of a loss to what to do now. Should I wait for your work Heikki? Or continue validating this patch? The best thing I can think to do right now is continue
Re: [HACKERS] WIP: Automatic view update rules
Bernd, Do you intend to submit an updated version of this patch for this commitfest? If not, I will move this to Returned with feedback. Thanks, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [WIP] In-place upgrade
Zdenek - I am a bit murky on where we stand with upgrade-in-place in terms of reviewing. Initially, you had submitted four patches for this commitfest: 1. htup and bufpage API clean up 2. HeapTuple version extension + code cleanup 3. In-place online upgrade 4. Extending pg_class info + more flexible TOAST chunk size I think that it was decided that replacing the heap tuple access macros with function calls was not acceptable, so I have moved patches #1 and #2 to the Returned with feedback section. I thought that perhaps the third patch could be salvaged, but the consensus seemed to be to go in a new direction, so I'm thinking that one should probably be moved to Returned with feedback as well. However, I'm not clear on whether you will be submitting something else instead and whether that thing should be considered material for this commitfest. Can you let me know how you are thinking about this? With respect to #4, I know that Alvaro submitted a draft patch, but I'm not clear on whether that needs to be reviewed, because: - I'm not sure whether it's close enough to being finished for a review to be a good use of time. - I'm not sure how much you and Heikki have already reviewed it. - I'm not sure whether this patch buys us anything by itself. Thoughts? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
井上です。 ITAGAKI Takahiro wrote: Hiroshi Saito [EMAIL PROTECTED] wrote: Um, It was not supported. http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/ITAGAKI_PATCH.txt Hmm... the implementation of wcsftime() in msvcrt seems to be completely broken. I ran the attached test (localetest.c) and got the following results. The point is that wcsftime() returns the same character codes as strftime() i.e, the result is not an unicode string :-( The bug might be fixed in recently msvcrts in VC2005 or VC2008, but at least mingw uses the old broken C runtime. 私はmingwを使っていないのでよくわからないのですが、ランタイムも一緒に 提供しているのですか？ We'd better to use strftime() and multiple conversions to avoid the Microsoft's bug. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
Hi. I think that MinGW does not have a direct relation. #define_UNICODE is required for wcsftime. Probably, ITAGAKI-san has only forgotten it.:-) P.S) 日本語になっていましたです:-) Regards, Hiroshi Saito - Original Message - From: Hiroshi Inoue [EMAIL PROTECTED] 井上です。 ITAGAKI Takahiro wrote: Hiroshi Saito [EMAIL PROTECTED] wrote: Um, It was not supported. http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/ITAGAKI_PATCH.txt Hmm... the implementation of wcsftime() in msvcrt seems to be completely broken. I ran the attached test (localetest.c) and got the following results. The point is that wcsftime() returns the same character codes as strftime() i.e, the result is not an unicode string :-( The bug might be fixed in recently msvcrts in VC2005 or VC2008, but at least mingw uses the old broken C runtime. 私はmingwを使っていないのでよくわからないのですが、ランタイムも一緒に 提供しているのですか？ We'd better to use strftime() and multiple conversions to avoid the Microsoft's bug. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Windowing Function Patch Review - Standard Conformance
2008/11/25 Hitoshi Harada [EMAIL PROTECTED]: 2008/11/25 Heikki Linnakangas [EMAIL PROTECTED]: Here's an updated patch, where the rows are fetched on-demand. Good! And I like the fetching args by number better. Let me take more time to look in detail... I read more, and your spooling approach seems flexible for both now and the furture. Looking at only current release, the frame with ORDER BY is done by detecting peers in WinFrameGetArg() and add row number of peers to winobj-currentpos. Actually if we have capability to spool all rows we need on demand, the frame would be only a boundary problem. It seems to me that eval_windowaggregate() also should use frame APIs. Only things we have to care is the shrinking frame, which is not supported in this release. So I'd suggest winobj-aggregatedupto to be removed. Is there objection? Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[REVIEW] (was Re: [HACKERS] usermap regexp support)
On Tue, Nov 11, 2008 at 02:59:01PM +0100, Magnus Hagander wrote: Gianni Ciolli wrote: WARNING: detected write past chunk end in Postmaster 0x9b13650 Yes, that's a stupid bug: (...) Attached is an updated version of the patch that fixes this. Hi Magnus, please find below the review of the second version of your patch, which I think is Ready for Committer now. Best regards, Dr. Gianni Ciolli - 2ndQuadrant Italia PostgreSQL Training, Services and Support [EMAIL PROTECTED] | www.2ndquadrant.it ---8--8--8--8--8--8--8--8--8--- = Introduction = The patch adds regexp support for username maps (see chapter Client Authentication, section Username maps). Basically, it allows a parametric approach to username maps, which can be useful whenever the database user name can be computed dynamically from the system user name. For example, this can simplify user provisioning operations; the pg_ident.conf file does no longer need to be updated each time an user is added/removed to a particular system. If the parameter SYSTEM-USERNAME begins with slash, then the remaining string is interpreted as a regexp, and the parameter DATABASE-USERNAME is then computed from the regexp match. Example: ---8--8--8--8--8--8--8--8--8--- mymap /(.*)@realm1.com$ \1 mymap /(.*)@realm2.com$ guest ---8--8--8--8--8--8--8--8--8--- means that [EMAIL PROTECTED] will have PostgreSQL username johnsmith, while [EMAIL PROTECTED] will connect as guest, and that similar rules will exist for any other user of these two realms. = En-passant remarks = * I found out that the comments in the pg_hba.conf file installed by default are obsolete; we should either update them or replace them with a reference to the Manual, chapter Client Authentication, section the pg_hba.conf file. = Review = (Note. I followed the guidelines found on the Wiki page.) Submission review * Is the patch in the standard form? Yes, it is. * Does it apply cleanly to the current CVS HEAD? patching file src/backend/libpq/hba.c Hunk #2 succeeded at 1404 (offset 78 lines). (CVS HEAD as of 2008-11-25 22:10+00) * Does it include reasonable tests, docs, etc? The patch modifies a single source file. No docs are enclosed; the submission e-mail ends with Obviously docs need to be updated as well. No tests are enclosed. Usability review Read what the patch is supposed to do, and consider: * Does the patch actually implement that? It seems so. * Do we want that? Yes, may be useful for several reasons. * Do we already have it? No. * Does it follow SQL spec, or the community-agreed behavior? It is a configuration option, having nothing to do with SQL spec. About the latter, I didn't find anything in the mailing lists archive. * Are there dangers? It seems not at a first glance. * Have all the bases been covered? Yes, it seems complete. Feature test Apply the patch, compile it and test: * Does the feature work as advertised? Yes. For the second revision of the patch I repeated exactly the same test procedure that I used for the first revision, and described in the mail reported in the Appendix. * Are there corner cases the author has failed to consider? I don't know if there are systems where the username can begin with /; however, on such systems it would be enough to add another slash to the beginning and to escape any regexp-like character, so that the remaining part will be interpreted as it really is. Performance review * Does the patch slow down simple tests? The patched code is triggered by a connection attempt. So in principle it could slow down, but in practice network lag time should be fairly larger than the time consumed by this code. * If it claims to improve performance, does it? - * Does it slow down other things? Should not, and seems not to. Coding review Read the changes to the code in detail and consider: * Does it follow the project coding guidelines? Yes. * Are there portability issues? No, it is all about string manipulation and regexp matching. * Will it work on Windows/BSD etc? I don't see any reason to believe the contrary. * Are the comments sufficient and accurate? Yes. There is a minor change: in the commented phrase This is replaced for $1 in the database username string, the dollar character $ should be replaced by the backslash \. * Does it do what it says, correctly? To check it I had to: * compile and install patched HEAD * edit postgresql.conf in order to make it listen from a different IP address * add to pg_hba.conf one line enabling ident connection mode for
