Thread: Comments on system tables and columns

Comments on system tables and columns

From
Thom Brown
Date:
Hi,

I notice that none of the system tables or columns thereof bear any
comments.  Is this intentional, or an oversight?  I would have thought
comments would be useful since the column names aren't exactly always
self-explanatory.

Thanks

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: Comments on system tables and columns

From
Euler Taveira de Oliveira
Date:
Em 28-03-2011 08:14, Thom Brown escreveu:
> I notice that none of the system tables or columns thereof bear any
> comments.  Is this intentional, or an oversight?  I would have thought
> comments would be useful since the column names aren't exactly always
> self-explanatory.
>
It could be useful in some cases. IIRC the comments are not there to avoid 
bloating the catalog. One month ago or so I saw a commit to comment operator 
support functions. Maybe it is worth comment system catalog too [1].


[1] http://eulerto.blogspot.com/2010/11/comment-on-catalog-tables.html


--   Euler Taveira de Oliveira  http://www.timbira.com/


Re: Comments on system tables and columns

From
Alvaro Herrera
Date:
Excerpts from Thom Brown's message of lun mar 28 08:14:07 -0300 2011:
> Hi,
> 
> I notice that none of the system tables or columns thereof bear any
> comments.  Is this intentional, or an oversight?  I would have thought
> comments would be useful since the column names aren't exactly always
> self-explanatory.

Bruce has been working on changes to have catalog objects (tables, views
and columns) contain comments, but he deferred it to 9.2 because it
involved nontrivial pieces of infrastructure (mainly to avoid
duplication with the SGML catalog documentation).

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Comments on system tables and columns

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Excerpts from Thom Brown's message of lun mar 28 08:14:07 -0300 2011:
> > Hi,
> >
> > I notice that none of the system tables or columns thereof bear any
> > comments.  Is this intentional, or an oversight?  I would have thought
> > comments would be useful since the column names aren't exactly always
> > self-explanatory.
>
> Bruce has been working on changes to have catalog objects (tables, views
> and columns) contain comments, but he deferred it to 9.2 because it
> involved nontrivial pieces of infrastructure (mainly to avoid
> duplication with the SGML catalog documentation).

Attached are diffs that change the Makefile and initdb, and a perl
script to pull the system view comments out of the SGML docs.  I need to
do more work to pull stuff for the system tables.  This does work in
testing.

I will work on this more for 9.2.

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

  + It's impossible for everything to be true. +
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
new file mode 100644
index 3a83461..68ed5fe
*** a/src/backend/catalog/Makefile
--- b/src/backend/catalog/Makefile
*************** OBJS = catalog.o dependency.o heap.o ind
*** 16,22 ****
         pg_operator.o pg_proc.o pg_db_role_setting.o pg_shdepend.o pg_type.o \
         storage.o toasting.o

! BKIFILES = postgres.bki postgres.description postgres.shdescription

  include $(top_srcdir)/src/backend/common.mk

--- 16,22 ----
         pg_operator.o pg_proc.o pg_db_role_setting.o pg_shdepend.o pg_type.o \
         storage.o toasting.o

! BKIFILES = postgres.bki postgres.description postgres.shdescription system_view_comments.sql

  include $(top_srcdir)/src/backend/common.mk

*************** schemapg.h: postgres.bki ;
*** 59,70 ****
--- 59,74 ----
  postgres.bki: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS)
      $(PERL) -I $(catalogdir) $< $(pg_includes) --set-version=$(MAJORVERSION) $(POSTGRES_BKI_SRCS)

+ system_view_comments.sql: $(top_builddir)/doc/src/sgml/catalogs.sgml gen_comments.pl
+     $(PERL) $(srcdir)/gen_comments.pl $< > $@
+
  .PHONY: install-data
  install-data: $(BKIFILES) installdirs
      $(INSTALL_DATA) $(call vpathsearch,postgres.bki) '$(DESTDIR)$(datadir)/postgres.bki'
      $(INSTALL_DATA) $(call vpathsearch,postgres.description) '$(DESTDIR)$(datadir)/postgres.description'
      $(INSTALL_DATA) $(call vpathsearch,postgres.shdescription) '$(DESTDIR)$(datadir)/postgres.shdescription'
      $(INSTALL_DATA) $(srcdir)/system_views.sql '$(DESTDIR)$(datadir)/system_views.sql'
+     $(INSTALL_DATA) $(srcdir)/system_view_comments.sql '$(DESTDIR)$(datadir)/system_view_comments.sql'
      $(INSTALL_DATA) $(srcdir)/information_schema.sql '$(DESTDIR)$(datadir)/information_schema.sql'
      $(INSTALL_DATA) $(srcdir)/sql_features.txt '$(DESTDIR)$(datadir)/sql_features.txt'

*************** installdirs:
*** 73,79 ****

  .PHONY: uninstall-data
  uninstall-data:
!     rm -f $(addprefix '$(DESTDIR)$(datadir)'/, $(BKIFILES) system_views.sql information_schema.sql sql_features.txt)

  # postgres.bki, postgres.description, postgres.shdescription, and schemapg.h
  # are in the distribution tarball, so they are not cleaned here.
