Comments on system tables and columns

Started by Thom Brownalmost 15 years ago4 messages
#1Thom Brown
thom@linux.com

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

In reply to: Thom Brown (#1)
Re: Comments on system tables and columns

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]http://eulerto.blogspot.com/2010/11/comment-on-catalog-tables.html.

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

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

#3Alvaro Herrera
alvherre@commandprompt.com
In reply to: Thom Brown (#1)
Re: Comments on system tables and columns

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

#4Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#3)
2 attachment(s)
Re: Comments on system tables and columns

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. +

Attachments:

/pgpatches/sysviews.difftext/x-diffDownload
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.sql sql_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();
  
/pgpatches/gen_comments.pltext/plainDownload