pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedv13: show extended stats target in \d
8+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2020-09-11 3c99230b4f aeb781107a psql: Display stats target of extended statistics
* v13: show extended stats target in \d
@ 2020-08-31 05:00 Justin Pryzby <pryzby@telsasoft.com>
2020-08-31 07:47 ` Re: v13: show extended stats target in \d gkokolatos@pm.me
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 2 replies; 8+ messages in thread
From: Justin Pryzby @ 2020-08-31 05:00 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>; Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
The stats target can be set since commit d06215d03, but wasn't shown by psql.
ALTER STATISISTICS .. SET STATISTICS n.
Normal (1-D) stats targets are shown in \d+ table.
Stats objects are shown in \d (no plus).
Arguably, this should be shown only in "verbose" mode (\d+).
Attachments:
0001-Show-stats-target-of-extended-statistics.patch (text/x-diff, 4.3K) download
From 60a4033a04fbaafbb01c2bb2d73bb2fbe4d1da75 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 30 Aug 2020 23:38:17 -0500
Subject: [PATCH 1/1] Show stats target of extended statistics
This can be set since d06215d03, but wasn't shown by psql.
Normal (1-D) stats targets are shown in \d+ table.
---
src/bin/psql/describe.c | 8 +++++++-
src/test/regress/expected/stats_ext.out | 18 ++++++++++++++++++
src/test/regress/sql/stats_ext.sql | 2 ++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d81f1575bf..73befa36ee 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2683,10 +2683,12 @@ describeOneTableDetails(const char *schemaname,
" a.attnum = s.attnum AND NOT attisdropped)) AS columns,\n"
" 'd' = any(stxkind) AS ndist_enabled,\n"
" 'f' = any(stxkind) AS deps_enabled,\n"
- " 'm' = any(stxkind) AS mcv_enabled\n"
+ " 'm' = any(stxkind) AS mcv_enabled,\n"
+ " %s"
"FROM pg_catalog.pg_statistic_ext stat "
"WHERE stxrelid = '%s'\n"
"ORDER BY 1;",
+ (pset.sversion >= 130000 ? "stxstattarget\n" : "-1\n"),
oid);
result = PSQLexec(buf.data);
@@ -2732,6 +2734,10 @@ describeOneTableDetails(const char *schemaname,
PQgetvalue(result, i, 4),
PQgetvalue(result, i, 1));
+ if (strcmp(PQgetvalue(result, i, 8), "-1") != 0)
+ appendPQExpBuffer(&buf, " STATISTICS %s",
+ PQgetvalue(result, i, 8));
+
printTableAddFooter(&cont, buf.data);
}
}
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 0ae779a3b9..4dea48a2e8 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -102,6 +102,15 @@ WARNING: statistics object "public.ab1_a_b_stats" could not be computed for rel
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+\d+ ab1
+ Table "public.ab1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | | | plain | |
+Statistics objects:
+ "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1 STATISTICS 0
+
ANALYZE ab1;
SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
FROM pg_statistic_ext s, pg_statistic_ext_data d
@@ -113,6 +122,15 @@ SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
(1 row)
ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
+\d+ ab1
+ Table "public.ab1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | | | plain | |
+Statistics objects:
+ "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1
+
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 2834a902a7..d1d49948b4 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -72,12 +72,14 @@ ANALYZE ab1;
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+\d+ ab1
ANALYZE ab1;
SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
FROM pg_statistic_ext s, pg_statistic_ext_data d
WHERE s.stxname = 'ab1_a_b_stats'
AND d.stxoid = s.oid;
ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
+\d+ ab1
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
ANALYZE ab1;
--
2.17.0
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-08-31 07:47 ` gkokolatos@pm.me
1 sibling, 0 replies; 8+ messages in thread
From: gkokolatos@pm.me @ 2020-08-31 07:47 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: pgsql-hackers; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, 31 August 2020 08:00, Justin Pryzby <pryzby@telsasoft.com> wrote:
> The stats target can be set since commit d06215d03, but wasn't shown by psql.
> ALTER STATISISTICS .. SET STATISTICS n.
>
> Normal (1-D) stats targets are shown in \d+ table.
> Stats objects are shown in \d (no plus).
> Arguably, this should be shown only in "verbose" mode (\d+).
This seems rather useful. May I suggest you add it to the commitfest?
//Georgios
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-09-09 21:36 ` Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 22:16 ` Re: v13: show extended stats target in \d Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-09 22:22 ` Re: v13: show extended stats target in \d Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-10 07:32 ` Re: v13: show extended stats target in \d Georgios <gkokolatos@protonmail.com>
2020-09-11 19:26 ` Re: v13: show extended stats target in \d Alvaro Herrera <alvherre@2ndquadrant.com>
1 sibling, 4 replies; 8+ messages in thread
From: Justin Pryzby @ 2020-09-09 21:36 UTC (permalink / raw)
To: Georgios Kokolatos <gkokolatos@protonmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Tue, Sep 01, 2020 at 12:35:19PM +0000, Georgios Kokolatos wrote:
> I will humbly disagree with the current review. I shall refrain from changing the status though because I do not have very strong feeling about it.
Sorry but this was in my spam, and didn't see until now.
>
> However the patch contains:
>
> - " 'm' = any(stxkind) AS mcv_enabled\n"
> + " 'm' = any(stxkind) AS mcv_enabled,\n"
> + " %s"
> "FROM pg_catalog.pg_statistic_ext stat "
> "WHERE stxrelid = '%s'\n"
> "ORDER BY 1;",
> + (pset.sversion >= 130000 ? "stxstattarget\n" : "-1\n"),
> oid);
>
> This seems to be breaking a bit the pattern in describeOneTableDetails().
> First, it is customary to write the whole query for the version in its own block. I do find this pattern rather helpful and clean. So in my humble opinion, the ternary expression should be replaced with a distinct if block that would implement stxstattarget. Second, setting the value to -1 as an indication of absence breaks the pattern a bit further. More on that bellow.
Hm, I did like this using the "hastriggers" code as a template. But you're
right that everywhere else does it differently.
> + if (strcmp(PQgetvalue(result, i, 8), "-1") != 0)
> + appendPQExpBuffer(&buf, " STATISTICS %s",
> + PQgetvalue(result, i, 8));
> +
>
> In the same function, one will find a bit of a different pattern for printing the statistics, e.g.
> if (strcmp(PQgetvalue(result, i, 7), "t") == 0)
> I will again hold the opinion that if the query gets crafted a bit differently, one can inspect if the table has `stxstattarget` and then, go ahead and print the value.
>
> Something in the lines of:
> if (strcmp(PQgetvalue(result, i, 8), "t") == 0)
> appendPQExpBuffer(&buf, " STATISTICS %s", PQgetvalue(result, i, 9));
I think what you've written wouldn't give the desired behavior, since it would
show the stats target even when it's set to the default. I don't see the point
of having additional, separate, version-specific boolean columns for 1) column
exists; 2) column is not default, in addition to 3) column value. But I added
comment about what Peter and I agree is desirable, anyway.
> Finally, the patch might be more complete if a note is added in the documentation.
> Have you considered adding something in doc/src/sgml/ref/psql-ref.sgml? If no, will you consider it? If yes, why did you discard it?
I don't think the details of psql output are currently documented. This shows
nothing about column statistics target, nor about MV stats at all.
https://www.postgresql.org/docs/13/app-psql.html
As for the discussion about a separator, I think maybe a comma is enough. I
doubt anyone is going to think that you can get a valid command by prefixing
this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
command without the stats target - after all, that's not true of indexes.
+ "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1, STATISTICS 0
This revision only shows the stats target in verbose mode (slash dee plus).
--
Justin
Attachments:
v2-0001-Show-stats-target-of-extended-statistics.patch (text/x-diff, 4.6K) download
From e5b351cc9b0518c9162a4b988b17ed920f09d952 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 30 Aug 2020 23:38:17 -0500
Subject: [PATCH v2] Show stats target of extended statistics
The stats target can be set since commit d06215d03, but wasn't shown by psql.
ALTER STATISISTICS .. SET STATISTICS n.
Traditional column stats targets are shown in \d+ table, so do the same for
extended/MV stats.
---
src/bin/psql/describe.c | 14 ++++++++++++--
src/test/regress/expected/stats_ext.out | 18 ++++++++++++++++++
src/test/regress/sql/stats_ext.sql | 2 ++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0861d74a6f..3c391fe686 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2683,8 +2683,13 @@ describeOneTableDetails(const char *schemaname,
" a.attnum = s.attnum AND NOT attisdropped)) AS columns,\n"
" 'd' = any(stxkind) AS ndist_enabled,\n"
" 'f' = any(stxkind) AS deps_enabled,\n"
- " 'm' = any(stxkind) AS mcv_enabled\n"
- "FROM pg_catalog.pg_statistic_ext stat "
+ " 'm' = any(stxkind) AS mcv_enabled");
+
+ if (pset.sversion >= 130000)
+ appendPQExpBufferStr(&buf, ",\n stxstattarget\n");
+ else
+ appendPQExpBufferStr(&buf, ",\n -1 AS stxstattarget\n");
+ appendPQExpBuffer(&buf, "FROM pg_catalog.pg_statistic_ext stat "
"WHERE stxrelid = '%s'\n"
"ORDER BY 1;",
oid);
@@ -2732,6 +2737,11 @@ describeOneTableDetails(const char *schemaname,
PQgetvalue(result, i, 4),
PQgetvalue(result, i, 1));
+ /* Show the stats target if it's not default */
+ if (verbose && strcmp(PQgetvalue(result, i, 8), "-1") != 0)
+ appendPQExpBuffer(&buf, ", STATISTICS %s",
+ PQgetvalue(result, i, 8));
+
printTableAddFooter(&cont, buf.data);
}
}
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 8c667d786a..aeea0e46a7 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -102,6 +102,15 @@ WARNING: statistics object "public.ab1_a_b_stats" could not be computed for rel
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+\d+ ab1
+ Table "public.ab1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | | | plain | |
+Statistics objects:
+ "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1, STATISTICS 0
+
ANALYZE ab1;
SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
FROM pg_statistic_ext s, pg_statistic_ext_data d
@@ -113,6 +122,15 @@ SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
(1 row)
ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
+\d+ ab1
+ Table "public.ab1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | | | plain | |
+Statistics objects:
+ "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1
+
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index f8d947af9e..ee522d83b1 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -72,12 +72,14 @@ ANALYZE ab1;
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+\d+ ab1
ANALYZE ab1;
SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
FROM pg_statistic_ext s, pg_statistic_ext_data d
WHERE s.stxname = 'ab1_a_b_stats'
AND d.stxoid = s.oid;
ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
+\d+ ab1
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
ANALYZE ab1;
--
2.17.0
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-09-09 22:16 ` Alvaro Herrera <alvherre@2ndquadrant.com>
3 siblings, 0 replies; 8+ messages in thread
From: Alvaro Herrera @ 2020-09-09 22:16 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; pgsql-hackers@lists.postgresql.org, Tomas Vondra <tomas.vondra@2ndquadrant.com>
On 2020-Sep-09, Justin Pryzby wrote:
> As for the discussion about a separator, I think maybe a comma is enough. I
> doubt anyone is going to think that you can get a valid command by prefixing
> this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
> command without the stats target - after all, that's not true of indexes.
>
> + "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1, STATISTICS 0
I vote to use a semicolon instead of comma, and otherwise +1.
> This revision only shows the stats target in verbose mode (slash dee plus).
I don't think this interferes enough with other stuff to relegate it to
the "plus" version, since it's not shown if default.
Tomas -- this needs to go in pg13, right?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-09-09 22:22 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-09 23:07 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
3 siblings, 1 reply; 8+ messages in thread
From: Alvaro Herrera @ 2020-09-09 22:22 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; pgsql-hackers@lists.postgresql.org
On 2020-Sep-09, Justin Pryzby wrote:
> As for the discussion about a separator, I think maybe a comma is enough. I
> doubt anyone is going to think that you can get a valid command by prefixing
> this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
> command without the stats target - after all, that's not true of indexes.
By the way, I got into this because of your comment in
https://postgr.es/m/20200901011429.GZ5450@telsasoft.com on whether we
needed CREATE STATISTICS to have a clause for this.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 22:22 ` Re: v13: show extended stats target in \d Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-09-09 23:07 ` Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 0 replies; 8+ messages in thread
From: Justin Pryzby @ 2020-09-09 23:07 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; pgsql-hackers@lists.postgresql.org
On Wed, Sep 09, 2020 at 07:22:30PM -0300, Alvaro Herrera wrote:
> On 2020-Sep-09, Justin Pryzby wrote:
>
> > As for the discussion about a separator, I think maybe a comma is enough. I
> > doubt anyone is going to think that you can get a valid command by prefixing
> > this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
> > command without the stats target - after all, that's not true of indexes.
>
> By the way, I got into this because of your comment in
> https://postgr.es/m/20200901011429.GZ5450@telsasoft.com on whether we
> needed CREATE STATISTICS to have a clause for this.
And I agree that it does not - the question was raised during development of
the feature, and nobody sees a need. I asked since I didn't know and I didn't
want it to be missed for lack of asking.
--
Justin
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-09-10 07:32 ` Georgios <gkokolatos@protonmail.com>
3 siblings, 0 replies; 8+ messages in thread
From: Georgios @ 2020-09-10 07:32 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, 10 September 2020 00:36, Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Tue, Sep 01, 2020 at 12:35:19PM +0000, Georgios Kokolatos wrote:
>
> > I will humbly disagree with the current review. I shall refrain from changing the status though because I do not have very strong feeling about it.
>
> Sorry but this was in my spam, and didn't see until now.
No worries at all. Thank you for replying.
>
> > However the patch contains:
> >
> > - " 'm' = any(stxkind) AS mcv_enabled\\n"
> >
> >
> >
> > - " 'm' = any(stxkind) AS mcv_enabled,\\n"
> >
> >
> > - " %s"
> > "FROM pg_catalog.pg_statistic_ext stat "
> > "WHERE stxrelid = '%s'\\n"
> > "ORDER BY 1;",
> >
> >
> > - (pset.sversion >= 130000 ? "stxstattarget\\n" : "-1\\n"),
> > oid);
> >
> >
> >
> > This seems to be breaking a bit the pattern in describeOneTableDetails().
> > First, it is customary to write the whole query for the version in its own block. I do find this pattern rather helpful and clean. So in my humble opinion, the ternary expression should be replaced with a distinct if block that would implement stxstattarget. Second, setting the value to -1 as an indication of absence breaks the pattern a bit further. More on that bellow.
>
> Hm, I did like this using the "hastriggers" code as a template. But you're
> right that everywhere else does it differently.
Thank you for taking my input.
>
> > - if (strcmp(PQgetvalue(result, i, 8), "-1") != 0)
> >
> >
> > - appendPQExpBuffer(&buf, " STATISTICS %s",
> >
> >
> > - PQgetvalue(result, i, 8));
> >
> >
> > -
> >
> > In the same function, one will find a bit of a different pattern for printing the statistics, e.g.
> > if (strcmp(PQgetvalue(result, i, 7), "t") == 0)
> > I will again hold the opinion that if the query gets crafted a bit differently, one can inspect if the table has `stxstattarget` and then, go ahead and print the value.
> > Something in the lines of:
> > if (strcmp(PQgetvalue(result, i, 8), "t") == 0)
> > appendPQExpBuffer(&buf, " STATISTICS %s", PQgetvalue(result, i, 9));
>
> I think what you've written wouldn't give the desired behavior, since it would
> show the stats target even when it's set to the default. I don't see the point
> of having additional, separate, version-specific boolean columns for 1) column
> exists; 2) column is not default, in addition to 3) column value. But I added
> comment about what Peter and I agree is desirable, anyway.
Fair enough. As I said above, I do not have a very strong feeling, so it gets my +1 if it is worth anything.
>
> > Finally, the patch might be more complete if a note is added in the documentation.
> > Have you considered adding something in doc/src/sgml/ref/psql-ref.sgml? If no, will you consider it? If yes, why did you discard it?
>
> I don't think the details of psql output are currently documented. This shows
> nothing about column statistics target, nor about MV stats at all.
> https://www.postgresql.org/docs/13/app-psql.html
Yeah, I have noticed that. Hence my question. If anything maybe the documentation can be expanded to cover these cases in a different patch. Thank you for your answer.
>
> As for the discussion about a separator, I think maybe a comma is enough. I
> doubt anyone is going to think that you can get a valid command by prefixing
> this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
> command without the stats target - after all, that's not true of indexes.
>
> - "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1, STATISTICS 0
>
> This revision only shows the stats target in verbose mode (slash dee plus).
>
> --
> Justin
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: v13: show extended stats target in \d
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 21:36 ` Re: v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
@ 2020-09-11 19:26 ` Alvaro Herrera <alvherre@2ndquadrant.com>
3 siblings, 0 replies; 8+ messages in thread
From: Alvaro Herrera @ 2020-09-11 19:26 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; pgsql-hackers@lists.postgresql.org
On 2020-Sep-09, Justin Pryzby wrote:
> As for the discussion about a separator, I think maybe a comma is enough. I
> doubt anyone is going to think that you can get a valid command by prefixing
> this by "CREATE STATISTICS". Actually, it didn't even occur to me it was valid
> command without the stats target - after all, that's not true of indexes.
>
> + "public"."ab1_a_b_stats" (ndistinct, dependencies, mcv) ON a, b FROM ab1, STATISTICS 0
>
> This revision only shows the stats target in verbose mode (slash dee plus).
I put it back to show in non-verbose mode; after all it's only in the
infrequent case that it's set to anything at all, and also it doesn't
take up another column, as is the case with per-column stats targets.
I changed the separator from comma to semicolon, because it seems a
little less ambiguous. It's easy to change if people hate that.
... and pushed. Thanks.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2020-09-11 19:26 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31 05:00 v13: show extended stats target in \d Justin Pryzby <pryzby@telsasoft.com>
2020-08-31 07:47 ` gkokolatos@pm.me
2020-09-09 21:36 ` Justin Pryzby <pryzby@telsasoft.com>
2020-09-09 22:16 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-09 22:22 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-09 23:07 ` Justin Pryzby <pryzby@telsasoft.com>
2020-09-10 07:32 ` Georgios <gkokolatos@protonmail.com>
2020-09-11 19:26 ` Alvaro Herrera <alvherre@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox