Re: Invisible PROMPT2

2019-12-22 Thread Maxence Ahlouche
On Wed, 27 Nov 2019 at 17:09, Tom Lane  wrote:

> Good idea, but I think you need to account for "visible" (ie, if the
> newline is inside RL_PROMPT_START_IGNORE, it shouldn't change the width).
> It might be best to add logic inside the existing "if (visible)" instead
> of making a new top-level case.
>

Right, I assumed that it was safe given that only terminal control
characters were invisible.
Since the title of the terminal window can be changed as well via control
characters, it's probably better not to make that assumption.

I updated the patch accordingly.


> Another special case that somebody's likely to whine about is \t, though
> to handle that we'd have to make assumptions about the tab stop distance.
> Maybe assuming that it's 8 is good enough.
>

The problem with tabs is that any user can set their tabstops to whatever
they want, and a tab doesn't have a fixed width, it just goes up to the
next tab stop.
One way to do it would be to add tabs wherever necessary in prompt2 to make
sure they have the same size as in prompt1 (a list of numbers of spaces,
which we would concatenate with a tab?), but I'm not sure it's worth the
effort.
commit 491cf173aad247299622796775feea580a8f9b13 (HEAD -> refs/heads/patch_psql_prompt)
Author: Maxence Ahlouche 
Date:   Wed Nov 27 16:21:35 2019 +0100

Fix %w length in PROMPT2 when PROMPT1 contains a newline, in psql.

The width of the invisible PROMPT2 must take into account, in order for user input to be aligned with the first line, that PROMPT1 can contain newlines.

diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 41c6f21ecf..24efb8f686 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -379,7 +379,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 if (visible)
 {
 	chwidth = PQdsplen(p, pset.encoding);
-	if (chwidth > 0)
+
+	if (*p == '\n')
+		last_prompt1_width = 0;
+	else if (chwidth > 0)
 		last_prompt1_width += chwidth;
 }
 


Re: Invisible PROMPT2

2019-11-27 Thread Maxence Ahlouche
Hi,

I noticed that this patch does not work when PROMPT1 contains a new line,
since the whole length of PROMPT1 is taken into account for the length of
%w.
Attached screenshot shows the issue on my psql, with the following PROMPT
variables (colors edited out for readability):

\set PROMPT1 '\n[pid:%p] %n :: %`hostname`:%> ‹%/› \n› '
\set PROMPT2 '%w'

Notice in the screenshot that just after inputting a newline, my cursor is
far to the right.
The length of %w should probably be computed starting from the last newline
in PROMPT1.

I could technically get rid of my newline, but since my prompt can get
pretty long, i like the comfort of having my first line of sql start right
at the left of my terminal.

Also attached is a trivial patch to fix this issue, which I have not
extensively tested (works for me at least), and might not be the right way
to do it, but it's a start.
Otherwise, nice feature, I like it!

Regards,
Maxence
commit 7fca5709d3ada8cf0b4219c707562cd841c997d2 (HEAD -> refs/heads/psql_prompt)
Author: Maxence Ahlouche 
Date:   Wed Nov 27 16:21:35 2019 +0100

Fix %w length in PROMPT2 when PROMPT1 contains a newline, in psql.

The width of the invisible PROMPT2 must take into account, in order for user input to be aligned with the first line, that PROMPT1 can contain newlines.

diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 41c6f21ecf..a844c384bb 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -366,6 +366,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 visible = true;
 ++p;
 			}
+			else if (*p == '\n')
+			{
+last_prompt1_width = 0;
+++p;
+			}
 			else
 #endif
 			{


Re: Show a human-readable n_distinct in pg_stats view

2019-03-15 Thread Maxence Ahlouche
On Wed, 13 Mar 2019 at 15:14, Maxence Ahlouche
 wrote:
> * Should I add this patch to the commitfest?

I added it: https://commitfest.postgresql.org/23/2061/



Show a human-readable n_distinct in pg_stats view

2019-03-13 Thread Maxence Ahlouche
Hi,

It seems to me that since the pg_stats view is supposed to be
human-readable, it would make sense to show a human-readable version
of n_distinct.
Currently, when the stats collector estimates that the number of
distinct values is more than 10% of the total row count, what is
stored in pg_statistic.stadistinct is -1 * n_distinct / totalrows, the
rationale being that if new rows are inserted in the table, they are
likely to introduce new values, and storing that value allows the
stadistinct not to get stale too fast.

You can find attached a simple WIP patch to show the proper n_distinct
value in pg_stats.

* Is this desired?
* Would it make sense to add a column in the pg_stats view to display
the information "lost", that is the fact that postgres will assume
that inserting new rows means a higher n_distinct?
* Am I right to assume that totalrows in the code
(src/backend/commands/analyze.c:2170) actually corresponds to
n_live_tup? That's what I gathered from glancing at the code, but I
might be wrong.
* Should the catalog version be changed for this kind of change?
* Should I add this patch to the commitfest?

If this patch is actually desired, I'll update the documentation as well.
I'm guessing this patch would break scripts relying on the pg_stats
view, but I do not know how much we want to avoid that, since they
should rely on the base tables rather than on the views.

Thanks in advance for your input!

Regards,
Maxence Ahlouche
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7723f01327..d0fd74075a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -194,7 +194,10 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
 stainherit AS inherited,
 stanullfrac AS null_frac,
 stawidth AS avg_width,
-stadistinct AS n_distinct,
+CASE
+WHEN stadistinct >= 0 THEN stadistinct
+ELSE -1 * stadistinct * pg_stat_get_live_tuples(c.oid)
+END AS n_distinct,
 CASE
 WHEN stakind1 = 1 THEN stavalues1
 WHEN stakind2 = 1 THEN stavalues2
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e0f2c543ef..e3dba42aef 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2142,7 +2142,10 @@ pg_stats| SELECT n.nspname AS schemaname,
 s.stainherit AS inherited,
 s.stanullfrac AS null_frac,
 s.stawidth AS avg_width,
-s.stadistinct AS n_distinct,
+CASE
+WHEN (s.stadistinct >= (0)::double precision) THEN (s.stadistinct)::double precision
+ELSE ((('-1'::integer)::double precision * s.stadistinct) * (pg_stat_get_live_tuples(c.oid))::double precision)
+END AS n_distinct,
 CASE
 WHEN (s.stakind1 = 1) THEN s.stavalues1
 WHEN (s.stakind2 = 1) THEN s.stavalues2