[HACKERS] RE: [HACKERS] Kerberos V5 required for PostgreSQL installation on Windows

2009-02-27 Thread Zeugswetter Andreas OSB sIT

We should delayload this dll since it is only needed
for specific configuration. No need to install when it is not used.

Andreas

 -Original Message-
 From: pgsql-hackers-ow...@postgresql.org 
 [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Dann Corbit
 Sent: Friday, February 27, 2009 2:31 AM
 To: pgsql-gene...@postgresql.org
 Cc: pgsql-hackers@postgresql.org
 Subject: [HACKERS] Kerberos V5 required for PostgreSQL 
 installation on Windows [bayes][heur]
 Importance: Low
 
 If Kerberos V5 is not installed on a Windows platform, the following
 error dialog is returned upon attempted installation:
 
 Posgres.exe - Unable to Locate Component
 
 This application has failed to start because krb5_32.dll was 
 not found.
 Re-installing the application may fix this problem.
 [OK]
 
 
 -- 
 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] RE: [HACKERS] Kerberos V5 required for PostgreSQL installation on Windows

2009-02-27 Thread Dave Page
On Fri, Feb 27, 2009 at 8:47 AM, Zeugswetter Andreas OSB sIT
andreas.zeugswet...@s-itsolutions.at wrote:

 We should delayload this dll since it is only needed
 for specific configuration. No need to install when it is not used.

That would require building knowledge of DLL names into the code,
which isn't practical as some projects have a habit of changing them
regularly (e.g. gettext).

I'm not sure why Dann would see this problem - all our installers
include the required DLLs, and they should also be in the
binaries-no-installer packages. If he's built the binary himself, then
he obviously has the import libraries for Kerberos, so where are the
DLLs?


-- 
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] Service not starting: Error 1053

2009-02-27 Thread Frank Featherlight
I did re-install Tune-Up Utilities, Kaspersky, Holdem Manager and Daemon
Tools, so those were probably not the problemcause since it's still working.

On Wed, Feb 25, 2009 at 4:37 PM, Magnus Hagander mag...@hagander.netwrote:

 Frank Featherlight wrote:
  1) Uninstalled the following programs+program files folder:
 
  File Shredder
  Holdem Manager (this is the program I need postgresql for)
  mIRC
  Proxifier

 This one sounds like a potential culprit.

  GetDataBack for FAT and NTFS

 This could be, but probably shouldn't.

  Registry Mechanic
  TuneUp Utilities
  SimpLite
  Daemon Tools
  Kaspersky (was not installed during the time of the errors so can't be
  to blame)
  PostgresQL

 Could be one of those, but the ones above sounds more likely.

 Now, the fact that this is Windows makes it actually possible that
 things will work even if you reinstall *all* of them :-) But it would be
 interesting to know if there is a specific one that breaks it.

 //Magnus




Re: [HACKERS] RE: [HACKERS] Kerberos V5 required for PostgreSQL installation on Windows

2009-02-27 Thread Magnus Hagander
Dave Page wrote:
 On Fri, Feb 27, 2009 at 8:47 AM, Zeugswetter Andreas OSB sIT
 andreas.zeugswet...@s-itsolutions.at wrote:
 We should delayload this dll since it is only needed
 for specific configuration. No need to install when it is not used.
 
 That would require building knowledge of DLL names into the code,
 which isn't practical as some projects have a habit of changing them
 regularly (e.g. gettext).

Are you sure?

http://msdn.microsoft.com/en-us/library/hf3f62bz.aspx

seems to indicate that you can do it on just the commandline if you want to?

(I haven't tried it so I don't know if it's actually doable)


 I'm not sure why Dann would see this problem - all our installers
 include the required DLLs, and they should also be in the
 binaries-no-installer packages. If he's built the binary himself, then
 he obviously has the import libraries for Kerberos, so where are the
 DLLs?

That is also a valid question, though.

//Magnus


-- 
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: [HACKERS] Kerberos V5 required for PostgreSQL installation on Windows

2009-02-27 Thread Dave Page
On Fri, Feb 27, 2009 at 9:39 AM, Magnus Hagander mag...@hagander.net wrote:
 Dave Page wrote:
 On Fri, Feb 27, 2009 at 8:47 AM, Zeugswetter Andreas OSB sIT
 andreas.zeugswet...@s-itsolutions.at wrote:
 We should delayload this dll since it is only needed
 for specific configuration. No need to install when it is not used.

 That would require building knowledge of DLL names into the code,
 which isn't practical as some projects have a habit of changing them
 regularly (e.g. gettext).

 Are you sure?

 http://msdn.microsoft.com/en-us/library/hf3f62bz.aspx

 seems to indicate that you can do it on just the commandline if you want to?

Hmm, didn't know you could do that. We'd still need code support to
figure out the DLL name from the import library so we can generate the
command line correctly though.


-- 
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


[HACKERS] Immediate shutdown and system(3)

2009-02-27 Thread Heikki Linnakangas
We're using SIGQUIT to signal immediate shutdown request. Upon receiving 
SIGQUIT, postmaster in turn kills all the child processes with SIGQUIT 
and exits.


This is a problem when child processes use system(3) to call other 
programs. We use system(3) in two places: to execute archive_command and 
restore_command. Fujii Masao identified this with pg_standby back in 
November:


http://archives.postgresql.org/message-id/3f0b79eb0811280156s78a3730en73aca49b6e95d...@mail.gmail.com
and recently discussed here
http://archives.postgresql.org/message-id/3f0b79eb0902260919l2675aaafq10e5b2d49ebfa...@mail.gmail.com

I'm starting a new thread to bring this to attention of those who 
haven't been following the hot standby stuff. pg_standby has a 
particular problem because it traps SIGQUIT to mean end recovery, 
promote standby to master, which it shouldn't do IMHO. But ignoring 
that for a moment, the problem is generic.


SIGQUIT by default dumps core. That's not what we want to happen on 
immediate shutdown. All PostgreSQL processes trap SIGQUIT to exit 
immediately instead, but external commands will dump core. system(3) 
ignores SIGQUIT, so we can't trap it in the parent process; it is always 
relayed to the child.


There's a few options on how to fix that:

1. Implement a custom version of system(3) using fork+exec that let's us 
trap SIGQUIT and send e.g SIGTERM or SIGINT to the child instead. It 
might be a bit tricky to get this right in a portable way; Windows would 
certainly need a completely separate implementation.


2. Use a signal other than SIGQUIT for immediate shutdown of child 
processes. We can't change the signal sent to postmaster for 
backwards-compatibility reasons, but the signal sent by postmaster to 
child processes we could change. We've already used all signals in 
normal backends, but perhaps we could rearrange them.


3. Use SIGINT instead of SIGQUIT for immediate shutdown of the two child 
processes that use system(3): the archiver process and the startup 
process. Neither of them use SIGINT currently. SIGINT is ignored by 
system(3), like SIGQUIT, but the default action is to terminate the 
process rather than core dump. Unfortunately pg_standby traps SIGINT too 
to mean promote to master, but we could change it to use SIGUSR1 
instead for that purpose. If someone has a script that uses killall 
-INT pg_standby to promote a standby server to master, it would need to 
be changed. Looking at the manual page of pg_standby, however, it seems 
that the kill-method of triggering a promotion isn't documented, so with 
a notice in release notes we could do that.


I'm leaning towards option 3, but I wonder if anyone sees a better solution.

This is all for CVS HEAD. In back-branches, I think we should just 
remove the signal handler for SIGQUIT from pg_standby and leave it at 
that. If you perform an immediate shutdown, you can get a core dump from 
archive_command or restore_command, but that's a minor inconvenience.


--
  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] Service not starting: Error 1053

