Re: [PATCHES] [PATCH] Have configure complain about unknown options
On 5/4/06, Tom Lane [EMAIL PROTECTED] wrote: Martijn van Oosterhout kleptog@svana.org writes: Currently, configure ignores unknown --enable/disable/with/without options. The autoconf people consider that a feature, not a bug. I'm disinclined to second-guess the designers of the tool, especially with a patch like this that mucks with the internals to the extent that it'll probably break in every future autoconf revision. Feel free to try to convince them to change it though ... AFAIK that 'feature' is there to support configuring a 'tree' of projects (like gcc), where subprojects have their own configure scripts with different options. That way you can give all options to top-level configure script which passes them to other scripts and each picks only whats needed. In such setting all scripts need to ignore unknown options. As PostgreSQL tree is not set up that way, I think for clarity sake it would be better to give explicit errors for unknown options. -- marko ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [PATCH] Have configure complain about unknown options
On Fri, May 05, 2006 at 02:22:25PM +0300, Marko Kreen wrote: AFAIK that 'feature' is there to support configuring a 'tree' of projects (like gcc), where subprojects have their own configure scripts with different options. That way you can give all options to top-level configure script which passes them to other scripts and each picks only whats needed. In such setting all scripts need to ignore unknown options. I was wondering about that. I think it's also because there are possibly a number of tools working together (autoheader/automake/etc) and autoconf is not in the position to know all possible options. You're not required to declare all the options you understand in configure.in because they may be used elsewhere. As PostgreSQL tree is not set up that way, I think for clarity sake it would be better to give explicit errors for unknown options. Someone in the past has gone to some effort to create a bunch of macros for postgres that declares all the options our configure script understands and simultaneously create help for them (this is also why the patch is so small, the hard work of identifying the options was done long ago). So we are in a position to know what is allowed and what isn't. One thing I've noticed so far is that the Debian package scripts use --enable-maintainer-mode. No idea why, that affects automake, which we don't use. It is however an excellent example of why complaining about unknown options can't be done in the general case. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] [BUGS] BUG #2401: spinlocks not available on amd64
Done, for Solaris and x86. --- Robert Lor wrote: Hi Bruce, The new SPARC assembly file src/backend/port/tas/solaris_sparc.s uses / instead of ! for comments, and as a result the compile fails with Sun Studio 11. Please modify the first 3 lines to look like the following. !=== ! solaris_sparc.s -- compare and swap for solaris_sparc !=== Thanks! -Robert Bruce Momjian wrote On 04/29/06 17:16,: Tom Lane wrote: Theo Schlossnagle [EMAIL PROTECTED] writes: I'd remind everyone that the spinlock stuff is entirely optional at build time. Not really. The performance hit for not having hardware spinlocks is so severe that it's not considered a reasonable fallback. I also think it immensely useful to replace all of the tas subsystem with cas so that one could reliabily lock these atomics with the process id of the locker. I cannot, ever once in my years working on Postgres, remember having wanted such a thing. I am strongly against mucking with the spinlock code for mere aesthetics --- it's too fragile and hard to test, especially on platforms you don't have ready access to. In short, it ain't broken and we don't need to fix it. Agreed. Should the new Solaris ASM code be modified? -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [PATCH] Have configure complain about unknown options
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Marko Kreen Sent: 05 May 2006 12:22 To: Tom Lane Cc: Martijn van Oosterhout; pgsql-patches@postgresql.org Subject: Re: [PATCHES] [PATCH] Have configure complain about unknown options As PostgreSQL tree is not set up that way, I think for clarity sake it would be better to give explicit errors for unknown options. I'm not in a position to argue about why autoconf works this way, but I can say that I'd like to see unsupported options rejected if there is a sensible way to do it. I've been bitten more than once by mistakenly using --enable-foo rather than --with-foo, or just plain mis-typing. Regards, Dave. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [PATCH] Have configure complain about unknown options
I am thinking we would need an option at the start like --strict that would throw an error for any later invalid options. --- Dave Page wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Marko Kreen Sent: 05 May 2006 12:22 To: Tom Lane Cc: Martijn van Oosterhout; pgsql-patches@postgresql.org Subject: Re: [PATCHES] [PATCH] Have configure complain about unknown options As PostgreSQL tree is not set up that way, I think for clarity sake it would be better to give explicit errors for unknown options. I'm not in a position to argue about why autoconf works this way, but I can say that I'd like to see unsupported options rejected if there is a sensible way to do it. I've been bitten more than once by mistakenly using --enable-foo rather than --with-foo, or just plain mis-typing. Regards, Dave. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [PATCH] Have configure complain about unknown options
On Fri, May 05, 2006 at 08:34:36AM -0400, Bruce Momjian wrote: I am thinking we would need an option at the start like --strict that would throw an error for any later invalid options. Well, --strict would be tricky, if it's possible. My reading of the autoconf code doesn't indicate a means of doing adding abitrary options. But something like --enable-strict-options would be fairly straight forward. Problem being, if you mistype that option, it'll seem to work even when it isn't :) Maybe an evironment variable: PGAC_STRICT Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] Have configure complain about unknown options
Martijn van Oosterhout wrote: On Fri, May 05, 2006 at 08:34:36AM -0400, Bruce Momjian wrote: I am thinking we would need an option at the start like --strict that would throw an error for any later invalid options. Well, --strict would be tricky, if it's possible. My reading of the autoconf code doesn't indicate a means of doing adding abitrary options. But something like --enable-strict-options would be fairly straight forward. Problem being, if you mistype that option, it'll seem to work even when it isn't :) I've been bitten by this in the past as well. I'd vote for applying the patch as-is, no strict mode necessary (because, as you say, it's easy to get it wrong). Can the Debian build script be fixed? Does the RPM spec have the same problem? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Have configure complain about unknown options
On Fri, May 05, 2006 at 09:13:54AM -0400, Alvaro Herrera wrote: Well, --strict would be tricky, if it's possible. My reading of the autoconf code doesn't indicate a means of doing adding abitrary options. But something like --enable-strict-options would be fairly straight forward. Problem being, if you mistype that option, it'll seem to work even when it isn't :) I've been bitten by this in the past as well. I'd vote for applying the patch as-is, no strict mode necessary (because, as you say, it's easy to get it wrong). How about the reverse, an option to relax the checking. --disable-strict-options for example? Can the Debian build script be fixed? Does the RPM spec have the same problem? Looking at the source it may be an artifact of the CDBS build system used to build the package. It knows that the autotools are in use and appends it automatically. FWIW, I'd just add a line to the case statement accepting the enable_maintainer_mode variable since it's harmless and we're trying to catch typos here, not actual options that don't apply in our case. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] plpython improvements
Ühel kenal päeval, N, 2006-05-04 kell 18:21, kirjutas Sven Suursoho: Hi, Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane [EMAIL PROTECTED]: Sven Suursoho [EMAIL PROTECTED] writes: Unfortunately, there is still one problem when using unpatched python, caused by too aggressive assert. http://mail.python.org/pipermail/python-checkins/2005-August/046571.html. I don't think we are going to be able to accept a patch that causes the server to crash when using any but a bleeding-edge copy of Python. Actually not bleeding-edge, but just version 2.4.x as distributed in Fedora Core (and possibly in RHAS), which have assert() enabled in python.so. The assert there is buggy (bug http://sourceforge.net/tracker/index.php?func=detailaid=1257960group_id=5470atid=105470) Did complete rewrite for SETOF functions: now accepts any python object for which iter(object) returns iterable object. In this way we don't have to deal with specific containers but can use unified python iterator API. It means that plpython is future-proof -- whenever python introduces new container, stored procedures already can use those without recompiling language handler. Also integrated with regression tests and updated existing tests to use named parameters. When using python interpreter with asserts enabled, generators still crash. But I don't think that we should drop this feature because of that. Reasons: 1) this is someone else's bug, we are using documented API correctly 2) it doesn't concern majority of users because probably there is no asserts in production packages (tested with gentoo, ubuntu, suse). This is true even for older python versions that are not patched. From reading the bug, it seems that older versions of python also don't have this bug, only 2.4. And after all, we can document using sets, lists, tuples, iterators etc and explicitly state that returning generator is undefined. I think that a less confusing way of saying it would be : Generators crash if python version used is 2.4.x and it is compiled with asserts. Currently only known linux distributions to distibute such python.so files are Fedora and possibly other RedHat distributions, while Gentoo, Ubuntu and Suse are OK. If you need to use generators on such a platform, compile your own python from source and make sure that configure uses your version. I think the patch should be commited so we can collect data about where else the buggy version of python exists. And if some buildfarm machines start crashing, python should be fixed there. Hannu ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [BUGS] BUG #2401: spinlocks not available on amd64
Bruce, the change was only needed for the SPARC version only. The x86 file worked just fine before and needs to be reverted back. Yes, they are different. Apologies if the message was not clear the first time! Thanks, Robert Bruce Momjian wrote: Done, for Solaris and x86. --- Robert Lor wrote: Hi Bruce, The new SPARC assembly file src/backend/port/tas/solaris_sparc.s uses / instead of ! for comments, and as a result the compile fails with Sun Studio 11. Please modify the first 3 lines to look like the following. !=== ! solaris_sparc.s -- compare and swap for solaris_sparc !=== ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plpython improvements
I think that a less confusing way of saying it would be : Generators crash if python version used is 2.4.x and it is compiled with asserts. Currently only known linux distributions to distibute such python.so files are Fedora and possibly other RedHat distributions, while Gentoo, Ubuntu and Suse are OK. Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime and it would be a mistake to assume that we won't run into this. People who are developing in Python are going to run 2.4 because it is the latest stable release of the language. Sincerely, Joshua D. Drake If you need to use generators on such a platform, compile your own python from source and make sure that configure uses your version. I think the patch should be commited so we can collect data about where else the buggy version of python exists. And if some buildfarm machines start crashing, python should be fixed there. Hannu ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [BUGS] BUG #2401: spinlocks not available on amd64
Robert Lor wrote: Bruce, the change was only needed for the SPARC version only. The x86 file worked just fine before and needs to be reverted back. Yes, they are different. OK, fixed, thanks. Apologies if the message was not clear the first time! Yes, you were clear, but it was so illogical I figured it must be both platforms. :-0 I added a comment to clarify. This was the way the comments were in 8.1.X. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Have configure complain about unknown options
I am worried if we change the default behavior that build systems will fail, but fail after our release when they go to package, and we will not get feedback until to late. --- Martijn van Oosterhout wrote: -- Start of PGP signed section. On Fri, May 05, 2006 at 09:13:54AM -0400, Alvaro Herrera wrote: Well, --strict would be tricky, if it's possible. My reading of the autoconf code doesn't indicate a means of doing adding abitrary options. But something like --enable-strict-options would be fairly straight forward. Problem being, if you mistype that option, it'll seem to work even when it isn't :) I've been bitten by this in the past as well. I'd vote for applying the patch as-is, no strict mode necessary (because, as you say, it's easy to get it wrong). How about the reverse, an option to relax the checking. --disable-strict-options for example? Can the Debian build script be fixed? Does the RPM spec have the same problem? Looking at the source it may be an artifact of the CDBS build system used to build the package. It knows that the autotools are in use and appends it automatically. FWIW, I'd just add a line to the case statement accepting the enable_maintainer_mode variable since it's harmless and we're trying to catch typos here, not actual options that don't apply in our case. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. -- End of PGP section, PGP failed! -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Page at a time index scan
I've just realized that the locking considerations in this patch are considerably more complicated than we thought. The discussion so far considered only half of what the deletion interlock needs to accomplish. We have to ensure that indexscans don't lose their place, which is solved in the patch by stopping between pages rather than on a specific item. But we also have to ensure that indexscans don't return pointers to heap tuples that have already been deleted by VACUUM --- at least in the case where the caller is using a non-MVCC snapshot. Else, if the tuple is replaced by a newer tuple before the caller is able to visit it, the caller might mistakenly accept that tuple as the one it wanted. The problematic scenario looks like this: 1. indexscan slurps a bunch of heap TIDs into local memory. It keeps pin, but not lock, on the index page it got the TIDs from. 2. someone else splits the index page, which is allowed since the page is only pinned not locked. Now, some of the index items are on a new page that the indexscan does not have pinned. 3. vacuum comes along and removes some of the moved items. Since they are on a page that no one has pinned, even the super-exclusive lock that vacuum takes won't prevent this. 4. vacuum removes the corresponding heap tuples. 5. someone else installs new, unrelated, tuples into the freed heap slots. 6. indexscan finally visits the heap. It finds tuples that are valid per its snapshot, but aren't what it thought it was finding (they don't necessarily match the index key). While we could prevent this by rechecking whether fetched heap tuples match the index search condition, that costs an awful lot of cycles for such an obviously rare problem. We need a locking-based solution instead. I believe an appropriate fix looks like this: 1. Indexscan is required to keep pin on the page it read the items from (even though we realize they may not still be on that page). The pin can only be dropped when we are no longer going to return any more items read from the page. 2. btbulkdelete is required to get super-exclusive lock on *every leaf page in the index*. It must not try to optimize this down to just the pages containing deletable items. With these rules, even though vacuum might have physically deleted the index item that the indexscan is reporting to its caller, the btbulkdelete call will not have been able to complete. (If bulkdelete arrived at the original page before the indexscan did, then of course it'd already have deleted the item and there's no problem. If it arrives after, then it'll be blocked by not being able to get super-exclusive lock.) Hence vacuum will not have removed the heap tuple yet, even though the index item might be gone. We could improve concurrency if we extend the API so that btgettuple knows whether it is fetching for an MVCC-safe snapshot or not. When the scan is using an MVCC-safe snapshot it's OK to release pin immediately. However I'm not sure if this is worth the trouble --- it probably makes sense to hold onto the pin anyway, in case we're told to mark some of the tuples killed. The patch as given gets point 2 right, but I think it doesn't consistently do point 1, and in any case it's certainly not documenting the issue properly. BTW, I just realized another bug in the patch: btbulkdelete fails to guarantee that it visits every page in the index. It was OK for btvacuumcleanup to ignore pages added to the index after it starts, but btbulkdelete has to deal with such pages. One thing I'm kind of wondering is whether amgetmulti should still exist as a separate API. Seems like if amgettuple is doing the equivalent thing under-the-hood, then amgetmulti isn't saving us anything except a few trips through FunctionCallN. It's not at all clear that it's worth the API complexity of having two different fetch functions. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [BUGS] BUG #2401: spinlocks not available on amd64
Bruce Momjian wrote: Robert Lor wrote: Bruce, the change was only needed for the SPARC version only. The x86 file worked just fine before and needs to be reverted back. Yes, they are different. OK, fixed, thanks. Apologies if the message was not clear the first time! Yes, you were clear, but it was so illogical I figured it must be both platforms. :-0 Don't blame x86 for SPARCs sillyness, that just isn't fair ;) Joshua D. Drake I added a comment to clarify. This was the way the comments were in 8.1.X. -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plpython improvements
Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake [EMAIL PROTECTED]: I think that a less confusing way of saying it would be : Generators crash if python version used is 2.4.x and it is compiled with asserts. Currently only known linux distributions to distibute such python.so files are Fedora and possibly other RedHat distributions, while Gentoo, Ubuntu and Suse are OK. Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime and it would be a mistake to assume that we won't run into this. Sure, but it is known problem and there is patch for this bug. In the documentation we can clearly state that python2.4 with asserts enabled causes problem and describe how it can be tested and fixed (regardless of distribution used). As an example of absurdity of this problem: let's assume there is known distribution with buggy gethostbyname(). Fact, that we know about this, shouldn't stop us developing TCP/IP applications. Especially, if there is also patch for this bug :) It would be real shame to prevent using generator for SETOF functions because it is most natural match for plpgsql's return next -- Sven Suursoho ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Page at a time index scan
I wrote: BTW, I just realized another bug in the patch: btbulkdelete fails to guarantee that it visits every page in the index. It was OK for btvacuumcleanup to ignore pages added to the index after it starts, but btbulkdelete has to deal with such pages. Actually, as written this patch does not work. At all. btbulkdelete has to guarantee that it removes every index entry it's told to, and it cannot guarantee that in the presence of concurrent page splits. A split could move index items from a page that btbulkdelete hasn't visited to one it's already passed over. This is not possible with an index-order traversal (because splits only move items to the right) but it's definitely possible with a physical-order traversal. I was toying with the idea of remembering deletable pages (which btvacuumcleanup does anyway), which are the only ones that page splits could move items to, and then rescanning those after the completion of the primary pass. This has a couple of pretty unpleasant consequences though: * We have to remember *every* deletable page for correctness, compared to the current situation where btvacuumcleanup can bound the number of pages it tracks. This creates a situation where VACUUM may fail outright if it doesn't have gobs of memory. Since one of the main reasons for developing lazy VACUUM was to ensure we could vacuum arbitrarily large tables in bounded memory, I'm not happy with this. * The rescan could be far from cheap if there are many such pages. Also, I'm not sure when you can stop: it certainly seems possible that items could be moved down during the primary scan and then moved down again while btbulkdelete is reconsidering the deletable pages. Without locking out splits entirely, it's far from obvious that we can make it work. Thoughts? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Have configure complain about unknown options
On Fri, May 05, 2006 at 12:28:48PM -0400, Bruce Momjian wrote: I am worried if we change the default behavior that build systems will fail, but fail after our release when they go to package, and we will not get feedback until to late. I guess there are a number of ways to deal with this: 1. Provide an escape option they can add 2. Package systems can usually apply patches prior to compiling, they can always remove the offending line if they like. 3. Try and get feedback from them now rather than wait enable_maintainer_mode is a kind of special case. It's a flag that disables a number of braindead things done by automake and libtool and probably others. autoconf OTOH doesn't know about it. So, independant of the options above, I think it should be accepted without warning. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] [COMMITTERS] pgsql: Don't try to compile SSL CRL support if local
Tom Lane wrote:
Kris Jurka [EMAIL PROTECTED] writes:
On Thu, 4 May 2006, Tom Lane wrote:
Don't try to compile SSL CRL support if local SSL installation hasn't
got it. Per buildfarm failure on 'canary'.
It seems a little bit dangerous to just not check the CRL without so much
as a warning message.
[ shrug... ] Anyone who's running openssl 0.9.6, or whatever that is on
canary, isn't expecting CRL support anyway. And all I did is restore
the behavior we've had for lo these past many years.
The problem is that we now document that we support CRL, so either we
log if we skip it, or we have to document which versions of OpenSSL do
not support CRL (yuck).
The attached patch checks for the file, and either user it or generates
a log message that it was skipped.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/libpq/be-secure.c
===
RCS file: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v
retrieving revision 1.67
diff -c -c -r1.67 be-secure.c
*** src/backend/libpq/be-secure.c 4 May 2006 22:18:38 - 1.67
--- src/backend/libpq/be-secure.c 5 May 2006 18:26:37 -
***
*** 795,801
}
else
{
- #ifdef X509_V_FLAG_CRL_CHECK
/*
* Check the Certificate Revocation List (CRL) if file
exists.
*
http://searchsecurity.techtarget.com/sDefinition/0,,sid14_gci803160,00.html
--- 795,800
***
*** 804,813
if (cvstore)
{
if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE,
NULL) != 0)
! /* setting the flags to check against the complete
CRL chain */
! X509_STORE_set_flags(cvstore,
X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL);
else
{
/* Not fatal - we do not require CRL */
--- 803,820
if (cvstore)
{
+ /* Set the flags to check against the complete CRL chain */
if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE,
NULL) != 0)
! /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
! #ifdef X509_V_FLAG_CRL_CHECK
! X509_STORE_set_flags(cvstore,
X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL);
+ #else
+ ereport(LOG,
+ (errmsg(SSL Certificate Revocation
List (CRL) file \%s\ ignored,
+ ROOT_CRL_FILE),
+errdetail(Installed SSL library does
not support CRL.)));
+ #endif
else
{
/* Not fatal - we do not require CRL */
***
*** 817,823
errdetail(Will not check certificates
against CRL.)));
}
}
- #endif /* X509_V_FLAG_CRL_CHECK */
SSL_CTX_set_verify(SSL_context,
(SSL_VERIFY_PEER |
--- 824,829
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [PATCHES] [COMMITTERS] pgsql: Don't try to compile SSL CRL support if local
Bruce Momjian pgman@candle.pha.pa.us writes: The attached patch checks for the file, and either user it or generates a log message that it was skipped. I still can't get excited about this. Who will it help? The DBA who is silly enough to think his ancient SSL library supports CRL is probably also silly enough not to read the postmaster log carefully. It would make a whole lot more sense just to document that OpenSSL whatever doesn't support CRL. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [COMMITTERS] pgsql: Don't try to compile SSL CRL support
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: The attached patch checks for the file, and either user it or generates a log message that it was skipped. I still can't get excited about this. Who will it help? The DBA who is silly enough to think his ancient SSL library supports CRL is probably also silly enough not to read the postmaster log carefully. It would make a whole lot more sense just to document that OpenSSL whatever doesn't support CRL. Why hard-code something if we can dynamically report it, and NetBSD 2.0 isn't that old. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Page at a time index scan
On Fri, 5 May 2006, Tom Lane wrote: I wrote: BTW, I just realized another bug in the patch: btbulkdelete fails to guarantee that it visits every page in the index. It was OK for btvacuumcleanup to ignore pages added to the index after it starts, but btbulkdelete has to deal with such pages. Actually, as written this patch does not work. At all. btbulkdelete has to guarantee that it removes every index entry it's told to, and it cannot guarantee that in the presence of concurrent page splits. A split could move index items from a page that btbulkdelete hasn't visited to one it's already passed over. This is not possible with an index-order traversal (because splits only move items to the right) but it's definitely possible with a physical-order traversal. True. :( The first solution that occurs to me is to force page splits to choose the target page so that it's blkno the original page's blkno during vacuum. It would cause the index to become more fragmented more quickly, which is bad but perhaps tolerable. I was toying with the idea of remembering deletable pages (which btvacuumcleanup does anyway), which are the only ones that page splits could move items to, and then rescanning those after the completion of the primary pass. This has a couple of pretty unpleasant consequences though: * We have to remember *every* deletable page for correctness, compared to the current situation where btvacuumcleanup can bound the number of pages it tracks. This creates a situation where VACUUM may fail outright if it doesn't have gobs of memory. Since one of the main reasons for developing lazy VACUUM was to ensure we could vacuum arbitrarily large tables in bounded memory, I'm not happy with this. * The rescan could be far from cheap if there are many such pages. Yep, that's not good. - Heikki ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] plpython improvements
As an example of absurdity of this problem: let's assume there is known distribution with buggy gethostbyname(). Fact, that we know about this, shouldn't stop us developing TCP/IP applications. Especially, if there is also patch for this bug :) It would be real shame to prevent using generator for SETOF functions because it is most natural match for plpgsql's return next All I was saying was that we need to account for the use of 2.4 :). So as long as we document (as you suggest) we should be good. Sorry if I wasn't clear. Sincerely, Joshua D. Drake --Sven Suursoho -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [COMMITTERS] pgsql: Don't try to compile SSL CRL support
pgman wrote: Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: The attached patch checks for the file, and either user it or generates a log message that it was skipped. I still can't get excited about this. Who will it help? The DBA who is silly enough to think his ancient SSL library supports CRL is probably also silly enough not to read the postmaster log carefully. It would make a whole lot more sense just to document that OpenSSL whatever doesn't support CRL. Why hard-code something if we can dynamically report it, and NetBSD 2.0 isn't that old. Sorry, I meant NetBSD 1.6/canary: http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=canarybr=HEAD Anyway, if we are doing CRL, we should do it right. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Page at a time index scan
Heikki Linnakangas [EMAIL PROTECTED] writes: The first solution that occurs to me is to force page splits to choose the target page so that it's blkno the original page's blkno during vacuum. I thought about that too, but don't like it for three reasons: * it encourages index bloat, the more the longer the vacuum runs. Not good, especially if you've got aggressive vacuum cost delay settings. * there's a locking problem with respect to how you turn that behavior on. * there's a failure mode where the behavior doesn't get turned off if vacuum fails partway through. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Page at a time index scan
Heikki Linnakangas [EMAIL PROTECTED] writes: On Fri, 5 May 2006, Tom Lane wrote: BTW, I just realized another bug in the patch: btbulkdelete fails to guarantee that it visits every page in the index. The first solution that occurs to me is to force page splits to choose the target page so that it's blkno the original page's blkno during vacuum. It would cause the index to become more fragmented more quickly, which is bad but perhaps tolerable. I have a sketch of a solution that doesn't require any change in page allocation behavior. Can anyone see any holes in this: Assume that we have some way to recognize whether a page has been split since the current btbulkdelete scan started. (A split would mark both the original page and the new right-hand page as newly split.) When btbulkdelete arrives at a page, it need take no special action unless the page is newly split *and* its right-link points to a lower physical address. If that's true, then after vacuuming the page, follow its right-link and vacuum that page; repeat until arriving at a page that is either not newly split or is above the current location of the outer loop. Then return to the outer, sequential-scan loop. We should also have btbulkdelete clear the newly-split marker whenever it processes a page; this ensures that no page is processed more than once, which is probably worth the possible extra I/O needed to clear the marker. (The cycles to re-scan a page are maybe not that important, but if we do reprocess pages we'll end up with a bogusly high tuple count. OTOH I'm not sure we can guarantee an accurate tuple count anyway, so maybe it doesn't matter.) AFAICS, this works correctly even if the test for a newly-split page sometimes yields false positives; thinking a page is newly split when it is not might cost a little extra I/O but won't lead to wrong results. This reduces the requirements for the newly-split marking mechanism. So, how do we do that marking? Noting that we have 16 bits we could use in BTPageOpaqueData without making that struct any bigger (because of alignment considerations), I'm thinking about a 16-bit counter for each index that is incremented at the start of each btbulkdelete operation and copied into the opaque data whenever a page is split. If the value's different from the current counter, the page definitely hasn't been split during btbulkdelete. There's a 1-in-65536 chance of a false positive, if the last split occurred some exact multiple of 65536 vacuums ago, but that's probably low enough to be acceptable. (We could reduce the odds of false positives in various ways, eg by saying that zero isn't a valid counter value and having btbulkdelete reset a page's counter to zero anytime it has to write the page anyway.) This still has the same sort of locking issues I complained of in regards to Heikki's idea, namely how do you make sure that everyone is using the new counter value before you start scanning? That seems soluble however. We'd just need to be willing to take one more lock during every page split operation, which does not seem an intolerable amount of overhead. Then we do something like take a sharelock while fetching the counter value during split (and hold the lock until the split's complete), and take a momentary exclusive lock while advancing the counter during btbulkdelete startup. Thoughts, better ideas? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] be-secure.c patch
Bruce Momjian wrote:
I am now wondering if fe-secure.c, the front-end code, should also check
for root.crl. The attached patch implents it.
Updated patch attached and applied. It adds CRL checking to libpq. It
returns an error if the CRL file exists, but the library can't process
it, just like the backend.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.79
diff -c -c -r1.79 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 27 Apr 2006 14:02:36 - 1.79
--- src/interfaces/libpq/fe-secure.c 6 May 2006 02:21:50 -
***
*** 125,135
--- 125,137
#define USER_CERT_FILE .postgresql/postgresql.crt
#define USER_KEY_FILE .postgresql/postgresql.key
#define ROOT_CERT_FILE .postgresql/root.crt
+ #define ROOT_CRL_FILE .postgresql/root.crl
#else
/* On Windows, the home directory is already PostgreSQL-specific */
#define USER_CERT_FILE postgresql.crt
#define USER_KEY_FILE postgresql.key
#define ROOT_CERT_FILE root.crt
+ #define ROOT_CRL_FILE root.crl
#endif
#ifdef NOT_USED
***
*** 784,789
--- 786,793
snprintf(fnbuf, sizeof(fnbuf), %s/%s, homedir, ROOT_CERT_FILE);
if (stat(fnbuf, buf) == 0)
{
+ X509_STORE *cvstore;
+
if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL))
{
char *err = SSLerrmessage();
***
*** 795,800
--- 799,826
return -1;
}
+ if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
+ {
+ /* setting the flags to check against the complete CRL chain */
+ if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE, NULL) != 0)
+ /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
+ #ifdef X509_V_FLAG_CRL_CHECK
+ X509_STORE_set_flags(cvstore,
+ X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL);
+ /* if not found, silently ignore; we do not require CRL */
+ #else
+ {
+ char *err = SSLerrmessage();
+
+ printfPQExpBuffer(conn-errorMessage,
+ libpq_gettext(Installed SSL library does not support CRL certificates, file \%s\\n),
+ fnbuf);
+ SSLerrfree(err);
+ return -1;
+ }
+ #endif
+ }
+
SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
}
}
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] cast bytea to/from bit strings
I am not sure this is of general enough usefulness to be in the backend. Can you add it as a pgfoundry project? --- Fabien COELHO wrote: Dear PostgreSQL developers, Please find attached a small patch to convert bytea to bit strings and vice versa. I used it in order to be able xor md5 results so as to checksum bundles of tuples together. The MD5 result is an hexa text convertible to bytea with decode, but then I was stuck... ISTM that having these types explicitely castable may be useful to others, hence this small contribution. The cast allows to work on a bytea at the bit level and to perform bitwise operations. ./src/backend/utils/adt/varbit.c - add two conversion functions ./src/include/catalog/pg_proc.h - declare the above functions in the catalog ./src/include/catalog/pg_cast.h - declare the 4 explicit casts ./src/test/regress/sql/bit.sql - test all those new casts ./src/test/regress/expected/bit.out - new regression results ./src/test/regress/expected/opr_sanity.out - pg figures out that bit and varbit are binary compatible, which is the case (well, at least I assumed it). -- Fabien. Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: current version: [PATCHES] Patch - Have psql show current values
As Tom asked, why not use seqname.last_value? Looking at your output: + if (showSeq !showTables) + appendPQExpBuffer(buf, + ,\n curval(c.oid) as \%s\ + ,\n CASE curvalcheck(c.oid) WHEN '1' THEN '%s' WHEN '0' THEN '%s' END as \%s\, + _(value),_( ***),_(),_(Start from)); What do you want to show that seqname.last_value doesn't give you? Curval? I don't see that as useful for a psql display. Now that I look at the TODO item: o Have psql show current values for a sequence It is confusing. It means the current values for the sequence, not curval for the sequence. I don't even understand what your function is returning. Just stick to last_value, though I think seqname.is_called might be what you were looking for. What fields do we want to show? Maybe the TODO item is not needed. Is this all we want to show? test= \x Expanded display is on. test= select * from xx; -[ RECORD 1 ]-+ sequence_name | xx last_value| 1 increment_by | 1 max_value | 9223372036854775807 min_value | 1 cache_value | 1 log_cnt | 32 is_cycled | f is_called | t --- Dhanaraj M wrote: Tom Lane wrote: Dhanaraj M [EMAIL PROTECTED] writes: sorry for sending the old version in the previous mail . Here I attach the recent version of the patch file. -- Surely this problem does not require adding any server-side code. Something like select last_value from seq would be more appropriate; and it'd have some hope of working with back-version servers. Also, please use something more helpful than *** as the column header. Your urge to add a footnote to explain it shows that you didn't try hard enough to devise a good header to begin with. [ btw, both fmgroids.h and fmgrtab.c are generated files. Patching them is unnecessary and inappropriate. ] -- The existing functions like lastval, currval dont provide the current sequence value always. They work only if the sequence is already cached (nextval is called atleast once for that sequence). Changing the internals of lastval/currval will give the solution. However, I feel that the functionality change may affect the customers who use the current version. Hence, I am sure that it requires the server side change. There are two options here 1. Modifying the exisitng functions (or) 2. adding new functions Thanks for your review Dhanaraj ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Dhanaraj M wrote: Tom Lane wrote: Dhanaraj M [EMAIL PROTECTED] writes: sorry for sending the old version in the previous mail . Here I attach the recent version of the patch file. -- Surely this problem does not require adding any server-side code. Something like select last_value from seq would be more appropriate; and it'd have some hope of working with back-version servers. Also, please use something more helpful than *** as the column header. Your urge to add a footnote to explain it shows that you didn't try hard enough to devise a good header to begin with. [ btw, both fmgroids.h and fmgrtab.c are generated files. Patching them is unnecessary and inappropriate. ] -- The existing functions like lastval, currval dont provide the current sequence value always. They work only if the sequence is already cached (nextval is called atleast once for that sequence). Changing the internals of lastval/currval will give the solution. However, I feel that the functionality change may affect the customers who use the current version. Hence, I am sure that it requires the server side change. There are two options here 1. Modifying the exisitng functions (or) 2. adding new functions Thanks for your review Dhanaraj ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to
Re: current version: [PATCHES] Patch - Have psql show current values
Bruce Momjian wrote: What fields do we want to show? Maybe the TODO item is not needed. Is this all we want to show? IRC what we want is something like this. regression=# \d abc Sequence public.abc Column| Type --+- sequence_name | abc last_value| 1 increment_by | 1 max_value | 9223372036854775807 min_value | 1 cache_value | 1 log_cnt | 1 is_cycled | f is_called | f Because \d abc doesn't show us any important information. regression=# \d abc Sequence public.abc Column | Type ---+- sequence_name | name last_value| bigint increment_by | bigint max_value | bigint min_value | bigint cache_value | bigint log_cnt | bigint is_cycled | boolean is_called | boolean Last year, I made a patch for this but it was so ugly that I didn't send to -patches. Maybe Bruce's solution (\x select * from seq) could be hardcoded in describe.c. -- Euler Taveira de Oliveira http://www.timbira.com/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: current version: [PATCHES] Patch - Have psql show current values
I am thinking we just add another column to the \d display for sequences showing the current value. --- Euler Taveira de Oliveira wrote: Bruce Momjian wrote: What fields do we want to show? Maybe the TODO item is not needed. Is this all we want to show? IRC what we want is something like this. regression=# \d abc Sequence public.abc Column| Type --+- sequence_name | abc last_value| 1 increment_by | 1 max_value | 9223372036854775807 min_value | 1 cache_value | 1 log_cnt | 1 is_cycled | f is_called | f Because \d abc doesn't show us any important information. regression=# \d abc Sequence public.abc Column | Type ---+- sequence_name | name last_value| bigint increment_by | bigint max_value | bigint min_value | bigint cache_value | bigint log_cnt | bigint is_cycled | boolean is_called | boolean Last year, I made a patch for this but it was so ugly that I didn't send to -patches. Maybe Bruce's solution (\x select * from seq) could be hardcoded in describe.c. -- Euler Taveira de Oliveira http://www.timbira.com/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] fori stmt with by keyword was:(Re: [HACKERS] for statement, adding a STEP clause?)
On 4/30/06, Jaime Casanova [EMAIL PROTECTED] wrote:
On 4/29/06, Andrew Dunstan [EMAIL PROTECTED] wrote:
Tom Lane wrote:
Jaime Casanova [EMAIL PROTECTED] writes:
there is a chance to add a STEP clause to the FOR statement in plpgsql?
This is not free: it'd require making STEP a reserved word (at least
within plpgsql) which is contrary to spec. I think you need to make
a pretty good case why the value of the feature outweighs breaking
applications that have perfectly-legally used step as an identifier.
This isn't available in PL/SQL, is it? That doesn't mean we shouldn't do it,
of course, but it might lessen any perceived imperative.
Maybe using BY instad of STEP as the keyword would make it easier, since its
occurrence in SQL makes it less likely to be used as a variable.
cheers
andrew
Hi,
i make a little patch using BY instead of STEP per Tom's complaint and
Andrew's suggestion.
the patch is ready, at least it seems to me... also i have added some
lines to the docs...
let me know what your decision is about this...
--
regards,
Jaime Casanova
Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning.
Richard Cook
diff -rcEib pgsql-8.2dev/doc/src/sgml/plpgsql.sgml pgsql-8.2fori/doc/src/sgml/plpgsql.sgml
*** pgsql-8.2dev/doc/src/sgml/plpgsql.sgml 2006-05-01 08:49:20.0 -0500
--- pgsql-8.2fori/doc/src/sgml/plpgsql.sgml 2006-05-05 17:31:36.0 -0500
***
*** 1960,1966
synopsis
optional lt;lt;replaceablelabel/replaceablegt;gt; /optional
! FOR replaceablename/replaceable IN optional REVERSE /optional replaceableexpression/replaceable .. replaceableexpression/replaceable LOOP
replaceablestatements/replaceable
END LOOP optional replaceablelabel/replaceable /optional;
/synopsis
--- 1960,1966
synopsis
optional lt;lt;replaceablelabel/replaceablegt;gt; /optional
! FOR replaceablename/replaceable IN optional REVERSE /optional replaceableexpression/replaceable .. replaceableexpression/replaceable optional BY replaceableexpression/replaceable /optional LOOP
replaceablestatements/replaceable
END LOOP optional replaceablelabel/replaceable /optional;
/synopsis
***
*** 1973,1980
definition of the variable name is ignored within the loop).
The two expressions giving
the lower and upper bound of the range are evaluated once when entering
! the loop. The iteration step is normally 1, but is -1 when literalREVERSE/ is
! specified.
/para
para
--- 1973,1982
definition of the variable name is ignored within the loop).
The two expressions giving
the lower and upper bound of the range are evaluated once when entering
! the loop. If the literalBY/ clause isn't specified the iteration
! step is 1 otherwise it's the value specified in the literalBY/
! clause. If literalREVERSE/ is specified then the step value is
! considered negative.
/para
para
***
*** 1988,1993
--- 1990,2000
FOR i IN REVERSE 10..1 LOOP
-- some computations here
END LOOP;
+
+ FOR i IN REVERSE 10..1 BY 2 LOOP
+ -- some computations here
+ RAISE NOTICE 'i is %', i;
+ END LOOP;
/programlisting
/para
diff -rcEib pgsql-8.2dev/src/pl/plpgsql/src/gram.y pgsql-8.2fori/src/pl/plpgsql/src/gram.y
*** pgsql-8.2dev/src/pl/plpgsql/src/gram.y 2006-05-01 08:49:39.0 -0500
--- pgsql-8.2fori/src/pl/plpgsql/src/gram.y 2006-05-05 17:42:08.0 -0500
***
*** 143,148
--- 143,149
%token K_ALIAS
%token K_ASSIGN
%token K_BEGIN
+ %token K_BY
%token K_CLOSE
%token K_CONSTANT
%token K_CONTINUE
***
*** 930,935
--- 931,937
{
/* Saw .., so it must be an integer loop */
PLpgSQL_expr *expr2;
+ PLpgSQL_expr *expr_by;
PLpgSQL_var *fvar;
PLpgSQL_stmt_fori *new;
char*varname;
***
*** 937,943
/* First expression is well-formed */
check_sql_expr(expr1-query);
! expr2 = plpgsql_read_expression(K_LOOP, LOOP);
/* should have had a single variable name */
plpgsql_error_lineno = $2.lineno;
--- 939,968
/* First expression is well-formed */
check_sql_expr(expr1-query);
!
! expr2 = read_sql_construct(K_BY,
! K_LOOP,
! LOOP,
! SELECT ,
! true,
! false,
! tok);
!
! if (tok == K_BY)
! expr_by = plpgsql_read_expression(K_LOOP, LOOP);
! else
! {
! /*
! * If there is no BY clause we will assume 1
!
