Include access method in listTables output
Hi,
Postgres 12 introduced TableAm api. Although as far as I can see, currently only heap is
included as access method, it is fair to imagine that users will start adding their own methods
and more methods to be included in Postgres core.
With that in mind, it might be desirable for a user to see the access method when describing
in verbose mode, eg. `\d+`.
A small patch is attached [1] to see if you think it makes sense. I have not included any
differences in the tests output yet, as the idea might get discarded. However if the patch is
found useful. I shall ament the test results as needed.
Cheers,
//Georgios
Attachments:
0001-Include-AM-in-listTables-output.patchapplication/octet-stream; name=0001-Include-AM-in-listTables-output.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cd..766aba218c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,13 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+ }
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3779,11 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000)
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
On Tue, 9 Jun 2020 at 23:03, Georgios <gkokolatos@protonmail.com> wrote:
A small patch is attached [1] to see if you think it makes sense. I have not included any
differences in the tests output yet, as the idea might get discarded. However if the patch is
found useful. I shall ament the test results as needed.
It seems like a fair thing to need/want. We do already show this in
\d+ tablename, so it seems pretty fair to want it in the \d+ output
too
Please add it to the commitfest at https://commitfest.postgresql.org/28/
David
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, June 9, 2020 1:34 PM, David Rowley <dgrowleyml@gmail.com> wrote:
On Tue, 9 Jun 2020 at 23:03, Georgios gkokolatos@protonmail.com wrote:
A small patch is attached [1] to see if you think it makes sense. I have not included any
differences in the tests output yet, as the idea might get discarded. However if the patch is
found useful. I shall ament the test results as needed.It seems like a fair thing to need/want. We do already show this in
\d+ tablename, so it seems pretty fair to want it in the \d+ output
tooPlease add it to the commitfest at https://commitfest.postgresql.org/28/
Thank you very much for your time. Added to the commitfest as suggested.
Show quoted text
David
On Tue, Jun 9, 2020 at 6:45 PM Georgios <gkokolatos@protonmail.com> wrote:
Please add it to the commitfest at https://commitfest.postgresql.org/28/
Thank you very much for your time. Added to the commitfest as suggested.
Patch applies cleanly, make check & make check-world passes.
Few comments:
+ if (pset.sversion >= 120000)
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");Should we include pset.hide_tableam check along with the version check?
+ if (pset.sversion >= 120000 && !pset.hide_tableam)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+ }Braces can be removed & the second line can be combined with earlier.
We could include the test in psql file by configuring HIDE_TABLEAM.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, June 15, 2020 3:20 AM, vignesh C <vignesh21@gmail.com> wrote:
On Tue, Jun 9, 2020 at 6:45 PM Georgios gkokolatos@protonmail.com wrote:
Please add it to the commitfest at https://commitfest.postgresql.org/28/
Thank you very much for your time. Added to the commitfest as suggested.
Patch applies cleanly, make check & make check-world passes.
Thank you for your time and comments! Please find v2 of the patch
attached.
Few comments:
- if (pset.sversion >= 120000)
- appendPQExpBufferStr(&buf,
- "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
Should we include pset.hide_tableam check along with the version check?
I opted against it, since it seems more intuitive to have a single
switch and placed on the display part. A similar pattern can be found
in describeOneTableDetails(). I do not hold a strong opinion and will
gladly ament if insisted upon.
- if (pset.sversion >= 120000 && !pset.hide_tableam)
- {
- appendPQExpBuffer(&buf,
- ",\n am.amname as \"%s\"",
- gettext_noop("Access Method"));
- }
Braces can be removed & the second line can be combined with earlier.
Agreed on the braces.
Disagree on the second line as this style is in line with the rest of
code. Consistency, buf, format string and gettext_noop() are found in
their own lines within this file.
We could include the test in psql file by configuring HIDE_TABLEAM.
Agreed.
Show quoted text
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Attachments:
0001-Include-AM-in-listTables-output-v2.patchapplication/octet-stream; name=0001-Include-AM-in-listTables-output-v2.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cd..12ead0b8b9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,11 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam)
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3777,11 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000)
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc..14a563e67f 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2796,19 +2796,22 @@ Type | func
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
-- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2819,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2827,34 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+---------------+-------+----------+-------------+---------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | georgios | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | georgios | permanent | heap_psql | 0 bytes |
+(2 rows)
+
\set HIDE_TABLEAM on
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------------+-------+----------+-------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | georgios | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | georgios | permanent | 0 bytes |
+(2 rows)
+
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index bd10aec6d6..795f39af41 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -456,8 +456,11 @@ select 1 where false;
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
-- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
@@ -466,9 +469,13 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
\set HIDE_TABLEAM on
+\d+
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
-- test numericlocale (as best we can without control of psql's locale)
On Tue, Jun 16, 2020 at 6:13 PM Georgios <gkokolatos@protonmail.com> wrote:
Few comments:
- if (pset.sversion >= 120000)
- appendPQExpBufferStr(&buf,
- "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
Should we include pset.hide_tableam check along with the version check?
I opted against it, since it seems more intuitive to have a single
switch and placed on the display part. A similar pattern can be found
in describeOneTableDetails(). I do not hold a strong opinion and will
gladly ament if insisted upon.
I felt we could add that check as we might be including
pg_catalog.pg_am in cases even though we really don't need it.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, June 20, 2020 3:15 PM, vignesh C <vignesh21@gmail.com> wrote:
On Tue, Jun 16, 2020 at 6:13 PM Georgios gkokolatos@protonmail.com wrote:
Few comments:
- if (pset.sversion >= 120000)
- appendPQExpBufferStr(&buf,
- "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
Should we include pset.hide_tableam check along with the version check?I opted against it, since it seems more intuitive to have a single
switch and placed on the display part. A similar pattern can be found
in describeOneTableDetails(). I do not hold a strong opinion and will
gladly ament if insisted upon.I felt we could add that check as we might be including
pg_catalog.pg_am in cases even though we really don't need it.
As promised, I gladly ament upon your request. Also included a fix for
a minor oversight in tests, they should now be stable. Finally in this
version, I extended a bit the logic to only include the access method column
if the relations displayed can have one, for example sequences.
Show quoted text
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Attachments:
0001-Include-AM-in-listTables-output-v3.patchapplication/octet-stream; name=0001-Include-AM-in-listTables-output-v3.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cd..d39e01dba7 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showViews || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showViews || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc..4089125b7b 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2796,19 +2796,24 @@ Type | func
\pset tuples_only false
-- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2821,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2829,43 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+(2 rows)
+
+-- access method column should not be displayed for sequences
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
\set HIDE_TABLEAM on
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | 0 bytes |
+(2 rows)
+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index bd10aec6d6..4e353e541d 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -456,9 +456,14 @@ select 1 where false;
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
@@ -466,9 +471,17 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
+-- access method column should not be displayed for sequences
+\ds+
\set HIDE_TABLEAM on
+\d+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
On Tue, Jun 30, 2020 at 2:53 PM Georgios <gkokolatos@protonmail.com> wrote:
As promised, I gladly ament upon your request. Also included a fix for
a minor oversight in tests, they should now be stable. Finally in this
version, I extended a bit the logic to only include the access method column
if the relations displayed can have one, for example sequences.
Patch applies cleanly, make check & make check-world passes.
One comment:
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showViews || showMatViews ||
showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n
am.amname as \"%s\"",
+
gettext_noop("Access Method"));I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.
+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().
--
Michael‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, July 6, 2020 3:12 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().
Thank you for the review.
Find attached v4 of the patch.
Show quoted text
---------------------------------------------------------------------------------------------------------------
Michael
Attachments:
0001-Include-AM-in-listTables-output-v4.patchapplication/octet-stream; name=0001-Include-AM-in-listTables-output-v4.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cd..520f92ec49 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc..4089125b7b 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2796,19 +2796,24 @@ Type | func
\pset tuples_only false
-- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2821,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2829,43 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+(2 rows)
+
+-- access method column should not be displayed for sequences
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
\set HIDE_TABLEAM on
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | 0 bytes |
+(2 rows)
+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index bd10aec6d6..4e353e541d 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -456,9 +456,14 @@ select 1 where false;
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
@@ -466,9 +471,17 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
+-- access method column should not be displayed for sequences
+\ds+
\set HIDE_TABLEAM on
+\d+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
On Mon, Jul 6, 2020 at 1:24 PM Georgios <gkokolatos@protonmail.com> wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, July 6, 2020 3:12 AM, Michael Paquier <michael@paquier.xyz> wrote:On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().Thank you for the review.
Find attached v4 of the patch.
Thanks for fixing the comments.
Patch applies cleanly, make check & make check-world passes.
I was not sure if any documentation change is required or not for this
in doc/src/sgml/ref/psql-ref.sgml. Thoughts?
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, July 11, 2020 3:16 PM, vignesh C <vignesh21@gmail.com> wrote:
On Mon, Jul 6, 2020 at 1:24 PM Georgios gkokolatos@protonmail.com wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, July 6, 2020 3:12 AM, Michael Paquier michael@paquier.xyz wrote:On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().Thank you for the review.
Find attached v4 of the patch.Thanks for fixing the comments.
Patch applies cleanly, make check & make check-world passes.
I was not sure if any documentation change is required or not for this
in doc/src/sgml/ref/psql-ref.sgml. Thoughts?
Thank you for the comment. I added a line in docs. Admittedly, might need tweaking.
Please find version 5 of the patch attached.
Show quoted text
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Attachments:
0001-Include-AM-in-listTables-output-v5.patchapplication/octet-stream; name=0001-Include-AM-in-listTables-output-v5.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 42e862cf17..1b6fc296de 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1177,7 +1177,9 @@ testdb=>
columns of the table are shown, as is the presence of OIDs in the
table, the view definition if the relation is a view, a non-default
<link linkend="sql-createtable-replica-identity">replica
- identity</link> setting.
+ identity</link> setting, the access method name
+ <link linkend="sql-create-access-method"> if the relation has access
+ method.
</para>
<para>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cd..520f92ec49 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc..4089125b7b 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2796,19 +2796,24 @@ Type | func
\pset tuples_only false
-- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2821,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2829,43 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+(2 rows)
+
+-- access method column should not be displayed for sequences
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
\set HIDE_TABLEAM on
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | 0 bytes |
+(2 rows)
+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index bd10aec6d6..4e353e541d 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -456,9 +456,14 @@ select 1 where false;
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
@@ -466,9 +471,17 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
+-- access method column should not be displayed for sequences
+\ds+
\set HIDE_TABLEAM on
+\d+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
On Thu, Jul 16, 2020 at 7:35 PM Georgios <gkokolatos@protonmail.com> wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, July 11, 2020 3:16 PM, vignesh C <vignesh21@gmail.com> wrote:On Mon, Jul 6, 2020 at 1:24 PM Georgios gkokolatos@protonmail.com wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, July 6, 2020 3:12 AM, Michael Paquier michael@paquier.xyz wrote:On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().Thank you for the review.
Find attached v4 of the patch.Thanks for fixing the comments.
Patch applies cleanly, make check & make check-world passes.
I was not sure if any documentation change is required or not for this
in doc/src/sgml/ref/psql-ref.sgml. Thoughts?Thank you for the comment. I added a line in docs. Admittedly, might need tweaking.
Please find version 5 of the patch attached.
Most changes looks fine, minor comments:
\pset tuples_only false
-- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO
conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;This comment might have been removed unintentionally, do you want to
add it back?
+-- access method column should not be displayed for sequences
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)We can include one test for view.
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char
*pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))If conditions in both the places are the same, do you want to make it a macro?
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, July 25, 2020 2:41 AM, vignesh C <vignesh21@gmail.com> wrote:
On Thu, Jul 16, 2020 at 7:35 PM Georgios gkokolatos@protonmail.com wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, July 11, 2020 3:16 PM, vignesh C vignesh21@gmail.com wrote:On Mon, Jul 6, 2020 at 1:24 PM Georgios gkokolatos@protonmail.com wrote:
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, July 6, 2020 3:12 AM, Michael Paquier michael@paquier.xyz wrote:On Sun, Jul 05, 2020 at 07:13:10AM +0530, vignesh C wrote:
I'm not sure if we should include showViews, I had seen that the
access method was not getting selected for view.+1. These have no physical storage, so you are looking here for
relkinds that satisfy RELKIND_HAS_STORAGE().Thank you for the review.
Find attached v4 of the patch.Thanks for fixing the comments.
Patch applies cleanly, make check & make check-world passes.
I was not sure if any documentation change is required or not for this
in doc/src/sgml/ref/psql-ref.sgml. Thoughts?Thank you for the comment. I added a line in docs. Admittedly, might need tweaking.
Please find version 5 of the patch attached.Most changes looks fine, minor comments:
Thank you for your comments. Please find the individual answers inline for completeness.
I'm having issues understanding where you are going with the reviews, can you fully describe
what is it that you wish to be done?
\pset tuples_only false -- check conditional tableam display --- Create a heap2 table am handler with heapam handler +\pset expanded off +CREATE SCHEMA conditional_tableam_display; +CREATE ROLE conditional_tableam_display_role; +ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role; +SET search_path TO conditional_tableam_display; CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;This comment might have been removed unintentionally, do you want to
add it back?
It was intentional as heap2 table am handler is not getting created. With
the code additions the comment does seem out of place and thus removed.
+-- access method column should not be displayed for sequences +\ds+- List of relations
- Schema | Name | Type | Owner | Persistence | Size | Description +--------+------+------+-------+-------------+------+------------- +(0 rows)We can include one test for view.
The list of cases in the test for both including and excluding storage is by no means
exhaustive. I thought that this is acceptable. Adding a test for the view, will still
not make it the test exhaustive. Are you sure you only suggest the view to be included
or you would suggest an exhaustive list of tests.
- if (pset.sversion >= 120000 && !pset.hide_tableam &&
- (showTables || showMatViews || showIndexes))
- appendPQExpBuffer(&buf,
- ",\n am.amname as \"%s\"",
- gettext_noop("Access Method"));
- /* - As of PostgreSQL 9.0, use pg_table_size() to show a more accurate - size of a table, including FSM, VM and TOAST tables. @@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_class c" "\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");-
- if (pset.sversion >= 120000 && !pset.hide_tableam &&- (showTables || showMatViews || showIndexes))
If conditions in both the places are the same, do you want to make it a macro?
No, I would rather not if you may. I think that a macro would not add to the readability
rather it would remove from it. Those are two simple conditionals used in the same function
very close to each other.
Show quoted text
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jul 29, 2020 at 7:30 PM Georgios <gkokolatos@protonmail.com> wrote:
I'm having issues understanding where you are going with the reviews, can you fully describe
what is it that you wish to be done?
I'm happy with the patch, that was the last of the comments that I had
from my side. Only idea here is to review every line of the code
before passing it to the committer.
\pset tuples_only false -- check conditional tableam display --- Create a heap2 table am handler with heapam handler +\pset expanded off +CREATE SCHEMA conditional_tableam_display; +CREATE ROLE conditional_tableam_display_role; +ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role; +SET search_path TO conditional_tableam_display; CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;This comment might have been removed unintentionally, do you want to
add it back?It was intentional as heap2 table am handler is not getting created. With
the code additions the comment does seem out of place and thus removed.
Thanks for clarifying it, I wasn't sure if it was completely intentional.
+-- access method column should not be displayed for sequences +\ds+- List of relations
- Schema | Name | Type | Owner | Persistence | Size | Description +--------+------+------+-------+-------------+------+------------- +(0 rows)We can include one test for view.
The list of cases in the test for both including and excluding storage is by no means
exhaustive. I thought that this is acceptable. Adding a test for the view, will still
not make it the test exhaustive. Are you sure you only suggest the view to be included
or you would suggest an exhaustive list of tests.
I had noticed this case while reviewing, you can take a call on it. It
is very difficult to come to a conclusion on what needs to be included
and what need not be included. I had noticed you have added a test
case for sequence. I felt views are similar in this case.
- if (pset.sversion >= 120000 && !pset.hide_tableam &&
- (showTables || showMatViews || showIndexes))
- appendPQExpBuffer(&buf,
- ",\n am.amname as \"%s\"",
- gettext_noop("Access Method"));
- /* - As of PostgreSQL 9.0, use pg_table_size() to show a more accurate - size of a table, including FSM, VM and TOAST tables. @@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_class c" "\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");-
- if (pset.sversion >= 120000 && !pset.hide_tableam &&- (showTables || showMatViews || showIndexes))
If conditions in both the places are the same, do you want to make it a macro?
No, I would rather not if you may. I think that a macro would not add to the readability
rather it would remove from it. Those are two simple conditionals used in the same function
very close to each other.
Ok, we can ignore this.
Regards,
Vignesh
On Sat, Aug 1, 2020 at 8:12 AM vignesh C <vignesh21@gmail.com> wrote:
+-- access method column should not be displayed for sequences +\ds+- List of relations
- Schema | Name | Type | Owner | Persistence | Size | Description +--------+------+------+-------+-------------+------+------------- +(0 rows)We can include one test for view.
I felt adding one test for view is good and added it.
Attached a new patch for the same.
I felt patch is in shape for committer to have a look at this.
Regards,
Vignesh
Attachments:
v6-0001-Include-access-method-in-listTables-output.patchapplication/x-patch; name=v6-0001-Include-access-method-in-listTables-output.patchDownload
From 59d102e21c0261c7349fc5ad55026d0125972836 Mon Sep 17 00:00:00 2001
From: Georgios <gkokolatos@protonmail.com>
Date: Mon, 17 Aug 2020 22:51:17 +0530
Subject: [PATCH v6] Include access method in listTables output
Author: Georgios <gkokolatos@protonmail.com>
Discussion: https://www.postgresql.org/message-id/svaS1VTOEscES9CLKVTeKItjJP1EEJuBhTsA0ESOdlnbXeQSgycYwVlliL5zt8Jwcfo4ATYDXtEqsExxjkSkkhCSTCL8fnRgaCAJdr0unUg%3D%40protonmail.com
---
doc/src/sgml/ref/psql-ref.sgml | 4 +++-
src/bin/psql/describe.c | 14 ++++++++++-
src/test/regress/expected/psql.out | 49 ++++++++++++++++++++++++++++++++++----
src/test/regress/sql/psql.sql | 17 ++++++++++++-
4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index fc16e6c..7667687 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1183,7 +1183,9 @@ testdb=>
columns of the table are shown, as is the presence of OIDs in the
table, the view definition if the relation is a view, a non-default
<link linkend="sql-createtable-replica-identity">replica
- identity</link> setting.
+ identity</link> setting, the access method name
+ <link linkend="sql-create-access-method"> if the relation has access
+ method.
</para>
<para>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d81f157..ac6a93c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 555d464..e497adf 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2796,19 +2796,24 @@ Type | func
\pset tuples_only false
-- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2821,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2829,50 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+(2 rows)
+
+-- access method column should not be displayed for sequences
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
+-- access method column should not be displayed for views
+\dv+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
\set HIDE_TABLEAM on
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------------+-------+----------------------------------+-------------+---------+-------------
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | 0 bytes |
+(2 rows)
+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 5a16080..920652b 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -456,9 +456,14 @@ select 1 where false;
\pset tuples_only false
-- check conditional tableam display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\d+ tbl_heap_psql
@@ -466,9 +471,19 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
+-- access method column should not be displayed for sequences
+\ds+
+-- access method column should not be displayed for views
+\dv+
\set HIDE_TABLEAM on
+\d+
+SET ROLE TO default;
DROP TABLE tbl_heap, tbl_heap_psql;
DROP ACCESS METHOD heap_psql;
+SET search_path TO default;
+DROP SCHEMA conditional_tableam_display;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
--
1.8.3.1
On Mon, Aug 17, 2020 at 11:10:05PM +0530, vignesh C wrote:
On Sat, Aug 1, 2020 at 8:12 AM vignesh C <vignesh21@gmail.com> wrote:
+-- access method column should not be displayed for sequences +\ds+- List of relations
- Schema | Name | Type | Owner | Persistence | Size | Description +--------+------+------+-------+-------------+------+------------- +(0 rows)We can include one test for view.
I felt adding one test for view is good and added it.
Attached a new patch for the same.I felt patch is in shape for committer to have a look at this.
The patch tester shows it's failing xmllint ; could you send a fixed patch ?
/usr/bin/xmllint --path . --noout --valid postgres.sgml
ref/psql-ref.sgml:1189: parser error : Opening and ending tag mismatch: link line 1187 and para
http://cfbot.cputube.org/georgios-kokolatos.html
--
Justin
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, 20 August 2020 07:31, Justin Pryzby <pryzby@telsasoft.com> wrote:
On Mon, Aug 17, 2020 at 11:10:05PM +0530, vignesh C wrote:
On Sat, Aug 1, 2020 at 8:12 AM vignesh C vignesh21@gmail.com wrote:
+-- access method column should not be displayed for sequences +\ds+- List of relations
- Schema | Name | Type | Owner | Persistence | Size | Description +--------+------+------+-------+-------------+------+------------- +(0 rows) We can include one test for view.I felt adding one test for view is good and added it.
Attached a new patch for the same.
I felt patch is in shape for committer to have a look at this.The patch tester shows it's failing xmllint ; could you send a fixed patch ?
/usr/bin/xmllint --path . --noout --valid postgres.sgml
ref/psql-ref.sgml:1189: parser error : Opening and ending tag mismatch: link line 1187 and para
Thank you for pointing it out!
Please find version 7 attached which hopefully addresses the error along with a proper
expansion of the test coverage and removal of recently introduced
whitespace warnings.
Show quoted text
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Justin
Attachments:
0001-Include-access-method-in-listTables-output-v7.patchapplication/octet-stream; name=0001-Include-access-method-in-listTables-output-v7.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index fc16e6c0c4..b96b5dedb6 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1183,7 +1183,9 @@ testdb=>
columns of the table are shown, as is the presence of OIDs in the
table, the view definition if the relation is a view, a non-default
<link linkend="sql-createtable-replica-identity">replica
- identity</link> setting.
+ identity</link> setting, the access method name
+ <link linkend="sql-create-access-method"> if the relation has access
+ method.</link>
</para>
<para>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d81f1575bf..ac6a93cd0b 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3778,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 555d464f91..2d0fc717a0 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2795,20 +2795,31 @@ Argument data types | numeric
Type | func
\pset tuples_only false
--- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+-- check conditional am display
+\pset expanded off
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
+CREATE VIEW view_heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE MATERIALIZED VIEW mat_view_heap_psql USING heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE INDEX tbl_heap_psql_idx ON tbl_heap_psql(f1);
+CREATE SEQUENCE foo_seq START WITH 1;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
+Indexes:
+ "tbl_heap_psql_idx" btree (f1)
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,24 +2827,92 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "conditional_tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
+Indexes:
+ "tbl_heap_psql_idx" btree (f1)
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "conditional_tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+--------------------+-------------------+----------------------------------+-------------+---------------+------------+-------------
+ conditional_tableam_display | foo_seq | sequence | conditional_tableam_display_role | permanent | | 8192 bytes |
+ conditional_tableam_display | mat_view_heap_psql | materialized view | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | heap | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+ conditional_tableam_display | view_heap_psql | view | conditional_tableam_display_role | permanent | | 0 bytes |
+(5 rows)
+
+\di+
+ List of relations
+ Schema | Name | Type | Owner | Table | Persistence | Access Method | Size | Description
+-----------------------------+-------------------+-------+----------------------------------+---------------+-------------+---------------+------------+-------------
+ conditional_tableam_display | tbl_heap_psql_idx | index | conditional_tableam_display_role | tbl_heap_psql | permanent | btree | 8192 bytes |
+(1 row)
+
+\dm+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------------------+--------------------+-------------------+----------------------------------+-------------+---------------+---------+-------------
+ conditional_tableam_display | mat_view_heap_psql | materialized view | conditional_tableam_display_role | permanent | heap_psql | 0 bytes |
+(1 row)
+
+-- following do not have am hence the column is not displayed
+\ds+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+---------+----------+----------------------------------+-------------+------------+-------------
+ conditional_tableam_display | foo_seq | sequence | conditional_tableam_display_role | permanent | 8192 bytes |
+(1 row)
+
+\dv+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+----------------+------+----------------------------------+-------------+---------+-------------
+ conditional_tableam_display | view_heap_psql | view | conditional_tableam_display_role | permanent | 0 bytes |
+(1 row)
+
+\dE+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+--------+------+------+-------+-------------+------+-------------
+(0 rows)
+
\set HIDE_TABLEAM on
-DROP TABLE tbl_heap, tbl_heap_psql;
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------------------+--------------------+-------------------+----------------------------------+-------------+------------+-------------
+ conditional_tableam_display | foo_seq | sequence | conditional_tableam_display_role | permanent | 8192 bytes |
+ conditional_tableam_display | mat_view_heap_psql | materialized view | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | tbl_heap_psql | table | conditional_tableam_display_role | permanent | 0 bytes |
+ conditional_tableam_display | view_heap_psql | view | conditional_tableam_display_role | permanent | 0 bytes |
+(5 rows)
+
+RESET role;
+RESET search_path;
+DROP SCHEMA conditional_tableam_display CASCADE;
+NOTICE: drop cascades to 5 other objects
+DETAIL: drop cascades to table conditional_tableam_display.tbl_heap_psql
+drop cascades to table conditional_tableam_display.tbl_heap
+drop cascades to view conditional_tableam_display.view_heap_psql
+drop cascades to materialized view conditional_tableam_display.mat_view_heap_psql
+drop cascades to sequence conditional_tableam_display.foo_seq
DROP ACCESS METHOD heap_psql;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 5a16080980..a21478ba09 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -455,20 +455,40 @@ select 1 where false;
\df exp
\pset tuples_only false
--- check conditional tableam display
+-- check conditional am display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA conditional_tableam_display;
+CREATE ROLE conditional_tableam_display_role;
+ALTER SCHEMA conditional_tableam_display OWNER TO conditional_tableam_display_role;
+SET search_path TO conditional_tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET role TO conditional_tableam_display_role;
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
+CREATE VIEW view_heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE MATERIALIZED VIEW mat_view_heap_psql USING heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE INDEX tbl_heap_psql_idx ON tbl_heap_psql(f1);
+CREATE SEQUENCE foo_seq START WITH 1;
\d+ tbl_heap_psql
\d+ tbl_heap
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+\d+
+\di+
+\dm+
+-- following do not have am hence the column is not displayed
+\ds+
+\dv+
+\dE+
\set HIDE_TABLEAM on
-DROP TABLE tbl_heap, tbl_heap_psql;
+\d+
+RESET role;
+RESET search_path;
+DROP SCHEMA conditional_tableam_display CASCADE;
DROP ACCESS METHOD heap_psql;
+DROP ROLE conditional_tableam_display_role;
-- test numericlocale (as best we can without control of psql's locale)
On Thu, Aug 20, 2020 at 08:16:19AM +0000, Georgios wrote:
Please find version 7 attached which hopefully addresses the error along with a proper
expansion of the test coverage and removal of recently introduced
whitespace warnings.
+CREATE ROLE conditional_tableam_display_role;
As a convention, regression tests need to have roles prefixed with
"regress_" or this would cause some buildfarm members to turn red.
Please see -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS (you could use
that in your environment for example).
So, as of the tests.. The role gets added to make sure that when
using \d+ on the full schema as well as the various \d*+ variants we
have a consistent owner. The addition of the relation size for the
sequence and the btree index in the output generated is a problem
though, because that's not really portable when compiling with other
page sizes. It is true that there are other tests failing in this
case, but I think that we should try to limit that if we can. In
short, I agree that having some tests is better than nothing, but I
would suggest to reduce their scope, as per the attached.
Adding \dE as there are no foreign tables does not make much sense,
and also I wondered why \dt+ was not added.
Does the attached look correct to you?
--
Michael
Attachments:
psql-am-michael.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d81f1575bf..cdfec34389 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3678,7 +3678,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PGresult *res;
printQueryOpt myopt = pset.popt;
int cols_so_far;
- bool translate_columns[] = {false, false, true, false, false, false, false, false};
+ bool translate_columns[] = {false, false, true, false, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3751,6 +3751,16 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* to; this might change with future additions to the output columns.
*/
+ /*
+ * Access methods exist for tables, materialized views and indexes.
+ * This has been introduced in PostgreSQL 12.
+ */
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBuffer(&buf,
+ ",\n am.amname as \"%s\"",
+ gettext_noop("Access Method"));
+
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
@@ -3772,6 +3782,12 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
+
+ if (pset.sversion >= 120000 && !pset.hide_tableam &&
+ (showTables || showMatViews || showIndexes))
+ appendPQExpBufferStr(&buf,
+ "\n LEFT JOIN pg_catalog.pg_am am ON am.oid = c.relam");
+
if (showIndexes)
appendPQExpBufferStr(&buf,
"\n LEFT JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid"
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 555d464f91..daac0ff49d 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2795,20 +2795,28 @@ Argument data types | numeric
Type | func
\pset tuples_only false
--- check conditional tableam display
--- Create a heap2 table am handler with heapam handler
+-- check conditional am display
+\pset expanded off
+CREATE SCHEMA tableam_display;
+CREATE ROLE regress_display_role;
+ALTER SCHEMA tableam_display OWNER TO regress_display_role;
+SET search_path TO tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET ROLE TO regress_display_role;
+-- Use only relations with a physical size of zero.
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
+CREATE VIEW view_heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE MATERIALIZED VIEW mat_view_heap_psql USING heap_psql AS SELECT f1 from tbl_heap_psql;
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2816,7 +2824,7 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
- Table "public.tbl_heap_psql"
+ Table "tableam_display.tbl_heap_psql"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
@@ -2824,16 +2832,68 @@ CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
Access method: heap_psql
\d+ tbl_heap
- Table "public.tbl_heap"
+ Table "tableam_display.tbl_heap"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+----------------+-----------+----------+---------+----------+--------------+-------------
f1 | integer | | | | plain | |
f2 | character(100) | | | | extended | |
Access method: heap
+-- AM is displayed for tables, indexes and materialized views.
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------+--------------------+-------------------+----------------------+-------------+---------------+---------+-------------
+ tableam_display | mat_view_heap_psql | materialized view | regress_display_role | permanent | heap_psql | 0 bytes |
+ tableam_display | tbl_heap | table | regress_display_role | permanent | heap | 0 bytes |
+ tableam_display | tbl_heap_psql | table | regress_display_role | permanent | heap_psql | 0 bytes |
+ tableam_display | view_heap_psql | view | regress_display_role | permanent | | 0 bytes |
+(4 rows)
+
+\dt+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------+---------------+-------+----------------------+-------------+---------------+---------+-------------
+ tableam_display | tbl_heap | table | regress_display_role | permanent | heap | 0 bytes |
+ tableam_display | tbl_heap_psql | table | regress_display_role | permanent | heap_psql | 0 bytes |
+(2 rows)
+
+\dm+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Access Method | Size | Description
+-----------------+--------------------+-------------------+----------------------+-------------+---------------+---------+-------------
+ tableam_display | mat_view_heap_psql | materialized view | regress_display_role | permanent | heap_psql | 0 bytes |
+(1 row)
+
+-- But not for views and sequences.
+\dv+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------+----------------+------+----------------------+-------------+---------+-------------
+ tableam_display | view_heap_psql | view | regress_display_role | permanent | 0 bytes |
+(1 row)
+
\set HIDE_TABLEAM on
-DROP TABLE tbl_heap, tbl_heap_psql;
+\d+
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------------+--------------------+-------------------+----------------------+-------------+---------+-------------
+ tableam_display | mat_view_heap_psql | materialized view | regress_display_role | permanent | 0 bytes |
+ tableam_display | tbl_heap | table | regress_display_role | permanent | 0 bytes |
+ tableam_display | tbl_heap_psql | table | regress_display_role | permanent | 0 bytes |
+ tableam_display | view_heap_psql | view | regress_display_role | permanent | 0 bytes |
+(4 rows)
+
+RESET ROLE;
+RESET search_path;
+DROP SCHEMA tableam_display CASCADE;
+NOTICE: drop cascades to 4 other objects
+DETAIL: drop cascades to table tableam_display.tbl_heap_psql
+drop cascades to table tableam_display.tbl_heap
+drop cascades to view tableam_display.view_heap_psql
+drop cascades to materialized view tableam_display.mat_view_heap_psql
DROP ACCESS METHOD heap_psql;
+DROP ROLE regress_display_role;
-- test numericlocale (as best we can without control of psql's locale)
\pset format aligned
\pset expanded off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 5a16080980..47b28d2a07 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -455,20 +455,38 @@ select 1 where false;
\df exp
\pset tuples_only false
--- check conditional tableam display
+-- check conditional am display
+\pset expanded off
--- Create a heap2 table am handler with heapam handler
+CREATE SCHEMA tableam_display;
+CREATE ROLE regress_display_role;
+ALTER SCHEMA tableam_display OWNER TO regress_display_role;
+SET search_path TO tableam_display;
CREATE ACCESS METHOD heap_psql TYPE TABLE HANDLER heap_tableam_handler;
+SET ROLE TO regress_display_role;
+-- Use only relations with a physical size of zero.
CREATE TABLE tbl_heap_psql(f1 int, f2 char(100)) using heap_psql;
CREATE TABLE tbl_heap(f1 int, f2 char(100)) using heap;
+CREATE VIEW view_heap_psql AS SELECT f1 from tbl_heap_psql;
+CREATE MATERIALIZED VIEW mat_view_heap_psql USING heap_psql AS SELECT f1 from tbl_heap_psql;
\d+ tbl_heap_psql
\d+ tbl_heap
\set HIDE_TABLEAM off
\d+ tbl_heap_psql
\d+ tbl_heap
+-- AM is displayed for tables, indexes and materialized views.
+\d+
+\dt+
+\dm+
+-- But not for views and sequences.
+\dv+
\set HIDE_TABLEAM on
-DROP TABLE tbl_heap, tbl_heap_psql;
+\d+
+RESET ROLE;
+RESET search_path;
+DROP SCHEMA tableam_display CASCADE;
DROP ACCESS METHOD heap_psql;
+DROP ROLE regress_display_role;
-- test numericlocale (as best we can without control of psql's locale)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 201946990f..e1e2236a71 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1183,7 +1183,9 @@ testdb=>
columns of the table are shown, as is the presence of OIDs in the
table, the view definition if the relation is a view, a non-default
<link linkend="sql-createtable-replica-identity">replica
- identity</link> setting.
+ identity</link> setting and the
+ <link linkend="sql-create-access-method">access method</link> name
+ if the relation has an access method.
</para>
<para>
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 1 September 2020 07:41, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Aug 20, 2020 at 08:16:19AM +0000, Georgios wrote:
Please find version 7 attached which hopefully addresses the error along with a proper
expansion of the test coverage and removal of recently introduced
whitespace warnings.+CREATE ROLE conditional_tableam_display_role;
As a convention, regression tests need to have roles prefixed with
"regress_" or this would cause some buildfarm members to turn red.
Please see -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS (you could use
that in your environment for example).
I was wondering about the name. I hoped that it would either come up during review, or that it would be fine.
Thank you for the detailed explanation.
So, as of the tests.. The role gets added to make sure that when
using \d+ on the full schema as well as the various \d*+ variants we
have a consistent owner. The addition of the relation size for the
sequence and the btree index in the output generated is a problem
though, because that's not really portable when compiling with other
page sizes. It is true that there are other tests failing in this
case, but I think that we should try to limit that if we can. In
short, I agree that having some tests is better than nothing, but I
would suggest to reduce their scope, as per the attached.
I could not agree more. I have only succumbed to the pressure of reviewing.
Adding \dE as there are no foreign tables does not make much sense,
and also I wondered why \dt+ was not added.Does the attached look correct to you?
You have my :+1:
Show quoted text
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Michael
On Tue, Sep 01, 2020 at 10:27:31AM +0000, Georgios wrote:
On Tuesday, 1 September 2020 07:41, Michael Paquier <michael@paquier.xyz> wrote:
Adding \dE as there are no foreign tables does not make much sense,
and also I wondered why \dt+ was not added.Does the attached look correct to you?
You have my :+1:
OK, applied then.
--
Michael