Обсуждение: wrapping in extended mode doesn't work well with default pager

Pavel Stehule <pavel.stehule@gmail.com>:
> Hello
>
> I am checking feature
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=6513633b94173fc1d9e2b213c43f9422ddbf5faa
>
> It works perfect with pager "less", but it works badly with default "more"
>
> see attached screenshots, pls
>
> It is expected behave?

I do not think so. It looks broken with or without any pager when
border != 2. Your less configuration might be hiding the problem from you.

I think it is because of miscalculation of the width used by
the separators. Increasing this variable for border = 0 and 1 fixed
the problem, but it might not be the right fix. The patch without
regression test changes attached.

While looking at it, I found another problem. It seems to me, a minus sign
is missing after -[RECORD  ] when border = 1.

Emre Hasegeli <emre@hasegeli.com> writes:
> Pavel Stehule <pavel.stehule@gmail.com>:
>> I am checking feature
>> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=6513633b94173fc1d9e2b213c43f9422ddbf5faa
>> 
>> It works perfect with pager "less", but it works badly with default "more"

> I do not think so. It looks broken with or without any pager when
> border != 2. Your less configuration might be hiding the problem from you.

This seems broken in several ways.  I tried this test case:

regression=# \x \pset format wrapped
Expanded display (expanded) is on.
Output format (format) is wrapped.
regression=# select * from pg_proc where prolang!=12;

In 9.3, the output looks like this:

-[ RECORD 1 ]---+---------------------------------------------------------------
--------------------------------------------------------------------------------
------------------------------------
proname         | to_timestamp
pronamespace    | 11
proowner        | 10
prolang         | 14
procost         | 1
prorows         | 0
provariadic     | 0
protransform    | -
...

In HEAD, I see:

-[ RECORD 1 ]---+---------------------------------------------------------------proname         | to_timestamp
                                       pronamespace    | 11
proowner       | 10                                                            prolang         | 14
                                      procost         | 1
prorows        | 0                                                             provariadic     | 0
                                      protransform    | -                                                             
 
After "\pset columns 77" it looks a little better:

-[ RECORD 1 ]---+------------------------------------------------------------proname         | to_timestamp
                                  pronamespace    | 11
proowner       | 10                                                          prolang         | 14
                                  procost         | 1                                                           prorows
       | 0                                                           provariadic     | 0
                          protransform    | -                                                           proisagg
|f                                                           proiswindow     | f
                  
 

but where did those leading spaces come from?  The header line is
definitely not on board with that, and I think those spaces are
contributing to the lines being too long for the window.  I think
possibly the code is also adding a space that shouldn't be there
at the end of the lines, because it prints lines that wrap around
if I \pset columns to either 79 or 80 in an 80-column window, so
the accounting is off by 2 someplace.

Also, this code looks quite broken:
        width = dwidth + swidth + hwidth;        if ((output_columns > 0) && (width > output_columns))        {
  dwidth = output_columns - hwidth - swidth;            width = output_columns;        }
 

What happens if output_columns is less than hwidth + swidth?  The code
goes crazy is what happens, because all of these are unsigned ints and so
wraparound leads to setting dwidth to something approaching 4 billion.
Try the same example after "\pset columns 10".  I don't necessarily expect
it to produce beautiful output, but I do expect it to not lock up.
        regards, tom lane

On Mon, May 12, 2014 at 2:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> but where did those leading spaces come from?  The header line is
> definitely not on board with that, and I think those spaces are
> contributing to the lines being too long for the window.  I think
> possibly the code is also adding a space that shouldn't be there
> at the end of the lines, because it prints lines that wrap around
> if I \pset columns to either 79 or 80 in an 80-column window, so
> the accounting is off by 2 someplace.

Hm, there was an off by one error earlier in some cases, maybe we
fixed it by breaking other case. Will investigate.


-- 
greg

On Mon, May 12, 2014 at 2:12 PM, Greg Stark <stark@mit.edu> wrote:
> Hm, there was an off by one error earlier in some cases, maybe we
> fixed it by breaking other case. Will investigate.

Those spaces are coming from the ascii wrapping indicators. i.e. the periods in:

+-----------------+--------------------+
|        a       +|         a         +|
|                +|         b          |
|        b        |                    |
+-----------------+--------------------+
| xx              | yyyyyyyyyyyyyyyyyy |
| xxxx           +| yyyyyyyyyyyyyyyy  +|
| xxxxxx         +| yyyyyyyyyyyyyy    +|
| xxxxxxxx       +| yyyyyyyyyyyy      +|
| xxxxxxxxxx     +| yyyyyyyyyy        +|
| xxxxxxxxxxxx   +| yyyyyyyy          +|
| xxxxxxxxxxxxxx +| yyyyyy            +|
| xxxxxxxxxxxxxxx.| yyyy              +|
|.x              +| yy                +|
| xxxxxxxxxxxxxxx.|                    |
|.xxx            +|                    |
| xxxxxxxxxxxxxxx.|                    |
|.xxxxx           |                    |
+-----------------+--------------------+

Apparently we used to print those with border=1 in normal mode but in
expanded mode we left out the space for those on the outermost edges
since there was no need for them. If we put them in for wrapped mode
then we'll be inconsistent if we don't for nonwrapped mode though. And
if we don't put them in for wrapped mode then there's no way to
indicate wrapping versus newlines.

The biggest difference it makes is that in the border=1 mode the lines
ended at the end of the data previously. Now it's expanded to fill the
rectangle because of the plus symbols. ie. It used to look like:

-[ RECORD 1 ]-----------
a | xx |
b |
a | yyyyyyyyyyyyyyyyyy
b |
-[ RECORD 2 ]-----------
a | xxxx | xxxxxx
b | xxxxxxxx | xxxxxxxxxx | xxxxxxxxxxxx | xxxxxxxxxxxxxx | xxxxxxxxxxxxxxxx | xxxxxxxxxxxxxxxxxx |
xxxxxxxxxxxxxxxxxxxx
a | yyyyyyyyyyyyyyyy
b | yyyyyyyyyyyyyy | yyyyyyyyyyyy | yyyyyyyyyy | yyyyyyyy | yyyyyy | yyyy | yy |

and now looks like:

-[ RECORD 1 ]-----------a+| xx +|b |a+| yyyyyyyyyyyyyyyyyyb |
-[ RECORD 2 ]-----------a+| xxxx                + +| xxxxxx              +b | xxxxxxxx            +  | xxxxxxxxxx
  +  | xxxxxxxxxxxx        +  | xxxxxxxxxxxxxx      +  | xxxxxxxxxxxxxxxx    +  | xxxxxxxxxxxxxxxxxx  +  |
xxxxxxxxxxxxxxxxxxxxa+|yyyyyyyyyyyyyyyy    +b | yyyyyyyyyyyyyy      +  | yyyyyyyyyyyy        +  | yyyyyyyyyy          +
| yyyyyyyy            +  | yyyyyy              +  | yyyy                +  | yy                  +  |
 



-- 
greg

Greg Stark <stark@mit.edu> writes:
> On Mon, May 12, 2014 at 2:12 PM, Greg Stark <stark@mit.edu> wrote:
>> Hm, there was an off by one error earlier in some cases, maybe we
>> fixed it by breaking other case. Will investigate.

> Those spaces are coming from the ascii wrapping indicators. i.e. the periods in:

Ah.  I wonder whether anyone will complain that the format changed?

> Apparently we used to print those with border=1 in normal mode but in
> expanded mode we left out the space for those on the outermost edges
> since there was no need for them. If we put them in for wrapped mode
> then we'll be inconsistent if we don't for nonwrapped mode though. And
> if we don't put them in for wrapped mode then there's no way to
> indicate wrapping versus newlines.

Barring anyone complaining that the format changed, I'd say the issue
is not that you added them but that the accounting for line length
fails to include them.
        regards, tom lane

I'm trying to review all the combinations of the options exhaustively
but in the process I noticed a few pre-existing psql oddities. Both of
these are present in 9.3:

Can anyone explain this? It's linestyle=old-style, border=1,
expanded=off, format=aligned. It looks like it's using new-style ascii
indicators in the header but old-style in the data cells:
         a           |         a
+                     |+        b
+         b           |+
----------------------+--------------------xx                   | yyyyyyyyyyyyyyyyyyxxxx                 |
yyyyyyyyyyyyyyyyxxxxxx              : yyyyyyyyyyyyyyxxxxxxxx             : yyyyyyyyyyyyxxxxxxxxxx           :
yyyyyyyyyyxxxxxxxxxxxx        : yyyyyyyyxxxxxxxxxxxxxx       : yyyyyyxxxxxxxxxxxxxxxx     : yyyyxxxxxxxxxxxxxxxxxx   :
yyxxxxxxxxxxxxxxxxxxxx:
 
(2 rows)

Also the line-ending white-space is very odd here. It's
linestyle=old-ascii, border=0, expanded=off, format=aligned. There's
an extra space on the header and the first line of the data, but not
on the subsequent lines of the data:
        a                   a                   +        b        b          +
-------------------- ------------------
xx                   yyyyyyyyyyyyyyyyyy
xxxx                 yyyyyyyyyyyyyyyy
xxxxxx               yyyyyyyyyyyyyy
xxxxxxxx             yyyyyyyyyyyy
xxxxxxxxxx           yyyyyyyyyy
xxxxxxxxxxxx         yyyyyyyy
xxxxxxxxxxxxxx       yyyyyy
xxxxxxxxxxxxxxxx     yyyy
xxxxxxxxxxxxxxxxxx   yy
xxxxxxxxxxxxxxxxxxxx
(2 rows)

Sorry, a couple things still look to not be quite right.

1) The width of the table when linestyle=old-ascii and border=0 or
border=1 (and expanded=on and format=wrapped) seems to off by one.

2) The hyphens following the RECORD NN are short by one

I'm surprised the last patch was so big since it sounded like a simple
off-by-one bug. It looks like you've removed the leading space on the
border=0 expanded case. I guess that makes sense but we should
probably stop making significant changes now and just focus on fixing
the off by one bugs.

Sergey Muraviov wrote:
> I found some new bugs and fix them.
> And I had to make many changes.

This version fixes some bugs I had noticed in expanded mode too.  For instance,
the original looked like this (five lines plus header):

-[ RECORD 49 ]-----+-------------------------------------------------------------------------pg_identify_object |
(rule,,,"""_RETURN""on pg_catalog.pg_available_extension_versions")     
 
pg_identify_object | (view,pg_catalog,pg_available_extension_versions,pg_catalog.pg_availabl
e.                   |._extension_versions)                                                     


whereas it's correctly only three lines plus header with this patch
applied.  I can't tell whether this patch is minimal enough.

Having a more comprehensive test case is good, of course, though I
didn't check the expected file.  Note that some things cannot be tested
correctly in this way, namely column count that actually match the
terminal -- to wit: I had to add line breaks manually to the above paste
so that it would look like what it does in my terminal.  If I just paste
it, it looks correct, but then my email terminal is wider than the one I
ran psql in.  I would presume that failure to account for this is what
caused (some of?) the bugs being fixed now ...

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, May 23, 2014 at 10:10:23AM -0400, Alvaro Herrera wrote:
> Sergey Muraviov wrote:
> > I found some new bugs and fix them.
> > And I had to make many changes.
>
> This version fixes some bugs I had noticed in expanded mode too.  For instance,
> the original looked like this (five lines plus header):
>
> -[ RECORD 49 ]-----+-------------------------------------------------------------------------
>  pg_identify_object | (rule,,,"""_RETURN"" on pg_catalog.pg_available_extension_versions")
>
>  pg_identify_object | (view,pg_catalog,pg_available_extension_versions,pg_catalog.pg_availabl
> e.
>                     |._extension_versions)
>
>
> whereas it's correctly only three lines plus header with this patch
> applied.

I had noticed a similar-looking behavior change with aligned format and
expanded display, so I gave psql-wrapped-expanded-fix-v4.patch a spin with
this test case:

\pset expanded on
\pset format aligned
select repeat('a', 2) union all select repeat('a', 500);

The patch did not restore 9.3 behavior for that one.  Starting with commit
6513633, the first line of letters is space-padded on the right to the width
of the second line of letters.  To illustrate, I have attached raw psql output
from both commit 6513633 and its predecessor.  Also note that
psql-wrapped-expanded-fix-v4.patch expands each [ RECORD x ] header from 509
bytes to 511 bytes; 509 is the longstanding width.

Thanks,
nm

--
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com

On 6/8/14, 11:29 PM, Noah Misch wrote:
> The patch did not restore 9.3 behavior for that one.  Starting with commit
> 6513633, the first line of letters is space-padded on the right to the width
> of the second line of letters.  To illustrate, I have attached raw psql output
> from both commit 6513633 and its predecessor.  Also note that
> psql-wrapped-expanded-fix-v4.patch expands each [ RECORD x ] header from 509
> bytes to 511 bytes; 509 is the longstanding width.

I noticed that (or perhaps a related) problem today.  Here is a simple demo:

psql -X -q -t -x -c 'select * from (values (1),(2)) as _ (col)'

9.3:

col | 1
----+--
col | 2

9.4:
col | 1
----+--col | 2


This breaks check_postgres.  (Why check_postgres doesn't use unaligned
output is beyond me.)

On Wed, Jun 11, 2014 at 7:52 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 6/8/14, 11:29 PM, Noah Misch wrote:
>> The patch did not restore 9.3 behavior for that one.  Starting with commit
>> 6513633, the first line of letters is space-padded on the right to the width
>> of the second line of letters.  To illustrate, I have attached raw psql output
>> from both commit 6513633 and its predecessor.  Also note that
>> psql-wrapped-expanded-fix-v4.patch expands each [ RECORD x ] header from 509
>> bytes to 511 bytes; 509 is the longstanding width.
>
> I noticed that (or perhaps a related) problem today.  Here is a simple demo:

I don't think these two issues are related.

The leading space that you (ie Peter) are complaining about in:
col | 1
----+--col | 2

Is there because if the cell wrapped it would get an ellipsis (ie
'...' but it's a single unicode character) in that column to indicate
that it's wrapped. However we don't wrap headers so the only reason to
change it is for the "old-ascii" linestyle:

stark=***# select * from (values (1),(2)) as _ ("col
col");
stark"***#col | 1
+col ;
-----+-----col | 2
+col ;

Noah's complaint is about the space padding on the *right*. Ie

stark=***# select * from (values ('foo'),('foo bar baz')) as _ ("col");col | foo        <- This is the end of the line
-----+--------------------------------------------------------------------------<-
This is the end of the linecol | foo bar baz        <- This is the end of the line

We didn't used to do that in expanded and I kind of agree it would be
nice to avoid. But then there are lots of cases where it would still
be necessary:

stark=***# select * from (values ('foo'),('foo barbaz')) as _ ("col");
stark'***#col | foo        <- This is the end of the line
-----+--------------------------------------------------------------------------<-
This is the end of the linecol | foo bar       +<- This is the end of the line    |  baz       <- This is the end of
theline
 

Obviously we would need to space padd to insert the "+" there.

I think this whole exercise has mostly just convinced me we should
implement an HTTP interface and reimplement psql as a browser app.

-- 
greg

And Gmail has thoroughly mangled that email. Let me see if I can
resend it from Emacs more clearly.

On Wed, Jun 11, 2014 at 08:59:34PM +0100, Greg Stark wrote:
> The leading space that you (ie Peter) are complaining about in:
> 
>  col | 1
> ----+--
>  col | 2
> 
> Is there because if the cell wrapped it would get an ellipsis (ie
> '...' but it's a single unicode character) in that column to indicate
> that it's wrapped. However we don't wrap headers so the only reason to
> change it is for the "old-ascii" linestyle:
> 
> stark=***# select * from (values (1),(2)) as _ ("col
> col");
> stark"***#
>  col | 1
> +col ;
> -----+-----
>  col | 2
> +col ;
> 
> Noah's complaint is about the space padding on the *right*. Ie
> 
> stark=***# select * from (values ('foo'),('foo bar baz')) as _ ("col");
>  col | foo
>          <- This is the end of the line
> -----+--------------------------------------------------------------------------<-
> This is the end of the line
>  col | foo bar baz
>          <- This is the end of the line
> 
> We didn't used to do that in expanded and I kind of agree it would be
> nice to avoid.

Based on the commit message and procedural history, I thought commit 6513633
was changing behavior solely for the combination of "\pset expanded" and
"\pset format wrapped".  Peter's and my test cases show that it also changed
behavior for "\pset expanded" alone.  That's a bug, unless someone sees to
argue that the new "\pset expanded" behavior is a desirable improvement in
spite of its origin as an accident.  Altering an entrenched psql output format
is a big deal.

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com

Noah Misch <noah@leadboat.com> writes:
> Based on the commit message and procedural history, I thought commit 6513633
> was changing behavior solely for the combination of "\pset expanded" and
> "\pset format wrapped".  Peter's and my test cases show that it also changed
> behavior for "\pset expanded" alone.  That's a bug, unless someone sees to
> argue that the new "\pset expanded" behavior is a desirable improvement in
> spite of its origin as an accident.  Altering an entrenched psql output format
> is a big deal.

TBH I'm wondering if we shouldn't just revert that patch (and the
subsequent fix attempts).  It was not a major feature and I'm thinking
we have better things to do right now than try to fix the multiple
logic holes it evidently has.  The author's certainly welcome to try
again with a more carefully thought-through patch for 9.5.
        regards, tom lane

On Wed, Jun 11, 2014 at 10:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Noah Misch <noah@leadboat.com> writes:
>> Based on the commit message and procedural history, I thought commit 6513633
>> was changing behavior solely for the combination of "\pset expanded" and
>> "\pset format wrapped".  Peter's and my test cases show that it also changed
>> behavior for "\pset expanded" alone.  That's a bug, unless someone sees to
>> argue that the new "\pset expanded" behavior is a desirable improvement in
>> spite of its origin as an accident.  Altering an entrenched psql output format
>> is a big deal.
>
> TBH I'm wondering if we shouldn't just revert that patch (and the
> subsequent fix attempts).  It was not a major feature and I'm thinking
> we have better things to do right now than try to fix the multiple
> logic holes it evidently has.  The author's certainly welcome to try
> again with a more carefully thought-through patch for 9.5.

So, it seems like we need to do something about this one way or
another.  Who's working on that?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> So, it seems like we need to do something about this one way or
> another.  Who's working on that?

So I'm fine finishing what I started. I've just been a bit busy this past week.

My inclination is to try to push forward and commit this patch,
document the changes and make sure we check for any consequences of
them.

The alternate plan is to revert it for 9.4 and commit the changes to
9.5 and that gives us more time to be sure we're ok with them.


-- 
greg

On Wed, Jun 11, 2014 at 12:59 PM, Greg Stark <stark@mit.edu> wrote:

> I think this whole exercise has mostly just convinced me we should
> implement an HTTP interface and reimplement psql as a browser app.

I certainly hope not.  I've seen lots of browser apps that were nice
enough to use for casual use by a casual user.  I've never seen one
that was an effective power tool for power users, the way psql is.
Now maybe they are out there and I just don't know about them, but I
have my doubts.

As an additional tool, to each his own.  But a browser-based
replacement for psql, -1 from me.

And as far browser-based things apply to this patch, I must say I've
tried micromanaging the way large amounts of data wrap in a HTML table
when I found the default to be inadequate, and I have not found that
to be noticeably easy, either.

The original version of this patch was only a few lines long and did
one very simple and useful thing: avoiding the printing of whole
screens full of hyphens when in 'expanded mode'.  If we end up
reverting the other parts of this patch, hopefully we don't lose that
part.

Cheers,

Jeff

On Sun, Jul 6, 2014 at 8:40 AM, Sergey Muraviov
<sergey.k.muraviov@gmail.com> wrote:
> Is there anyone who can commit the patch?

So what I'm inclined to do here (sigh) is commit it into 9.5 and
revert it in 9.4. I think it's an improvement but I there's enough
confusion and surprise about the changes from people who weren't
expecting them and enough surprising side effects from things like
check_postgres that letting it simmer for a full release so people can
get used to it might be worthwhile.

-- 
greg

On Mon, Jul 7, 2014 at 8:35 AM, Sergey Muraviov
<sergey.k.muraviov@gmail.com> wrote:
> So what's wrong with the patch?
> And what should I change in it for 9.5?

Possibly nothing. The concern was tha it's modifying the output in
cases where the output is not \expanded and/or not wrapped. Now I've
mostly convinced myself that those cases should change to be
consistent with the wrapped output but there was at least one tool
depending on that format (check_postgres) so perhaps it's not
worthwhile and having it be inconsistent would be safer.

-- 
greg

This remains open for 9.4.  Your proposal to revert the feature in 9.4 and fix
it in 9.5 sounds reasonable.

On Thu, Jul 10, 2014 at 04:15:35PM +0100, Greg Stark wrote:
> On Mon, Jul 7, 2014 at 8:35 AM, Sergey Muraviov
> <sergey.k.muraviov@gmail.com> wrote:
> > So what's wrong with the patch?
> > And what should I change in it for 9.5?
> 
> Possibly nothing. The concern was tha it's modifying the output in
> cases where the output is not \expanded and/or not wrapped. Now I've
> mostly convinced myself that those cases should change to be
> consistent with the wrapped output but there was at least one tool
> depending on that format (check_postgres) so perhaps it's not
> worthwhile and having it be inconsistent would be safer.

I did try psql-wrapped-expanded-fix-v5.patch with the tests Peter and I posted
upthread, and those tests now behave as they do in released versions.  What
cases did you find that still change vs. 9.3?

Thanks,
nm

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com

On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch <noah@leadboat.com> wrote:
> This remains open for 9.4.  Your proposal to revert the feature in 9.4 and fix
> it in 9.5 sounds reasonable.

Ok, I've gone ahead and done this. I'm sorry for the delays and confusion.

> On Thu, Jul 10, 2014 at 04:15:35PM +0100, Greg Stark wrote:
>> On Mon, Jul 7, 2014 at 8:35 AM, Sergey Muraviov
>> <sergey.k.muraviov@gmail.com> wrote:
>> > So what's wrong with the patch?
>> > And what should I change in it for 9.5?
>>
>> Possibly nothing. The concern was tha it's modifying the output in
>> cases where the output is not \expanded and/or not wrapped. Now I've
>> mostly convinced myself that those cases should change to be
>> consistent with the wrapped output but there was at least one tool
>> depending on that format (check_postgres) so perhaps it's not
>> worthwhile and having it be inconsistent would be safer.
>
> I did try psql-wrapped-expanded-fix-v5.patch with the tests Peter and I posted
> upthread, and those tests now behave as they do in released versions.  What
> cases did you find that still change vs. 9.3?

I was trying to build a spreadsheet of every combination of these
options. It turns out that 4-dimensional spreadsheets are kind of
awkward.

I think the fundamental dilemma was the same that was discussed
upthread. If wrapped expanded mode should have an extra space padding
column for the wrap indicators then all expanded modes should have
that column to be consistent since wrapping shouldn't change the
amount of padding. But that means unrelated queries changes format in
ways people weren't expecting. They were depending on the basically
inconsistent formatting where expanded didn't have the same padding
that non-expanded formats had which was only the case because nobody
had anticiapted expanded format needing space for wrapping indicators.

-- 
greg

On Mon, Aug 18, 2014 at 8:30 PM, Greg Stark <stark@mit.edu> wrote:
> On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch <noah@leadboat.com> wrote:
>> This remains open for 9.4.  Your proposal to revert the feature in 9.4 and fix
>> it in 9.5 sounds reasonable.
>
> Ok, I've gone ahead and done this. I'm sorry for the delays and confusion.
I imagine that you also need to fix the release notes accordingly.
Patch attached for master and REL9_4_STABLE.
Regards,
--
Michael

On Mon, Aug 18, 2014 at 12:55 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I imagine that you also need to fix the release notes accordingly.
> Patch attached for master and REL9_4_STABLE.

Thanks.

Done for 9.4 but the patch is still in master. In fact it's the most
recent version and I'm still pretty convinced it's a good patch.


-- 
greg

On Mon, Aug 18, 2014 at 10:02 PM, Greg Stark <stark@mit.edu> wrote:
> On Mon, Aug 18, 2014 at 12:55 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> Done for 9.4 but the patch is still in master. In fact it's the most
> recent version and I'm still pretty convinced it's a good patch.
If this feature is not going to be part of 9.4 anymore, the related
release notes should be updated in consequence on REL9_4_STABLE and
master, no? Am I missing something?
-- 
Michael

On Mon, Aug 18, 2014 at 12:30:40PM +0100, Greg Stark wrote:
> On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch <noah@leadboat.com> wrote:
> > This remains open for 9.4.  Your proposal to revert the feature in 9.4 and fix
> > it in 9.5 sounds reasonable.
> 
> Ok, I've gone ahead and done this. I'm sorry for the delays and confusion.

Thanks.

> > I did try psql-wrapped-expanded-fix-v5.patch with the tests Peter and I posted
> > upthread, and those tests now behave as they do in released versions.  What
> > cases did you find that still change vs. 9.3?
> 
> I was trying to build a spreadsheet of every combination of these
> options. It turns out that 4-dimensional spreadsheets are kind of
> awkward.

What's one query that still behaves differently in 9.5 vs. 9.3 (under
formatting options that exist in both versions)?

> I think the fundamental dilemma was the same that was discussed
> upthread. If wrapped expanded mode should have an extra space padding
> column for the wrap indicators then all expanded modes should have
> that column to be consistent since wrapping shouldn't change the
> amount of padding.

I might agree for a greenfield design, but -1 for changing expanded mode now
to improve consistency in this way.  I predict the complaints from users of
expanded mode in automation would overpower any applause for the consistency.