patch: psql - enforce constant width of last column

Started by Pavel Stehuleover 6 years ago8 messages
#1Pavel Stehule
pavel.stehule@gmail.com
1 attachment(s)

Hi

When I played with vertical cursor support I got badly displayed last
columns when border was not 2. Only when border is 2, then psql displays
last column with same width for each row.

I think so we can force column width alignment for any border styles today
(for alignment and wrapping styles) or as minimum this behave can be
optional.

I wrote a patch with pset option "final_spaces", but I don't see a reason
why we trim rows today.

Regards

Pavel

Attachments:

psql-use-final-spaces-always.patchtext/x-patch; charset=US-ASCII; name=psql-use-final-spaces-always.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7789fc6177..4849086c0f 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2641,6 +2641,15 @@ lo_import 152801
           </listitem>
           </varlistentry>
 
+          <varlistentry>
+          <term><literal>final_spaces</literal></term>
+          <listitem>
+          <para>
+          With value <literal>always</literal> last column will have same width.
+          </para>
+          </listitem>
+          </varlistentry>
+
           <varlistentry>
           <term><literal>footer</literal></term>
           <listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b981ae81ff..a91bb6ef6e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1980,7 +1980,7 @@ exec_command_pset(PsqlScanState scan_state, bool active_branch)
 			int			i;
 			static const char *const my_list[] = {
 				"border", "columns", "csv_fieldsep", "expanded", "fieldsep",
-				"fieldsep_zero", "footer", "format", "linestyle", "null",
+				"fieldsep_zero", "final_spaces", "footer", "format", "linestyle", "null",
 				"numericlocale", "pager", "pager_min_lines",
 				"recordsep", "recordsep_zero",
 				"tableattr", "title", "tuples_only",
@@ -3746,6 +3746,19 @@ _unicode_linestyle2string(int linestyle)
 	return "unknown";
 }
 
+static const char *
+_final_spaces_style2string(int finalspaces)
+{
+	switch (finalspaces)
+	{
+		case FINAL_SPACES_AUTO:
+			return "auto";
+		case FINAL_SPACES_ALWAYS:
+			return "always";
+	}
+	return "unknown";
+}
+
 /*
  * do_pset
  *
@@ -3820,6 +3833,21 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 		}
 	}
 
+	else if (strcmp(param, "final_spaces") == 0)
+	{
+		if (!value)
+			;
+		if (pg_strncasecmp("auto", value, vallen) == 0)
+			popt->topt.final_spaces = FINAL_SPACES_AUTO;
+		else if (pg_strncasecmp("always", value, vallen) == 0)
+			popt->topt.final_spaces = FINAL_SPACES_ALWAYS;
+		else
+		{
+			pg_log_error("\\pset: allowed final space styles are auto or always");
+			return false;
+		}
+	}
+
 	/* set table line style */
 	else if (strcmp(param, "linestyle") == 0)
 	{
@@ -4247,6 +4275,12 @@ printPsetInfo(const char *param, struct printQueryOpt *popt)
 			   _unicode_linestyle2string(popt->topt.unicode_header_linestyle));
 	}
 
