Re: Include access method in listTables output

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