Re: [PATCHES] still alive?
Peter Eisentraut wrote: Simon Riggs wrote: On Thu, 2008-09-11 at 15:39 +0300, Peter Eisentraut wrote: Bruce Momjian wrote: Abhijit Menon-Sen wrote: I thought -patches was supposed to die. What happened? I was wondering the same thing. Peter? Hmm, let's try this: Anyone who thinks the patches list should remain as separate from hackers, shout now (with rationale)! Kill it now, long enough before the next patchfest for it to stick. I think what we need now, for patches, ports, and the others, is someone to actually kill it. All the talk has been talked, everything has been decided, now someone with the right permission bits just turn it off. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] still alive?
Marc, care to do the honors? --- Peter Eisentraut wrote: Simon Riggs wrote: On Thu, 2008-09-11 at 15:39 +0300, Peter Eisentraut wrote: Bruce Momjian wrote: Abhijit Menon-Sen wrote: I thought -patches was supposed to die. What happened? I was wondering the same thing. Peter? Hmm, let's try this: Anyone who thinks the patches list should remain as separate from hackers, shout now (with rationale)! Kill it now, long enough before the next patchfest for it to stick. I think what we need now, for patches, ports, and the others, is someone to actually kill it. All the talk has been talked, everything has been decided, now someone with the right permission bits just turn it off. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] hash index improving v3
Can we consider this hash thread closed/completed? --- Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Thinks: Why not just sort all of the time and skip the debate entirely? The sort is demonstrably a loser for smaller indexes. Admittedly, if the index is small then the sort can't cost all that much, but if the (correct) threshold is some large fraction of shared_buffers then it could still take awhile on installations with lots-o-buffers. The other side of that coin is that it's not clear this is really worth arguing about, much less exposing a separate parameter for. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] still alive?
Abhijit Menon-Sen wrote: I thought -patches was supposed to die. What happened? I was wondering the same thing. Peter? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Add missing descriptions for aggregates, functions and conversions
Patch applied; always nice to beef up our descriptions, thanks. --- Bernd Helmle wrote: Please find attached a patch that adds some missing descriptions for aggregates, functions and conversions. This will add COMMENTs to the conversion sql script as well. Most of the descriptions are taken from the documentation (especially for the statistic functions). I didn't bother with some internal functions like text_pattern_lt, if we agree they should be described as well i can add them, too. -- Thanks Bernd [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Re: [HACKERS] Solaris ident authentication using unix domain sockets
Garick Hamlin wrote: On Thu, Jul 03, 2008 at 02:01:22PM -0400, Tom Lane wrote: Garick Hamlin [EMAIL PROTECTED] writes: I have a patch that I have been using to support postgresql's notion of ident authentication when using unix domain sockets on Solaris. This patch basically just adds support for using getupeercred() on Solaris so unix sockets and ident auth works just like it does on Linux and elsewhere. Cool. + #if defined(HAVE_GETPEERUCRED) + #include ucred.h + #endif But this is not cool. There might be systems out there that have getpeerucred() but not ucred.h, and this coding would cause a compile failure (even if they actually wouldn't be trying to use getpeerucred() because they have some other way to do it). You need an explicit configure probe for the header file too, I think. Ok, I can fix that. Garick, have you made any progress on an updated patch? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pgbench minor fixes
Committed, commit fest page updated. --- Simon Riggs wrote: Minor patch on pgbench 1. -i option should run vacuum analyze only on pgbench tables, not *all* tables in database. 2. pre-run cleanup step was DELETE FROM HISTORY then VACUUM HISTORY. This is just a slow version of TRUNCATE HISTORY. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Userset logging
Simon Riggs wrote: On Mon, 2008-07-07 at 16:13 +0100, Simon Riggs wrote: I notice log_temp_files is a PGC_USERSET parameter, which is out of step with our current thinking on who is allowed to initiate logging. Is there a rationale for this? Or should we set this to PGC_SUSET like all the other logging functions? Patch enclosed. Patch applied, docs updated. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] review: table function support
Added to September commit fest. --- Pavel Stehule wrote: 2008/7/10 Marko Kreen [EMAIL PROTECTED]: On 7/10/08, Pavel Stehule [EMAIL PROTECTED] wrote: I am sending actualized patch Regards Pavel Stehule 2008/7/9 Pavel Stehule [EMAIL PROTECTED]: 2008/7/9 Marko Kreen [EMAIL PROTECTED]: Generally, the patch looks fine. There are few issues still: - plpgsql: the result columns _do_ create local variables. AIUI, they should not? it was my mistake - it doesn't do local variables - fixed - pg_dump: is the psql_assert() introduction necessary, considering it is used only in one place? removed - argmode variables is checked before - There should be regression test for plpgsql too, that test if the behaviour is correct. addeded - The documentation should mention behaviour difference from OUT parameters. I will do it. Wishlist (probably out of scope for this patch): this is in my wishlist too, but postgresql doesn't support types like result of functions. - plpgsql: a way to create record variable for result row. Something like: CREATE FUNCTION foo(..) RETURNS TABLE (..) AS $$ DECLARE retval foo%ROWTYPE; Currently the OUT parameters are quite painful to use due to bad name resolving logic. Such feature would be perfect replacement. -- marko I'll send patch early, thank you much Ok, last items: - Attached is a patch that fixes couple C comments. - I think plpgsql 38.1.2 chapter of Supported Argument and Result Data Types should also have a mention of TABLE functions. Then I'm content with the patch. applyed Regards and thank you very much Pavel -- marko [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] doc patch - archive/restore_command on windows
Patch applied to CVS HEAD and 8.3.X. Not sure how this was overlooked in the past. Thanks. --- ITAGAKI Takahiro wrote: I found the examples of documentation about archive_command and restore_command for Windows are incorrect or improper. - copy doesn't accept / (slash) as a path separator. We should use \\ (double backslash) for the purpose. - Windows path is typically starts with a drive letter (C:\\). - We'd better to quote a whole path, not only the last filename with double-quotes. It can work, but is not a windows manner. Index: doc/src/sgml/backup.sgml === --- doc/src/sgml/backup.sgml (HEAD) +++ doc/src/sgml/backup.sgml (working copy) @@ -1122,7 +1122,7 @@ when so asked. Examples: programlisting restore_command = 'cp /mnt/server/archivedir/%f %p' -restore_command = 'copy /mnt/server/archivedir/%f %p' # Windows +restore_command = 'copy C:\\server\\archivedir/%f %p' # Windows /programlisting /para /listitem Index: doc/src/sgml/config.sgml === --- doc/src/sgml/config.sgml (HEAD) +++ doc/src/sgml/config.sgml (working copy) @@ -1698,7 +1698,7 @@ and only if it succeeds. Examples: programlisting archive_command = 'cp %p /mnt/server/archivedir/%f' -archive_command = 'copy %p /mnt/server/archivedir/%f' # Windows +archive_command = 'copy %p C:\\server\\archivedir\\%f' # Windows /programlisting /para /listitem Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] doc patch - archive/restore_command on windows
This is the patch version I applied. Thanks again. --- ITAGAKI Takahiro wrote: Sorry, this is a correct patch. Index: doc/src/sgml/backup.sgml === --- doc/src/sgml/backup.sgml (HEAD) +++ doc/src/sgml/backup.sgml (working copy) @@ -1122,7 +1122,7 @@ when so asked. Examples: programlisting restore_command = 'cp /mnt/server/archivedir/%f %p' -restore_command = 'copy /mnt/server/archivedir/%f %p' # Windows +restore_command = 'copy C:\\server\\archivedir\\%f %p' # Windows /programlisting /para /listitem Index: doc/src/sgml/config.sgml === --- doc/src/sgml/config.sgml (HEAD) +++ doc/src/sgml/config.sgml (working copy) @@ -1698,7 +1698,7 @@ and only if it succeeds. Examples: programlisting archive_command = 'cp %p /mnt/server/archivedir/%f' -archive_command = 'copy %p /mnt/server/archivedir/%f' # Windows +archive_command = 'copy %p C:\\server\\archivedir\\%f' # Windows /programlisting /para /listitem Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] array_fill function
Patch applied, with minor adjustments in error message wording, with documntation added; committed patch attached. --- Pavel Stehule wrote: Hello Proposal: http://archives.postgresql.org/pgsql-hackers/2008-06/msg00057.php I changed name to array_fill and order of arguments. postgres=# SELECT array_fill(0, ARRAY[2,3]); array_fill --- {{0,0,0},{0,0,0}} (1 row) postgres=# SELECT array_fill(0, ARRAY[2,3], ARRAY[1,2]); array_fill -- [1:2][2:4]={{0,0,0},{0,0,0}} (1 row) postgres=# SELECT array_fill(0, ARRAY[4], ARRAY[2]); array_fill - [2:5]={0,0,0,0} (1 row) postgres=# SELECT array_fill(NULL::int, ARRAY[4]); array_fill --- {NULL,NULL,NULL,NULL} (1 row) Regards Pavel Stehule [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/func.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.440 diff -c -c -r1.440 func.sgml *** doc/src/sgml/func.sgml 15 Jul 2008 18:24:59 - 1.440 --- doc/src/sgml/func.sgml 16 Jul 2008 00:42:25 - *** *** 9374,9379 --- 9374,9392 row entry literal + functionarray_fill/function(typeanyelement/type, typeanyarray/type, + optional, typeanyarray/type/optional) + /literal + /entry + entrytypeanyarray/type/entry + entryreturns an array initialized with supplied value, + dimensions, and lower bounds/entry + entryliteralarray_fill(7, ARRAY[3], ARRAY[2])/literal/entry + entryliteral[2:4]={7,7,7}/literal/entry +/row +row + entry + literal functionarray_lower/function(typeanyarray/type, typeint/type) /literal /entry Index: src/backend/utils/adt/arrayfuncs.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/arrayfuncs.c,v retrieving revision 1.145 diff -c -c -r1.145 arrayfuncs.c *** src/backend/utils/adt/arrayfuncs.c 12 May 2008 00:00:51 - 1.145 --- src/backend/utils/adt/arrayfuncs.c 16 Jul 2008 00:42:26 - *** *** 95,100 --- 95,105 int *st, int *endp, int typlen, bool typbyval, char typalign); static int array_cmp(FunctionCallInfo fcinfo); + static ArrayType *create_array_envelope(int ndims, int *dimv, int *lbv, int nbytes, + Oid elmtype, int dataoffset); + static ArrayType *array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, + Oid elmtype, bool isnull, + FunctionCallInfo fcinfo); /* *** *** 4314,4316 --- 4319,4590 /* just call the other one -- it can handle both cases */ return generate_subscripts(fcinfo); } + + /* + * array_fill_with_lower_bounds + * Create and fill array with defined lower bounds. + */ + Datum + array_fill_with_lower_bounds(PG_FUNCTION_ARGS) + { + ArrayType *dims; + ArrayType *lbs; + ArrayType *result; + Oid elmtype; + Datum value; + bool isnull; + + if (PG_ARGISNULL(1) || PG_ARGISNULL(2)) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg(dimension array or low bound array cannot be NULL))); + + dims = PG_GETARG_ARRAYTYPE_P(1); + lbs = PG_GETARG_ARRAYTYPE_P(2); + + if (!PG_ARGISNULL(0)) + { + value = PG_GETARG_DATUM(0); + isnull = false; + } + else + { + value = 0; + isnull = true; + } + + elmtype = get_fn_expr_argtype(fcinfo-flinfo, 0); + if (!OidIsValid(elmtype)) + elog(ERROR, could not determine data type of input); + + result = array_fill_internal(dims, lbs, value, elmtype, isnull, fcinfo); + PG_RETURN_ARRAYTYPE_P(result); + } + + /* + * array_fill + * Create and fill array with default lower bounds. + */ + Datum + array_fill(PG_FUNCTION_ARGS) + { + ArrayType *dims; + ArrayType *result; + Oid elmtype; + Datum value; + bool isnull; + + if (PG_ARGISNULL(1)) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg(dimension array or low bound array cannot be NULL))); + + dims = PG_GETARG_ARRAYTYPE_P(1); + + if (!PG_ARGISNULL(0)) + { + value = PG_GETARG_DATUM(0); + isnull = false; + } + else + { + value = 0; + isnull = true; + } + + elmtype = get_fn_expr_argtype(fcinfo-flinfo, 0); + if (!OidIsValid(elmtype)) + elog(ERROR, could not determine data type of input); + + result
Re: [PATCHES] \d+ should display the storage options for columns
Alvaro Herrera wrote: Bruce Momjian wrote: Update patch applied; I also adjusted some translation function calls. The new output of psql \d+ is: test= \d+ test Table public.test Column | Type | Modifiers | Storage | Description +-+---+-+- x | integer | | plain | Has OIDs: no Thanks for fixing the header problem. This patch introduces other strings, plain, main and so on that are not gettext_noop()ed. Should we mark those for translation too? I admit I am not sure, if only because the untranslated strings are what gets passed to ALTER TABLE ... SET STORAGE. But if that's the decision, then it oughtta be commented in the code ... I thought about that, but those strings are literal used in our syntax, so translating them seemed wrong. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] odd output in restore mode
Simon Riggs wrote: * recommendation to use GnuWin32 cp on Windows * provide holdtime delay, default 0 (on all platforms) * default stays same on Windows=copy to ensure people upgrading don't get stung This seems pretty kludgey to me. I wouldn't want to install GnuWin32 utilities on a production system just for the cp command, and I don't know how I would tune holdtime properly for using copy. And it seems risky to have defaults that are known to not work reliably. How about implementing a replacement function for cp ourselves? It seems pretty trivial to do. We could use that on Unixes as well, which would keep the differences between Win32 and other platforms smaller, and thus ensure the codepath gets more testing. (Sorry for jumping into the discussion so late, I didn't follow this thread earlier, and just read it now in the archives while looking at the patch.) If you've heard complaints about any of this from users, I haven't. AFAIK we're doing this because it *might* cause a problem. Bear in mind that link is the preferred performance option, not copy. So AFAICS we're tuning a secondary option on one specific port, without it being a raised issue and in an area of code that will be superceded in the next release. So further embellishments would be a long way down my own priority list, putting it politely. Yet I have no objections to the suggestion overall; we have done that already for alter tablespace. OK, based on these observations I think we need to learn more about the issues before making any changes to our code. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] TRUNCATE TABLE with IDENTITY
Alvaro Herrera wrote: Tom Lane wrote: 2. I had first dismissed Neil's idea of transactional sequence updates as impossible, but on second look it could be done. Suppose RESTART IDENTITY does this for each sequence; * obtain AccessExclusiveLock; * assign a new relfilenode; * insert a sequence row with all parameters copied except last_value copies start_value; * hold AccessExclusiveLock till commit. Hmm, this kills the idea of moving sequence data to a single non-transactional catalog :-( So what I think we should do is leave the patch there, revise the warning per Neil's complaint, and add a TODO item to reimplement RESTART IDENTITY transactionally. I think the TODO item did not make it, but the docs do seem updated. Done: * Fix TRUNCATE ... RESTART IDENTITY so its affect on sequences is rolled back on transaction abort -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] LOCK_DEBUG documentation
Greg Sabino Mullane wrote: [ There is text before PGP section. ] -BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Tom Lane replied: Documentation patch by Kevin L. McBride explaining LOCK_DEBUG options in detail. Should this stuff really go into the SGML documentation, when these options will certainly never be enabled anywhere except in developers' private builds? A few lines of comments in pg_config_manual.h seems a more appropriate solution. Call me a traditionalist, but I like all the documentation in one place, even if some of it it seldom used. For the record, these options have been enabled by myself and others in production systems for debugging purposes, and the lack of detail in that section while doing so led to the patch. The docs also has the advantage of being more available, searchable, and found via the web. Agreed, applied. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Cliff Nieuwenhuis wrote: On Fri, 9 May 2008 08:38:01 -0400 Alvaro Herrera [EMAIL PROTECTED] wrote: Bruce Momjian escribi?: Guillaume Smet wrote: On Thu, May 8, 2008 at 9:11 PM, Bruce Momjian [EMAIL PROTECTED] wrote: As I mentioned it before, is there any chance for this fix to be backported to 8.3 branch? IMHO it's a usability regression. No, we don't change behaviors in back branches unless we get lots of complaints, and we haven't in this case. complaints++ I suppose this a Me Too post, but Bruce Momjian invites it. You folks take this to a level way beyond me, but I can tell you that the idea of using spaces instead of the terminal hard tabs would solve my problem -- I'd prefer _any_ choice of whitespace over seeing \x09 on the terminal. FYI, this was corrected in 8.3.3. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Explain XML patch
Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: You probably need to say a whole lot more about this patch. I've updated the wiki with things I've learned while submitting patches. http://wiki.postgresql.org/wiki/Submitting_a_Patch Anybody mind if I update that to put more emphasis on the need for documentation? As in documentation patches are *required* if your patch adds or changes user-visible behavior? Sure, but I do documentation updates for non-English speakers occasionally. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. OK, I started looking at what it would take to backpatch this and found another bug I have fixed in CVS HEAD. What back branchs (8.0-8.3.X) are doing is pretty odd. On non-Win32 systems, it is looking for the null byte, then putting a null byte before it, and passing a NULL back as the options and binary location. The test: if (postgres_path != NULL) postgres_path = optline; is backwards, which means that if in 8.3.X you start the server with any arguments, like: /usr/var/local/postgres/bin/postgres -i -o -d5 and you use pg_ctl to specify the binary location: pg_ctl -p /u/pg/bin/postmaster restart the server actually fails to restart because it chops off the last byte (a bug) and the test above is wrong (another bug), and it thinks the binary name is the full string, in quotes: /usr/var/local/postgres/bin/postgres -i -o -d and you get this error from pg_ctl: sh: /usr/var/local/postgres/bin/postgres -i -o -d: not found This is more than just ignoring the documentation, it is a failure. I am attaching a minimal patch that will fix the bug in back branches. Keep in mind that a patched pg_ctl will not be able to restart a backend that was not patched. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/postmaster/postmaster.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.551 diff -c -c -r1.551 postmaster.c *** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551 --- src/backend/postmaster/postmaster.c 26 Jun 2008 18:53:42 - *** *** 4163,4169 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE); fputs(\n, fp); if (fclose(fp)) --- 4163,4169 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, \%s\, argv[i]); fputs(\n, fp); if (fclose(fp)) Index: src/bin/pg_ctl/pg_ctl.c === RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.92.2.3 diff -c -c -r1.92.2.3 pg_ctl.c *** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3 --- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 18:53:43 - *** *** 613,627 { char *arg1; ! arg1 = strchr(optline, *SYSTEMQUOTE); ! if (arg1 == NULL || arg1 == optline) ! post_opts = ; ! else { ! *(arg1 - 1) = '\0'; /* this should be a space */ ! post_opts = arg1; } ! if (postgres_path != NULL) postgres_path = optline; } else --- 613,628 { char *arg1; ! /* !* Are we at the first option, as defined by space and !* double-quote? !*/ ! if ((arg1 = strstr(optline, \)) != NULL) { ! *arg1 = '\0'; /* terminate so we get only program name */ ! post_opts = arg1 + 1; /* point past whitespace */ } ! if (postgres_path == NULL) postgres_path = optline; } else -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: I am attaching a minimal patch that will fix the bug in back branches. Keep in mind that a patched pg_ctl will not be able to restart a backend that was not patched. I think this patch will work for unpatched backends as well. I am still uncertain if it should be backpatched. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/postmaster/postmaster.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.551 diff -c -c -r1.551 postmaster.c *** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551 --- src/backend/postmaster/postmaster.c 26 Jun 2008 19:11:37 - *** *** 4163,4169 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE); fputs(\n, fp); if (fclose(fp)) --- 4163,4169 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, \%s\, argv[i]); fputs(\n, fp); if (fclose(fp)) Index: src/bin/pg_ctl/pg_ctl.c === RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.92.2.3 diff -c -c -r1.92.2.3 pg_ctl.c *** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3 --- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 19:11:37 - *** *** 613,627 { char *arg1; ! arg1 = strchr(optline, *SYSTEMQUOTE); ! if (arg1 == NULL || arg1 == optline) ! post_opts = ; ! else { ! *(arg1 - 1) = '\0'; /* this should be a space */ ! post_opts = arg1; } ! if (postgres_path != NULL) postgres_path = optline; } else --- 613,629 { char *arg1; ! /* ! * Are we at the first option, as defined by space and ! * double-quote? ! */ ! if ((arg1 = strstr(optline, \)) != NULL || ! (arg1 = strstr(optline, -)) != NULL) { ! *arg1 = '\0'; /* terminate so we get only program name */ ! post_opts = arg1 + 1; /* point past whitespace */ } ! if (postgres_path == NULL) postgres_path = optline; } else -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. I think the policy is we fix the bugs in supported releases. If you start making exceptions, it becomes needlessly complex. I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [Fwd: Re: [HACKERS] pg_dump additional options for performance]
Added to July patch queue. Thanks. --- Simon Riggs wrote: Re-sending post as discussed with Bruce... On Sun, 2008-03-23 at 12:45 -0300, Alvaro Herrera wrote: Bruce Momjian wrote: Added to TODO: o Allow pre/data/post files when dumping a single object, for performance reasons http://archives.postgresql.org/pgsql-hackers/2008-02/msg00205.php When dumping a single object?? Do you mean database? It would be for whatever set of objects are specified through the use of databases, table include/exclude switches. I've written a patch that implements these new switches on the commands as shown pg_dump --schema-pre-load pg_dump --schema-post-load pg_restore --schema-pre-load pg_restore --schema-post-load I have not implemented --schema-pre-file=xxx style because they don't make any sense when using pg_restore in direct database connection mode. On reflection I don't see any particular need to produce multiple files as output, which just complicates an already horrendous user interface. This is a minimal set of changes and includes nothing at all about directories, parallelisation in the code etc.. This has the following use cases amongst others... * dump everything to a file, then use pg_restore first --schema-pre-load and then --data-only directly into the database, then pg_restore --schema-post-load to a file so we can edit that file into multiple pieces to allow index creation in parallel * dump of database into multiple files by manually specifying which tables go where, then reload in parallel using multiple psql sessions The patch tests OK after some testing, though without a test suite that probably isn't more than a few percentage points of all the possible code paths. There are no docs for it, as yet. --- Further thinking on this Some further refinement might replace --data-only and --schema-only with --want-schema-pre --want-data --want-schema-post --want-schema (same as --want-schema-pre --want-schema-post) These could be used together e.g. --want-schema-pre --want-data whereas the existing --data-only type switches cannot. Which would be a straightforward and useful change to the enclosed patch. That way of doing things is hierarchically extensible to include further subdivisions of the set of SQL commands produced, e.g. divide --want-post-schema into objects required to support various inter-table dependencies and those that don't such as additional indexes. I don't personally think we need that though. Comments? -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Removal of the patches email list
We have come to agreement that there is no longer a need for a separate 'patches' email list --- the size of patches isn't a significant issue anymore, and tracking threads between the patches and hackers lists is confusing. I propose we close the patches list and tell everyone to start using only the hackers list. This will require email server changes and web site updates, and some people who are only subscribed to patches have to figure out if they want to subscribe to hackers. I have CC'ed hackers, patches, and www because this does affect all those lists. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Alvaro Herrera wrote: I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. Actually, Bruce, this *is* a minor bug; if it were major we'd have heard about it from the field. My take on it is that pg_ctl restart must be practically unused. Given that we now know it's completely broken, the only way that patching it could make the situation worse would be if the patch affected some other code path that people actually do use. As long as you're sure it doesn't do that, I see no downside to an attempted fix, even if it fails. OK, done; backpatched from 8.0.X to 8.3.X. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Re: [BUGS] BUG #4128: The postmaster.opts.default file is begin ignored
Tom Lane wrote: Peter Eisentraut [EMAIL PROTECTED] writes: Tom Lane wrote: We've mostly deprecated setting options on the postmaster command line already, so why do we need another obscure way to do that? As long as we support the pg_ctl -o option, the file still necessary so that you get the same options after a restart. No, it's the postmaster.opts.default file that I'm complaining about, not the postmaster.opts file. The attached applied patch removes the use of the postmaster.opts.default file by pg_ctl. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/pg_ctl-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/pg_ctl-ref.sgml,v retrieving revision 1.45 diff -c -c -r1.45 pg_ctl-ref.sgml *** doc/src/sgml/ref/pg_ctl-ref.sgml 23 Apr 2008 13:44:58 - 1.45 --- doc/src/sgml/ref/pg_ctl-ref.sgml 26 Jun 2008 01:04:44 - *** *** 406,425 /varlistentry varlistentry - termfilenamepostmaster.opts.default/filename/term - - listitem - para - If this file exists in the data directory, - applicationpg_ctl/application (in optionstart/option - mode) will pass the contents of the file as options to the - commandpostgres/command command, unless overridden by the - option-o/option option. - /para - /listitem -/varlistentry - -varlistentry termfilenamepostmaster.opts/filename/term listitem --- 406,411 Index: src/bin/pg_ctl/pg_ctl.c === RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.98 diff -c -c -r1.98 pg_ctl.c *** src/bin/pg_ctl/pg_ctl.c 24 Apr 2008 14:23:43 - 1.98 --- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 01:04:44 - *** *** 140,146 static bool test_postmaster_connection(bool); static bool postmaster_is_alive(pid_t pid); - static char def_postopts_file[MAXPGPATH]; static char postopts_file[MAXPGPATH]; static char pid_file[MAXPGPATH]; static char conf_file[MAXPGPATH]; --- 140,145 *** *** 575,616 static void read_post_opts(void) { - char *optline = NULL; - if (post_opts == NULL) { char **optlines; - int len; ! optlines = readfile(ctl_command == RESTART_COMMAND ? ! postopts_file : def_postopts_file); ! if (optlines == NULL) { ! if (ctl_command == START_COMMAND || ctl_command == RUN_AS_SERVICE_COMMAND) ! post_opts = ; ! else { write_stderr(_(%s: could not read file \%s\\n), progname, postopts_file); exit(1); } ! } ! else if (optlines[0] == NULL || optlines[1] != NULL) ! { ! write_stderr(_(%s: option file \%s\ must have exactly one line\n), ! progname, ctl_command == RESTART_COMMAND ? ! postopts_file : def_postopts_file); ! exit(1); ! } ! else ! { ! optline = optlines[0]; ! len = strcspn(optline, \r\n); ! optline[len] = '\0'; ! ! if (ctl_command == RESTART_COMMAND) { char *arg1; arg1 = strchr(optline, *SYSTEMQUOTE); if (arg1 == NULL || arg1 == optline) post_opts = ; --- 574,608 static void read_post_opts(void) { if (post_opts == NULL) { char **optlines; ! post_opts = ; /* defatult */ ! if (ctl_command == RESTART_COMMAND) { ! optlines = readfile(postopts_file); ! if (optlines == NULL) { write_stderr(_(%s: could not read file \%s\\n), progname, postopts_file); exit(1); } ! else if (optlines[0] == NULL || optlines[1] != NULL) { + write_stderr(_(%s: option file \%s\ must have exactly one line\n), + progname, postopts_file); + exit(1); + } + else + { + int len; + char *optline = NULL; char *arg1; + optline = optlines[0]; + len = strcspn(optline, \r\n); + optline[len] = '\0'; + arg1 = strchr(optline, *SYSTEMQUOTE); if (arg1 == NULL || arg1 == optline) post_opts = ; *** *** 622,629 if (postgres_path != NULL) postgres_path = optline; } - else - post_opts = optline; } } } --- 614,619 *** *** 1894,1900 if (pg_data) { - snprintf(def_postopts_file, MAXPGPATH, %s/postmaster.opts.default, pg_data); snprintf(postopts_file, MAXPGPATH, %s/postmaster.opts, pg_data); snprintf(pid_file, MAXPGPATH, %s/postmaster.pid, pg_data); snprintf(conf_file, MAXPGPATH, %s/postgresql.conf, pg_data); --- 1884,1889 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Fix pg_ctl restart bug
We document 'pg_ctl restart' to preserve any command-line arguments passed when the server was started: Restarting the server is almost equivalent to stopping the server and starting it again except that commandpg_ctl/command saves and reuses the command line options that were passed to the previously running instance. To restart However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. The second attached applied patch fixes the problem. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/postmaster/postmaster.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.433 retrieving revision 1.434 diff -c -r1.433 -r1.434 *** src/backend/postmaster/postmaster.c 14 Oct 2004 20:23:45 - 1.433 --- src/backend/postmaster/postmaster.c 15 Oct 2004 04:54:31 - 1.434 *** *** 3301,3307 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, '%s', argv[i]); fputs(\n, fp); if (fclose(fp)) --- 3301,3307 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE); fputs(\n, fp); if (fclose(fp)) Index: src/bin/pg_ctl/pg_ctl.c === RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.37 retrieving revision 1.38 diff -c -r1.37 -r1.38 *** src/bin/pg_ctl/pg_ctl.c 15 Oct 2004 04:32:14 - 1.37 --- src/bin/pg_ctl/pg_ctl.c 15 Oct 2004 04:54:33 - 1.38 *** *** 501,507 { char *arg1; ! arg1 = strchr(optline, '\''); if (arg1 == NULL || arg1 == optline) post_opts = ; else --- 501,507 { char *arg1; ! arg1 = strchr(optline, *SYSTEMQUOTE); if (arg1 == NULL || arg1 == optline) post_opts = ; else Index: src/backend/postmaster/postmaster.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.560 diff -c -c -r1.560 postmaster.c *** src/backend/postmaster/postmaster.c 26 Jun 2008 01:35:45 - 1.560 --- src/backend/postmaster/postmaster.c 26 Jun 2008 02:41:04 - *** *** 4184,4190 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, SYSTEMQUOTE %s SYSTEMQUOTE, argv[i]); fputs(\n, fp); if (fclose(fp)) --- 4184,4190 fprintf(fp, %s, fullprogname); for (i = 1; i argc; i++) ! fprintf(fp, \%s\, argv[i]); fputs(\n, fp); if (fclose(fp)) Index: src/bin/pg_ctl/pg_ctl.c === RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.100 diff -c -c -r1.100 pg_ctl.c *** src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 01:35:45 - 1.100 --- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 02:41:04 - *** *** 573,583 { if (post_opts == NULL) { - char **optlines; - post_opts = ; /* defatult */ if (ctl_command == RESTART_COMMAND) { optlines = readfile(postopts_file); if (optlines == NULL) { --- 573,583 { if (post_opts == NULL) { post_opts = ; /* defatult */ if (ctl_command == RESTART_COMMAND) { + char **optlines; + optlines = readfile(postopts_file); if (optlines == NULL) { *** *** 593,612 else { int len; ! char *optline = NULL; char *arg1; optline = optlines[0]; len = strcspn(optline, \r\n); optline[len] = '\0'; ! arg1 = strchr(optline, *SYSTEMQUOTE); ! if (arg1 == NULL || arg1 == optline) ! post_opts = ; ! else { ! *(arg1 - 1) = '\0'; /* this should be a space */ ! post_opts = arg1; } if (postgres_path != NULL) postgres_path = optline; --- 593,618 else { int len; ! char *optline; char *arg1; optline = optlines[0]; + /* trim off line endings */ len = strcspn(optline, \r\n); optline[len] = '\0'; ! for (arg1 = optline; *arg1; arg1++) { ! /* ! * Are we at the first option, as defined by space, ! * double-quote, and a dash? ! */ ! if (*arg1 == ' ' *(arg1+1) == '' *(arg1+2) == '-') ! { ! *arg1 = '\0'; /* terminate so we get only program name */ ! post_opts = arg1 + 1; /* point past whitespace */ ! break; ! } } if (postgres_path != NULL) postgres_path
Re: [PATCHES] Fix pg_ctl restart bug
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Alex Hunsaker wrote: On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian [EMAIL PROTECTED] wrote: I would like to get do this without adding a new --use-statement-timeout flag. Is anyone going to want to honor statement_timeout during pg_dump/pg_restore? I thought we were just going to disable it. I believe so. This was when not everyone was convinced. Im fairly certain Josh original patch is in the commit fest. So feel free to drop this one. I certainly don't see any version of Drake's patch in the July commitfest: http://wiki.postgresql.org/wiki/CommitFest I am thinking I will just remove the option and commit it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Joshua D. Drake wrote: Alex Hunsaker wrote: On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian [EMAIL PROTECTED] wrote: I would like to get do this without adding a new --use-statement-timeout flag. Is anyone going to want to honor statement_timeout during pg_dump/pg_restore? I thought we were just going to disable it. I believe so. This was when not everyone was convinced. Im fairly certain Josh original patch is in the commit fest. So feel free to drop this one. My patch has been committed. Ah, I see, but with no switch. Thanks. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Simplify formatting.c
Bruce Momjian wrote: I am starting to think that the simplest case is to keep the single-copy version in there for single-byte encodings and not worry about the overhead of the multi-byte case. My new idea is if we pass the length to str_initcap, we can eliminate the string copy from text to char *. That leaves us with just one extra string copy from char * to text, which seems acceptable. We still have the wide char copy but I don't see any easy way to eliminate that because the multi-byte code is complex and not something we want to duplicate. I ended up going in this direction, and did the same for upper and lower. Patch attached and applied. I don't see any other cleanups in this area. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/formatting.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v retrieving revision 1.142 diff -c -c -r1.142 formatting.c *** src/backend/utils/adt/formatting.c 17 Jun 2008 16:09:06 - 1.142 --- src/backend/utils/adt/formatting.c 23 Jun 2008 19:24:35 - *** *** 925,933 static char *str_numth(char *dest, char *num, int type); static int strspace_len(char *str); static int strdigits_len(char *str); - static char *str_toupper(char *buff); - static char *str_tolower(char *buff); - static char *str_initcap(char *buff); static int seq_search(char *name, char **array, int type, int max, int *len); static void do_to_timestamp(text *date_txt, text *fmt, --- 925,930 *** *** 1424,1435 return dest; } /* -- ! * Convert string to upper case. It is designed to be multibyte-aware. * -- */ ! static char * ! str_toupper(char *buff) { char *result; --- 1421,1444 return dest; } + /* + * If the system provides the needed functions for wide-character manipulation + * (which are all standardized by C99), then we implement upper/lower/initcap + * using wide-character functions, if necessary. Otherwise we use the + * traditional ctype.h functions, which of course will not work as desired + * in multibyte character sets. Note that in either case we are effectively + * assuming that the database character encoding matches the encoding implied + * by LC_CTYPE. + */ + /* -- ! * wide-character-aware lower function ! * We pass the number of bytes so we can pass varlena and char* ! * to this function. * -- */ ! char * ! str_tolower(char *buff, size_t nbytes) { char *result; *** *** 1438,1464 #ifdef USE_WIDE_UPPER_LOWER if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) ! result = wstring_upper(buff); else #endif /* USE_WIDE_UPPER_LOWER */ { char *p; ! result = pstrdup(buff); for (p = result; *p; p++) ! *p = pg_toupper((unsigned char) *p); } return result; } /* -- ! * Convert string to lower case. It is designed to be multibyte-aware. * -- */ ! static char * ! str_tolower(char *buff) { char *result; --- 1447,1492 #ifdef USE_WIDE_UPPER_LOWER if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) ! { ! wchar_t *workspace; ! int curr_char = 0; ! ! /* Output workspace cannot have more codes than input bytes */ ! workspace = (wchar_t *) palloc((nbytes + 1) * sizeof(wchar_t)); ! ! char2wchar(workspace, nbytes + 1, buff, nbytes + 1); ! ! for (curr_char = 0; workspace[curr_char] != 0; curr_char++) ! workspace[curr_char] = towlower(workspace[curr_char]); ! ! /* Make result large enough; case change might change number of bytes */ ! result = palloc(curr_char * MB_CUR_MAX + 1); ! ! wchar2char(result, workspace, curr_char * MB_CUR_MAX + 1); ! pfree(workspace); ! } else #endif /* USE_WIDE_UPPER_LOWER */ { char *p; ! result = pnstrdup(buff, nbytes); for (p = result; *p; p++) ! *p = pg_tolower((unsigned char) *p); } return result; } /* -- ! * wide-character-aware upper function ! * We pass the number of bytes so we can pass varlena and char* ! * to this function. * -- */ ! char * ! str_toupper(char *buff, size_t nbytes) { char *result; *** *** 1467,1493 #ifdef USE_WIDE_UPPER_LOWER if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) ! result = wstring_lower(buff); else #endif /* USE_WIDE_UPPER_LOWER */ { char *p; ! result = pstrdup(buff); for (p = result; *p; p++) ! *p = pg_tolower((unsigned char) *p); } return result; } ! /* -- * wide-character-aware initcap function * -- */ ! static char * ! str_initcap(char *buff) { char *result; bool
Re: [PATCHES] Simplify formatting.c
Euler Taveira de Oliveira wrote: Tom Lane wrote: Also, it seems a bit inconsistent to be relying on oracle_compat.c for upper/lower but not initcap. I saw this inconsistence while I'm doing the patch. What about moving that upper/lower/initcap and wcs* code to another file. pg_locale.c? BTW, formatting.c and oracle_compat.c already include pg_locale.h. I researched this idea but is seems pg_locale.c contains only locale-specific stuff, while these functions have locale and non-locale versions; I ended up moving the common stuff into formatting.c. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Alex Hunsaker wrote: On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Joshua D. Drake escribi?: That is an interesting idea. Something like: pg_restore -E SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G ? We already have it -- it's called PGOPTIONS. Ok but is not the purpose of the patch to turn off statement_timeout by *default* in pg_restore/pg_dump? Here is an updated patch for I posted above (with the command line option --use-statement-timeout) for pg_dump and pg_restore. I would like to get do this without adding a new --use-statement-timeout flag. Is anyone going to want to honor statement_timeout during pg_dump/pg_restore? I thought we were just going to disable it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
daveg wrote: On Mon, Jun 23, 2008 at 06:51:28PM -0400, Bruce Momjian wrote: Alex Hunsaker wrote: On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Joshua D. Drake escribi?: That is an interesting idea. Something like: pg_restore -E SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G ? We already have it -- it's called PGOPTIONS. Ok but is not the purpose of the patch to turn off statement_timeout by *default* in pg_restore/pg_dump? Here is an updated patch for I posted above (with the command line option --use-statement-timeout) for pg_dump and pg_restore. I would like to get do this without adding a new --use-statement-timeout flag. Is anyone going to want to honor statement_timeout during pg_dump/pg_restore? I thought we were just going to disable it. I have a patch in the queue to use set statement timeout while pg_dump is taking locks to avoid pg_dump hanging for other long running transactions that may have done ddl. Do I need to repost for discussion now? I see it now, but I forgot how it would interact with this patch. We would have to prevent --use-statement-timeout when lock timeout was being used, but my point is that I see no value in having --use-statement-timeout. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Simplify formatting.c
Bruce Momjian wrote: Actually it seems like the hard part is not so much the input representation as the output representation --- what should the base-level initcap routine return, to be reasonably efficient for both cases? I hadn't gotten to trying it out yet, but I can see the output being a problem. You can't even really pre-allocate the storage before passing it because you don't know the length after case change. You could pass back a char* and repalloc to get the varlena header in there but that is very messy. Add to that that the multi-byte case also has to be converted to wide characters, so you have text - char * - wide chars - char * - text for the most complex case. I am starting to think that the simplest case is to keep the single-copy version in there for single-byte encodings and not worry about the overhead of the multi-byte case. My new idea is if we pass the length to str_initcap, we can eliminate the string copy from text to char *. That leaves us with just one extra string copy from char * to text, which seems acceptable. We still have the wide char copy but I don't see any easy way to eliminate that because the multi-byte code is complex and not something we want to duplicate. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Simplify formatting.c
Bruce Momjian wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: I moved str_initcap() over into oracle_compat.c and then had initcap() convert to/from TEXT to call it. The code is a little weird because str_initcap() needs to convert to text to use texttowcs(), so in multibyte encodings initcap converts the string to text, then to char, then to text to call texttowcs(). I didn't see a cleaner way to do this. Why not use wchar2char? It seems there's room for extra cleanup here. Also, the prototype of str_initcap in builtins.h looks out of place. I talked to Alvaro on IM, and there is certainly much more cleanup to do in this area. I will work from the bottom up. First, is moving the USE_WIDE_UPPER_LOWER define to c.h, and removing TS_USE_WIDE and using USE_WIDE_UPPER_LOWER instead. Patch attached and applied. The second step is to move wchar2char() and char2wchar() from tsearch into /mb to be easier to use for other modules; also move pnstrdup(). The third step is for oracle_compat.c::initcap() to use formatting.c::str_initcap(). You can see the result; patch attached (not applied). This greatly reduces the size of initcap(), with the downside that we are making two extra copies of the string to convert it to/from char*. Is this acceptable? If it is I will do the same for uppper()/lower() with similar code size reduction and modularity. If not perhaps I should keep the non-multibyte code in initcap() and have only the multi-byte use str_initcap(). -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/formatting.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v retrieving revision 1.142 diff -c -c -r1.142 formatting.c *** src/backend/utils/adt/formatting.c 17 Jun 2008 16:09:06 - 1.142 --- src/backend/utils/adt/formatting.c 21 Jun 2008 20:00:45 - *** *** 1499,1526 if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) { wchar_t *workspace; ! text *in_text; ! text *out_text; ! int i; ! in_text = cstring_to_text(buff); ! workspace = texttowcs(in_text); ! for (i = 0; workspace[i] != 0; i++) { if (wasalnum) ! workspace[i] = towlower(workspace[i]); else ! workspace[i] = towupper(workspace[i]); ! wasalnum = iswalnum(workspace[i]); } ! out_text = wcstotext(workspace, i); ! result = text_to_cstring(out_text); pfree(workspace); - pfree(in_text); - pfree(out_text); } else #endif /* USE_WIDE_UPPER_LOWER */ --- 1499,1525 if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) { wchar_t *workspace; ! int curr_char = 0; ! /* Output workspace cannot have more codes than input bytes */ ! workspace = (wchar_t *) palloc((strlen(buff) + 1) * sizeof(wchar_t)); ! char2wchar(workspace, strlen(buff) + 1, buff, strlen(buff) + 1); ! ! for (curr_char = 0; workspace[curr_char] != 0; curr_char++) { if (wasalnum) ! workspace[curr_char] = towlower(workspace[curr_char]); else ! workspace[curr_char] = towupper(workspace[curr_char]); ! wasalnum = iswalnum(workspace[curr_char]); } ! /* Make result large enough; case change might change number of bytes */ ! result = palloc(curr_char * MB_CUR_MAX + 1); + wchar2char(result, workspace, curr_char * MB_CUR_MAX + 1); pfree(workspace); } else #endif /* USE_WIDE_UPPER_LOWER */ Index: src/backend/utils/adt/oracle_compat.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/oracle_compat.c,v retrieving revision 1.80 diff -c -c -r1.80 oracle_compat.c *** src/backend/utils/adt/oracle_compat.c 17 Jun 2008 16:09:06 - 1.80 --- src/backend/utils/adt/oracle_compat.c 21 Jun 2008 20:00:45 - *** *** 467,530 Datum initcap(PG_FUNCTION_ARGS) { ! #ifdef USE_WIDE_UPPER_LOWER ! /* ! * Use wide char code only when max encoding length 1 and ctype != C. ! * Some operating systems fail with multi-byte encodings and a C locale. ! * Also, for a C locale there is no need to process as multibyte. ! */ ! if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) ! { ! text *string = PG_GETARG_TEXT_PP(0); ! text *result; ! wchar_t*workspace; ! int wasalnum = 0; ! int i; ! ! workspace = texttowcs(string); ! ! for (i = 0; workspace[i] != 0; i++) ! { ! if (wasalnum) ! workspace[i] = towlower(workspace[i]); ! else ! workspace[i] = towupper(workspace[i]); ! wasalnum = iswalnum(workspace[i]); ! } ! ! result = wcstotext(workspace, i); ! ! pfree(workspace); ! ! PG_RETURN_TEXT_P(result); ! } ! else ! #endif
Re: [PATCHES] Simplify formatting.c
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: The third step is for oracle_compat.c::initcap() to use formatting.c::str_initcap(). You can see the result; patch attached (not applied). This greatly reduces the size of initcap(), with the downside that we are making two extra copies of the string to convert it to/from char*. Is this acceptable? I'd say not. Can't we do some more refactoring and avoid so many useless conversions? Seems like str_initcap is the wrong primitive API --- the work ought to be done by a function that takes a char pointer and a length. That would be a suitable basis for functions operating on both text datums and C strings. Yea, I thought about that idea too but it is going to add a strlen() calls in some places, but not in critical ones. (Perhaps what I should be asking is whether the performance of upper() and lower() is equally bad. Certainly all three should have comparable code, so maybe they all need refactoring.) Yes, they do. I will work on the length idea and see how that goes. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Simplify formatting.c
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: I'd say not. Can't we do some more refactoring and avoid so many useless conversions? Seems like str_initcap is the wrong primitive API --- the work ought to be done by a function that takes a char pointer and a length. That would be a suitable basis for functions operating on both text datums and C strings. Yea, I thought about that idea too but it is going to add a strlen() calls in some places, but not in critical ones. Sure, but the cost-per-byte of the strlen should be a good bit less than the cost-per-byte of the actual conversion, so that doesn't bother me too much. Actually it seems like the hard part is not so much the input representation as the output representation --- what should the base-level initcap routine return, to be reasonably efficient for both cases? I hadn't gotten to trying it out yet, but I can see the output being a problem. You can't even really pre-allocate the storage before passing it because you don't know the length after case change. You could pass back a char* and repalloc to get the varlena header in there but that is very messy. Add to that that the multi-byte case also has to be converted to wide characters, so you have text - char * - wide chars - char * - text for the most complex case. I am starto to think that the simplest case is to keep the single-copy version in there for single-byte encodings and not worry about the overhead of the multi-byte case. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Simplify formatting.c
Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: I moved str_initcap() over into oracle_compat.c and then had initcap() convert to/from TEXT to call it. The code is a little weird because str_initcap() needs to convert to text to use texttowcs(), so in multibyte encodings initcap converts the string to text, then to char, then to text to call texttowcs(). I didn't see a cleaner way to do this. Why not use wchar2char? It seems there's room for extra cleanup here. Also, the prototype of str_initcap in builtins.h looks out of place. I talked to Alvaro on IM, and there is certainly much more cleanup to do in this area. I will work from the bottom up. First, is moving the USE_WIDE_UPPER_LOWER define to c.h, and removing TS_USE_WIDE and using USE_WIDE_UPPER_LOWER instead. Patch attached and applied. The second step is to move wchar2char() and char2wchar() from tsearch into /mb to be easier to use for other modules; also move pnstrdup(). Patch attached and applied. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/tsearch/ts_locale.c === RCS file: /cvsroot/pgsql/src/backend/tsearch/ts_locale.c,v retrieving revision 1.8 diff -c -c -r1.8 ts_locale.c *** src/backend/tsearch/ts_locale.c 17 Jun 2008 16:09:06 - 1.8 --- src/backend/tsearch/ts_locale.c 18 Jun 2008 18:37:02 - *** *** 16,140 #include tsearch/ts_locale.h #include tsearch/ts_public.h - #ifdef USE_WIDE_UPPER_LOWER - /* - * wchar2char --- convert wide characters to multibyte format - * - * This has the same API as the standard wcstombs() function; in particular, - * tolen is the maximum number of bytes to store at *to, and *from must be - * zero-terminated. The output will be zero-terminated iff there is room. - */ - size_t - wchar2char(char *to, const wchar_t *from, size_t tolen) - { - if (tolen == 0) - return 0; - - #ifdef WIN32 - if (GetDatabaseEncoding() == PG_UTF8) - { - int r; - - r = WideCharToMultiByte(CP_UTF8, 0, from, -1, to, tolen, - NULL, NULL); - - if (r = 0) - return (size_t) -1; - - Assert(r = tolen); - - /* Microsoft counts the zero terminator in the result */ - return r - 1; - } - #endif /* WIN32 */ - - return wcstombs(to, from, tolen); - } - - /* - * char2wchar --- convert multibyte characters to wide characters - * - * This has almost the API of mbstowcs(), except that *from need not be - * null-terminated; instead, the number of input bytes is specified as - * fromlen. Also, we ereport() rather than returning -1 for invalid - * input encoding. tolen is the maximum number of wchar_t's to store at *to. - * The output will be zero-terminated iff there is room. - */ - size_t - char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen) - { - if (tolen == 0) - return 0; - - #ifdef WIN32 - if (GetDatabaseEncoding() == PG_UTF8) - { - int r; - - /* stupid Microsloth API does not work for zero-length input */ - if (fromlen == 0) - r = 0; - else - { - r = MultiByteToWideChar(CP_UTF8, 0, from, fromlen, to, tolen - 1); - - if (r = 0) - { - /* see notes in oracle_compat.c about error reporting */ - pg_verifymbstr(from, fromlen, false); - ereport(ERROR, - (errcode(ERRCODE_CHARACTER_NOT_IN_REPERTOIRE), - errmsg(invalid multibyte character for locale), - errhint(The server's LC_CTYPE locale is probably incompatible with the database encoding.))); - } - } - - Assert(r tolen); - to[r] = 0; - - return r; - } - #endif /* WIN32 */ - - if (lc_ctype_is_c()) - { - /* - * pg_mb2wchar_with_len always adds trailing '\0', so 'to' should be - * allocated with sufficient space - */ - return pg_mb2wchar_with_len(from, (pg_wchar *) to, fromlen); - } - else - { - /* - * mbstowcs requires ending '\0' - */ - char *str = pnstrdup(from, fromlen); - size_t result; - - result = mbstowcs(to, str, tolen); - - pfree(str); - - if (result == (size_t) -1) - { - pg_verifymbstr(from, fromlen, false); - ereport(ERROR, - (errcode(ERRCODE_CHARACTER_NOT_IN_REPERTOIRE), - errmsg(invalid multibyte character for locale), - errhint(The server's LC_CTYPE locale is probably incompatible with the database encoding.))); - } - - if (result tolen) - to[result] = 0; - - return result; - } - } - - int t_isdigit(const char *ptr) { --- 16,23 Index: src/backend/tsearch/ts_utils.c === RCS file: /cvsroot/pgsql/src/backend/tsearch/ts_utils.c,v retrieving revision 1.9 diff -c -c -r1.9 ts_utils.c *** src/backend/tsearch/ts_utils.c 1 Jan 2008 19:45:52 - 1.9 --- src/backend
Re: [PATCHES] Simplify formatting.c
Tom Lane wrote: Euler Taveira de Oliveira [EMAIL PROTECTED] writes: Tom Lane wrote: Also, it seems a bit inconsistent to be relying on oracle_compat.c for upper/lower but not initcap. I saw this inconsistence while I'm doing the patch. What about moving that upper/lower/initcap and wcs* code to another file. pg_locale.c? That doesn't seem a particularly appropriate place for them. pg_locale is about dealing with the locale state, not about doing actual operations based on the locale data. I was just thinking of having oracle_compat expose an initcap routine. You mean like the attached? I moved str_initcap() over into oracle_compat.c and then had initcap() convert to/from TEXT to call it. The code is a little weird because str_initcap() needs to convert to text to use texttowcs(), so in multibyte encodings initcap converts the string to text, then to char, then to text to call texttowcs(). I didn't see a cleaner way to do this. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/formatting.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v retrieving revision 1.141 diff -c -c -r1.141 formatting.c *** src/backend/utils/adt/formatting.c 20 May 2008 01:41:02 - 1.141 --- src/backend/utils/adt/formatting.c 13 Jun 2008 22:01:18 - *** *** 927,933 static int strdigits_len(char *str); static char *str_toupper(char *buff); static char *str_tolower(char *buff); - static char *str_initcap(char *buff); static int seq_search(char *name, char **array, int type, int max, int *len); static void do_to_timestamp(text *date_txt, text *fmt, --- 927,932 *** *** 1484,1549 } /* -- - * wide-character-aware initcap function - * -- - */ - static char * - str_initcap(char *buff) - { - char *result; - bool wasalnum = false; - - if (!buff) - return NULL; - - #ifdef USE_WIDE_UPPER_LOWER - if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) - { - wchar_t *workspace; - text *in_text; - text *out_text; - int i; - - in_text = cstring_to_text(buff); - workspace = texttowcs(in_text); - - for (i = 0; workspace[i] != 0; i++) - { - if (wasalnum) - workspace[i] = towlower(workspace[i]); - else - workspace[i] = towupper(workspace[i]); - wasalnum = iswalnum(workspace[i]); - } - - out_text = wcstotext(workspace, i); - result = text_to_cstring(out_text); - - pfree(workspace); - pfree(in_text); - pfree(out_text); - } - else - #endif /* USE_WIDE_UPPER_LOWER */ - { - char *p; - - result = pstrdup(buff); - - for (p = result; *p; p++) - { - if (wasalnum) - *p = pg_tolower((unsigned char) *p); - else - *p = pg_toupper((unsigned char) *p); - wasalnum = isalnum((unsigned char) *p); - } - } - - return result; - } - - /* -- * Sequential search with to upper/lower conversion * -- */ --- 1483,1488 Index: src/backend/utils/adt/oracle_compat.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/oracle_compat.c,v retrieving revision 1.79 diff -c -c -r1.79 oracle_compat.c *** src/backend/utils/adt/oracle_compat.c 19 May 2008 18:08:16 - 1.79 --- src/backend/utils/adt/oracle_compat.c 13 Jun 2008 22:01:18 - *** *** 471,478 Datum initcap(PG_FUNCTION_ARGS) { ! #ifdef USE_WIDE_UPPER_LOWER /* * Use wide char code only when max encoding length 1 and ctype != C. * Some operating systems fail with multi-byte encodings and a C locale. --- 471,496 Datum initcap(PG_FUNCTION_ARGS) { ! text *string = PG_GETARG_TEXT_PP(0); ! char *str2; ! ! str2 = str_initcap(DatumGetCString(string)); ! string = cstring_to_text(str2); ! pfree(str2); ! PG_RETURN_TEXT_P(string); ! } ! + char * + str_initcap(char *str) + { + char *result; + int wasalnum = 0; + + if (!str) + return NULL; + + #ifdef USE_WIDE_UPPER_LOWER /* * Use wide char code only when max encoding length 1 and ctype != C. * Some operating systems fail with multi-byte encodings and a C locale. *** *** 480,492 */ if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) { - text *string = PG_GETARG_TEXT_PP(0); - text *result; wchar_t*workspace; ! int wasalnum = 0; int i; ! workspace = texttowcs(string); for (i = 0; workspace[i] != 0; i++) { --- 498,510 */ if (pg_database_encoding_max_length() 1 !lc_ctype_is_c()) { wchar_t*workspace; ! text *in_text; ! text *out_text; int i; ! in_text = cstring_to_text(str); ! workspace = texttowcs(in_text); for (i = 0
Re: [PATCHES] [HACKERS] Upcoming back-branch update releases
Guillaume Smet wrote: On Wed, May 28, 2008 at 4:10 PM, Tom Lane [EMAIL PROTECTED] wrote: If you've got any bug fixes you've been working on, now is a good time to get them finished up and sent in... Has the s/\x09//g patch for psql from Bruce and you been backported to 8.3? I didn't see it on pgsql-commiters. No. I have not backpatched it because Tom found a problem with my applied patch and did a second patch. I am attaching both patches. The second one is Tom's and I don't understand it well enough to backpatch it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/mbprint.c === RCS file: /cvsroot/pgsql/src/bin/psql/mbprint.c,v retrieving revision 1.31 retrieving revision 1.32 diff -c -r1.31 -r1.32 *** src/bin/psql/mbprint.c 8 May 2008 17:04:26 - 1.31 --- src/bin/psql/mbprint.c 8 May 2008 19:11:36 - 1.32 *** *** 3,9 * * Copyright (c) 2000-2008, PostgreSQL Global Development Group * ! * $PostgreSQL: pgsql/src/bin/psql/mbprint.c,v 1.31 2008/05/08 17:04:26 momjian Exp $ * * XXX this file does not really belong in psql/. Perhaps move to libpq? * It also seems that the mbvalidate function is redundant with existing --- 3,9 * * Copyright (c) 2000-2008, PostgreSQL Global Development Group * ! * $PostgreSQL: pgsql/src/bin/psql/mbprint.c,v 1.32 2008/05/08 19:11:36 momjian Exp $ * * XXX this file does not really belong in psql/. Perhaps move to libpq? * It also seems that the mbvalidate function is redundant with existing *** *** 321,326 --- 321,334 linewidth += 2; ptr += 2; } + else if (*pwcs == '\t') /* Tab */ + { + do + { + *ptr++ = ' '; + linewidth++; + } while (linewidth % 8 != 0); + } else if (w 0) /* Other control char */ { sprintf((char *) ptr, \\x%02X, *pwcs); Index: src/test/regress/expected/prepare.out === RCS file: /cvsroot/pgsql/src/test/regress/expected/prepare.out,v retrieving revision 1.15 retrieving revision 1.16 diff -c -r1.15 -r1.16 *** src/test/regress/expected/prepare.out 18 Jun 2007 21:40:58 - 1.15 --- src/test/regress/expected/prepare.out 8 May 2008 19:11:36 - 1.16 *** *** 155,169 name |statement|parameter_types --+-+ q2 | PREPARE q2(text) AS | {text} ! : \x09SELECT datname, datistemplate, datallowconn ! : \x09FROM pg_database WHERE datname = $1; q3 | PREPARE q3(text, int, float, boolean, oid, smallint) AS | {text,integer,double precision,boolean,oid,smallint} ! : \x09SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 OR ! : \x09ten = $3::bigint OR true = $4 OR oid = $5 OR odd = $6::int) ! : \x09ORDER BY unique1; q5 | PREPARE q5(int, text) AS| {integer,text} ! : \x09SELECT * FROM tenk1 WHERE unique1 = $1 OR stringu1 = $2 ! : \x09ORDER BY unique1; q6 | PREPARE q6 AS | {integer,name} : SELECT * FROM tenk1 WHERE unique1 = $1 AND stringu1 = $2; q7 | PREPARE q7(unknown) AS | {path} --- 155,171 name |statement|parameter_types --+-+ q2 | PREPARE q2(text) AS | {text} ! : SELECT datname, datistemplate, datallowconn ! : FROM pg_database WHERE datname = $1; q3 | PREPARE q3(text, int, float, boolean, oid, smallint) AS | {text,integer,double precision,boolean,oid,smallint} ! : SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 O ! ; R ! : ten = $3::bigint OR true = $4 OR oid = $5 OR odd = $6
[PATCHES] Simplify formatting.c
Now that upper/lower/initcase do not modify the passed string, I have simplified formatting.c with the attached patch. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/formatting.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v retrieving revision 1.140 diff -c -c -r1.140 formatting.c *** src/backend/utils/adt/formatting.c 19 May 2008 18:08:15 - 1.140 --- src/backend/utils/adt/formatting.c 20 May 2008 01:37:23 - *** *** 1894,1903 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_full_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_toupper(workbuff)); ! } else { strcpy(workbuff, months_full[tm-tm_mon - 1]); --- 1894,1900 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_toupper(localized_full_months[tm-tm_mon - 1])); else { strcpy(workbuff, months_full[tm-tm_mon - 1]); *** *** 1910,1923 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_full_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_initcap(workbuff)); ! } else - { sprintf(s, %*s, S_FM(n-suffix) ? 0 : -9, months_full[tm-tm_mon - 1]); - } s += strlen(s); break; case DCH_month: --- 1907,1915 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_initcap(localized_full_months[tm-tm_mon - 1])); else sprintf(s, %*s, S_FM(n-suffix) ? 0 : -9, months_full[tm-tm_mon - 1]); s += strlen(s); break; case DCH_month: *** *** 1925,1934 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_full_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_tolower(workbuff)); ! } else { sprintf(s, %*s, S_FM(n-suffix) ? 0 : -9, months_full[tm-tm_mon - 1]); --- 1917,1923 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_tolower(localized_full_months[tm-tm_mon - 1])); else { sprintf(s, %*s, S_FM(n-suffix) ? 0 : -9, months_full[tm-tm_mon - 1]); *** *** 1941,1955 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_abbrev_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_toupper(workbuff)); ! } else ! { ! strcpy(workbuff, months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_toupper(workbuff)); ! } s += strlen(s); break; case DCH_Mon: --- 1930,1938 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_toupper(localized_abbrev_months[tm-tm_mon - 1])); else ! strcpy(s, str_toupper(months[tm-tm_mon - 1])); s += strlen(s); break; case DCH_Mon: *** *** 1957,1970 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_abbrev_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_initcap(workbuff)); ! } else - { strcpy(s, months[tm-tm_mon - 1]); - } s += strlen(s); break; case DCH_mon: --- 1940,1948 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_initcap(localized_abbrev_months[tm-tm_mon - 1])); else strcpy(s, months[tm-tm_mon - 1]); s += strlen(s); break; case DCH_mon: *** *** 1972,1981 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_abbrev_months[tm-tm_mon - 1]); ! sprintf(s, %*s, 0, str_tolower(workbuff)); ! } else { strcpy(s, months[tm-tm_mon - 1]); --- 1950,1956 if (!tm-tm_mon) break; if (S_TM(n-suffix)) ! strcpy(s, str_tolower(localized_abbrev_months[tm-tm_mon - 1])); else { strcpy(s, months[tm-tm_mon - 1]); *** *** 1992,2001 case DCH_DAY: INVALID_FOR_INTERVAL; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_full_days[tm-tm_wday]); ! sprintf(s, %*s, 0, str_toupper(workbuff)); ! } else { strcpy(workbuff, days[tm-tm_wday]); --- 1967,1973 case DCH_DAY: INVALID_FOR_INTERVAL; if (S_TM(n-suffix)) ! strcpy(s, str_toupper(localized_full_days[tm-tm_wday])); else { strcpy(workbuff, days[tm-tm_wday]); *** *** 2006,2028 case DCH_Day: INVALID_FOR_INTERVAL; if (S_TM(n-suffix)) ! { ! strcpy(workbuff, localized_full_days[tm-tm_wday]); ! sprintf(s, %*s, 0, str_initcap(workbuff)); ! } else
Re: [PATCHES] [HACKERS] use of pager on Windows psql
Andrew Dunstan wrote: In fact, it looks to me like it would be much more sensible to #include settings.h and then simply test pset.notty for all platforms. Yes, we could do that but does the isatty() value ever change while psql is running? When you do '\g filename' does stdout then have isatty as false? Good point. I think the best thing would just be to remove the #ifndef WIN32 / #endif lines OK, patch applied to remove the Win32 test in both places. This broke the buildfarm and finally explains the following kluge which has been puzzling me for four years: /* * for some reason MinGW (and MSVC) outputs an extra newline, so this * suppresses it */ #ifndef WIN32 fputc('\n', fout); #endif I have removed the kluge (and yes, I tested it). Oh, that kluge. Why did the isatty() addition fix this? Was the pager being used on Win32 for the regression tests and somehow eating a line or something? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [COMMITTERS] pgsql: Don't call rm with empty file list.
Peter Eisentraut wrote: Log Message: --- Don't call rm with empty file list. Modified Files: -- pgsql/src: nls-global.mk (r1.12 - r1.13) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/nls-global.mk?r1=1.12r2=1.13) FYI, I had to also apply the attached patch to fix it, but your example on the line above was very helpful. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/nls-global.mk === RCS file: /cvsroot/pgsql/src/nls-global.mk,v retrieving revision 1.13 diff -c -c -r1.13 nls-global.mk *** src/nls-global.mk 17 May 2008 20:24:05 - 1.13 --- src/nls-global.mk 17 May 2008 20:59:27 - *** *** 78,84 clean-po: $(if $(MO_FILES),rm -f $(MO_FILES)) ! @rm -f $(addsuffix .old, $(PO_FILES)) rm -f po/$(CATALOG_NAME).pot --- 78,84 clean-po: $(if $(MO_FILES),rm -f $(MO_FILES)) ! @$(if $(PO_FILES),rm -f $(addsuffix .old, $(PO_FILES))) rm -f po/$(CATALOG_NAME).pot -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] use of pager on Windows psql
Andrew Dunstan wrote: Not sure why ware are not. Should we enabled that code on Win32 and see how it works? Can you test it? Was it some MinGW limitation? I do see isatty() being used on lots of platforms. This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I think we had working on Win32 long before we had psql working and perhaps I copied it from there. I don't see the Win32 checks around isatty() anywhere else. In fact, it looks to me like it would be much more sensible to #include settings.h and then simply test pset.notty for all platforms. Yes, we could do that but does the isatty() value ever change while psql is running? When you do '\g filename' does stdout then have isatty as false? Good point. I think the best thing would just be to remove the #ifndef WIN32 / #endif lines OK, patch applied to remove the Win32 test in both places. I was wrong about \g filename changing stdout, I think. It keeps stdout but uses a different output stream. I am just unsure, given all the features of psql, wether it could change stdin/stdout while running in some way. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/print.c === RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v retrieving revision 1.105 diff -c -c -r1.105 print.c *** src/bin/psql/print.c 17 May 2008 21:40:44 - 1.105 --- src/bin/psql/print.c 17 May 2008 23:32:36 - *** *** 1912,1924 PageOutput(int lines, unsigned short int pager) { /* check whether we need / can / are supposed to use pager */ ! if (pager ! #ifndef WIN32 ! ! isatty(fileno(stdin)) ! isatty(fileno(stdout)) ! #endif ! ) { const char *pagerprog; FILE *pagerpipe; --- 1912,1918 PageOutput(int lines, unsigned short int pager) { /* check whether we need / can / are supposed to use pager */ ! if (pager isatty(fileno(stdin)) isatty(fileno(stdout))) { const char *pagerprog; FILE *pagerpipe; Index: src/interfaces/libpq/fe-print.c === RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-print.c,v retrieving revision 1.75 diff -c -c -r1.75 fe-print.c *** src/interfaces/libpq/fe-print.c 1 Jan 2008 19:46:00 - 1.75 --- src/interfaces/libpq/fe-print.c 17 May 2008 23:32:36 - *** *** 147,159 if (fout == NULL) fout = stdout; ! if (po-pager fout == stdout ! #ifndef WIN32 ! ! isatty(fileno(stdin)) ! isatty(fileno(stdout)) ! #endif ! ) { /* * If we think there'll be more than one screen of output, try to --- 147,154 if (fout == NULL) fout = stdout; ! if (po-pager fout == stdout isatty(fileno(stdin)) ! isatty(fileno(stdout))) { /* * If we think there'll be more than one screen of output, try to -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] lc_time and localized dates
I have reviewed this patch. I like the method you used, but I did find a few things I had to change. First, I found the abbreviated variable names confusing so I used longer ones, like: extern char *localized_abbrev_days[7]; extern char *localized_full_days[7]; extern char *localized_abbrev_months[12]; extern char *localized_full_months[12]; Second, I found that the code doing upper/lower case didn't work. It was copying the buffer into a 'result' variable, but then incrementing 'result' so by the end 'result' pointed to only the null byte, and that is the pointer that was returned. Third, there were a few places where the code assumed str_toupper() modified the passed buffer, rather than returning a new one. And finally, while you used strdup() to save values in the cache (good), you never free()'ed the old values when you were reloading the cache. I have fixed all these items and the updated patch is at: ftp://momjian.us/pub/postgresql/mypatches/lc_time The original patch was here: http://archives.postgresql.org/message-id/[EMAIL PROTECTED] --- Euler Taveira de Oliveira wrote: Bruce Momjian wrote: Euler, have you updated this patch yet? Here is an updated patch. It follows the Oracle behaviour and uses a cache mechanism to avoid calling setlocale() all the time. I unified the localized_* and str_* functions. I didn't test it on Windows. I would appreciate some feedback. euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu Qui Quinta apr ABR abril 2008 (1 registro) euler=# set lc_time to 'it_IT.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char - thu Gio Gioved? apr APR aprile 2008 (1 registro) euler=# set lc_time to 'es_ES.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu Jue Jueves apr ABR abril 2008 (1 registro) euler=# set lc_time to 'fr_FR.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char -- thu Jeu Jeudi apr AVR avril 2008 (1 registro) euler=# set lc_time to 'cs_CZ.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu ?t ?tvrtek apr DUB duben 2008 (1 registro) -- Euler Taveira de Oliveira http://www.timbira.com/ [ application/x-gzip is not supported, skipping... ] -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq thread-locking
Magnus Hagander wrote: Bruce Momjian wrote: Bruce Momjian wrote: Magnus Hagander wrote: Attached patch adds some error checking to the thread locking stuff in libpq. Previously, if thread locking failed for some reason, we would just fall through and do things without locking. This patch makes us abort() instead. It's not the greatest thing probably, but our API doesn't let us pass back return values... I have looked over the patch and it seems fine, though I am concerned about the abort() case with no output. I realize stderr might be going nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures so for consistency I think we should do the same here. If there is concern about code bloat, I suggest a macro at the top of the file for thread failure exits: #define THEAD_FAILURE(str) \ do { \ fprintf(stderr, libpq_gettext(Thread failure: %s\n)); \ exit(1); \ } while(0) Oh, this is Tom saying he doesn't like stderr and the added code lines for failure: http://archives.postgresql.org/pgsql-patches/2008-04/msg00254.php I think the macro and consistency suggest doing as I outlined. Does this one look like what you're suggesting? Yep. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix for psql pager computations
Bruce Momjian wrote: The attached patch causes psql to use the pager if newlines or 'format=wrapped' has caused a single row to span more than one line and the output is then too long for the screen. It also uses the pager if psql thinks the data will wrap off the edge of the screen. The display width as defined by \pset columns, $COLUMNS, or the ioctl (as previously discussed) We don't have similar settings for the number of display rows. I assume that is OK. I believe this makes the pager more reliable, and in fact I have removed the psql manual mention that pager computations are somewhat imperfect. Update patch applied. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.204 diff -c -c -r1.204 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 May 2008 04:07:01 - 1.204 --- doc/src/sgml/ref/psql-ref.sgml 16 May 2008 16:49:19 - *** *** 1555,1561 termliteralcolumns/literal/term listitem para ! Controls the target width for the literalwrapped/ format. Zero (the default) causes the literalwrapped/ format to affect only screen output. /para --- 1555,1562 termliteralcolumns/literal/term listitem para ! Controls the target width for the literalwrapped/ format, ! and width for determining if wide output requires the pager. Zero (the default) causes the literalwrapped/ format to affect only screen output. /para *** *** 1717,1726 When the pager is literaloff/, the pager is not used. When the pager is literalon/, the pager is used only when appropriate, i.e. the output is to a terminal and will not fit on the screen. ! (applicationpsql/ does not do a perfect job of estimating ! when to use the pager.) literal\pset pager/ turns the ! pager on and off. Pager can also be set to literalalways/, ! which causes the pager to be always used. /para /listitem /varlistentry --- 1718,1726 When the pager is literaloff/, the pager is not used. When the pager is literalon/, the pager is used only when appropriate, i.e. the output is to a terminal and will not fit on the screen. ! literal\pset pager/ turns the pager on and off. Pager can ! also be set to literalalways/, which causes the pager to be ! always used. /para /listitem /varlistentry *** *** 2734,2741 listitem para ! Used for the literalwrapped/ output format if ! literal\pset columns/ is zero. /para /listitem /varlistentry --- 2734,2742 listitem para ! If literal\pset columns/ is zero, controls the ! width for the literalwrapped/ format and width for determining ! if wide output requires the pager. /para /listitem /varlistentry Index: src/bin/psql/print.c === RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v retrieving revision 1.101 diff -c -c -r1.101 print.c *** src/bin/psql/print.c 13 May 2008 00:14:11 - 1.101 --- src/bin/psql/print.c 16 May 2008 16:49:20 - *** *** 45,50 --- 45,52 /* Local functions */ static int strlen_max_width(unsigned char *str, int *target_width, int encoding); + static void IsPagerNeeded(const printTableContent *cont, const int extra_lines, + FILE **fout, bool *is_pager); static void * *** *** 394,400 * Print pretty boxes around cells. */ static void ! print_aligned_text(const printTableContent *cont, bool is_pager, FILE *fout) { bool opt_tuples_only = cont-opt-tuples_only; bool opt_numeric_locale = cont-opt-numericLocale; --- 396,402 * Print pretty boxes around cells. */ static void ! print_aligned_text(const printTableContent *cont, FILE *fout) { bool opt_tuples_only = cont-opt-tuples_only; bool opt_numeric_locale = cont-opt-numericLocale; *** *** 416,421 --- 418,425 unsigned char **format_buf; unsigned int width_total; unsigned int total_header_width; + unsigned int extra_row_output_lines = 0; + unsigned int extra_output_lines = 0; const char * const *ptr; *** *** 424,429 --- 428,434 bool *header_done; /* Have all header lines been output? */ int *bytes_output; /* Bytes output for column value
Re: [PATCHES] Patch to change psql default banner v6
Bruce Momjian wrote: OK, here is the mega-print: $ psql test psql (8.4devel, server 8.4devel) WARNING: psql version 8.4, server version 8.4. Some psql features might not work. WARNING: Console code page (44) differs from Windows code page (55) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. SSL connection (cipher: 55, bits: 512) Type help for help. test= Updated patch applied, docs adjusted for new psql startup banner. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/start.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/start.sgml,v retrieving revision 1.46 diff -c -c -r1.46 start.sgml *** doc/src/sgml/start.sgml 23 Jan 2008 02:04:47 - 1.46 --- doc/src/sgml/start.sgml 16 May 2008 17:06:38 - *** *** 329,341 In commandpsql/command, you will be greeted with the following message: screen ! Welcome to psql version;, the PostgreSQL interactive terminal. ! ! Type: \copyright for distribution terms !\h for help with SQL commands !\? for help with psql commands !\g or terminate with semicolon to execute query !\q to quit mydb=gt; /screen --- 329,336 In commandpsql/command, you will be greeted with the following message: screen ! psql (version;) ! Type help for help. mydb=gt; /screen Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.205 diff -c -c -r1.205 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 16 May 2008 16:59:05 - 1.205 --- doc/src/sgml/ref/psql-ref.sgml 16 May 2008 17:06:38 - *** *** 571,583 the string literal=gt;/literal. For example: programlisting $ userinputpsql testdb/userinput ! Welcome to psql version;, the PostgreSQL interactive terminal. ! Type: \copyright for distribution terms !\h for help with SQL commands !\? for help with psql commands !\g or terminate with semicolon to execute query !\q to quit testdb=gt; /programlisting --- 571,580 the string literal=gt;/literal. For example: programlisting $ userinputpsql testdb/userinput ! psql (version;) ! Type help for help. ! test= testdb=gt; /programlisting Index: src/bin/psql/help.c === RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.127 diff -c -c -r1.127 help.c *** src/bin/psql/help.c 14 May 2008 15:30:22 - 1.127 --- src/bin/psql/help.c 16 May 2008 17:06:39 - *** *** 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); - fprintf(output, _( \\g [FILE] send query buffer to server (and results to file or |pipe)\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE --- 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); + fprintf(output, _( \\g [FILE] or ; execute query (and send results to file or |pipe)\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE Index: src/bin/psql/mainloop.c === RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.90 diff -c -c -r1.90 mainloop.c *** src/bin/psql/mainloop.c 5 Apr 2008 03:40:15 - 1.90 --- src/bin/psql/mainloop.c 16 May 2008 17:06:39 - *** *** 177,186 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(You are using psql, the command-line interface
Re: [PATCHES] Patch to change psql default banner v6
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Ah, OK. I had forgotten. Here is the new output: $ sql test psql (8.4devel) Type help for help. test= help You are being unreasonably cryptic here. What happens when there is optional output --- ie, version mismatch warning and/or SSL info? Oh, good point. Let me look at that. Thanks. You prefer: $ sql test psql (8.4devel) Type help for help. test= help That looked so sparse to me. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Joshua D. Drake wrote: O.k. I am not trying to start an argument here but... I already sent 6 revisions of this patch that received comments and had thorough review via Alvaro. I even took into account Tom's original comments from the previous thread. This much effort on something so simple makes it not worth the effort in the first place. Bruce with respect the only useful thing I have seen you do to the patch in all this wrangling is realign the \? General options and frankly even that is suspect in my opinion. Can we either throw it away and say, Nice try JD or just commit the thing. Your patch is getting the same review any other patch would have. If you want someone else to apply it I will stop working on it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Oh, good point. Let me look at that. Thanks. You prefer: $ sql test psql (8.4devel) Type help for help. test= help Well, the question is still where is the optional info going to go? I think what I'd find nice looking is $ psql test psql 8.4devel [ server version warning here, if needed ] [ line with SSL info here, if needed ] Type help for help. test= I do feel that the help statement ought to be on its own line; the other way is going to look cluttered, particularly as soon as there's a version warning in there. OK, here is the normal startup now: $ sql test psql (8.4.0) Type help for help. test= Here is a minor version mismatch: $ sql test psql (8.4.0, server 8.4.1) Type help for help. test= Here is a major version mismatch: $ sql test psql (8.4.0, server 8.3.1) WARNING: psql version 8.4.0, server version 8.3.1. Some psql features might not work. Type help for help. test= I have also added '\g' to the 'help' display: test= help You are using psql, the command-line interface to PostgreSQL. \? for psql help \h or \help for SQL help \g or ; to execute a query \q to quit psql \copyright to view the copyright test= I don't know how to generate an SSL message. With the new smaller \? General section, I though it was worth considering if we still want to do the help banner the same. Obviously we do, but I wanted to explore it. Patch attached. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/help.c === RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.127 diff -c -c -r1.127 help.c *** src/bin/psql/help.c 14 May 2008 15:30:22 - 1.127 --- src/bin/psql/help.c 15 May 2008 16:05:51 - *** *** 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); - fprintf(output, _( \\g [FILE] send query buffer to server (and results to file or |pipe)\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE --- 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); + fprintf(output, _( \\g [FILE] or ; execute query (and send results to file or |pipe)\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE Index: src/bin/psql/mainloop.c === RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.90 diff -c -c -r1.90 mainloop.c *** src/bin/psql/mainloop.c 5 Apr 2008 03:40:15 - 1.90 --- src/bin/psql/mainloop.c 15 May 2008 16:05:51 - *** *** 177,186 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(You are using psql, the command-line interface to PostgreSQL.)); ! puts(_(Enter SQL commands, or type \\? for a list of backslash options.)); ! puts(_(Use \\h for SQL command help.)); ! puts(_(Use \\q to quit.)); fflush(stdout); continue; } --- 177,189 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4
Re: [PATCHES] Patch to change psql default banner v6
Ron Mayer wrote: Alvaro Herrera wrote: Andrew Dunstan wrote: Welcome to UI development. There is always *far* more argument of minor matters of appearance than over anything else, in my experience. Which is a good thing (in this case at least), because otherwise we would end up with a crappy UI just because a single person thinks it's good enough. This makes me think we shouldn't be hard-coding anything at all as the welcome message; but rather having a default .psqlrc in much the same way that that there's a default /etc/bash.bashrc and /etc/csh.login. Within that default .psqlrc we can put \qecho Whatever the default message is or select my message +version(); to create the default, but then anyone with their own .psqlrc can re-define it to whatever they think is a good enough UI. We could do that but we still have to design the default banner. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Alvaro Herrera wrote: I'm OK with thisG but please move the printSSLInfo() call just before echoing the help line. Oh, good catch, moved. I also moved the Win32 code page message up too. Patch attached. I hacked up an example that shows both SSL and Win32 code page messages: $ psql test psql (8.4devel) SSL connection (cipher: 2343, bits: 512) WARNING: Console code page (323) differs from Windows code page (2323) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. Type help for help. test= With major version mismatches it looks like this: $ psql test psql (8.4devel) SSL connection (cipher: 2343, bits: 512) WARNING: Console code page (323) differs from Windows code page (2323) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. WARNING: psql version 8.4.0, server version 8.3.1. Some psql features might not work. Type help for help. test= By indenting those messages the 'help' message still stands out. Adjustments? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/help.c === RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.127 diff -c -c -r1.127 help.c *** src/bin/psql/help.c 14 May 2008 15:30:22 - 1.127 --- src/bin/psql/help.c 15 May 2008 19:17:27 - *** *** 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); - fprintf(output, _( \\g [FILE] send query buffer to server (and results to file or |pipe)\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE --- 170,182 */ fprintf(output, _(General\n)); fprintf(output, _( \\copyright show PostgreSQL usage and distribution terms\n)); + fprintf(output, _( \\g [FILE] or ; execute query (and send results to file or |pipe)\n)); fprintf(output, _( \\h [NAME] help on syntax of SQL commands, * for all commands\n)); fprintf(output, _( \\q quit psql\n)); fprintf(output, \n); fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE Index: src/bin/psql/mainloop.c === RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.90 diff -c -c -r1.90 mainloop.c *** src/bin/psql/mainloop.c 5 Apr 2008 03:40:15 - 1.90 --- src/bin/psql/mainloop.c 15 May 2008 19:17:27 - *** *** 177,186 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(You are using psql, the command-line interface to PostgreSQL.)); ! puts(_(Enter SQL commands, or type \\? for a list of backslash options.)); ! puts(_(Use \\h for SQL command help.)); ! puts(_(Use \\q to quit.)); fflush(stdout); continue; } --- 177,189 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(\nYou are using psql, the command-line interface to PostgreSQL.)); ! puts(_(\t\\? for psql help)); ! puts(_(\t\\h or \\help for SQL help\n)); ! puts(_(\t\\g or \;\ to execute a query)); ! puts(_(\t\\q to quit psql\n)); ! puts(_(\t\\copyright to view the copyright\n)); ! fflush(stdout); continue; } Index: src/bin/psql/startup.c === RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v retrieving revision 1.147 diff -c -c -r1.147 startup.c *** src/bin/psql/startup.c 8 May 2008 17:04:26 - 1.147 --- src/bin/psql/startup.c 15 May 2008 19:17:27 - *** *** 300,305 --- 300,312 { int client_ver = parse_version(PG_VERSION); + #ifdef USE_SSL + printSSLInfo(); + #endif + #ifdef WIN32
Re: [PATCHES] Patch to change psql default banner v6
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: With major version mismatches it looks like this: $ psql test psql (8.4devel) SSL connection (cipher: 2343, bits: 512) WARNING: Console code page (323) differs from Windows code page (2323) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. WARNING: psql version 8.4.0, server version 8.3.1. Some psql features might not work. Type help for help. By indenting those messages the 'help' message still stands out. My advice: don't do that, it just looks weird. Both of these look fine to me: $ psql test psql (8.4devel) SSL connection (cipher: 2343, bits: 512) Type help for help. $ psql test psql (8.4devel) SSL connection (cipher: 2343, bits: 512) WARNING: Console code page (323) differs from Windows code page (2323) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. WARNING: psql version 8.4.0, server version 8.3.1. Some psql features might not work. Type help for help. Also, maybe it's just me, but I think you have put the order of these optional things exactly backwards. I'd do $ psql test psql (8.4devel) WARNING: psql version 8.4.0, server version 8.3.1. Some psql features might not work. WARNING: Console code page (323) differs from Windows code page (2323) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. SSL connection (cipher: 2343, bits: 512) Type help for help. Why? Well, it's just more nearly the way it used to be. OK, here is the mega-print: $ psql test psql (8.4devel, server 8.4devel) WARNING: psql version 8.4, server version 8.4. Some psql features might not work. WARNING: Console code page (44) differs from Windows code page (55) 8-bit characters might not work correctly. See psql reference page Notes for Windows users for details. SSL connection (cipher: 55, bits: 512) Type help for help. test= Patch at ftp://momjian.us/pub/postgresql/mypatches/help. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
daveg wrote: On Thu, May 15, 2008 at 10:20:53AM -0700, Ron Mayer wrote: Alvaro Herrera wrote: Andrew Dunstan wrote: Welcome to UI development. There is always *far* more argument of minor matters of appearance than over anything else, in my experience. Which is a good thing (in this case at least), because otherwise we would end up with a crappy UI just because a single person thinks it's good enough. This makes me think we shouldn't be hard-coding anything at all as the welcome message; but rather having a default .psqlrc in much the same way that that there's a default /etc/bash.bashrc and /etc/csh.login. Within that default .psqlrc we can put \qecho Whatever the default message is or select my message +version(); to create the default, but then anyone with their own .psqlrc can re-define it to whatever they think is a good enough UI. +1 Including no banner at all please. There is a psql quiet option that shows no banner: -q --quiet Specifies that psql should do its work quietly. By default, it prints welcome messages and various informational output. If this option is used, none of this happens. This is useful with the -c option. Within psql you can also set the QUIET variable to achieve the same effect. You could then use \echo in .psqlrc to make your own banner. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Fix for psql pager computations
The attached patch causes psql to use the pager if newlines or 'format=wrapped' has caused a single row to span more than one line and the output is then too long for the screen. It also uses the pager if psql thinks the data will wrap off the edge of the screen. The display width as defined by \pset columns, $COLUMNS, or the ioctl (as previously discussed) We don't have similar settings for the number of display rows. I assume that is OK. I believe this makes the pager more reliable, and in fact I have removed the psql manual mention that pager computations are somewhat imperfect. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.204 diff -c -c -r1.204 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 May 2008 04:07:01 - 1.204 --- doc/src/sgml/ref/psql-ref.sgml 14 May 2008 19:56:10 - *** *** 1555,1561 termliteralcolumns/literal/term listitem para ! Controls the target width for the literalwrapped/ format. Zero (the default) causes the literalwrapped/ format to affect only screen output. /para --- 1555,1562 termliteralcolumns/literal/term listitem para ! Controls the target width for the literalwrapped/ format, ! and width for determining if wide output requires the pager. Zero (the default) causes the literalwrapped/ format to affect only screen output. /para *** *** 1717,1726 When the pager is literaloff/, the pager is not used. When the pager is literalon/, the pager is used only when appropriate, i.e. the output is to a terminal and will not fit on the screen. ! (applicationpsql/ does not do a perfect job of estimating ! when to use the pager.) literal\pset pager/ turns the ! pager on and off. Pager can also be set to literalalways/, ! which causes the pager to be always used. /para /listitem /varlistentry --- 1718,1726 When the pager is literaloff/, the pager is not used. When the pager is literalon/, the pager is used only when appropriate, i.e. the output is to a terminal and will not fit on the screen. ! literal\pset pager/ turns the pager on and off. Pager can ! also be set to literalalways/, which causes the pager to be ! always used. /para /listitem /varlistentry *** *** 2734,2741 listitem para ! Used for the literalwrapped/ output format if ! literal\pset columns/ is zero. /para /listitem /varlistentry --- 2734,2742 listitem para ! If literal\pset columns/ is zero, controls the ! width for the literalwrapped/ format and width for determining ! if wide output requires the pager. /para /listitem /varlistentry Index: src/bin/psql/print.c === RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v retrieving revision 1.101 diff -c -c -r1.101 print.c *** src/bin/psql/print.c 13 May 2008 00:14:11 - 1.101 --- src/bin/psql/print.c 14 May 2008 19:56:12 - *** *** 45,50 --- 45,52 /* Local functions */ static int strlen_max_width(unsigned char *str, int *target_width, int encoding); + static void IsPagerNeeded(const printTableContent *cont, const int extra_lines, + FILE **fout, bool *is_pager); static void * *** *** 394,400 * Print pretty boxes around cells. */ static void ! print_aligned_text(const printTableContent *cont, bool is_pager, FILE *fout) { bool opt_tuples_only = cont-opt-tuples_only; bool opt_numeric_locale = cont-opt-numericLocale; --- 396,402 * Print pretty boxes around cells. */ static void ! print_aligned_text(const printTableContent *cont, FILE *fout) { bool opt_tuples_only = cont-opt-tuples_only; bool opt_numeric_locale = cont-opt-numericLocale; *** *** 416,421 --- 418,425 unsigned char **format_buf; unsigned int width_total; unsigned int total_header_width; + unsigned int extra_row_output_lines = 0; + unsigned int extra_output_lines = 0; const char * const *ptr; *** *** 424,429 --- 428,434 bool *header_done; /* Have all header lines been output? */ int *bytes_output; /* Bytes output for column value */ int output_columns = 0; /* Width of interactive console
Re: [PATCHES] Patch to change psql default banner v6
Joshua D. Drake wrote: On Wed, 23 Apr 2008 14:41:20 -0700 Joshua D. Drake [EMAIL PROTECTED] wrote: Hello, Per final discussion here: http://archives.postgresql.org/pgsql-hackers/2008-04/msg01607.php I have looked over this patch and made a few adjustments. You used for a help startup banner: Type: help for help. I feel this has too much indirection because 'help' then produced: You are using psql, the command-line interface to PostgreSQL. \h or \\help for SQL help. \? for psql help. \q to quit psql. \copyright to view the copyright. Because \? now has \h, \q, and \copyright alone at the top I think we should just use: $ psql test psql (8.4devel) Type \? for help. test= If you type 'help' it just repeats the startup banner suggestion: test= help You are using psql, the command-line interface to PostgreSQL. Type \? for help. test= I think that consistency will be clearer. In the past we were trying to avoid \?, but I think now it is clean enough to be used by new people without confusion. I also put the version number in parentheses so it wouldn't be as prominent. Updated patch attached. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/help.c === RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.127 diff -c -c -r1.127 help.c *** src/bin/psql/help.c 14 May 2008 15:30:22 - 1.127 --- src/bin/psql/help.c 14 May 2008 23:09:06 - *** *** 176,182 fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); ! fprintf(output, _( \\g [FILE] send query buffer to server (and results to file or |pipe)\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE --- 176,182 fprintf(output, _(Query Buffer\n)); fprintf(output, _( \\e [FILE] edit the query buffer (or file) with external editor\n)); ! fprintf(output, _( \\g [FILE] or ; send query buffer to server (and results to file or |pipe)\n)); fprintf(output, _( \\p show the contents of the query buffer\n)); fprintf(output, _( \\r reset (clear) the query buffer\n)); #ifdef USE_READLINE Index: src/bin/psql/mainloop.c === RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.90 diff -c -c -r1.90 mainloop.c *** src/bin/psql/mainloop.c 5 Apr 2008 03:40:15 - 1.90 --- src/bin/psql/mainloop.c 14 May 2008 23:09:06 - *** *** 177,186 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(You are using psql, the command-line interface to PostgreSQL.)); ! puts(_(Enter SQL commands, or type \\? for a list of backslash options.)); ! puts(_(Use \\h for SQL command help.)); ! puts(_(Use \\q to quit.)); fflush(stdout); continue; } --- 177,185 (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4]))) { free(line); ! puts(_(\nYou are using psql, the command-line interface to PostgreSQL.)); ! puts(_(Type \\? for help.\n)); ! fflush(stdout); continue; } Index: src/bin/psql/startup.c === RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v retrieving revision 1.147 diff -c -c -r1.147 startup.c *** src/bin/psql/startup.c 8 May 2008 17:04:26 - 1.147 --- src/bin/psql/startup.c 14 May 2008 23:09:06 - *** *** 317,342 server_version = server_ver_str; } ! printf(_(Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n), ! pset.progname, PG_VERSION, server_version); } else ! printf(_(Welcome to %s %s, the PostgreSQL interactive terminal.\n\n), ! pset.progname, PG_VERSION); ! ! printf(_(Type: \\copyright for distribution terms\n ! \\h for help with SQL commands\n ! \\? for help with psql commands\n ! \\g or terminate with semicolon to execute query\n ! \\q to quit\n\n)); if (pset.sversion / 100 != client_ver / 100) ! printf(_(WARNING: You are connected to a server with major version %d.%d,\n ! but your %s client is major version %d.%d. Some backslash commands,\n ! such as \\d, might not work properly.\n\n), ! pset.sversion / 1, (pset.sversion / 100) % 100, ! pset.progname, ! client_ver
Re: [PATCHES] Patch to change psql default banner v6
I think that consistency will be clearer. In the past we were trying to avoid \?, but I think now it is clean enough to be used by new people without confusion. I also put the version number in parentheses so it wouldn't be as prominent. Updated patch attached. FYI, after the patch is applied I will update psql banner examples in our documentation. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Alvaro Herrera wrote: Bruce Momjian wrote: If you type 'help' it just repeats the startup banner suggestion: test= help You are using psql, the command-line interface to PostgreSQL. Type \? for help. I think we wanted to have more information in 'help', not less. Making it just repeat the startup info is not very helpful. I thought about that, but aren't we just repeating the top of \?. Is that helpful? Should we just display \?. I know we decided not to do that, but I am trying to figure out what the goal if 'help' is? To display the most frequently-used help commands? Aren't they at the top of \?. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Alvaro Herrera wrote: Bruce Momjian wrote: I know we decided not to do that, but I am trying to figure out what the goal if 'help' is? To display the most frequently-used help commands? Aren't they at the top of \?. The purpose of 'help' is to provide useful help. If it only says see \? then it's just redirecting you somewhere else, which isn't useful. I don't think the various help commands need to be completely orthogonal. If you agree with that, then making 'help' repeat part of what \? says is acceptable. (Of course, the idea is not just to repeat, but also to provide useful advice to the unwary.) OK. Remember, the people who is going to type 'help' is not the 10-year-Pg- experience types. It's the newbies. The larger issue is whether we want to advertise only help in the startup banner. The patch has just: Type: help for help. Now, aside from being confusing (we need quotes around help), I thought we should only mention \?. My question is whether we agreed that suggesting help as the best way to get help was what we agreed upon? If we did, I forgot. I thought the 'help' ideas was just for people who forgot the help commands. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Alvaro Herrera wrote: Bruce Momjian wrote: My question is whether we agreed that suggesting help as the best way to get help was what we agreed upon? If we did, I forgot. I thought the 'help' ideas was just for people who forgot the help commands. Please review the previous discussion: http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop OK, I just read the thread and saw no one say we should be promoting _only_ 'help' in the startup banner. Where is that email discussion? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Joshua D. Drake wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: My question is whether we agreed that suggesting help as the best way to get help was what we agreed upon? If we did, I forgot. I thought the 'help' ideas was just for people who forgot the help commands. Please review the previous discussion: http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop OK, I just read the thread and saw no one say we should be promoting _only_ 'help' in the startup banner. Where is that email discussion? http://archives.postgresql.org/pgsql-hackers/2008-04/msg01476.php And most specifically: http://archives.postgresql.org/pgsql-hackers/2008-04/msg01376.php Ah, OK. I had forgotten. Here is the new output: $ sql test psql (8.4devel) Type help for help. test= help You are using psql, the command-line interface to PostgreSQL. \h or \help for SQL help \? for psql help \q to quit psql \copyright to view the copyright test= \? General \copyright show PostgreSQL usage and distribution terms \g [FILE] or ; send query buffer to server (and results to file or |pipe) \h [NAME] help on syntax of SQL commands, * for all commands \q quit psql Query Buffer \e [FILE] edit the query buffer (or file) with external editor ... I moved '\g' up into the General section rather than make it a single-entry section. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Joshua D. Drake wrote: Bruce Momjian wrote: test= \? General \copyright show PostgreSQL usage and distribution terms \g [FILE] or ; send query buffer to server (and results to file or |pipe) \h [NAME] help on syntax of SQL commands, * for all commands \q quit psql Query Buffer \e [FILE] edit the query buffer (or file) with external editor ... I moved '\g' up into the General section rather than make it a single-entry section. send query buffer to server means nothing to a newbie. You execute queries, you don't send buffers (from a user perspective). Yep, good, updated: General \copyright show PostgreSQL usage and distribution terms \g [FILE] or ; execute query (and send results to file or |pipe) \h [NAME] help on syntax of SQL commands, * for all commands \q quit psql -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Replace offnum++ by OffsetNumberNext
Patch applied. Thanks. --- Fujii Masao wrote: This is the patch replace offnum++ by OffsetNumberNext. According to off.h, OffsetNumberNext is the macro prepared to disambiguate the different manipulations on OffsetNumbers. But, increment operator was used in some places instead of the macro. -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center TEL (03)5860-5115 FAX (03)5463-5490 ? patch.diff Index: src/backend/access/heap/pruneheap.c === RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/pruneheap.c,v retrieving revision 1.9 diff -c -r1.9 pruneheap.c *** src/backend/access/heap/pruneheap.c 26 Mar 2008 21:10:37 - 1.9 --- src/backend/access/heap/pruneheap.c 4 Apr 2008 14:34:19 - *** *** 789,795 MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber)); maxoff = PageGetMaxOffsetNumber(page); ! for (offnum = FirstOffsetNumber; offnum = maxoff; offnum++) { ItemId lp = PageGetItemId(page, offnum); HeapTupleHeader htup; --- 789,795 MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber)); maxoff = PageGetMaxOffsetNumber(page); ! for (offnum = FirstOffsetNumber; offnum = maxoff; offnum = OffsetNumberNext(offnum)) { ItemId lp = PageGetItemId(page, offnum); HeapTupleHeader htup; Index: src/backend/executor/nodeBitmapHeapscan.c === RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeBitmapHeapscan.c,v retrieving revision 1.25 diff -c -r1.25 nodeBitmapHeapscan.c *** src/backend/executor/nodeBitmapHeapscan.c 26 Mar 2008 21:10:38 - 1.25 --- src/backend/executor/nodeBitmapHeapscan.c 4 Apr 2008 14:34:19 - *** *** 301,307 OffsetNumber maxoff = PageGetMaxOffsetNumber(dp); OffsetNumber offnum; ! for (offnum = FirstOffsetNumber; offnum = maxoff; offnum++) { ItemId lp; HeapTupleData loctup; --- 301,307 OffsetNumber maxoff = PageGetMaxOffsetNumber(dp); OffsetNumber offnum; ! for (offnum = FirstOffsetNumber; offnum = maxoff; offnum = OffsetNumberNext(offnum)) { ItemId lp; HeapTupleData loctup; Index: src/backend/storage/page/bufpage.c === RCS file: /projects/cvsroot/pgsql/src/backend/storage/page/bufpage.c,v retrieving revision 1.78 diff -c -r1.78 bufpage.c *** src/backend/storage/page/bufpage.c10 Feb 2008 20:39:08 - 1.78 --- src/backend/storage/page/bufpage.c4 Apr 2008 14:34:19 - *** *** 533,539 * Since this is just a hint, we must confirm that there is * indeed a free line pointer */ ! for (offnum = FirstOffsetNumber; offnum = nline; offnum++) { ItemId lp = PageGetItemId(page, offnum); --- 533,539 * Since this is just a hint, we must confirm that there is * indeed a free line pointer */ ! for (offnum = FirstOffsetNumber; offnum = nline; offnum = OffsetNumberNext(offnum)) { ItemId lp = PageGetItemId(page, offnum); *** *** 736,742 totallen = 0; nused = 0; nextitm = 0; ! for (offnum = 1; offnum = nline; offnum++) { lp = PageGetItemId(page, offnum); Assert(ItemIdHasStorage(lp)); --- 736,742 totallen = 0; nused = 0; nextitm = 0; ! for (offnum = FirstOffsetNumber; offnum = nline; offnum = OffsetNumberNext(offnum)) { lp = PageGetItemId(page, offnum); Assert(ItemIdHasStorage(lp)); -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org
Re: [PATCHES] libpq object hooks
Andrew Dunstan wrote: The first thing it needs is lots and lots of documentation. I think it probably needs a Section in the libpq chapter all on its own, preferably with some examples. I think that lack alone is enough to keep it from being committed for now. Second, the hook names are compared case insensitively and by linear search. I don't see any justification for using case insensitive names for hooks in a C program, so I think that part should go. And if we expect to keep anything other than trivial numbers of hooks we should look at some sort of binary or hashed search. The thing that is a bit disturbing is that the whole style of this scheme is very different from the fairly simple APIs that the rest of libpq presents. It's going to make libpq look rather odd, I think. I would have felt happier if the authors had been able to come up with a simple scheme to add API calls to export whatever information they needed, rather than using this callback scheme. My personal opinion is still that I would like to see a more general usefulness for these functions before adding them to libpq. The complexity of the API just mirrors my gut feeling on this. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [HACKERS] [PATCHES] [NOVICE] encoding problems
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Guillaume Smet wrote: I understand your point of view but I really think it's more a regression fix than a behavior change. If I can get other hackers to say we should backpatch we can consider it. Well, 8.3 is already different from 8.2, and a lot of people will see this particular aspect of it as a regression. I'm okay with backpatching to 8.3 ... though the patch needed rather more testing than you gave it. OK, so Alvaro and Tom want this backpatched. However, it isn't going to match 8.2 behavior --- is that OK? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Replace offnum++ by OffsetNumberNext
Tom Lane wrote: Fujii Masao [EMAIL PROTECTED] writes: This is the patch replace offnum++ by OffsetNumberNext. According to off.h, OffsetNumberNext is the macro prepared to disambiguate the different manipulations on OffsetNumbers. But, increment operator was used in some places instead of the macro. I wonder if we shouldn't go the other way, ie, use ++ everywhere. OffsetNumberNext seems like just useless obscurantism ... The only value I saw to OffsetNumberNext was the fact is does int16 increment, with casting. There is also the comment: * Increments/decrements the argument. These macros look pointless * but they help us disambiguate the different manipulations on * OffsetNumbers (e.g., sometimes we subtract one from an * OffsetNumber to move back, and sometimes we do so to form a * real C array index). -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [PATCH] sh: Add support Renesas SuperH
Where are we on this? --- Tom Lane wrote: I wrote: Nobuhiro Iwamatsu [EMAIL PROTECTED] writes: +#if defined(__sh__) /* Renesas SuperH */ Do they have any longer form of that macro? I looked into the gcc sources, and the answer seems to be no :-(. So we're stuck with __sh__. I'm still pretty skeptical about the adequacy of the asm parameters, though. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Database owner installable modules patch
looking for modules under some module path for e.g. /usr/local module installs - Convert existing contrib to modules where appropriate :) - I really have no idea what happens if non-ascii characters are in an install script at the moment. What happens if funky characters are passed to an SPI_execute call? Very far future: - Have pgxs auto-generate rpm .spec files for modules, plus e.g. .deb equivalent, wix files for windows etc etc. - Versioning on modules? General discussion: I see this work as orthogonal to both the CPAN-style distribution / repository discussion, and the fate-of-contrib discussion. If contrib modules are reworked as this sort of module and left in the distribution, they'll be easier to use and more likely to be installed than they are now. If, as Tom suggested, they're mostly moved out of the pgsql source tree and to e.g. pgfoundry or whatever, this mechanism should make them (and every other extension out there) easy to package, install and enable in a user's database. Similarly, I don't personally care for a CPAN-style distribution setup - on my preferred unix-like system I use yum and on windows I prefer installers. Nonetheless, a standardised system to install and enable/disable modules acts as an enabler for all packaging and distribution systems. I'm not sure about the command names - there was already a tendency in the recent discussion to mix the notion of installation of code on the filesystem versus installation into a particular user's database. The convention for doing stuff in a db is CREATE/DROP, but CREATE MODULE doesn't feel right to me, just as I don't really like CREATE LANGUAGE. How about ENABLE/DISABLE MODULE? Makes it clear that the module is installed, it's just not available in this database yet. Thoughts? Anyway, discussion and feedback hereby solicited! Cheers Tom [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq thread-locking
Magnus Hagander wrote: Attached patch adds some error checking to the thread locking stuff in libpq. Previously, if thread locking failed for some reason, we would just fall through and do things without locking. This patch makes us abort() instead. It's not the greatest thing probably, but our API doesn't let us pass back return values... I have looked over the patch and it seems fine, though I am concerned about the abort() case with no output. I realize stderr might be going nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures so for consistency I think we should do the same here. If there is concern about code bloat, I suggest a macro at the top of the file for thread failure exits: #define THEAD_FAILURE(str) \ do { \ fprintf(stderr, libpq_gettext(Thread failure: %s\n)); \ exit(1); \ } while(0) -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq thread-locking
Bruce Momjian wrote: Magnus Hagander wrote: Attached patch adds some error checking to the thread locking stuff in libpq. Previously, if thread locking failed for some reason, we would just fall through and do things without locking. This patch makes us abort() instead. It's not the greatest thing probably, but our API doesn't let us pass back return values... I have looked over the patch and it seems fine, though I am concerned about the abort() case with no output. I realize stderr might be going nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures so for consistency I think we should do the same here. If there is concern about code bloat, I suggest a macro at the top of the file for thread failure exits: #define THEAD_FAILURE(str) \ do { \ fprintf(stderr, libpq_gettext(Thread failure: %s\n)); \ exit(1); \ } while(0) Oh, this is Tom saying he doesn't like stderr and the added code lines for failure: http://archives.postgresql.org/pgsql-patches/2008-04/msg00254.php I think the macro and consistency suggest doing as I outlined. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Applied. --- Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Surely psql computes the width of all cells before printing anything. It does, but if you have a value that has a tab, how do you know what tab stop you are on because you don't know the final width of the previous columns at that time, so there is no way to know the width of that cell. My point is that you don't need to align the tabstops with the start of the line, but with the start of the _column_. So the width of the previous column doesn't matter. Alvaro, using spaces instead of the terminal hard tabs was a very good idea. The output is now: test= \x Expanded display is on. test= \df+ xx List of functions -[ RECORD 1 ]---+ Schema | public Name| xx Result data type| text Argument data types | Volatility | volatile Owner | postgres Language| sql Source code | SELECT 'a'::text : WHERE 1 = 1 Description | Patch attached. It substitutes spaces for the tab. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] bug in numeric_power() function
Applied. --- Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I have developed the attached patch which fixes 0 ^ 123.3. Did you actually read the wikipedia entry you cited? But that's about 0^0, not about 0^123.3. See this other subsection: http://en.wikipedia.org/wiki/Exponentiation#Powers_of_zero 0^123.3 is 0, not 1. Ah, got it, and I updated the patch to remove the commment about discrete. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Guillaume Smet wrote: On Thu, May 8, 2008 at 9:11 PM, Bruce Momjian [EMAIL PROTECTED] wrote: Applied. As I mentioned it before, is there any chance for this fix to be backported to 8.3 branch? IMHO it's a usability regression. No, we don't change behaviors in back branches unless we get lots of complaints, and we haven't in this case. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Guillaume Smet wrote: On Fri, May 9, 2008 at 4:38 AM, Bruce Momjian [EMAIL PROTECTED] wrote: No, we don't change behaviors in back branches unless we get lots of complaints, and we haven't in this case. I suspect it's annoying for a lot of people, just not annoying enough to make them complain about it. I understand your point of view but I really think it's more a regression fix than a behavior change. That said, if nobody is following me on this one, I'll live with it - it's just annoying, not blocking. If I can get other hackers to say we should backpatch we can consider it. Let me add though that as the patch is coded it is not the same as 8.2, but better, so it is hard to say we should actually improve 8.3 over 8.2 in a minor release. As you can see 8.3.X behavior will not match 8.3 and that might be worse than just keeping it constant until 8.4. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Cliff Nieuwenhuis wrote: On Tuesday 11 March 2008 11:41:35 Tom Lane wrote: Cliff Nieuwenhuis [EMAIL PROTECTED] writes: I'm not sure how to ask this question. I have written a function, and with PostgreSQL 8.0.13 I can do a \df+ and see something like this under Source Code: DECLARE result text; ... If I create the same function on my computer running PostgreSQL 8.3.0 and try the \df+ then the Source Code shows: \x09DECLARE \x09\x09result text; ... That's not an encoding problem, that's an intentional behavioral change in the way that psql formats strings for display. I guess it's a bit annoying if you were hoping that tabs would be useful for pretty-printing purposes. Should we reconsider what's done with a tab in mbprint.c? regards, tom lane My vote would be to go back to the old way, or at least have that as an option of some sort. I use command-line psql all the time -- to me, psql offers the same advantages as using a command-line interface for other work. I find the extra characters really get in the way. Yes, I think our psql display of tabs needs improving too. Our current behavior is to output tab as 0x09: test= SELECT E'\011'; ?column? -- \x09 (1 row) test= CREATE FUNCTION xx() RETURNS text AS E' test' SELECT ''a''::text test' WHERE1 = 1' test- LANGUAGE SQL; CREATE FUNCTION test= SELECT prosrc FROM pg_proc WHERE proname = 'xx'; prosrc - SELECT\x09'a'::text WHERE\x091 = 1 (1 row) test= \x Expanded display is on. test= \df+ xx List of functions -[ RECORD 1 ]---+ Schema | public Name| xx Result data type| text Argument data types | Volatility | volatile Owner | postgres Language| sql Source code | : SELECT\x09'a'::text : WHERE\x091 = 1 Description | I have implemented the following patch which outputs tab as a tab. It also assumes a tab has a width of 4, which is its average width: test= SELECT E'\011'; ?column? -- (1 row) test= SELECT prosrc FROM pg_proc WHERE proname = 'xx'; prosrc - SELECT 'a'::text WHERE 1 = 1 (1 row) test= \x Expanded display is on. test= \df+ xx List of functions -[ RECORD 1 ]---+ Schema | public Name| xx Result data type| text Argument data types | Volatility | volatile Owner | postgres Language| sql Source code | : SELECT'a'::text : WHERE 1 = 1 Description | The only downside I see for this patch is that we are never sure of the display width of tab because we don't know what tab stop we are at. With '\x09' we knew exactly how wide it was. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/mbprint.c === RCS file: /cvsroot/pgsql/src/bin/psql/mbprint.c,v retrieving revision 1.30 diff -c -c -r1.30 mbprint.c *** src/bin/psql/mbprint.c 16 Apr 2008 18:18:00 - 1.30 --- src/bin/psql/mbprint.c 7 May 2008 15:18:25 - *** *** 315,320 --- 315,330 linewidth += 2; ptr += 2; } + else if (*pwcs == '\t') /* Tab */ + { + strcpy((char *) ptr, \t); + /* + * We don't know what tab stop we are on, so assuming 8-space + * tabs, the average width of a tab is 4. + */ + linewidth += 4; + ptr += 1; + } else if (w 0) /* Other control char */ { sprintf((char *) ptr, \\x%02X, *pwcs); -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I have implemented the following patch which outputs tab as a tab. It also assumes a tab has a width of 4, which is its average width: That pretty much completely sucks; it will undo all the hard work we've put into nice formatting of the output, because seven times out of eight this assumption is wrong. An actually acceptable solution would involve emitting the correct number of spaces depending on how much we've put out so far. Even if we knew the column position at output time, when we are doing aligned column width computations, we don't know the width of the previous columns so we would have no way to know how far the tab would extend in the current column. The only other idea I have is to output four spaces rather than '\x09' for a tab. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Alvaro Herrera wrote: Bruce Momjian wrote: Even if we knew the column position at output time, when we are doing aligned column width computations, we don't know the width of the previous columns so we would have no way to know how far the tab would extend in the current column. If you start counting every line from the start of the current column, it will align correctly regardless of the previous columns. At this stage you don't know the width of previous columns because you don't know if a very wide value is coming in a later row, so there is no way to output the width of the cell with a tab you are looking at now. Unless I am misunderstanding you. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: If you start counting every line from the start of the current column, it will align correctly regardless of the previous columns. At this stage you don't know the width of previous columns because you don't know if a very wide value is coming in a later row, so there is no way to output the width of the cell with a tab you are looking at now. Unless I am misunderstanding you. Surely psql computes the width of all cells before printing anything. It does, but if you have a value that has a tab, how do you know what tab stop you are on because you don't know the final width of the previous columns at that time, so there is no way to know the width of that cell. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Surely psql computes the width of all cells before printing anything. It does, but if you have a value that has a tab, how do you know what tab stop you are on because you don't know the final width of the previous columns at that time, so there is no way to know the width of that cell. My point is that you don't need to align the tabstops with the start of the line, but with the start of the _column_. So the width of the previous column doesn't matter. OK, so I am not really using tabs in the output, but outputting the proper number of spaces to make it look like a tab? That works. Let me try it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] bug in numeric_power() function
Richard Wang wrote: I expected 0 ^ 123.3 to be 0, but it reported error as follows postgres=# select 0 ^ 123.3; ERROR: cannot take logarithm of zero I find that there is a bug in numeric_power() function the function caculates a ^ b based on the algorithm e ^ (lna * b) as you see, ln0 is not valid I have developed the attached patch which fixes 0 ^ 123.3. It also fixes the case for 0 ^ 0.0 so it returns 1 instead of an error --- see the C comment for why one is the proper return value. float pow() already returned one in this case: test= select 0 ^ 0; ?column? -- 1 (1 row) test= select 0 ^ 0.0; ?column? -- 1 (1 row) test= select 0 ^ 3.4; ?column? -- 1 (1 row) -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/numeric.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v retrieving revision 1.110 diff -c -c -r1.110 numeric.c *** src/backend/utils/adt/numeric.c 21 Apr 2008 00:26:45 - 1.110 --- src/backend/utils/adt/numeric.c 7 May 2008 20:05:01 - *** *** 5170,5175 --- 5170,5187 int local_rscale; double val; + /* + * This avoids log(0) for cases of 0 raised to a non-integer. + * We also treat 0 ^ 0 == 1 because it is the best value for discrete + * mathematics, and most programming languages do this. + * http://en.wikipedia.org/wiki/Exponentiation#Zero_to_the_zero_power + */ + if (cmp_var(base, const_zero) == 0) + { + set_var_from_var(const_one, result); + return; + } + /* If exp can be represented as an integer, use power_var_int */ if (exp-ndigits == 0 || exp-ndigits = exp-weight + 1) { *** *** 5266,5280 NumericVar base_prod; int local_rscale; - /* Detect some special cases, particularly 0^0. */ - switch (exp) { case 0: - if (base-ndigits == 0) - ereport(ERROR, - (errcode(ERRCODE_FLOATING_POINT_EXCEPTION), - errmsg(zero raised to zero is undefined))); set_var_from_var(const_one, result); result-dscale = rscale; /* no need to round */ return; --- 5278,5286 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Surely psql computes the width of all cells before printing anything. It does, but if you have a value that has a tab, how do you know what tab stop you are on because you don't know the final width of the previous columns at that time, so there is no way to know the width of that cell. My point is that you don't need to align the tabstops with the start of the line, but with the start of the _column_. So the width of the previous column doesn't matter. Alvaro, using spaces instead of the terminal hard tabs was a very good idea. The output is now: test= \x Expanded display is on. test= \df+ xx List of functions -[ RECORD 1 ]---+ Schema | public Name| xx Result data type| text Argument data types | Volatility | volatile Owner | postgres Language| sql Source code | SELECT 'a'::text : WHERE 1 = 1 Description | Patch attached. It substitutes spaces for the tab. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/mbprint.c === RCS file: /cvsroot/pgsql/src/bin/psql/mbprint.c,v retrieving revision 1.30 diff -c -c -r1.30 mbprint.c *** src/bin/psql/mbprint.c 16 Apr 2008 18:18:00 - 1.30 --- src/bin/psql/mbprint.c 7 May 2008 20:27:39 - *** *** 315,320 --- 315,328 linewidth += 2; ptr += 2; } + else if (*pwcs == '\t') /* Tab */ + { + do + { + *ptr++ = ' '; + linewidth++; + } while (linewidth % 8 != 0); + } else if (w 0) /* Other control char */ { sprintf((char *) ptr, \\x%02X, *pwcs); -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Tom Lane wrote: Greg Smith [EMAIL PROTECTED] writes: On Fri, 14 Mar 2008, Tom Lane wrote: Yeah, -s is only meaningful when given with -i. Maybe someday we ought to fix pgbench to complain if you try to set it at other times. You have to pass -s in to the actual run if you're specifying your own custom script(s) using -f and you want the :scale variable to be defined. Right, I knew that at one time ;-) The way the option parsing code is done would make complaining in the case where your parameter is ignored a bit of a contortion. The part that detects based on the database is after all the other parsing because the connection has to be brought up first. Yeah. But couldn't we have that part issue a warning if -s had been set on the command line? Patch attached that issues a warning. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: contrib/pgbench/pgbench.c === RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v retrieving revision 1.79 diff -c -c -r1.79 pgbench.c *** contrib/pgbench/pgbench.c 19 Mar 2008 03:33:21 - 1.79 --- contrib/pgbench/pgbench.c 7 May 2008 21:36:42 - *** *** 1627,1632 --- 1627,1635 } } + if (!is_init_mode scale) + fprintf(stderr, Scale specification ignored because init mode (-i) not specified\n); + if (argc optind) dbName = argv[optind]; else -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] bug in numeric_power() function
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I have developed the attached patch which fixes 0 ^ 123.3. Did you actually read the wikipedia entry you cited? Yes: The evaluation of 0^0 presents a problem, because different mathematical reasoning leads to different results. The best choice for its value depends on the context. According to Benson (1999), The choice whether to define 00 is based on convenience, not on correctness.[2] There are two principal treatments in practice, one from discrete mathematics and the other from analysis. ... The computer programming languages that evaluate 00 to be 1[8] include J, Java, Python, Ruby, Haskell, ML, Scheme, MATLAB, bc, R programming language, and Microsoft Windows' Calculator. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] bug in numeric_power() function
Alvaro Herrera wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I have developed the attached patch which fixes 0 ^ 123.3. Did you actually read the wikipedia entry you cited? But that's about 0^0, not about 0^123.3. See this other subsection: http://en.wikipedia.org/wiki/Exponentiation#Powers_of_zero 0^123.3 is 0, not 1. Ah, got it, and I updated the patch to remove the commment about discrete. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/numeric.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v retrieving revision 1.110 diff -c -c -r1.110 numeric.c *** src/backend/utils/adt/numeric.c 21 Apr 2008 00:26:45 - 1.110 --- src/backend/utils/adt/numeric.c 7 May 2008 23:18:31 - *** *** 5170,5175 --- 5170,5190 int local_rscale; double val; + /* + * This avoids log(0) for cases of 0 raised to a non-integer. + * Also, while 0 ^ 0 can be either 1 or indeterminate (error), we + * treat it as one because most programming languages do this. + * http://en.wikipedia.org/wiki/Exponentiation#Zero_to_the_zero_power + */ + if (cmp_var(base, const_zero) == 0) + { + if (cmp_var(exp, const_zero) == 0) + set_var_from_var(const_one, result); + else + set_var_from_var(const_zero, result); + return; + } + /* If exp can be represented as an integer, use power_var_int */ if (exp-ndigits == 0 || exp-ndigits = exp-weight + 1) { *** *** 5266,5280 NumericVar base_prod; int local_rscale; - /* Detect some special cases, particularly 0^0. */ - switch (exp) { case 0: - if (base-ndigits == 0) - ereport(ERROR, - (errcode(ERRCODE_FLOATING_POINT_EXCEPTION), - errmsg(zero raised to zero is undefined))); set_var_from_var(const_one, result); result-dscale = rscale; /* no need to round */ return; --- 5281,5289 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] bug in numeric_power() function
Alvaro Herrera wrote: Bruce Momjian wrote: Ah, got it, and I updated the patch to remove the commment about discrete. The page also says that 0^x is undefined when x is negative, not sure about that one but I don't see it in your patch. That one was already handled: test= select 0 ^ -1; ERROR: invalid argument for power function test= select 0 ^ -1.0; ERROR: invalid argument for power function -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] [GENERAL] psql \pset pager
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: If we're going to change it, we should make it match GUC's parse_bool, which has had some actual thought put into it. Good idea. Do I copy the C code into /psql or somehow share the function? Just copy it --- it's not large enough to be worth doing something like inventing a /port module for, and besides it's not clear that you want exactly the same API. Patch attached and applied. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/variables.c === RCS file: /cvsroot/pgsql/src/bin/psql/variables.c,v retrieving revision 1.28 diff -c -c -r1.28 variables.c *** src/bin/psql/variables.c 1 Jan 2008 19:45:56 - 1.28 --- src/bin/psql/variables.c 7 May 2008 02:10:55 - *** *** 48,68 return NULL; } bool ! ParseVariableBool(const char *val) { ! if (val == NULL) return false; /* not set - assume off */ - if (pg_strcasecmp(val, off) == 0) - return false; /* accept off or OFF as true */ ! /* ! * for backwards compatibility, anything except off or OFF is taken as ! * true ! */ return true; } /* * Read numeric variable, or defaultval if it is not set, or faultval if its * value is not a valid numeric string. If allowtrail is false, this will --- 48,95 return NULL; } + /* + * Try to interpret value as boolean value. Valid values are: true, + * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof. + */ bool ! ParseVariableBool(const char *value) { ! size_t len; ! ! if (value == NULL) return false; /* not set - assume off */ ! len = strlen(value); ! ! if (pg_strncasecmp(value, true, len) == 0) ! return true; ! else if (pg_strncasecmp(value, false, len) == 0) ! return false; ! else if (pg_strncasecmp(value, yes, len) == 0) ! return true; ! else if (pg_strncasecmp(value, no, len) == 0) ! return false; ! /* 'o' is not unique enough */ ! else if (pg_strncasecmp(value, on, (len 2 ? len : 2)) == 0) ! return true; ! else if (pg_strncasecmp(value, off, (len 2 ? len : 2)) == 0) ! return false; ! else if (pg_strcasecmp(value, 1) == 0) ! return true; ! else if (pg_strcasecmp(value, 0) == 0) ! return false; ! else ! { ! /* NULL is treated as false, so a non-matching value is 'true' */ ! psql_error(unrecognized boolean value; assuming \on\.\n); ! return true; ! } ! /* suppress compiler warning */ return true; } + /* * Read numeric variable, or defaultval if it is not set, or faultval if its * value is not a valid numeric string. If allowtrail is false, this will -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [BUGS] BUG #3975: tsearch2 index should not bomb out of 1Mb limit
Added to TODO: o Consider changing error to warning for strings larger than one megabyte http://archives.postgresql.org/pgsql-bugs/2008-02/msg00190.php http://archives.postgresql.org/pgsql-patches/2008-03/msg00062.php --- Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: I don't think that follows. A tsearch index is lossy anyway, so there's Uh, the index is lossy but I thought it was lossy in a way that just required additional heap accesses, not lossy in that it doesn't index everything. Sure it's lossy. It doesn't index stopwords, and it doesn't index the difference between various forms of a word (when the dictionaries reduce them to a common root). I am concerned a 1mb limit is too low though. Exactly why can't we have a higher limit? Is positional information that significant? That's pretty much exactly the point: it's not very significant, and it doesn't justify a total inability to index large documents. One thing we could do is index words that are past the limit but not store a position, or perhaps have the convention that the maximum position value means somewhere past here. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [HACKERS] Re: [PATCHES] a tsearch2 (8.2.4) dictionary that only filters out stopwords
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Added to TODO: * Allow text search dictionary to filter out only stop words http://archives.postgresql.org/pgsql-patches/2007-11/msg00081.php That's a poor description. I thought the TODO was something more like allow dictionaries to change the token that is passed on to later dictionaries. TODO updated as described. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Proposed patch - psql wraps at window width
Bryce Nesbitt wrote: Bruce Momjian wrote: I have updated the documentation for this patch. I consider it ready to apply. I think it is as close to a middle ground as we are going to get. Further adjustment will have to happen when we have more reports from the field. I heard a pretty compelling argument to make wrapped part of aligned, and thus I think the patch is ready to go only after adjusting the user-facing syntax: \pset border 2 \pset format aligned Output format is aligned, no wrapping. \pset format aligned autowrap Output format is aligned, with automatic wrapping to the window width for terminals. \pset format aligned 80 Output format is aligned, with a target width of 80 characters. \a Output format is unaligned, no wrapping. \a Output format is aligned, with a target width of 80 characters. If the wrapping code can't fit the column headings in the wrap width, it gives up and produces aligned output. To do otherwise is totally unreadable. Please give the patch a try, before complaining about this particular aspect of it. Uh, well, we can do that, though looking at the psql code \pset only wants two arguments. We would have to modify how \pset works. Also, I am afraid making wrapping part of aligned is overly complicating the API. For example, I specificially kept \pset columns rather than allowing a third argument to pset because it was starting to look too complicated to describe in the manual. I am not 100% sure myself so hopefully others will comment. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Proposed patch - psql wraps at window width
I have updated the documentation for this patch. I consider it ready to apply. I think it is as close to a middle ground as we are going to get. Further adjustment will have to happen when we have more reports from the field. --- Bruce Momjian wrote: I have moved this discussion to hackers in hopes of getting more feedback, and moved the patch to a static URL: ftp://momjian.us/pub/postgresql/mypatches/wrap This patch adds a new '\pset format wrapped' mode that wraps long values to fit the table on the user's screen, or in '\pset columns' columns in an output to file or pipe. The documentation additions are at the top of the patch. Sample: \pset format wrapped Output format is wrapped. \pset columns 70 Target column width for wrap format is 70. SELECT 1, 2, repeat('a', 80), repeat('b', 80), E'a\nb\nc', 1 FROM generate_series(1,2)\g ?column? | ?column? | repeat | repeat| ?column? | ?column? --+--++-+--+-- 1 |2 | aa | bbb | a|1 ; aa ; bbb : b ; aa ; bbb : c ; aa ; bbb ; aa ; bbb ; aa ; bbb ; aa ; bbb ; aa ; bbb 1 |2 | aa | bbb | a|1 ; aa ; bbb : b ; aa ; bbb : c ; aa ; bbb ; aa ; bbb ; aa ; bbb ; aa ; bbb ; aa ; bbb (2 rows) You will notice: o I have pulled up newline values to appear in the same rows as the wrapped text o Colons are used on the left for newline values o Semicolons are used on the left for wrapped values o There are no vertical bars for values that don't extend to the wrapped or newline rows. This is how our newline display has always worked so it was copied by the wrapped code o The left column has no indicator of wrapping or newlines because there is no left border We could use dashes to indicated wrapped values, but we don't. It would be nice if someone could do some multi-byte testing of this, particularly for characters that have a display width greater than one. I think this patch is ready for application. Should 'wrapped' be the default for certain operations, like \df? 'wrapped' mode is really good for a table that would fit the screen width except for a few wide values. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.199 diff -c -c -r1.199 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 30 Mar 2008 18:10:20 - 1.199 --- doc/src/sgml/ref/psql-ref.sgml 29 Apr 2008 01:24:44 - *** *** 1513,1519 listitem para Sets the output format to one of literalunaligned/literal, ! literalaligned/literal, literalhtml/literal, literallatex/literal, or literaltroff-ms/literal. Unique abbreviations are allowed. (That would mean one letter is enough.) --- 1513,1520 listitem para Sets the output format to one of literalunaligned/literal, ! literalaligned/literal, literalwrapped/literal, ! literalhtml/literal, literallatex/literal, or literaltroff-ms/literal. Unique abbreviations are allowed. (That would mean one letter is enough.) *** *** 1525,1531 is intended to create output that might be intended to be read in by other programs (tab-separated, comma-separated). quoteAligned/quote mode is the standard, human-readable, ! nicely formatted text output that is default
Re: [PATCHES] 64-bit CommandIds
So, is this an option we want for configure? --- Zoltan Boszormenyi wrote: Alvaro Herrera ?rta: Zoltan Boszormenyi wrote: attached is our patch against HEAD which enables extending CommandIds to 64-bit. This is for enabling long transactions that really do that much non-read-only work in one transaction. I think you should add a pg_control field and corresponding check, to avoid a 64bit-Cid postmaster to start on a 32bit-Cid data area and vice versa. I added the check but I needed to add it BEFORE checking for toast_max_chunk_size otherwise it complained about this more cryptic problem. I think it's cleaner to report this failure to know why toast_max_chunk_size != TOAST_MAX_CHUNK_SIZE. Best regards, Zolt?n B?sz?rm?nyi -- -- Zolt?n B?sz?rm?nyi Cybertec Sch?nig Sch?nig GmbH http://www.postgresql.at/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your Subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Sun Studio on Linux spinlock patch
So, is this a feature we want? --- Julius Stroffek wrote: Tom Lane wrote: This patch seems broken in a number of ways. Why are you removing -DLINUX_PROFILE, for example? Are you sure you don't need -D_GNU_SOURCE? And why add -DSUNOS4_CC, which is a Solaris-specific define (not that we seem to be using it anywhere anymore)? Do we really have to have a configure-time probe to detect this particular compiler? You are right, removing -DLINUX_PROFILE seems to break profiling on linux when compiled with sun studio. I am not quite sure about the desired usage of _GNU_SOURCE and SUNOS4_CC macros. I would not expect _GNU_SOURCE to be defined when compiling sources with Sun Studio. I am not quite sure why SUNOS4_CC was supposed to be defined at all for Solaris as well. There are already enough macros defined -- __sun is defined on Solaris by both Sun Studio and gcc and __SUNPRO_C is defined by Sun Studio on both Linux and Solaris. Should we then remove _GNU_SOURCE and SUNOS4_CC macro definitions from the build scripts since they are not used at all in the source code? Configure-time probe for sun studio is required to create tas.s link to the proper file - sunstudio_x86.s (or sunstudio_sparc.s). This is done during a run of a configure script based on settings for the platform. Since these settings may vary on the same platform based on the compiler we need to have a configure-time probe. But I guess the *real* question is why anyone would care ... what benefit is there to using Sun's compiler on Linux? Some tools bundled with sun studio might be used. I personally run into this when I wanted to debug postgres with sun studio ide and wanted to compile it first. It is based on netbeans, written in java so it needs a big enough memory, however it offers a great possibility to explore postgres internals during a query execution, etc. It is especially useful, if you do not know what you are interested in during a compilation. I am using this to step over join order search plugins. I mostly use Solaris for this but I switched to linux for a while. I wrote a blog with more details about this. http://blogs.sun.com/databases/entry/debugging_postgresql_backends_on_solaris There is also a screenshot showing how it looks in action http://mediacast.sun.com/users/%7Ejulo/media/pgss_debugging.png Also, there was some message a while back on pgsql-bugs from Len Zaifman requesting this as well. Cheers Julo -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] 64-bit CommandIds
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: So, is this an option we want for configure? I think the case for it got a whole lot weaker in 8.3, with lazy consumption of CIDs. If someone had tables big enough to make the 32-bit-CID limit still be a problem despite that fix, I'd think they'd not be very happy about adding another 4 bytes of tuple header overhead (or more likely 8 bytes, on the kind of machine where this patch would make some sense). I don't foresee many people paying that cost rather than breaking up their transactions. My feeling is we should avoid the extra complexity, at least till such time as we see whether there are still any real field complaints about this with 8.3. In any case the patch is broken by --enable-float8-byval, and would need some adjustments to play nice with that. Agreed. Let's see if we get requests for it in = 8.3 releases. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Patch to change psql default banner v6
Joshua D. Drake wrote: On Wed, 23 Apr 2008 14:41:20 -0700 Joshua D. Drake [EMAIL PROTECTED] wrote: Hello, Per final discussion here: http://archives.postgresql.org/pgsql-hackers/2008-04/msg01607.php Isn't this going to mean \g is listed twice? + fprintf(output, _(Execution\n)); + fprintf(output, _( \\g or ;execute query\n\n)); If you want I can look at reorganizing the \? help. I have a larger reorganization mind. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Proposed patch - psql wraps at window width
Bryce Nesbitt wrote: The newline handling code was, by far, the most complex part of this patch. While I think it would be nicer to filter up past the newline, I just don't see this as a common need. Large text fields with newlines are difficult to really view in psql anyway, and they are likely to be the longest columns in any given query. Bottom line: not worth messing with. OK, we can always return to it. I'm split on the new formatting style. It looks a lot less grid-like, which I might like once I get used to it (which will be a while, because I can't run my own patch in daily use, as my servers are too old). I use a version of the wrapping patch that I backported to postgres 8.1, which was prior to multibyte support, and much much simpler. What new formatting style are you referring to above? Did I modify what you sent as the original patch? I got this warning compiling: print.c:784: warning: pointer targets in passing argument 1 of ?mb_strlen_max_width? differ in signedness And I did have trouble applying the patch -- I had to manually give it the filename, and tell it to reverse the patch. OK, fixed. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Proposed patch - psql wraps at window width
Bryce Nesbitt wrote: 1) \pset columns XX should make it clear that's for file output only. OK, docs updated. 2) There's an extra space, which breaks \pset border 2 717c717 fputc(' ', fout);; --- fputc(' ', fout); 842c842 fputs( | , fout); --- fputs( |, f OK, got them fixed. 2) With \pset border 2, the far left border, for symmetry, should work like the middle borders. OK, how does it look now with this patch? 3) I'm getting bolder: how about having \pset format wrapped as the default? Any downsides? I think we are going to want it as the default for many psql informational commands, like \df. Not sure about a more general default. We were just discussing using \x as a default for wide output but it seems this wrap style is probably a better solution than switching for \x for wide columns (less distracting for the user and cleaner). That will have to be a separate discussion once we are done. Oh, I found a problem in my coding of the new wrap function I added. While I handled the case where a character might span multiple bytes, I assumed all characters had a display width of one. You can see from pg_wcsformat()'s use of PQdsplen() that this isn't always the case. I have modified the patch to properly use PQdsplen() but we are going to need multi-byte users to test this once we are done. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.199 diff -c -c -r1.199 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 30 Mar 2008 18:10:20 - 1.199 --- doc/src/sgml/ref/psql-ref.sgml 21 Apr 2008 15:27:55 - *** *** 1513,1519 listitem para Sets the output format to one of literalunaligned/literal, ! literalaligned/literal, literalhtml/literal, literallatex/literal, or literaltroff-ms/literal. Unique abbreviations are allowed. (That would mean one letter is enough.) --- 1513,1520 listitem para Sets the output format to one of literalunaligned/literal, ! literalaligned/literal, literalwrapped/literal, ! literalhtml/literal, literallatex/literal, or literaltroff-ms/literal. Unique abbreviations are allowed. (That would mean one letter is enough.) *** *** 1525,1532 is intended to create output that might be intended to be read in by other programs (tab-separated, comma-separated). quoteAligned/quote mode is the standard, human-readable, ! nicely formatted text output that is default. The ! quoteacronymHTML/acronym/quote and quoteLaTeX/quote modes put out tables that are intended to be included in documents using the respective mark-up language. They are not complete documents! (This might not be --- 1526,1535 is intended to create output that might be intended to be read in by other programs (tab-separated, comma-separated). quoteAligned/quote mode is the standard, human-readable, ! nicely formatted text output that is default. ! quoteWrapped/quote is like literalaligned/ but wraps ! the output to fit the detected screen width or literal\pset ! columns/. The quoteacronymHTML/acronym/quote and quoteLaTeX/quote modes put out tables that are intended to be included in documents using the respective mark-up language. They are not complete documents! (This might not be *** *** 1537,1542 --- 1540,1556 /varlistentry varlistentry + termliteralcolumns/literal/term + listitem + para + Controls the target width for the literalwrap/ format when + output is to a file, or the operating system does not support + screen width detection. + /para + /listitem + /varlistentry + + varlistentry termliteralborder/literal/term listitem para Index: src/bin/psql/command.c === RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v retrieving revision 1.186 diff -c -c -r1.186 command.c *** src/bin/psql/command.c 1 Jan 2008 19:45:55 - 1.186 --- src/bin/psql/command.c 21 Apr 2008 15:27:56 - *** *** 1526,1531 --- 1526,1534 case PRINT_ALIGNED
Re: [PATCHES] Testing pg_terminate_backend()
Magnus Hagander wrote: Done that. Also, I needed to replace gmake with make, since I'm on linux... Anyway. It's been running for about 12 hours now, and I have *nothing* in the output file. That tells me that the script doesn't appear to be working - correct? It should output *something* there, right? (It's obviously running, because I've got about 400,000 lines in nohup.out..) Argh. So here I am looking at it now for details, and it seems the script should be run from src/test/regress, and I ran it from the root directory.. Oops. Also, I needed to place psql in the path - it failed to find it, but hid the error message. Just a hint if someone else is running it ;-) Yea, basically you need to rewrite my script. :-( -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] float4/float8/int64 passed by value with tsearchfixup
Tom Lane wrote: Gregory Stark [EMAIL PROTECTED] writes: So are you killing V0 for non-integral types? Because if not we should keep some sacrificial module to the regression tests to use to test for this problem. Well, we could potentially continue to have, say, the oldstyle geo_distance function used when not FLOAT8PASSBYVAL. Not clear how useful that really is though. So is this whole float4/float8/int64 issue closed? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches