On Thu, Apr 10, 2014 at 01:10:35PM -0400, Bruce Momjian wrote:
> On Thu, Apr 10, 2014 at 01:05:32PM -0400, Greg Stark wrote:
> > If it's conditional I think "when it matches a guc" is too hard for users to
> > use.
> 
> Yes, we gave up on having the OID display match the GUC;  we just
> display something if and only if it oids are present.
> 
> Robert is talking about the "Identity Replica" display, which is new in
> 9.4 and should match how we display oids.  Right now "Identity Replica"
> only displays something if the user changed the default.
> 
> > I think "say nothing if oids are off and say something of their on" would be
> > fine. It would result in clutter for users which oids on by default but 
> > that's
> > a non default setting.
> 
> Yes, that is what the proposed patch does.

I talked to Robert via voice to understand his concerns.  He clearly
explained the complexity of how "Replica Identity" is set.  I, of
course, am concerned about user confusion in showing these values.

What I did was develop the attached patch which clarifies the default
for system and non-system tables, documents when psql displays it, and
improves the display for pg_catalog tables that aren't equal to NONE.

It also has changed the OID status to only display if it exists.  One
question that came up with Robert is whether OID status should appear
for \d as well, now that is only shows up when present.

I am hopeful this patch is closer to general agreement.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
new file mode 100644
index 0b08f83..ac6a4a4
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
*************** ALTER TABLE [ IF EXISTS ] <replaceable c
*** 608,619 ****
       <para>
        This form changes the information which is written to the write-ahead log
        to identify rows which are updated or deleted.  This option has no effect
!       except when logical replication is in use.  <literal>DEFAULT</> records the 
        old values of the columns of the primary key, if any.  <literal>USING INDEX</>
        records the old values of the columns covered by the named index, which
        must be unique, not partial, not deferrable, and include only columns marked
        <literal>NOT NULL</>.  <literal>FULL</> records the old values of all columns
        in the row.  <literal>NOTHING</> records no information about the old row.
        In all cases, no old values are logged unless at least one of the columns
        that would be logged differs between the old and new versions of the row.
       </para>
--- 608,621 ----
       <para>
        This form changes the information which is written to the write-ahead log
        to identify rows which are updated or deleted.  This option has no effect
!       except when logical replication is in use.  <literal>DEFAULT</>
!       (the default for non-system tables) records the 
        old values of the columns of the primary key, if any.  <literal>USING INDEX</>
        records the old values of the columns covered by the named index, which
        must be unique, not partial, not deferrable, and include only columns marked
        <literal>NOT NULL</>.  <literal>FULL</> records the old values of all columns
        in the row.  <literal>NOTHING</> records no information about the old row.
+       (This is the default for system tables.)
        In all cases, no old values are logged unless at least one of the columns
        that would be logged differs between the old and new versions of the row.
       </para>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 85899d7..0b91d45
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=&gt;
*** 951,957 ****
          The command form <literal>\d+</literal> is identical, except that
          more information is displayed: any comments associated with the
          columns of the table are shown, as is the presence of OIDs in the
!         table, the view definition if the relation is a view.
          </para>
  
          <para>
--- 951,959 ----
          The command form <literal>\d+</literal> is identical, except that
          more information is displayed: any comments associated with the
          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.
          </para>
  
          <para>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index d1447fe..0ff7950
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 2350,2357 ****
  			 * No need to display default values;  we already display a
  			 * REPLICA IDENTITY marker on indexes.
  			 */
! 			tableinfo.relreplident != 'd' && tableinfo.relreplident != 'i' &&
! 			strcmp(schemaname, "pg_catalog") != 0)
  		{
  			const char *s = _("Replica Identity");
  
--- 2350,2358 ----
  			 * No need to display default values;  we already display a
  			 * REPLICA IDENTITY marker on indexes.
  			 */
! 			tableinfo.relreplident != 'i' &&
! 			((strcmp(schemaname, "pg_catalog") != 0 && tableinfo.relreplident != 'd') ||
! 			 (strcmp(schemaname, "pg_catalog") == 0 && tableinfo.relreplident != 'n')))
  		{
  			const char *s = _("Replica Identity");
  
*************** describeOneTableDetails(const char *sche
*** 2365,2378 ****
  		}
  
  		/* OIDs, if verbose and not a materialized view */
! 		if (verbose && tableinfo.relkind != 'm')
! 		{
! 			const char *s = _("Has OIDs");
! 
! 			printfPQExpBuffer(&buf, "%s: %s", s,
! 							  (tableinfo.hasoids ? _("yes") : _("no")));
! 			printTableAddFooter(&cont, buf.data);
! 		}
  
  		/* Tablespace info */
  		add_tablespace_footer(&cont, tableinfo.relkind, tableinfo.tablespace,
--- 2366,2373 ----
  		}
  
  		/* OIDs, if verbose and not a materialized view */
! 		if (verbose && tableinfo.relkind != 'm' && tableinfo.hasoids)
! 			printTableAddFooter(&cont, _("Has OIDs: yes"));
  
  		/* Tablespace info */
  		add_tablespace_footer(&cont, tableinfo.relkind, tableinfo.tablespace,
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
new file mode 100644
index 5f29b39..a5fac7b
*** a/src/test/regress/expected/create_table_like.out
--- b/src/test/regress/expected/create_table_like.out
*************** CREATE TABLE ctlt12_storage (LIKE ctlt1
*** 115,121 ****
   a      | text | not null  | main     |              | 
   b      | text |           | extended |              | 
   c      | text |           | external |              | 
- Has OIDs: no
  
  CREATE TABLE ctlt12_comments (LIKE ctlt1 INCLUDING COMMENTS, LIKE ctlt2 INCLUDING COMMENTS);
  \d+ ctlt12_comments
--- 115,120 ----
*************** CREATE TABLE ctlt12_comments (LIKE ctlt1
*** 125,131 ****
   a      | text | not null  | extended |              | A
   b      | text |           | extended |              | B
   c      | text |           | extended |              | C
- Has OIDs: no
  
  CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1);
  NOTICE:  merging column "a" with inherited definition
--- 124,129 ----
*************** NOTICE:  merging constraint "ctlt1_a_che
*** 140,146 ****
  Check constraints:
      "ctlt1_a_check" CHECK (length(a) > 2)
  Inherits: ctlt1
- Has OIDs: no
  
  SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass;
   description 
--- 138,143 ----
*************** Check constraints:
*** 162,168 ****
      "ctlt3_a_check" CHECK (length(a) < 5)
  Inherits: ctlt1,
            ctlt3
- Has OIDs: no
  
  CREATE TABLE ctlt13_like (LIKE ctlt3 INCLUDING CONSTRAINTS INCLUDING COMMENTS INCLUDING STORAGE) INHERITS (ctlt1);
  NOTICE:  merging column "a" with inherited definition
--- 159,164 ----
*************** Check constraints:
*** 177,183 ****
      "ctlt1_a_check" CHECK (length(a) > 2)
      "ctlt3_a_check" CHECK (length(a) < 5)
  Inherits: ctlt1
- Has OIDs: no
  
  SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt13_like'::regclass;
   description 
--- 173,178 ----
*************** Indexes:
*** 198,204 ****
      "ctlt_all_expr_idx" btree ((a || b))
  Check constraints:
      "ctlt1_a_check" CHECK (length(a) > 2)
- Has OIDs: no
  
  SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
      relname     | objsubid | description 
--- 193,198 ----
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
new file mode 100644
index c34c9b4..ff203b2
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
*************** COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
*** 684,690 ****
   c3     | date    |           |                                | plain    |              | 
  Server: s0
  FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
- Has OIDs: no
  
  \det+
                                   List of foreign tables
--- 684,689 ----
*************** ALTER FOREIGN TABLE ft1 ALTER COLUMN c8
*** 743,749 ****
   c10    | integer |           | (p1 'v1')                      | plain    |              | 
  Server: s0
  FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
- Has OIDs: no
  
  -- can't change the column type if it's used elsewhere
  CREATE TABLE use_ft1_column_type (x ft1);
--- 742,747 ----
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
new file mode 100644
index c84c435..56e2c99
*** a/src/test/regress/expected/inherit.out
--- b/src/test/regress/expected/inherit.out
*************** ALTER TABLE inhts RENAME d TO dd;
*** 913,919 ****
   dd     | integer |           | plain   |              | 
  Inherits: inht1,
            inhs1
- Has OIDs: no
  
  DROP TABLE inhts;
  -- Test for renaming in diamond inheritance
--- 913,918 ----
*************** ALTER TABLE inht1 RENAME aa TO aaa;
*** 934,940 ****
   z      | integer |           | plain   |              | 
  Inherits: inht2,
            inht3
- Has OIDs: no
  
  CREATE TABLE inhts (d int) INHERITS (inht2, inhs1);
  NOTICE:  merging multiple inherited definitions of column "b"
--- 933,938 ----
*************** ERROR:  cannot rename inherited column "
*** 952,958 ****
   d      | integer |           | plain   |              | 
  Inherits: inht2,
            inhs1
- Has OIDs: no
  
  WITH RECURSIVE r AS (
    SELECT 'inht1'::regclass AS inhrelid
--- 950,955 ----
*************** CREATE TABLE test_constraints_inh () INH
*** 999,1005 ****
  Indexes:
      "test_constraints_val1_val2_key" UNIQUE CONSTRAINT, btree (val1, val2)
  Child tables: test_constraints_inh
- Has OIDs: no
  
  ALTER TABLE ONLY test_constraints DROP CONSTRAINT test_constraints_val1_val2_key;
  \d+ test_constraints
--- 996,1001 ----
*************** ALTER TABLE ONLY test_constraints DROP C
*** 1010,1016 ****
   val1   | character varying |           | extended |              | 
   val2   | integer           |           | plain    |              | 
  Child tables: test_constraints_inh
- Has OIDs: no
  
  \d+ test_constraints_inh
                        Table "public.test_constraints_inh"
--- 1006,1011 ----
*************** Has OIDs: no
*** 1020,1026 ****
   val1   | character varying |           | extended |              | 
   val2   | integer           |           | plain    |              | 
  Inherits: test_constraints
- Has OIDs: no
  
  DROP TABLE test_constraints_inh;
  DROP TABLE test_constraints;
--- 1015,1020 ----
*************** CREATE TABLE test_ex_constraints_inh ()
*** 1037,1043 ****
  Indexes:
      "test_ex_constraints_c_excl" EXCLUDE USING gist (c WITH &&)
  Child tables: test_ex_constraints_inh
- Has OIDs: no
  
  ALTER TABLE test_ex_constraints DROP CONSTRAINT test_ex_constraints_c_excl;
  \d+ test_ex_constraints
--- 1031,1036 ----
*************** ALTER TABLE test_ex_constraints DROP CON
*** 1046,1052 ****
  --------+--------+-----------+---------+--------------+-------------
   c      | circle |           | plain   |              | 
  Child tables: test_ex_constraints_inh
- Has OIDs: no
  
  \d+ test_ex_constraints_inh
                 Table "public.test_ex_constraints_inh"
--- 1039,1044 ----
*************** Has OIDs: no
*** 1054,1060 ****
  --------+--------+-----------+---------+--------------+-------------
   c      | circle |           | plain   |              | 
  Inherits: test_ex_constraints
- Has OIDs: no
  
  DROP TABLE test_ex_constraints_inh;
  DROP TABLE test_ex_constraints;
--- 1046,1051 ----
*************** Indexes:
*** 1071,1077 ****
      "test_primary_constraints_pkey" PRIMARY KEY, btree (id)
  Referenced by:
      TABLE "test_foreign_constraints" CONSTRAINT "test_foreign_constraints_id1_fkey" FOREIGN KEY (id1) REFERENCES test_primary_constraints(id)
- Has OIDs: no
  
  \d+ test_foreign_constraints
                 Table "public.test_foreign_constraints"
--- 1062,1067 ----
*************** Has OIDs: no
*** 1081,1087 ****
  Foreign-key constraints:
      "test_foreign_constraints_id1_fkey" FOREIGN KEY (id1) REFERENCES test_primary_constraints(id)
  Child tables: test_foreign_constraints_inh
- Has OIDs: no
  
  ALTER TABLE test_foreign_constraints DROP CONSTRAINT test_foreign_constraints_id1_fkey;
  \d+ test_foreign_constraints
--- 1071,1076 ----
*************** ALTER TABLE test_foreign_constraints DRO
*** 1090,1096 ****
  --------+---------+-----------+---------+--------------+-------------
   id1    | integer |           | plain   |              | 
  Child tables: test_foreign_constraints_inh
- Has OIDs: no
  
  \d+ test_foreign_constraints_inh
               Table "public.test_foreign_constraints_inh"
--- 1079,1084 ----
*************** Has OIDs: no
*** 1098,1104 ****
  --------+---------+-----------+---------+--------------+-------------
   id1    | integer |           | plain   |              | 
  Inherits: test_foreign_constraints
- Has OIDs: no
  
  DROP TABLE test_foreign_constraints_inh;
  DROP TABLE test_foreign_constraints;
--- 1086,1091 ----
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
new file mode 100644
index b0b6e27..6c51d0d
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** Rules:
*** 2609,2615 ****
      r3 AS
      ON DELETE TO rules_src DO
   NOTIFY rules_src_deletion
- Has OIDs: no
  
  --
  -- check alter rename rule
--- 2609,2614 ----
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to