2009-02-27 Thread Zeugswetter Andreas OSB sIT

 I did re-install Tune-Up Utilities, Kaspersky, Holdem Manager 
 and Daemon
 Tools, so those were probably not the problemcause since it's 
 still working.

I think unfortunately it may be the case, that only initdb has a problem.

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


[HACKERS] Hot Standby - 8.5

2009-02-27 Thread Heikki Linnakangas
As discussed at 
http://archives.postgresql.org/message-id/603c8f070902251956s16eee4a7l495d75d3ddccc...@mail.gmail.com, 
it's time to stop pushing hot standby into 8.4, take the time to work 
out the remaining details, and schedule it for 8.5. It will be a great 
feature when it's released, and in 8.5 we should also have the built-in 
replication capability, making it even greater.


I'll start looking at other patches, to help with getting 8.4 out of the 
door. Meanwhile, I'll also continue to review and discuss any updates to 
Hot Standby, but at a lower priority. I'm expecting it to be ready for 
the first or second commit fest of 8.5, meaning we have the full cycle 
left to iron out any bugs or usability issues.


--
  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] Immediate shutdown and system(3)

2009-02-27 Thread Greg Stark
On Fri, Feb 27, 2009 at 9:52 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:

 2. Use a signal other than SIGQUIT for immediate shutdown of child
 processes. We can't change the signal sent to postmaster for
 backwards-compatibility reasons, but the signal sent by postmaster to child
 processes we could change. We've already used all signals in normal
 backends, but perhaps we could rearrange them.

This isn't the first time we've run into the problem that we've run
out of signals. I think we need to multiplex all our event signals
onto a single signal and use some other mechanism to indicate the type
of message.

Perhaps we do need two signals though, so subprocesses don't need to
connect to shared memory to distinguish exit now from other events.
SIGINT for exit now and USR1 for every postgres-internal signal
using shared memory to determine the meaning sounds like the most
logical arrangement to me.

Do we really need a promote to master message at all? Is pg_standby
responsible for this or could the master write out the configuration
changes necessary itself?

-- 
greg

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


[HACKERS] RE: [HACKERS] RE: [HACKERS] Kerberos V5 required for PostgreSQL installation on Windows

2009-02-27 Thread Zeugswetter Andreas OSB sIT

  We should delayload this dll since it is only needed
  for specific configuration. No need to install when it is not used.
 
 That would require building knowledge of DLL names into the code,
 which isn't practical as some projects have a habit of changing them
 regularly (e.g. gettext).

Yup, that is bad. Seems the krb5_32.dll name is quite stable though.
Not sure if you can specify a list of anticipated names ?

 I'm not sure why Dann would see this problem - all our installers
 include the required DLLs, and they should also be in the

Oh, didn't know that. I guess that shifts it to pilot error then.

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


[HACKERS] Error codes for LIMIT and OFFSET

2009-02-27 Thread Peter Eisentraut
I was looking into adding new specific SQL:2008 error codes for invalid 
LIMIT and OFFSET values (see attached patch), when I came across an 
existing error code definition:


#define ERRCODE_INVALID_LIMIT_VALUE  MAKE_SQLSTATE('2','2', '0','2','0')

This definition has been in our sources since error codes were first 
added, but I don't find this code in the standard (it uses a 
standard-space SQLSTATE code), and as far as I can tell, it hasn't been 
actually used anywhere.  Except that PL/pgSQL defines it in plerrcodes.h 
(and Google shows that various other interfaces list it as well), but it 
can never happen, I think.


What should we do here, if anything?  Redefine 
ERRCODE_INVALID_LIMIT_VALUE to the new SQL:2008 code?  Or remove the 
whole thing (assuming that no PL/pgSQL code actually referes to it)?
Index: src/backend/executor/nodeLimit.c
===
RCS file: /cvsroot/pgsql/src/backend/executor/nodeLimit.c,v
retrieving revision 1.35
diff -u -3 -p -r1.35 nodeLimit.c
--- src/backend/executor/nodeLimit.c	1 Jan 2009 17:23:41 -	1.35
+++ src/backend/executor/nodeLimit.c	27 Feb 2009 11:23:13 -
@@ -247,7 +247,7 @@ recompute_limits(LimitState *node)
 			node-offset = DatumGetInt64(val);
 			if (node-offset  0)
 ereport(ERROR,
-		(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+		(errcode(ERRCODE_INVALID_ROW_COUNT_IN_RESULT_OFFSET_CLAUSE),
 		 errmsg(OFFSET must not be negative)));
 		}
 	}
@@ -274,7 +274,7 @@ recompute_limits(LimitState *node)
 			node-count = DatumGetInt64(val);
 			if (node-count  0)
 ereport(ERROR,
-		(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+		(errcode(ERRCODE_INVALID_ROW_COUNT_IN_LIMIT_CLAUSE),
 		 errmsg(LIMIT must not be negative)));
 			node-noCount = false;
 		}
Index: src/include/utils/errcodes.h
===
RCS file: /cvsroot/pgsql/src/include/utils/errcodes.h,v
retrieving revision 1.28
diff -u -3 -p -r1.28 errcodes.h
--- src/include/utils/errcodes.h	1 Jan 2009 17:24:02 -	1.28
+++ src/include/utils/errcodes.h	27 Feb 2009 11:23:13 -
@@ -136,6 +136,8 @@
 #define ERRCODE_INVALID_LIMIT_VALUE			MAKE_SQLSTATE('2','2', '0','2','0')
 #define ERRCODE_INVALID_PARAMETER_VALUE		MAKE_SQLSTATE('2','2', '0','2','3')
 #define ERRCODE_INVALID_REGULAR_EXPRESSION	MAKE_SQLSTATE('2','2', '0','1','B')
+#define ERRCODE_INVALID_ROW_COUNT_IN_LIMIT_CLAUSE	MAKE_SQLSTATE('2', '2', '0', '1', 'W')
+#define ERRCODE_INVALID_ROW_COUNT_IN_RESULT_OFFSET_CLAUSE	MAKE_SQLSTATE('2', '2', '0', '1', 'X')
 #define ERRCODE_INVALID_TIME_ZONE_DISPLACEMENT_VALUE	MAKE_SQLSTATE('2','2', '0','0','9')
 #define ERRCODE_INVALID_USE_OF_ESCAPE_CHARACTER		MAKE_SQLSTATE('2','2', '0','0','C')
 #define ERRCODE_MOST_SPECIFIC_TYPE_MISMATCH MAKE_SQLSTATE('2','2', '0','0','G')

-- 
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] Immediate shutdown and system(3)

2009-02-27 Thread Heikki Linnakangas

Greg Stark wrote:

This isn't the first time we've run into the problem that we've run
out of signals. I think we need to multiplex all our event signals
onto a single signal and use some other mechanism to indicate the type
of message.


Yeah. A patch to do that was discussed a while ago, as Fujii's 
synchronous replication patch bumped into that as well. I don't feel 
like changing the signaling so dramatically right now, however.



Do we really need a promote to master message at all? Is pg_standby
responsible for this or could the master write out the configuration
changes necessary itself?


The way pg_standby works is that it keeps waiting for new WAL files to 
arrive, until it's told to stop and return a non-zero exit code. 
Non-zero exit code from restore_command basically means file not 
found, making the startup process to end recovery and start up the 
database. There's two ways to tell pg_standby to stop: create a trigger 
file with a particular name, or signal it with SIGINT or SIGQUIT.


--
  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


[HACKERS] Patch for not going beyond NOFILE system limit (updated)

2009-02-27 Thread Peter Eisentraut
This is an updated and properly autoconf-guarded patch for this 
previously reported issue:



Jacek Drobiecki recently sent me a patch which stops postgresql to
actively violate the system limit of maximum open files
(RLIMIT_NOFILE) in src/backend/storage/file/fd.c, function
count_usable_fds().

This avoids irritating kernel logs (if system overstep violations are
enabled) and also the grsecurity alert when starting PostgreSQL.


References:
http://archives.postgresql.org/pgsql-bugs/2004-05/msg00103.php
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=248967

I think we can put this into 8.4 or the first commit fest of 8.5.
Index: src/backend/storage/file/fd.c
===
RCS file: /cvsroot/pgsql/src/backend/storage/file/fd.c,v
retrieving revision 1.147
diff -u -3 -p -r1.147 fd.c
--- src/backend/storage/file/fd.c	12 Jan 2009 05:10:44 -	1.147
+++ src/backend/storage/file/fd.c	27 Feb 2009 12:45:25 -
@@ -45,6 +45,9 @@
 #include sys/stat.h
 #include unistd.h
 #include fcntl.h
+#ifdef HAVE_SYS_RESOURCE_H
+#include sys/resource.h		/* for getrlimit */
+#endif
 
 #include miscadmin.h
 #include access/xact.h
@@ -361,15 +364,35 @@ count_usable_fds(int max_to_probe, int *
 	int			used = 0;
 	int			highestfd = 0;
 	int			j;
+#ifdef HAVE_GETRLIMIT
+	struct rlimit rlim;
+	int			getrlimit_status;
+#endif
 
 	size = 1024;
 	fd = (int *) palloc(size * sizeof(int));
 
+#ifdef HAVE_GETRLIMIT
+# ifdef RLIMIT_NOFILE/* most platforms use RLIMIT_NOFILE */
+	getrlimit_status = getrlimit(RLIMIT_NOFILE, rlim);
+# else   /* but BSD doesn't ... */
+	getrlimit_status = getrlimit(RLIMIT_OFILE, rlim);
+# endif /* RLIMIT_NOFILE */
+	if (getrlimit_status != 0)
+		ereport(WARNING, (errmsg(getrlimit failed: %m)));
+#endif /* HAVE_GETRLIMIT */
+
 	/* dup until failure or probe limit reached */
 	for (;;)
 	{
 		int			thisfd;
 
+#ifdef HAVE_GETRLIMIT
+		/* don't go beyond RLIMIT_NOFILE */
+		if (getrlimit_status == 0  highestfd = rlim.rlim_cur - 1)
+			break;
+#endif
+
 		thisfd = dup(0);
 		if (thisfd  0)
 		{

-- 
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] Hot Standby - 8.5

2009-02-27 Thread Bruce Momjian
Heikki Linnakangas wrote:
 As discussed at 
 http://archives.postgresql.org/message-id/603c8f070902251956s16eee4a7l495d75d3ddccc...@mail.gmail.com,
  
 it's time to stop pushing hot standby into 8.4, take the time to work 
 out the remaining details, and schedule it for 8.5. It will be a great 
 feature when it's released, and in 8.5 we should also have the built-in 
 replication capability, making it even greater.
 
 I'll start looking at other patches, to help with getting 8.4 out of the 
 door. Meanwhile, I'll also continue to review and discuss any updates to 
 Hot Standby, but at a lower priority. I'm expecting it to be ready for 
 the first or second commit fest of 8.5, meaning we have the full cycle 
 left to iron out any bugs or usability issues.

Disappointing, but sounds good.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] Service not starting: Error 1053

2009-02-27 Thread Heikki Linnakangas

Zeugswetter Andreas OSB sIT wrote:
I did re-install Tune-Up Utilities, Kaspersky, Holdem Manager 
and Daemon
Tools, so those were probably not the problemcause since it's 
still working.


I think unfortunately it may be the case, that only initdb has a problem.


Hmm, wait a minute, initdb doesn't run postmaster, but only launches 
single-mode and bootstrap backends. I don't think those need to attach 
to an existing shared memory block.


--
  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] Service not starting: Error 1053

2009-02-27 Thread Andrew Dunstan



Heikki Linnakangas wrote:

Zeugswetter Andreas OSB sIT wrote:
I did re-install Tune-Up Utilities, Kaspersky, Holdem Manager and 
Daemon
Tools, so those were probably not the problemcause since it's still 
working.


I think unfortunately it may be the case, that only initdb has a 
problem.


Hmm, wait a minute, initdb doesn't run postmaster, but only launches 
single-mode and bootstrap backends. I don't think those need to attach 
to an existing shared memory block.




In any case, starting the service doesn't run initdb either, so I think 
Andreas was having a little thinko.


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] Error codes for LIMIT and OFFSET

2009-02-27 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 What should we do here, if anything?  Redefine 
 ERRCODE_INVALID_LIMIT_VALUE to the new SQL:2008 code?

If you're going to spell the errcode macros as suggested in the
patch, just remove ERRCODE_INVALID_LIMIT_VALUE.

Note that this patch misses at least two places where new errcodes
need to be listed (plerrcodes.h and the documentation appendix)

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] Immediate shutdown and system(3)

2009-02-27 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Greg Stark wrote:
 This isn't the first time we've run into the problem that we've run
 out of signals. I think we need to multiplex all our event signals
 onto a single signal and use some other mechanism to indicate the type
 of message.

 Yeah. A patch to do that was discussed a while ago, as Fujii's 
 synchronous replication patch bumped into that as well. I don't feel 
 like changing the signaling so dramatically right now, however.

It's not really a feasible answer anyway for auxiliary processes that
have no need to be connected to shared memory.

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] Index correlation versus multi-column indexes

2009-02-27 Thread Tom Lane
I looked into Maxim Boguk's complaint here:
http://archives.postgresql.org/pgsql-general/2009-02/msg01226.php
in which the planner preferred to use an index despite the column
being searched on being a lower-order column in that index.

It turns out that the reason the planner is preferring the wrong index
is that that index has a very high indexCorrelation score, evidently
because its first column is well correlated with the table ordering.
This causes cost_index to compute very low estimated heap access costs,
outweighing the increased index access costs due to the index's
relatively poor match to the query.

Now we already knew that btcostestimate's estimate of index correlation
was pretty bogus for multicolumn indexes.  However, I now realize that
there's another issue here as well, which would apply even if the index
ordering correlation estimate were perfect.  If you look closely at what
cost_index is doing with the number, you'll realize that it is
effectively assuming that high index correlation means that an indexscan
returns TIDs that are adjacent or nearly so in the heap.  Even given
a perfect match of index and heap order, this fails to hold when the
indexscan quals contain constraints on lower-order index columns,
because we'll be skipping sections of the index in such cases.

So apparently we need to rethink this, and derate the correlation effect
somehow when there are constraints on non-first columns.  I'm not
entirely sure what the model ought to be.  Thoughts?

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] Synchronous replication Hot standby patches

2009-02-27 Thread Hannu Krosing
On Tue, 2009-02-24 at 10:34 -0800, Joshua D. Drake wrote:
 On Tue, 2009-02-24 at 17:36 +, Simon Riggs wrote:
  On Wed, 2009-02-25 at 00:51 +0900, Fujii Masao wrote:
  
   On Tue, Feb 24, 2009 at 3:47 PM, K, Niranjan (NSN - IN/Bangalore)
   niranja...@nsn.com wrote:
Could you please let me know what are the outstanding features that are 
still to be developed in the respective patches?
   
I'am currently referring the wiki: Todo and Claim for NTT and for 
HotStandby, i see that almost all issues are closed. Are there any 
features / refactoring / bugs still need to be fixed.
   
   At least I'm planning to work on the following two items of Synch Rep for 
   v8.5.
   Of course, Synch Rep works fine without these features.
  
   - Add new feature which transfers all WAL records via the direct 
   connection
 between the primary and the standby. In other words, get rid of
 file-based log shipping part from the patch.
 
   http://archives.postgresql.org/message-id/496b9495.4010...@enterprisedb.com
  
  Please bear in mind my strong objection to this. Attempting to transfer
  all data via a single connection destroys VLDB usage of this feature. So
  for me its just additional code for ease-of-use in the simplest case,
  not code replacement.
 
 Well VLDB is like 2% of what we need. If the above will remove all the
 B.S. currently associated with actually doing PITR (rsync, scp, nfs,
 pg_standby pick your poison) then I am all for it.

If you use walmgr.py, then all you need is writing a conf file and
making sure that ssh and rsync work.

Actually the best way to do Sync Rep would have been to just move to C
what walmgr.py does. That the patch could have started off from a
well-tested foundation.

 Log shipping should be:
 
 I am master, my slave is here.
 I am slave, I understand my master is here.
 Here is our mutual authentication love token.
 Let congress begin.
 
 Anything more and we are being difficult for the sake of being
 difficult.

Actually I'd leave out the first line, and start with just

- I am slave, my master accepts me, start replicationg

So there could be several slaves, of both hot standby postgresql,
wal-file-store and store-and-forward-to-many types.


-- 
Hannu Krosing   http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability 
   Services, Consulting and 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] Synchronous replication Hot standby patches

2009-02-27 Thread Joshua D. Drake
On Fri, 2009-02-27 at 22:17 +0200, Hannu Krosing wrote:

  Well VLDB is like 2% of what we need. If the above will remove all the
  B.S. currently associated with actually doing PITR (rsync, scp, nfs,
  pg_standby pick your poison) then I am all for it.
 
 If you use walmgr.py, then all you need is writing a conf file and
 making sure that ssh and rsync work.
 
 Actually the best way to do Sync Rep would have been to just move to C
 what walmgr.py does. That the patch could have started off from a
 well-tested foundation.
 

For sync?

  Log shipping should be:
  
  I am master, my slave is here.
  I am slave, I understand my master is here.
  Here is our mutual authentication love token.
  Let congress begin.
  
  Anything more and we are being difficult for the sake of being
  difficult.
 
 Actually I'd leave out the first line, and start with just
 
 - I am slave, my master accepts me, start replicationg
 

Heh fair enough.

 So there could be several slaves, of both hot standby postgresql,
 wal-file-store and store-and-forward-to-many types.

THat should be optional not the default.


Joshua D. Drake

-- 
PostgreSQL - XMPP: jdr...@jabber.postgresql.org
   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] Synchronous replication Hot standby patches

2009-02-27 Thread Hannu Krosing
On Fri, 2009-02-27 at 12:21 -0800, Joshua D. Drake wrote:
 On Fri, 2009-02-27 at 22:17 +0200, Hannu Krosing wrote:
 
   Well VLDB is like 2% of what we need. If the above will remove all the
   B.S. currently associated with actually doing PITR (rsync, scp, nfs,
   pg_standby pick your poison) then I am all for it.
  
  If you use walmgr.py, then all you need is writing a conf file and
  making sure that ssh and rsync work.
  
  Actually the best way to do Sync Rep would have been to just move to C
  what walmgr.py does. That the patch could have started off from a
  well-tested foundation.
  
 
 For sync?

For everything up to starting sync.

It somehow feels like current patch concentrated mainly on doing the
sync part and the need to get also existing stete over automatically
became as an afterthought. That's why I'm proposing to start (at least
conceptually) from an existing, fully automated and working solution.

Currently walmgr.py is doing everything from setting up replica to
getting up-to-last-second changes to slave's disk.

   Log shipping should be:
   
   I am master, my slave is here.
   I am slave, I understand my master is here.
   Here is our mutual authentication love token.
   Let congress begin.
   
   Anything more and we are being difficult for the sake of being
   difficult.
  
  Actually I'd leave out the first line, and start with just
  
  - I am slave, my master accepts me, start replicationg
  
 
 Heh fair enough.
 
  So there could be several slaves, of both hot standby postgresql,
  wal-file-store and store-and-forward-to-many types.
 
 That should be optional not the default.

Of course. One slave is a sub-case of any number of slaves :)

But doing 1-1 replica _only_ would make us laughing stock for everybody
else.

-- 
Hannu Krosing   http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability 
   Services, Consulting and 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] xpath processing brain dead

2009-02-27 Thread Hannu Krosing
On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote:
  What I have proposed for 8.3 should not break a single case that currently
  behaves usefully. If anyone has a counter-example please show it.
 
  What I have proposed for 8.4 possibly would break current useful behaviour
  (FSVO useful), but should be done anyway on correctness grounds.
 
 I dunno, aren't XML document fragments sort of a pretty common case?

I'd rather argue that xml datatype should not even accept anything but
complete xml documents. Same as int field does not accept int[].

Or maybe we rather need separate xmldocument and xmlforest/xmlfragments
types in next releases and leave the base xml as it is but deprecated
due to inability to fix it without breaking backwards compatibility.


-- 
Hannu Krosing   http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability 
   Services, Consulting and 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] B-tree finish incomplete split bug

2009-02-27 Thread Alvaro Herrera
Tom Lane wrote:
 Heikki Linnakangas hei...@enterprisedb.com writes:
  The trivial fix is to not call CacheInvalidateRelcache() in recovery 
  (patch attached). Another option is to put the check into 
  CacheInvalidateRelcache() itself, but in the name of consistency we 
  should then put the same check into the other CacheInvalidate* variants 
  as well. As nbtinsert.c is the only place that calls 
  CacheInvalidateRelcache during WAL replay, I'm going to do the trivial fix.
 
 This will need to be revisited if we ever hope to get read-only slaves
 working.  But I agree with the trivial fix for now (especially in the
 back branches).

Is this being handled by the hot standby patch?

-- 
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] xpath processing brain dead

2009-02-27 Thread Andrew Dunstan



Hannu Krosing wrote:

On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote:
  

What I have proposed for 8.3 should not break a single case that currently
behaves usefully. If anyone has a counter-example please show it.

What I have proposed for 8.4 possibly would break current useful behaviour
(FSVO useful), but should be done anyway on correctness grounds.
  

I dunno, aren't XML document fragments sort of a pretty common case?



I'd rather argue that xml datatype should not even accept anything but
complete xml documents. Same as int field does not accept int[].

Or maybe we rather need separate xmldocument and xmlforest/xmlfragments
types in next releases and leave the base xml as it is but deprecated
due to inability to fix it without breaking backwards compatibility.

  


Some of the functions, including some specified in the standard, produce 
fragments. That's why we have the 'IS DOCUMENT' test.


You can also force validation as a document by saying  SET XML OPTION 
DOCUMENT;


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] xpath processing brain dead

2009-02-27 Thread Andrew Dunstan



Andrew Dunstan wrote:



Tom Lane wrote:

Hmm, does this proposal require adding a test of well-formed-ness to
a code path that doesn't currently have one?  If so, is that likely
to contribute any noticeable slowdown?

I can't offhand see an objection to this other than possible performance
impact.

   
  


Yeah, testing the well-formedness might cost a bit. We could 
short-circuit the test by applying some comparatively fast heuristic 
tests.


Or we could decide that we'll just fix the xpath prefix part for 8.3 
and keep the wrapping. I don't want to spend a huge effort on fixing 
something I regard as fundamentally broken.


I'll do some tests to see what the cost of extra xml parsing might be.



The extra cost appears to be fairly negligible.

regression=# create table xpathtest3 as select xmlconcat(xmlelement(name 
unique1, unique1), '\n\t',xmlelement(name unique2, unique2), 
'\n\t',xmlelement(name two, two), '\n\t',xmlelement(name four, 
four),'\n\t',xmlelement(name ten,ten),'\n\t',xmlelement(name 
twenty,twenty),'\n\t',xmlelement(name 
hundred,hundred),'\n\t',xmlelement(name 
thousand,thousand),'\n\t',xmlelement(name 
twothusand,twothousand),'\n\t',xmlelement(name 
fivethous,fivethous),'\n\t',xmlelement(name 
tenthous,tenthous),'\n\t',xmlelement(name 
odd,odd),'\n\t',xmlelement(name even,even),'\n\t',xmlelement(name 
stringu1,stringu1),'\n\t',xmlelement(name 
stringu2,stringu2),'\n\t',xmlelement(name string4,string4),'\n') from tenk1;