+	else if (strcmp(param, "final_spaces") == 0)
+	{
+		printf(_("final spaces style is \"%s\".\n"),
+			   _final_spaces_style2string(popt->topt.final_spaces));
+	}
+
 	else
 	{
 		pg_log_error("\\pset: unknown option: %s", param);
@@ -4357,6 +4391,8 @@ pset_value_string(const char *param, struct printQueryOpt *popt)
 		return pstrdup(_unicode_linestyle2string(popt->topt.unicode_column_linestyle));
 	else if (strcmp(param, "unicode_header_linestyle") == 0)
 		return pstrdup(_unicode_linestyle2string(popt->topt.unicode_header_linestyle));
+	else if (strcmp(param, "final_spaces") == 0)
+		return pstrdup(_final_spaces_style2string(popt->topt.final_spaces));
 	else
 		return pstrdup("ERROR");
 }
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bcc7404c55..3162f77d68 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3707,7 +3707,7 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH_QUERY(Query_for_list_of_roles);
 	else if (TailMatchesCS("\\pset"))
 		COMPLETE_WITH_CS("border", "columns", "csv_fieldsep", "expanded",
-						 "fieldsep", "fieldsep_zero", "footer", "format",
+						 "fieldsep", "final_spaces", "fieldsep_zero", "footer", "format",
 						 "linestyle", "null", "numericlocale",
 						 "pager", "pager_min_lines",
 						 "recordsep", "recordsep_zero",
@@ -3717,6 +3717,8 @@ psql_completion(const char *text, int start, int end)
 						 "unicode_header_linestyle");
 	else if (TailMatchesCS("\\pset", MatchAny))
 	{
+		if (TailMatchesCS("final_spaces"))
+			COMPLETE_WITH_CS("auto", "always");
 		if (TailMatchesCS("format"))
 			COMPLETE_WITH_CS("aligned", "asciidoc", "csv", "html", "latex",
 							 "latex-longtable", "troff-ms", "unaligned",
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index e41f42ea98..77b9da357d 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -586,6 +586,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
 	unsigned short opt_border = cont->opt->border;
 	const printTextFormat *format = get_line_style(cont->opt);
 	const printTextLineFormat *dformat = &format->lrule[PRINT_RULE_DATA];
+	final_spaces_style opt_final_spaces = cont->opt->final_spaces;
 
 	unsigned int col_count = 0,
 				cell_count = 0;
@@ -1005,8 +1006,14 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
 				struct lineptr *this_line = &col_lineptrs[j][curr_nl_line[j]];
 				int			bytes_to_output;
 				int			chars_to_output = width_wrap[j];
-				bool		finalspaces = (opt_border == 2 ||
-										   (col_count > 0 && j < col_count - 1));
+				bool		finalspaces;
+
+				if (opt_final_spaces == FINAL_SPACES_AUTO)
+					finalspaces = opt_border == 2 ||
+										   (col_count > 0 && j < col_count - 1);
+				else
+					/* FINAL_SPACES_ALWAYS */
+					finalspaces = true;
 
 				/* Print left-hand wrap or newline mark */
 				if (opt_border != 0)
@@ -1100,7 +1107,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
 					fputs(format->wrap_right, fout);
 				else if (wrap[j] == PRINT_LINE_WRAP_NEWLINE)
 					fputs(format->nl_right, fout);
-				else if (opt_border == 2 || (col_count > 0 && j < col_count - 1))
+				else if (finalspaces)
 					fputc(' ', fout);
 
 				/* Print column divider, if not the last column */
diff --git a/src/include/fe_utils/print.h b/src/include/fe_utils/print.h
index f138d963d3..40fe50c655 100644
--- a/src/include/fe_utils/print.h
+++ b/src/include/fe_utils/print.h
@@ -88,6 +88,12 @@ typedef enum unicode_linestyle
 	UNICODE_LINESTYLE_DOUBLE
 } unicode_linestyle;
 
+typedef enum final_spaces_style
+{
+	FINAL_SPACES_AUTO = 0,
+	FINAL_SPACES_ALWAYS
+} final_spaces_style;
+
 struct separator
 {
 	char	   *separator;
@@ -123,6 +129,8 @@ typedef struct printTableOpt
 	unicode_linestyle unicode_border_linestyle;
 	unicode_linestyle unicode_column_linestyle;
 	unicode_linestyle unicode_header_linestyle;
+	final_spaces_style final_spaces;	/* allows final spaces for last column
+									 * for other border styles than 2 */
 } printTableOpt;
 
 /*
#2Ahsan Hadi
ahsan.hadi@gmail.com
In reply to: Pavel Stehule (#1)
Re: patch: psql - enforce constant width of last column

Hi Pavel,

I have been trying to reproduce the case of badly displaying last columns
of a query result-set. I played around with the legal values for psql
border variable but not able to find a case where last columns are badly
displayed. Can you please share an example that I can use to reproduce this
problem. I will try out your patch once I am able to reproduce the problem.

Thanks,

-- Ahsan

On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Show quoted text

Hi

When I played with vertical cursor support I got badly displayed last
columns when border was not 2. Only when border is 2, then psql displays
last column with same width for each row.

I think so we can force column width alignment for any border styles today
(for alignment and wrapping styles) or as minimum this behave can be
optional.

I wrote a patch with pset option "final_spaces", but I don't see a reason
why we trim rows today.

Regards

Pavel

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Ahsan Hadi (#2)
1 attachment(s)
Re: patch: psql - enforce constant width of last column

út 17. 9. 2019 v 17:06 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com> napsal:

Hi Pavel,

I have been trying to reproduce the case of badly displaying last columns
of a query result-set. I played around with the legal values for psql
border variable but not able to find a case where last columns are badly
displayed. Can you please share an example that I can use to reproduce this
problem. I will try out your patch once I am able to reproduce the problem.

you need to use pspg, and vertical cursor.

https://github.com/okbob/pspg
vertical cursor should be active

\pset border 1
\pset linestyle ascii
\pset pager always

select * from generate_series(1,3);

Regards

Pavel

Show quoted text

Thanks,

-- Ahsan

On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

When I played with vertical cursor support I got badly displayed last
columns when border was not 2. Only when border is 2, then psql displays
last column with same width for each row.

I think so we can force column width alignment for any border styles
today (for alignment and wrapping styles) or as minimum this behave can be
optional.

I wrote a patch with pset option "final_spaces", but I don't see a reason
why we trim rows today.

Regards

Pavel

Attachments:

Snímek z 2019-09-17 17-13-51.pngimage/png; name="=?UTF-8?B?U27DrW1layB6IDIwMTktMDktMTcgMTctMTMtNTEucG5n?="Download
#4Ahsan Hadi
ahsan.hadi@gmail.com
In reply to: Pavel Stehule (#3)
Re: patch: psql - enforce constant width of last column

On Tue, Sep 17, 2019 at 8:16 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

út 17. 9. 2019 v 17:06 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com>
napsal:

Hi Pavel,

I have been trying to reproduce the case of badly displaying last columns
of a query result-set. I played around with the legal values for psql
border variable but not able to find a case where last columns are badly
displayed. Can you please share an example that I can use to reproduce this
problem. I will try out your patch once I am able to reproduce the problem.

you need to use pspg, and vertical cursor.

https://github.com/okbob/pspg
vertical cursor should be active

okay thanks for the info. I don't think it was possible to figure this out
by reading the initial post. I will check it out.

does this patch have any value for psql without pspg?

Show quoted text

\pset border 1
\pset linestyle ascii
\pset pager always

select * from generate_series(1,3);

Regards

Pavel

Thanks,

-- Ahsan

On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

When I played with vertical cursor support I got badly displayed last
columns when border was not 2. Only when border is 2, then psql displays
last column with same width for each row.

I think so we can force column width alignment for any border styles
today (for alignment and wrapping styles) or as minimum this behave can be
optional.

I wrote a patch with pset option "final_spaces", but I don't see a
reason why we trim rows today.

Regards

Pavel

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Ahsan Hadi (#4)
Re: patch: psql - enforce constant width of last column

st 18. 9. 2019 v 12:52 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com> napsal:

On Tue, Sep 17, 2019 at 8:16 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

út 17. 9. 2019 v 17:06 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com>
napsal:

Hi Pavel,

I have been trying to reproduce the case of badly displaying last
columns of a query result-set. I played around with the legal values for
psql border variable but not able to find a case where last columns are
badly displayed. Can you please share an example that I can use to
reproduce this problem. I will try out your patch once I am able to
reproduce the problem.

you need to use pspg, and vertical cursor.

https://github.com/okbob/pspg
vertical cursor should be active

okay thanks for the info. I don't think it was possible to figure this out
by reading the initial post. I will check it out.

does this patch have any value for psql without pspg?

The benefit of this patch is just for pspg users today.

Pavel

Show quoted text

\pset border 1
\pset linestyle ascii
\pset pager always

select * from generate_series(1,3);

Regards

Pavel

Thanks,

-- Ahsan

On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

When I played with vertical cursor support I got badly displayed last
columns when border was not 2. Only when border is 2, then psql displays
last column with same width for each row.

I think so we can force column width alignment for any border styles
today (for alignment and wrapping styles) or as minimum this behave can be
optional.

I wrote a patch with pset option "final_spaces", but I don't see a
reason why we trim rows today.

Regards

Pavel

#6Bruce Momjian
bruce@momjian.us
In reply to: Pavel Stehule (#3)
Re: patch: psql - enforce constant width of last column

On Tue, Sep 17, 2019 at 05:15:42PM +0200, Pavel Stehule wrote:

�t 17. 9. 2019 v�17:06 odes�latel Ahsan Hadi <ahsan.hadi@gmail.com> napsal:

Hi Pavel,

I have been trying to reproduce the case of badly displaying last columns
of a query result-set. I played around with the legal values for psql
border variable but not able to find a case where last columns are badly
displayed. Can you please share an example that I can use to reproduce this
problem. I will try out your patch once I am able to reproduce the problem.

you need to use pspg, and vertical cursor.

https://github.com/okbob/pspg
vertical cursor should be active

\pset border 1
\pset linestyle ascii
\pset pager always

select * from generate_series(1,3);

I was able to reproduce the failure, but with a little more work:

$ export PSQL_PAGER='pspg --vertical-cursor'
$ psql test
\pset border 1
\pset linestyle ascii
\pset pager always
select * from generate_series(1,3);

Line '1' has highlighted trailing space.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#5)
Re: patch: psql - enforce constant width of last column

Pavel Stehule <pavel.stehule@gmail.com> writes:

st 18. 9. 2019 v 12:52 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com> napsal:

does this patch have any value for psql without pspg?

The benefit of this patch is just for pspg users today.

TBH, I think we should just reject this patch. It makes psql's
table-printing behavior even more complicated than it was before.
And I don't see how pspg gets any benefit --- you'll still have
to deal with the old code, for an indefinite time into the future.

Moreover, *other* programs that pay close attention to the output
format will be forced to deal with the possibility that this flag
has been turned on, which typically they wouldn't even have a way
to find out. So I think you're basically trying to export your
problems onto everyone else.

regards, tom lane

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#7)
Re: patch: psql - enforce constant width of last column

Hi

po 4. 11. 2019 v 21:55 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

st 18. 9. 2019 v 12:52 odesílatel Ahsan Hadi <ahsan.hadi@gmail.com>

napsal:

does this patch have any value for psql without pspg?

The benefit of this patch is just for pspg users today.

TBH, I think we should just reject this patch. It makes psql's
table-printing behavior even more complicated than it was before.
And I don't see how pspg gets any benefit --- you'll still have
to deal with the old code, for an indefinite time into the future.

I don't think so it increase printing rules too much. A default value
"auto" doesn't any change against current state, "always" ensure same line
width of any row.

The problem, that this patch try to solve, is different width of rows -
although the result is aligned.

Personally I think so current behave is not correct. Correct solution
should be set "finalspaces true" every time - for aligned output. But I
don't know a motivation of authors and as solution with minimal impacts I
wrote a possibility to set (it's not default) to finalspace to "always" as
fix of some possible visual artefact (although these artefacts are almost
time invisible).

The patch maybe looks not trivial (although it is trivial), but it is due I
try to reduce possible impact on any other application to zero.

Moreover, *other* programs that pay close attention to the output
format will be forced to deal with the possibility that this flag
has been turned on, which typically they wouldn't even have a way
to find out. So I think you're basically trying to export your
problems onto everyone else.

I try to fix this issue where this issue coming. For this patch is
important to get a agreement (or not) if this problem is a issue that
should be fixed.

I think so in aligned mode all rows should to have same width.

On second hand, really I don't know why the last space is not printed, and
if some applications had a problem with it. I have not any idea. Current
code where last spaces are not printed is little bit complex than if the
align was really complete.

Sure, "the issue of last invisible space" is not big issue (it's
triviality) - I really think so it should be fixed on psql side, but if
there will not be a agreement, I can fix it on pspg side (although it will
not be elegant - because I have to print chars that doesn't exists).

Is here any man who remember this implementation, who can say, why the code
is implemented how it is implemented?

Regards

Pavel

Show quoted text

regards, tom lane