Re: [PATCHES] Proposed patch - psql wraps at window width

2008-04-20 Thread Bryce Nesbitt
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.


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.


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.



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

2008-04-20 Thread Bryce Nesbitt

1) \pset columns XX should make it clear that's for file output only.

2) There's an extra space, which breaks \pset border 2

717c717
   fputc(' ', fout);;
---
   fputc(' ', fout);
842c842
   fputs( | , fout);
---
   fputs( |, f

2) With \pset border 2, the far left border, for symmetry, should work 
like the middle borders.


3) I'm getting bolder: how about having \pset format wrapped as the 
default?  Any downsides?





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

2008-04-20 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 BTW, I trolled the contrib files for other v0 functions taking or
 returning float4 or float8.  I found seg_size (fixed it) and a whole
 bunch of functions in earthdistance.  Those use float8 not float4,
 so they are not broken by this patch, but that module will have to
 be v1-ified before we can consider applying the other part of the
 patch.

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.

I don't see any way not to kill v0 for non-integral types if we want to make
float4 and float8 pass-by-value. We could leave those pass-by-reference and
just make bigint pass-by-value.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

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

2008-04-20 Thread Zoltan Boszormenyi

Gregory Stark írta:

Tom Lane [EMAIL PROTECTED] writes:

  

BTW, I trolled the contrib files for other v0 functions taking or
returning float4 or float8.  I found seg_size (fixed it) and a whole
bunch of functions in earthdistance.  Those use float8 not float4,
so they are not broken by this patch, but that module will have to
be v1-ified before we can consider applying the other part of the
patch.



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.

I don't see any way not to kill v0 for non-integral types if we want to make
float4 and float8 pass-by-value. We could leave those pass-by-reference and
just make bigint pass-by-value.
  


Just a note: time[stamp[tz]] also depend on either float8 or int64 and
they have to be the same pass-by-value or pass-by-reference  as their
base storage types. There were crashes or regression failures if not.

--
--
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://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] float4/float8/int64 passed by value with tsearchfixup

2008-04-20 Thread Tom Lane
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.

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


Re: [PATCHES] Coding standards

2008-04-20 Thread Peter Eisentraut
Alvaro Herrera wrote:
 The developer's FAQ is supposed to contain this kind of thing, but I
 think it's rather thin on actual details.  (Some time ago it was
 proposed to create a style guide, but people here thought it was a
 waste of time and it will not cover what's really important, so we're
 stuck with repeating the advice over and over.)

Excellent wiki material ...

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

2008-04-20 Thread Tom Lane
Zoltan Boszormenyi [EMAIL PROTECTED] writes:
 I tried to split the previous patch up to see where the tsearch regression
 comes from. So, it turned out that:
 - float4 conversion is risk free (patch #1)
 - float8 conversion is okay, too, if coupled with time[stamp[tz]] conversion
  (patch #2) but with int64 timestamps enabled, the next one is also 
 needed:
 - int64 conversion (patch #3) is mostly okay but it is the one that's 
 causing
   the tsearch regression

Applied with revisions --- mostly, supporting configure-time control
over whether pass-by-value is used.

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


Re: [PATCHES] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1

2008-04-20 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 4) Your problems with tsearch and timestamp etc raise an interesting problem.
We don't need to mark this in pg_control because it's a purely a run-time
issue and doesn't affect on-disk storage.

Just for the record, that's wrong.  It's true that on-disk data isn't
affected, but the system catalog contents are, and they'd better match
what the postgres binary is going to do.

However it does affect ABI
compatibility with modules. Perhaps it should be added to
PG_MODULE_MAGIC_DATA. 

Very good point, especially now that it's configurable.  Included
in committed patch.

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


Re: [PATCHES] float4/float8/int64 passed by value with tsearch fixup

2008-04-20 Thread Guillaume Smet
On Mon, Apr 21, 2008 at 2:28 AM, Tom Lane [EMAIL PROTECTED] wrote:
  Applied with revisions --- mostly, supporting configure-time control
  over whether pass-by-value is used.

Do we need buildfarm coverage of these options?

-- 
Guillaume

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

2008-04-20 Thread Tom Lane
Guillaume Smet [EMAIL PROTECTED] writes:
 On Mon, Apr 21, 2008 at 2:28 AM, Tom Lane [EMAIL PROTECTED] wrote:
 Applied with revisions --- mostly, supporting configure-time control
 over whether pass-by-value is used.

 Do we need buildfarm coverage of these options?

Wouldn't hurt, I guess, though it's not very urgent since the
non-default cases were default up till these patches.

Note that we are already exercising both ends of the
float8-byval option, so probably only --disable-float4-byval
is very interesting to test explicitly.

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