regression=# select count(*) from (select 
xpath('//two[text()=0]/text()',xmlconcat) as elems from xpathtest3, 
generate_series(1,10) ) x ;
count 


10
(1 row)

Time: 27.722 ms


Proposed patch for 8.3 attached. (Note: it only reparses in the 
non-document case)


cheers

andrew


Index: src/backend/utils/adt/xml.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/xml.c,v
retrieving revision 1.68.2.6
diff -c -r1.68.2.6 xml.c
*** src/backend/utils/adt/xml.c	10 Nov 2008 18:02:27 -	1.68.2.6
--- src/backend/utils/adt/xml.c	27 Feb 2009 20:59:28 -
***
*** 3320,3360 
  
  	xml_init();
  
! 	/*
! 	 * To handle both documents and fragments, regardless of the fact whether
! 	 * the XML datum has a single root (XML well-formedness), we wrap the XML
! 	 * datum in a dummy element (x.../x) and extend the XPath expression
! 	 * accordingly.  To do it, throw away the XML prolog, if any.
! 	 */
! 	if (len = 5 
! 		xmlStrncmp((xmlChar *) datastr, (xmlChar *) ?xml, 5) == 0)
! 	{
! 		i = 5;
! 		while (i  len 
! 			   !(datastr[i - 1] == '?'  datastr[i] == ''))
! 			i++;
! 
! 		if (i == len)
! 			xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
! 		could not parse XML data);
  
! 		++i;
  
! 		datastr += i;
! 		len -= i;
! 	}
! 
! 	string = (xmlChar *) palloc((len + 8) * sizeof(xmlChar));
! 	memcpy(string, x, 3);
! 	memcpy(string + 3, datastr, len);
! 	memcpy(string + 3 + len, /x, 5);
! 	len += 7;
! 
! 	xpath_expr = (xmlChar *) palloc((xpath_len + 3) * sizeof(xmlChar));
! 	memcpy(xpath_expr, /x, 2);
! 	memcpy(xpath_expr + 2, VARDATA(xpath_expr_text), xpath_len);
! 	xpath_expr[xpath_len + 2] = '\0';
! 	xpath_len += 2;
  
  	xmlInitParser();
  
--- 3320,3332 
  
  	xml_init();
  
! 	string = (xmlChar *) palloc((len + 8) * sizeof(xmlChar));
  
! 	xpath_expr = (xmlChar *) palloc((xpath_len + 5) * sizeof(xmlChar));
  
! 	memcpy (string, datastr, len);
! 	string[len] = '\0';
! 	
  
  	xmlInitParser();
  
***
*** 3367,3375 
  		xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
  	could not allocate parser context);
  	doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
! 	if (doc == NULL)
! 		xml_ereport(ERROR, ERRCODE_INVALID_XML_DOCUMENT,
! 	could not parse XML data);
  	xpathctx = xmlXPathNewContext(doc);
  	if (xpathctx == NULL)
  		xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
--- 3339,3410 
  		xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
  	could not allocate parser context);
  	doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
! 
! 	if (doc == NULL || xmlDocGetRootElement(doc) == NULL)
! 	{
! 
! 		/*
! 		 * In case we have a fragment rather than a well-formed XML document,
! 		 * which has a single root (XML well-formedness), we try again after
! 		 * transforming the xml by stripping away the XML prolog, if any, and
! 		 * wrapping the remainder in a dummy element (x.../x),
! 		 * and later extending the XPath expression accordingly. 
! 		 */
! 		if (len = 5 
! 			xmlStrncmp((xmlChar *) datastr, (xmlChar *) ?xml, 5) == 0)
! 		{
! 			i = 5;
! 			while (i  len 
!    !(datastr[i - 1] == '?'  datastr[i] == ''))
! i++;
! 			
! 			if (i == len)
! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
! 			could not parse XML data);
! 			
! 			++i;
! 			
! 			datastr += i;
! 			len -= i;
! 		}
! 
! 		memcpy(string, x, 3);
! 		memcpy(string + 3, datastr, len);
! 		memcpy(string + 3 + len, /x, 5);
! 		len += 7;
! 
! 		doc = 

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)

2009-02-27 Thread Jaime Casanova
On Sat, Feb 14, 2009 at 12:46 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
 Jaime Casanova wrote:

 i only have ubuntu at hand, and i can install selinux from
 repositories... do you see any problem with that?
 i'm creating the VM now... will test on weekend...

 Now I checked the repository of Ubuntu, however, I can find
 several matters to build/run SE-PostgreSQL.
 - The default security policy is quite old.
  Is does not contain SE-PostgreSQL support which is merged
  at the refpolicy-20080702.
 - The libselinux is a bit old. It does not contains several
  symbols required by SE-PostgreSQL, so it is not possible
  to compile.

 If you set up a VM from scratch, Fedora enables to install
 via remote repository using minimum bootable image.
  http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/


at last i managed to install fedora 10 and it seems like it have
selinux installed (both locate selinux and locate libselinux gives
results).

executing: yum install libselinux gives Package
libselinux-2.0.73-1.fc10.i386 already installed and latest version

nevertheless, when i run:

./configure --prefix=/usr/local/pgsql/8.4.se --enable-cassert
--enable-debug --enable-depend --enable-selinux

i get this message:
checking for getpeercon in -lselinux... no
configure: error: --enable-selinux requires libselinux.

attached config.log

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157


config.log.gz
Description: GNU Zip compressed 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] xpath processing brain dead

2009-02-27 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 I'll do some tests to see what the cost of extra xml parsing might be.

 The extra cost appears to be fairly negligible.

Uh, you didn't actually show a comparison of before and after?
What it looks like to me is that this approach is free or nearly so for
well-formed documents, but doubles the parsing cost for forests.
Which is likely to annoy anyone who's really depending on the
capability.

Also,

 ! if (*VARDATA(xpath_expr_text) == '/')

This is risking a core dump if the xpath expr is of zero length.  You
need something like

if (xpath_len  0  *VARDATA(xpath_expr_text) == '/')

It would also be a good idea if the allocation of string and xpath_expr
had a comment about why it's allocating extra space (something like see
hacks below for use of this extra space would be sufficient).

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 (r1530)

2009-02-27 Thread Tom Lane
Jaime Casanova jcasa...@systemguards.com.ec writes:
 executing: yum install libselinux gives Package
 libselinux-2.0.73-1.fc10.i386 already installed and latest version

Yeah, but have you got libselinux-devel?  A general rule on Red Hat
based systems is that compiling a program that depends on library
package foo will require foo-devel to be installed too.

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] xpath processing brain dead

2009-02-27 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:
  

I'll do some tests to see what the cost of extra xml parsing might be.
  


  

The extra cost appears to be fairly negligible.



Uh, you didn't actually show a comparison of before and after?
What it looks like to me is that this approach is free or nearly so for
well-formed documents, but doubles the parsing cost for forests.
Which is likely to annoy anyone who's really depending on the
capability.
  



The difference is lost in the noise.

Without fix:

Time: 24.619 ms
Time: 24.245 ms
Time: 25.179 ms

With fix:

Time: 24.084 ms
Time: 21.980 ms
Time: 23.765 ms

The test is done on 10,000 short fragments each parsed 10 times (or 20 
times with the fix).


I'll test again on some longer fragments since you don't seem convinced.


Also,

  

!   if (*VARDATA(xpath_expr_text) == '/')



This is risking a core dump if the xpath expr is of zero length.  You
need something like

if (xpath_len  0  *VARDATA(xpath_expr_text) == '/')
  


OK.


It would also be a good idea if the allocation of string and xpath_expr
had a comment about why it's allocating extra space (something like see
hacks below for use of this extra space would be sufficient).
  



OK.

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] Updates of SE-PostgreSQL 8.4devel patches (r1530)

2009-02-27 Thread Jaime Casanova
On Fri, Feb 27, 2009 at 5:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Jaime Casanova jcasa...@systemguards.com.ec writes:
 executing: yum install libselinux gives Package
 libselinux-2.0.73-1.fc10.i386 already installed and latest version

 Yeah, but have you got libselinux-devel?  A general rule on Red Hat
 based systems is that compiling a program that depends on library
 package foo will require foo-devel to be installed too.


ah! ok, i tried libselinux-dev and nothing was found (i'm more
familiar with the debian naming convention ;)

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

-- 
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] Index correlation versus multi-column indexes

2009-02-27 Thread Jeff Davis
On Fri, 2009-02-27 at 13:25 -0500, Tom Lane wrote:
 So apparently we need to rethink this, and derate the correlation effect
 somehow when there are constraints on non-first columns.  I'm not
 entirely sure what the model ought to be.  Thoughts?

This seems similar to the problem of estimating correlation for a GiST
index (as I recall you mentioned before that we should be tracking
correlation per-index rather than per-attribute).

Unless we get significantly smarter about what correlation means, I
think its only purpose is for very simple range scans. And, as you point
out, a selective predicate on a non-first attribute means that it's not
really a range scan.

I don't see an easy solution to this other than just saying that a
predicate on a second attribute is not a range scan at all, unless the
predicate is not very selective.

Regards,
Jeff Davis


-- 
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] xpath processing brain dead

2009-02-27 Thread Hannu Krosing
On Fri, 2009-02-27 at 16:37 -0500, Andrew Dunstan wrote:
 
 Hannu Krosing wrote:
  On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote:

  What I have proposed for 8.3 should not break a single case that currently
  behaves usefully. If anyone has a counter-example please show it.
 
  What I have proposed for 8.4 possibly would break current useful 
  behaviour
  (FSVO useful), but should be done anyway on correctness grounds.

  I dunno, aren't XML document fragments sort of a pretty common case?
  
 
  I'd rather argue that xml datatype should not even accept anything but
  complete xml documents. Same as int field does not accept int[].
 
  Or maybe we rather need separate xmldocument and xmlforest/xmlfragments
  types in next releases and leave the base xml as it is but deprecated
  due to inability to fix it without breaking backwards compatibility.
 

 
 Some of the functions, including some specified in the standard, produce 
 fragments. That's why we have the 'IS DOCUMENT' test.

But then you could use xmlfragments as the functions return type, no ?

Does tha standard require that the same field type must store both
documents and fragments ?

 You can also force validation as a document by saying  SET XML OPTION 
 DOCUMENT;
 
 cheers
 
 andrew
-- 
Hannu Krosing   http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability 
   Services, Consulting and 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] add_path optimization

2009-02-27 Thread Tom Lane
After a lot of distractions, I've finished applying the planner fixes
that seem necessary in view of your report about poorer planning in 8.4
than 8.3.  When you have a chance, it would be useful to do a thorough
test of CVS HEAD on your data and query mix --- are there any other
places where we have regressed relative to 8.3?

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] xpath processing brain dead

2009-02-27 Thread James Pye

On Feb 26, 2009, at 7:03 PM, Andrew Dunstan wrote:
can you point me at any call in libxml2 which will evaluate an xpath  
expression in the context of a nodeset instead of a document?


No, I can't. node-sets are XPath objects not xmlNode objects, so I  
don't think it would be as simple as modifying:


xml.c:xpath() {
   ...
   xpathctx-node = xmlDocGetRootElement(doc);

with the result of xmlXPathNewNodeSet..

[snip other questions]

My *guess* would be that if we were to use a node-set instead, we'd  
still have to prefix the XPath query. In this case, with a function  
call to an xpath extension function that creates the NodeSet from the  
content fragment(s?) of the document created by xml_parse(ie, more or  
less, a re-implementation of exsl:node-set() tailored for our use- 
case). Well, that or force the user to call it explicitly. Possible or  
not--wrt using a content fragment/document as the context node, I find  
this less desirable than the current mangling, so I'm becoming quite  
indifferent. :)


--
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] add_path optimization

2009-02-27 Thread Tom Lane
[ This patch is on the 2009-First commitfest list, but there was earlier
discussion to the effect that it might be a good idea to include in 8.4,
so I made some time to look at it. ]

Robert Haas robertmh...@gmail.com writes:
 I've been doing some benchmarking and profiling on the PostgreSQL
 query analyzer, and it seems that (at least for the sorts of queries
 that I typically run) the dominant cost is add_path().  I've been able
 to find two optimizations that seem to help significantly:

I got around to testing this patch a bit today, and I'm afraid it
doesn't look very good from here.  The test case I used was a psql
script with 1 repetitions of

explain select * from information_schema.table_constraints
  join information_schema.referential_constraints
  using (constraint_catalog,constraint_schema,constraint_name);

