Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Aug 18, 2014 at 12:30:40PM +0100, Greg Stark wrote: On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch n...@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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch n...@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.murav...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Aug 18, 2014 at 8:30 PM, Greg Stark st...@mit.edu wrote: On Tue, Aug 5, 2014 at 3:41 AM, Noah Misch n...@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 diff --git a/doc/src/sgml/release-9.4.sgml b/doc/src/sgml/release-9.4.sgml index 5330c42..1782be4 100644 --- a/doc/src/sgml/release-9.4.sgml +++ b/doc/src/sgml/release-9.4.sgml @@ -1813,14 +1813,6 @@ listitem para -Add ability to wrap long lines in applicationpsql/'s -literalexpanded/ mode by using command\pset format wrapped/ -(Sergey Muraviov) - /para - /listitem - - listitem - para Suppress quoteNo rows/quote output in applicationpsql/ link linkend=APP-PSQL-meta-commandsoptionexpanded//link mode when the footer is disabled (Bruce Momjian) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Aug 18, 2014 at 12:55 PM, Michael Paquier michael.paqu...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Aug 18, 2014 at 10:02 PM, Greg Stark st...@mit.edu wrote: On Mon, Aug 18, 2014 at 12:55 PM, Michael Paquier michael.paqu...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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.murav...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Jul 7, 2014 at 8:35 AM, Sergey Muraviov sergey.k.murav...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
So what's wrong with the patch? And what should I change in it for 9.5? 2014-07-07 3:12 GMT+04:00 Greg Stark st...@mit.edu: On Sun, Jul 6, 2014 at 8:40 AM, Sergey Muraviov sergey.k.murav...@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 -- Best regards, Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hi. Is there anyone who can commit the patch? 2014-06-25 20:17 GMT+04:00 Pavel Stehule pavel.steh...@gmail.com: 2014-06-24 19:45 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com: Hi. Is there any problem with the patch? I tested it and I had not any issue with last version So, please, commit it Regards Pavel 2014-06-17 0:21 GMT+04:00 Greg Stark st...@mit.edu: On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas robertmh...@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 -- Best regards, Sergey Muraviov -- Best regards, Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Sun, Jul 6, 2014 at 8:40 AM, Sergey Muraviov sergey.k.murav...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
2014-06-24 19:45 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com: Hi. Is there any problem with the patch? I tested it and I had not any issue with last version So, please, commit it Regards Pavel 2014-06-17 0:21 GMT+04:00 Greg Stark st...@mit.edu: On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas robertmh...@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 -- Best regards, Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hi. Is there any problem with the patch? 2014-06-17 0:21 GMT+04:00 Greg Stark st...@mit.edu: On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas robertmh...@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 -- Best regards, Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
2014-06-16 23:28 GMT+02:00 Jeff Janes jeff.ja...@gmail.com: On Wed, Jun 11, 2014 at 12:59 PM, Greg Stark st...@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. We can integrate a text console browsers like links, elinks or lynx instead and we can call a BROWSER instead PAGER when \pset is html pavel@localhost postgresql92]$ PAGER=elinks -force-html psql postgres psql (9.4beta1) Type help for help. postgres=# \pset format html Output format (format) is html. postgres=# SELECT * FROM pg_proc; works perfect [pavel@localhost postgresql92]$ PAGER=lynx -stdin psql postgres psql (9.4beta1) Type help for help. postgres=# \pset format html Output format (format) is html. postgres=# SELECT * FROM pg_proc; Writing html browsing into psql is useless now and I don't think so it is good idea. On second hand better integration mentioned browsers can be very useful. Regards Pavel 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
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Wed, Jun 11, 2014 at 10:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Noah Misch n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas robertmh...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Wed, Jun 11, 2014 at 12:59 PM, Greg Stark st...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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.) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Wed, Jun 11, 2014 at 7:52 PM, Peter Eisentraut pete...@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 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. But then there are lots of cases where it would still
be necessary:
stark=***# select * from (values ('foo'),('foo bar
baz')) as _ (col);
stark'***#
col | foo
- This is the end of the line
-+---
This is the end of the line
col | foo bar
+- This is the end of the line
| baz
- This is the end of the line
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
And Gmail has thoroughly mangled that email. Let me see if I can resend it from Emacs more clearly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Noah Misch n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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
Expanded display (expanded) is on.
Output format (format) is aligned.
-[ RECORD 1
]
repeat | aa
-[ RECORD 2
]
repeat |
Expanded display (expanded) is on.
Output format (format) is aligned.
-[ RECORD 1
]
repeat | aa
-[ RECORD 2
]
repeat |
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hello where we are with this feature? Is there some barriers to commit bugfix? Regards Pavel 2014-05-18 19:46 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com: I found some new bugs and fix them. And I had to make many changes. 2014-05-17 21:31 GMT+04:00 Greg Stark st...@mit.edu: 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. -- Best regards, Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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 Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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 | yy | xx : yy : xx : yy : xx : yy : xx : yy : (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 yy xx yy xx yy xx yy xx yy (2 rows) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hello
2014-05-15 15:04 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com:
Hi.
Please review the new patch.
This version works perfect
Regards
Pavel
PS
Issues which were described by Tom and Pavel were relevant to single-line
headers.
So I've added appropriate regression tests to the patch.
I've also attached complex regression tests for unicode linestyle and
multibyte symbols.
2014-05-14 10:55 GMT+04:00 Pavel Stehule pavel.steh...@gmail.com:
sorry
there is still small issue
I have a plpgsql function:
CREATE OR REPLACE FUNCTION public.foo_update_trg()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
DECLARE t text;
BEGIN
EXECUTE format('SELECT $1.%I', TG_ARGV[0]) INTO t USING old;
RAISE NOTICE 'original value of % is %', TG_ARGV[0], t;
RETURN NULL;
END;
$function$
Default expanded view of select * from pg_proc where proname =
'foo_update_trg'; is little bit broken (screenshoot 1)
After wrap mode, it add useless new line into source code (screenshoot 2)
but border2 fixes it (screenshots 3)
Regards
Pavel
2014-05-14 8:32 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
Hello
With this patch it works perfect
Thank you
Regards
Pavel
2014-05-13 21:33 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com
:
Please check this patch.
2014-05-12 22:56 GMT+04:00 Sergey Muraviov sergey.k.murav...@gmail.com
:
Hi.
I'll try to fix it tomorrow.
2014-05-12 18:42 GMT+04:00 Tom Lane t...@sss.pgh.pa.us:
Greg Stark st...@mit.edu writes:
On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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
--
Best regards,
Sergey Muraviov
--
Best regards,
Sergey MuraviovH
--
Best regards,
Sergey Muraviov
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hello With this patch it works perfect Thank you Regards Pavel 2014-05-13 21:33 GMT+02:00 Sergey Muraviov sergey.k.murav...@gmail.com: Please check this patch. 2014-05-12 22:56 GMT+04:00 Sergey Muraviov sergey.k.murav...@gmail.com: Hi. I'll try to fix it tomorrow. 2014-05-12 18:42 GMT+04:00 Tom Lane t...@sss.pgh.pa.us: Greg Stark st...@mit.edu writes: On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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 -- Best regards, Sergey Muraviov -- Best regards, Sergey MuraviovH
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Please check this patch.
2014-05-12 22:56 GMT+04:00 Sergey Muraviov sergey.k.murav...@gmail.com:
Hi.
I'll try to fix it tomorrow.
2014-05-12 18:42 GMT+04:00 Tom Lane t...@sss.pgh.pa.us:
Greg Stark st...@mit.edu writes:
On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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
--
Best regards,
Sergey Muraviov
--
Best regards,
Sergey MuraviovH
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 62850d8..69f4efe 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1258,45 +1258,67 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
if (cont-opt-format == PRINT_WRAPPED)
{
/*
- * Calculate the available width to wrap the columns to after
- * subtracting the maximum header width and separators. At a minimum
- * enough to print [ RECORD N ]
+ * Separators width
*/
unsigned int width,
+ min_width,
swidth;
if (opt_border == 0)
- swidth = 1; /* header data */
+ /*
+ * For border = 0, one space in the middle.
+ */
+ swidth = 1;
else if (opt_border == 1)
- swidth = 3; /* header | data */
- else
- swidth = 7; /* | header | data | */
-
- /* Wrap to maximum width */
- width = dwidth + swidth + hwidth;
- if ((output_columns 0) (width output_columns))
{
- dwidth = output_columns - hwidth - swidth;
- width = output_columns;
+ /*
+ * For border = 1, one for the pipe (|) in the middle
+ * between the two spaces.
+ */
+ swidth = 3;
}
+ else
+ /*
+ * For border = 2, two more for the pipes (|) at the begging and
+ * at the end of the lines.
+ */
+ swidth = 7;
- /* Wrap to minimum width */
+ min_width = hwidth + swidth + 3;
+
+ /*
+ * Record header width
+ */
if (!opt_tuples_only)
{
- int delta = 1 + log10(cont-nrows) - width;
-
+ /*
+ * Record number
+ */
+ unsigned int rwidth = 1 + log10(cont-nrows);
if (opt_border == 0)
-delta += 6; /* * RECORD */
+rwidth += 9; /* * RECORD */
else if (opt_border == 1)
-delta += 10; /* -[ RECORD ] */
+rwidth += 12; /* -[ RECORD ] */
else
-delta += 15; /* +-[ RECORD ]-+ */
+rwidth += 15; /* +-[ RECORD ]-+ */
- if (delta 0)
-dwidth += delta;
+ if (rwidth min_width)
+min_width = rwidth;
}
- else if (dwidth 3)
- dwidth = 3;
+
+ if ((hheight 1) (opt_border 2))
+ hwidth++; /* for wrapping indicator*/
+
+ /* Wrap to minimum width */
+ width = hwidth + swidth + dwidth;
+ if ((width min_width) || (output_columns min_width))
+ dwidth = min_width - hwidth - swidth;
+ else if ((output_columns 0)
+ (width output_columns))
+ /*
+ * Wrap to maximum width
+ */
+ dwidth = output_columns - hwidth - swidth;
}
/* print records */
@@ -1357,12 +1379,17 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
int swidth,
twidth = hwidth + 1;
-fputs(hline ? format-header_nl_left : , fout);
+if ((hheight 1) || (opt_border == 2))
+ fputs(hline ? format-header_nl_left : , fout);
strlen_max_width(hlineptr[hline].ptr, twidth,
encoding);
fprintf(fout, %-s, hlineptr[hline].ptr);
swidth = hwidth - twidth;
+if ((hheight 1)
+ (opt_border 2)
+ (cont-opt-format == PRINT_WRAPPED))
+ swidth--;
if (swidth 0) /* spacer */
fprintf(fout, %*s, swidth, );
@@ -1382,7 +1409,11 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
else
{
/* Header exhausted but more data for column */
-fprintf(fout, %*s, hwidth + 2, );
+unsigned int ewidth = hwidth + 2;
+if ((opt_border 2)
+ (cont-opt-format == PRINT_WRAPPED))
+ ewidth--;
+fprintf(fout, %*s, ewidth, );
}
/* Separator */
@@ -1401,13 +1432,24 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
/* Data */
if (!dcomplete)
{
-int target_width,
+int target_width = dwidth,
bytes_to_output,
swidth;
-fputs(!dcomplete !offset ? : format-wrap_left, fout);
+if (dheight 1)
+{
+
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Pavel Stehule pavel.steh...@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. psql-wrapped-expanded-fix.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Emre Hasegeli e...@hasegeli.com writes:
Pavel Stehule pavel.steh...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, May 12, 2014 at 2:00 PM, Tom Lane t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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 | yy | | +| +| | xx +| yy+| | +| +| | xx +| yy+| | +| +| | xx +| yy+| | xxx.| +| |.x +| yy+| | xxx.|| |.xxx+|| | xxx.|| |.x || +-++ 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 | yy b | -[ RECORD 2 ]--- a | | xx b | | xx | | xx | | xx | a | b | yy | | yy | | yy | | yy | and now looks like: -[ RECORD 1 ]--- a+| xx +| b | a+| yy b | -[ RECORD 2 ]--- a+| + +| xx + b | + | xx + | + | xx + | + | xx + | a+| + b | yy + | + | yy + | + | yy + | + | yy + | -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Greg Stark st...@mit.edu writes: On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] wrapping in extended mode doesn't work well with default pager
Hi. I'll try to fix it tomorrow. 2014-05-12 18:42 GMT+04:00 Tom Lane t...@sss.pgh.pa.us: Greg Stark st...@mit.edu writes: On Mon, May 12, 2014 at 2:12 PM, Greg Stark st...@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 -- Best regards, Sergey Muraviov
