On Fri, 18 Jan 2019 at 10:13, Amit Khandekar <[email protected]> wrote:
> On Tue, 15 Jan 2019 at 17:58, Dmitry Dolgov <[email protected]> wrote:
> > Also I guess another attached patch should address the psql part, namely
> > displaying a table access method with \d+ and possibility to hide it with a
> > psql variable (HIDE_TABLEAM, but I'm open for suggestion about the name).
I am ok with the name.
>
> Will have a look at this one.
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -1,3 +1,4 @@
+\set HIDE_TABLEAM on
CREATE TEMP TABLE x (
I thought we wanted to avoid having to add this setting in individual
regression tests. Can't we do this in pg_regress as a common setting ?
+ /* Access method info */
+ if (pset.sversion >= 120000 && verbose && tableinfo.relam != NULL &&
+ !(pset.hide_tableam && tableinfo.relam_is_default))
+ {
+ printfPQExpBuffer(&buf, _("Access method: %s"),
fmtId(tableinfo.relam));
So this will make psql hide the access method if it's same as the
default. I understand that this was kind of concluded in the other
thread "Displaying and dumping of table access methods". But IMHO, if
the hide_tableam is false, we should *always* show the access method,
regardless of the default value. I mean, we can make it simple : off
means never show table-access, on means always show table-access,
regardless of the default access method. And this also will work with
regression tests. If some regression test wants specifically to output
the access method, it can have a "\SET HIDE_TABLEAM off" command.
If we hide the method if it's default, then for a regression test that
wants to forcibly show the table access method of all tables, it won't
show up for tables that have default access method.
------------
+ if (pset.sversion >= 120000 && verbose && tableinfo.relam != NULL &&
If the server does not support relam, tableinfo.relam will be NULL
anyways. So I think sversion check is not needed.
------------
+ printfPQExpBuffer(&buf, _("Access method: %s"), fmtId(tableinfo.relam));
fmtId is not required. In fact, we should display the access method
name as-is. fmtId is required only for identifiers present in SQL
queries.
-----------
+ printfPQExpBuffer(&buf, _("Access method: %s"), fmtId(tableinfo.relam));
+ printTableAddFooter(&cont, buf.data);
+ }
+
+
}
Last two blank lines are not needed.
-----------
+ bool hide_tableam;
} PsqlSettings;
These variables, it seems, are supposed to be grouped together by type.
-----------
I believe you are going to add a new regression testcase for the change ?
--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company