which isn't terribly creative, but those two views incorporate multiple
joins so I figured this was a reasonably middle-of-the-road planner
exercise.

I first tried just the compare_fuzzy_path_costs() change and really
couldn't measure any reliable difference.  oprofile output for CVS HEAD
looks like

PU: P4 / Xeon with 2 hyper-threads, speed 2792.99 MHz (estimated)
Counted GLOBAL_POWER_EVENTS events (time during which processor is not stopped) 
with a unit mask of 0x01 (mandatory) count 10
samples  %image name   symbol name
1492949.7519  postgres AllocSetAlloc
85437 5.5808  postgres SearchCatCache
45520 2.9734  postgres copyObject
39525 2.5818  postgres add_path
35878 2.3436  postgres expression_tree_walker
29733 1.9422  libc-2.8.so  memcpy
25560 1.6696  postgres MemoryContextAllocZeroAligned
20879 1.3638  libc-2.8.so  strlen
20521 1.3404  postgres add_paths_to_joinrel
20054 1.3099  postgres AllocSetFree
20023 1.3079  postgres cost_mergejoin
19363 1.2648  postgres nocachegetattr
17689 1.1554  libc-2.8.so  vfprintf
17335 1.1323  postgres generate_join_implied_equalities
16085 1.0507  postgres MemoryContextAlloc

and with the compare_fuzzy_path_costs() change

CPU: P4 / Xeon with 2 hyper-threads, speed 2792.99 MHz (estimated)
Counted GLOBAL_POWER_EVENTS events (time during which processor is not stopped) 
with a unit mask of 0x01 (mandatory) count 10
samples  %image name   symbol name
1448829.6951  postgres AllocSetAlloc
84205 5.6348  postgres SearchCatCache
43956 2.9414  postgres copyObject
37418 2.5039  postgres add_path
34865 2.3331  postgres expression_tree_walker
28718 1.9217  libc-2.8.so  memcpy
23643 1.5821  postgres MemoryContextAllocZeroAligned
20835 1.3942  libc-2.8.so  strlen
20065 1.3427  postgres cost_mergejoin
19672 1.3164  postgres AllocSetFree
18992 1.2709  postgres add_paths_to_joinrel
18802 1.2582  postgres nocachegetattr
17109 1.1449  libc-2.8.so  __printf_fp
16901 1.1310  libc-2.8.so  vfprintf
16159 1.0813  postgres hash_search_with_hash_value
16039 1.0733  postgres generate_join_implied_equalities
15554 1.0408  postgres MemoryContextAlloc
14981 1.0025  postgres expression_tree_mutator

Note that the compiler is inlining compare_fuzzy_path_costs in both
cases.

I then added the add_similar_paths change, and got this:

CPU: P4 / Xeon with 2 hyper-threads, speed 2792.99 MHz (estimated)
Counted GLOBAL_POWER_EVENTS events (time during which processor is not stopped) 
with a unit mask of 0x01 (mandatory) count 10
samples  %image name   symbol name
1426459.6668  postgres AllocSetAlloc
83161 5.6357  postgres SearchCatCache
44660 3.0265  postgres copyObject
35762 2.4235  postgres expression_tree_walker
28427 1.9264  postgres add_path
27970 1.8955  libc-2.8.so  memcpy
23317 1.5801  postgres MemoryContextAllocZeroAligned
20685 1.4018  libc-2.8.so  strlen
19646 1.3314  postgres add_paths_to_joinrel
19129 1.2963  postgres AllocSetFree
18065 1.2242  postgres cost_mergejoin
17768 1.2041  postgres nocachegetattr
17249 1.1689  postgres generate_join_implied_equalities
16784 1.1374  libc-2.8.so  vfprintf
15740 1.0667  libc-2.8.so  __printf_fp
15642 1.0600  postgres MemoryContextAlloc
15194 

Re: [HACKERS] add_path optimization

2009-02-27 Thread Robert Haas
Thanks for taking a look at it.

 I first tried just the compare_fuzzy_path_costs() change and really
 couldn't measure any reliable difference.  oprofile output for CVS HEAD
 looks like

Well, there's obviously something different between your case and
mine, because in my query add_path was the #1 CPU hog and it wasn't
close.  I suspect my query had more joins - I'll take a look at yours.

 It gets worse though: add_similar_paths is flat *wrong*.  It compares
 each successive path to only the current cheapest-total candidate,
 and thus is quite likely to seize on the wrong cheapest-startup path.
 I tried fixing its loop logic like so:

Nuts.  That's probably a fatal defect.

 Now, maybe if we could force the compiler to inline
 compare_fuzzy_path_costs we could buy some of that back.  Another
 possibility is to contort add_similar_path some more so that it avoids
 double comparisons so long as cheapest_total and cheapest_startup are
 the same path; I have a feeling that's not going to win much though.

Agreed.

 So it's kind of looking like a dead end from here.  But in any case the
 patch isn't acceptable as-is with that fundamental logic error in
 add_similar_paths, so I'm bouncing it back for rework.

...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] xpath processing brain dead

2009-02-27 Thread Andrew Dunstan



Hannu Krosing wrote:
  
  
Some of the functions, including some specified in the standard, produce 
fragments. That's why we have the 'IS DOCUMENT' test.



But then you could use xmlfragments as the functions return type, no ?

  



Not in the case of xpath, no.

There is a very complete standard for the Xpath data model, and we need 
to adhere to it.


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] add_path optimization

2009-02-27 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I first tried just the compare_fuzzy_path_costs() change and really
 couldn't measure any reliable difference.  oprofile output for CVS HEAD
 looks like

 Well, there's obviously something different between your case and
 mine, because in my query add_path was the #1 CPU hog and it wasn't
 close.  I suspect my query had more joins - I'll take a look at yours.

Interesting.  I'd like to see your example too, if you can publish it.
You had noted that add_path seemed to iterate clear to the end of the
list in most cases, which it's designed not to do, so there might be
something else going on in your example.

One other thought to roll around in your head: at the time that the
current add_path logic was designed, compare_pathkeys was ungodly
expensive, which is why the code tries to compare costs first.
We've since introduced the canonical pathkey representation, which
makes compare_pathkeys enough cheaper that it might not be insane
to do it in the other order.  I don't immediately see an easy win
there, because as things are structured we typically want the cost
comparison result anyway for list sorting purposes.  But maybe there's
a way around that.

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