--- 77,83 ----

  .PHONY: uninstall-data
  uninstall-data:
!     rm -f $(addprefix '$(DESTDIR)$(datadir)'/, $(BKIFILES) system_views.sql system_view_comments.sql
information_schema.sqlsql_features.txt) 

  # postgres.bki, postgres.description, postgres.shdescription, and schemapg.h
  # are in the distribution tarball, so they are not cleaned here.
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index acd2514..f0d72e9
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** static char *dictionary_file;
*** 102,107 ****
--- 102,108 ----
  static char *info_schema_file;
  static char *features_file;
  static char *system_views_file;
+ static char *system_view_comments_file;
  static bool made_new_pgdata = false;
  static bool found_existing_pgdata = false;
  static bool made_new_xlogdir = false;
*************** static void setup_auth(void);
*** 166,171 ****
--- 167,173 ----
  static void get_set_pwd(void);
  static void setup_depend(void);
  static void setup_sysviews(void);
+ static void append_sysviews(char **lines);
  static void setup_description(void);
  static void setup_collation(void);
  static void setup_conversion(void);
*************** setup_depend(void)
*** 1419,1432 ****
  static void
  setup_sysviews(void)
  {
!     PG_CMD_DECL;
!     char      **line;
!     char      **sysviews_setup;

      fputs(_("creating system views ... "), stdout);
      fflush(stdout);

!     sysviews_setup = readfile(system_views_file);

      /*
       * We use -j here to avoid backslashing stuff in system_views.sql
--- 1421,1452 ----
  static void
  setup_sysviews(void)
  {
!     char      **sysview_lines;
!     char      **sysview_comment_lines;

      fputs(_("creating system views ... "), stdout);
      fflush(stdout);

!     sysview_lines = readfile(system_views_file);
!     append_sysviews(sysview_lines);
!     free(sysview_lines);
!
!     sysview_comment_lines = readfile(system_view_comments_file);
!     append_sysviews(sysview_comment_lines);
!     free(sysview_comment_lines);
!
!     check_ok();
! }
!
!
! /*
!  * append system view lines
!  */
! static void
! append_sysviews(char **lines)
! {
!     char      **line;
!     PG_CMD_DECL;

      /*
       * We use -j here to avoid backslashing stuff in system_views.sql
*************** setup_sysviews(void)
*** 1438,1454 ****

      PG_CMD_OPEN;

!     for (line = sysviews_setup; *line != NULL; line++)
      {
          PG_CMD_PUTS(*line);
          free(*line);
      }

      PG_CMD_CLOSE;
-
-     free(sysviews_setup);
-
-     check_ok();
  }

  /*
--- 1458,1470 ----

      PG_CMD_OPEN;

!     for (line = lines; *line != NULL; line++)
      {
          PG_CMD_PUTS(*line);
          free(*line);
      }

      PG_CMD_CLOSE;
  }

  /*
*************** main(int argc, char *argv[])
*** 2806,2811 ****
--- 2822,2828 ----
      set_input(&info_schema_file, "information_schema.sql");
      set_input(&features_file, "sql_features.txt");
      set_input(&system_views_file, "system_views.sql");
+     set_input(&system_view_comments_file, "system_view_comments.sql");

      set_info_version();

*************** main(int argc, char *argv[])
*** 2839,2844 ****
--- 2856,2862 ----
      check_input(info_schema_file);
      check_input(features_file);
      check_input(system_views_file);
+     check_input(system_view_comments_file);

      setlocales();

#!/usr/bin/perl
#
# Generate the system_comments.sql file from sgml/catalogs.sgml
# Copyright (c) 2000-2011, PostgreSQL Global Development Group

use warnings;
use strict;

print "-- autogenerated from /doc/src/sgml/catalogs.sgml, do not edit\n";

open my $catalogs, $ARGV[0] or die;

my $in_view_table = 0;
my $in_struct = 0;
my $in_entry = 0;
my $view_name = '';
my $description = '';

while (<$catalogs>) {
    # In system view table?
    $in_view_table = 1 if (m{id="view-table"});

    if ($in_view_table) {
        # concatenate text, might span lines
        if (m{</?structname>} || $in_struct) {
            $view_name .= $_;
            $description = '';
        } elsif (m{</?entry>} || $in_entry) {
            $description .= $_;
        }

        # update status
        $in_struct = 1 if (m{<structname>});
        $in_struct = 0 if (m{</structname>});
        $in_entry = 1 if (m{<entry>});
        $in_entry = 0 if (m{</entry>});

        if (! $in_struct && ! $in_entry && $view_name && $description) {
            # remove tags, eat newlines as well ("s")
            $view_name =~ s{^.*<structname>([^<]*).*$}{$1}s;
            $description =~ s{^.*<entry>([^<]*).*$}{$1}s;

            # convert whitespaces to one space
            $view_name =~ s/\s+/ /g;
            $description =~ s/\s+/ /g;

            print 'COMMENT ON VIEW ' . $view_name . " IS '" . $description . "';\n";
            $view_name = '';
            $description = '';
        }
    }

    $in_view_table = 0 if (m{</table>});
}

close $catalogs;