remove separate postgres.(sh)description files

Started by John Naylorabout 6 years ago5 messages
#1John Naylor
john.naylor@2ndquadrant.com
1 attachment(s)

I'm guessing the initial data for pg_(sh)description is output into
separate files because it was too difficult for the traditional shell
script to maintain enough state to do otherwise. With Perl, it's just
as easy to assemble the data into the same format as the rest of the
catalogs and then let the generic code path output it into
postgres.bki. The attached patch does that and simplifies the catalog
makefile and initdb.c. I'll add a commitfest entry for this.
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

v1-0001-Remove-separate-files-for-the-initial-contents-of.patchapplication/octet-stream; name=v1-0001-Remove-separate-files-for-the-initial-contents-of.patchDownload
From 4a383dbb7c0ea009df593bd5589e08a6b22343f0 Mon Sep 17 00:00:00 2001
From: John Naylor <john.naylor@2ndquadrant.com>
Date: Mon, 23 Dec 2019 11:31:41 -0500
Subject: [PATCH v1] Remove separate files for the initial contents of
 pg_(sh)description

This data was only in separate files because it was the most
convenient way to handle it with a shell script. Now that we
use a general-purpose programming language, it's easy to assemble
the data into the same format as the rest of the catalogs and
output it into postgres.bki. This allows removal of some special-
purpose code from initdb.c.
---
 src/backend/catalog/Makefile  |  8 ++---
 src/backend/catalog/genbki.pl | 62 +++++++++++++++--------------------
 src/bin/initdb/initdb.c       | 44 +------------------------
 3 files changed, 30 insertions(+), 84 deletions(-)

diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index a51153236a..252534c82b 100644
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -44,8 +44,6 @@ OBJS = \
 	storage.o \
 	toasting.o
 
-BKIFILES = postgres.bki postgres.description postgres.shdescription
-
 include $(top_srcdir)/src/backend/common.mk
 
 # Note: the order of this list determines the order in which the catalog
@@ -127,8 +125,6 @@ $(top_builddir)/src/include/catalog/header-stamp: bki-stamp
 .PHONY: install-data
 install-data: bki-stamp 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)/information_schema.sql '$(DESTDIR)$(datadir)/information_schema.sql'
 	$(INSTALL_DATA) $(srcdir)/sql_features.txt '$(DESTDIR)$(datadir)/sql_features.txt'
@@ -138,7 +134,7 @@ installdirs:
 
 .PHONY: uninstall-data
 uninstall-data:
-	rm -f $(addprefix '$(DESTDIR)$(datadir)'/, $(BKIFILES) system_views.sql information_schema.sql sql_features.txt)
+	rm -f $(addprefix '$(DESTDIR)$(datadir)'/, postgres.bki system_views.sql information_schema.sql sql_features.txt)
 
 # postgres.bki, postgres.description, postgres.shdescription,
 # and the generated headers are in the distribution tarball,
@@ -146,4 +142,4 @@ uninstall-data:
 clean:
 
 maintainer-clean: clean
-	rm -f bki-stamp $(BKIFILES) $(GENERATED_HEADERS)
+	rm -f bki-stamp postgres.bki $(GENERATED_HEADERS)
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 6eff045bd9..ac18c000e7 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -2,10 +2,9 @@
 #----------------------------------------------------------------------
 #
 # genbki.pl
-#    Perl script that generates postgres.bki, postgres.description,
-#    postgres.shdescription, and symbol definition headers from specially
-#    formatted header files and data files.  The BKI files are used to
-#    initialize the postgres template database.
+#    Perl script that generates postgres.bki and symbol definition
+#    headers from specially formatted header files and data files.
+#    postgres.bki is used to initialize the postgres template database.
 #
 # Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
 # Portions Copyright (c) 1994, Regents of the University of California
@@ -93,9 +92,29 @@ foreach my $header (@ARGV)
 		my $data = Catalog::ParseData($datfile, $schema, 0);
 		$catalog_data{$catname} = $data;
 
-		# Check for duplicated OIDs while we're at it.
 		foreach my $row (@$data)
 		{
+			# Generate entries for pg_description and pg_shdescription.
+			if (defined $row->{descr})
+			{
+				my %descr = (
+					objoid      => $row->{oid},
+					classoid    => $catalog->{relation_oid},
+					objsubid    => 0,
+					description => $row->{descr});
+
+				if ($catalog->{shared_relation})
+				{
+					delete $descr{objsubid};
+					push @{ $catalog_data{pg_shdescription} }, \%descr;
+				}
+				else
+				{
+					push @{ $catalog_data{pg_description}}, \%descr;
+				}
+			}
+
+			# Check for duplicated OIDs while we're at it.
 			$oidcounts{ $row->{oid} }++ if defined $row->{oid};
 		}
 	}
@@ -361,15 +380,8 @@ open my $bki, '>', $bkifile . $tmpext
 my $schemafile = $output_path . 'schemapg.h';
 open my $schemapg, '>', $schemafile . $tmpext
   or die "can't open $schemafile$tmpext: $!";
-my $descrfile = $output_path . 'postgres.description';
-open my $descr, '>', $descrfile . $tmpext
-  or die "can't open $descrfile$tmpext: $!";
-my $shdescrfile = $output_path . 'postgres.shdescription';
-open my $shdescr, '>', $shdescrfile . $tmpext
-  or die "can't open $shdescrfile$tmpext: $!";
 
-# Generate postgres.bki, postgres.description, postgres.shdescription,
-# and pg_*_d.h headers.
+# Generate postgres.bki and pg_*_d.h headers.
 
 # version marker for .bki file
 print $bki "# PostgreSQL $major_version\n";
@@ -579,22 +591,6 @@ EOM
 		# Write to postgres.bki
 		print_bki_insert(\%bki_values, $schema);
 
-		# Write comments to postgres.description and
-		# postgres.shdescription
-		if (defined $bki_values{descr})
-		{
-			if ($catalog->{shared_relation})
-			{
-				printf $shdescr "%s\t%s\t%s\n",
-				  $bki_values{oid}, $catname, $bki_values{descr};
-			}
-			else
-			{
-				printf $descr "%s\t%s\t0\t%s\n",
-				  $bki_values{oid}, $catname, $bki_values{descr};
-			}
-		}
-
 		# Emit OID symbol
 		if (defined $bki_values{oid_symbol})
 		{
@@ -673,14 +669,10 @@ print $schemapg "\n#endif\t\t\t\t\t\t\t/* SCHEMAPG_H */\n";
 # We're done emitting data
 close $bki;
 close $schemapg;
-close $descr;
-close $shdescr;
 
 # Finally, rename the completed files into place.
 Catalog::RenameTempFile($bkifile,     $tmpext);
 Catalog::RenameTempFile($schemafile,  $tmpext);
-Catalog::RenameTempFile($descrfile,   $tmpext);
-Catalog::RenameTempFile($shdescrfile, $tmpext);
 
 exit 0;
 
@@ -967,9 +959,9 @@ Options:
     --set-version    PostgreSQL version number for initdb cross-check
     --include-path   Include path in source tree
 
-genbki.pl generates BKI files and symbol definition
+genbki.pl generates postgres.bki and symbol definition
 headers from specially formatted header files and .dat
-files.  The BKI files are used to initialize the
+files.  postgres.bki is used to initialize the
 postgres template database.
 
 Report bugs to <pgsql-bugs\@lists.postgresql.org>.
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 1f6d8939be..5cba453489 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -151,8 +151,6 @@ static int	wal_segment_size_mb;
 static const char *progname;
 static int	encodingid;
 static char *bki_file;
-static char *desc_file;
-static char *shdesc_file;
 static char *hba_file;
 static char *ident_file;
 static char *conf_file;
@@ -1644,38 +1642,11 @@ setup_sysviews(FILE *cmdfd)
 }
 
 /*
- * load description data
+ * fill in extra description data
  */
 static void
 setup_description(FILE *cmdfd)
 {
-	PG_CMD_PUTS("CREATE TEMP TABLE tmp_pg_description ( "
-				"	objoid oid, "
-				"	classname name, "
-				"	objsubid int4, "
-				"	description text);\n\n");
-
-	PG_CMD_PRINTF("COPY tmp_pg_description FROM E'%s';\n\n",
-				   escape_quotes(desc_file));
-
-	PG_CMD_PUTS("INSERT INTO pg_description "
-				" SELECT t.objoid, c.oid, t.objsubid, t.description "
-				"  FROM tmp_pg_description t, pg_class c "
-				"    WHERE c.relname = t.classname;\n\n");
-
-	PG_CMD_PUTS("CREATE TEMP TABLE tmp_pg_shdescription ( "
-				" objoid oid, "
-				" classname name, "
-				" description text);\n\n");
-
-	PG_CMD_PRINTF("COPY tmp_pg_shdescription FROM E'%s';\n\n",
-				   escape_quotes(shdesc_file));
-
-	PG_CMD_PUTS("INSERT INTO pg_shdescription "
-				" SELECT t.objoid, c.oid, t.description "
-				"  FROM tmp_pg_shdescription t, pg_class c "
-				"   WHERE c.relname = t.classname;\n\n");
-
 	/* Create default descriptions for operator implementation functions */
 	PG_CMD_PUTS("WITH funcdescs AS ( "
 				"SELECT p.oid as p_oid, o.oid as o_oid, oprname "
@@ -1689,13 +1660,6 @@ setup_description(FILE *cmdfd)
 				"  AND NOT EXISTS (SELECT 1 FROM pg_description "
 				"   WHERE objoid = o_oid AND classoid = 'pg_operator'::regclass"
 				"         AND description LIKE 'deprecated%');\n\n");
-
-	/*
-	 * Even though the tables are temp, drop them explicitly so they don't get
-	 * copied into template0/postgres databases.
-	 */
-	PG_CMD_PUTS("DROP TABLE tmp_pg_description;\n\n");
-	PG_CMD_PUTS("DROP TABLE tmp_pg_shdescription;\n\n");
 }
 
 /*
@@ -2586,8 +2550,6 @@ void
 setup_data_file_paths(void)
 {
 	set_input(&bki_file, "postgres.bki");
-	set_input(&desc_file, "postgres.description");
-	set_input(&shdesc_file, "postgres.shdescription");
 	set_input(&hba_file, "pg_hba.conf.sample");
 	set_input(&ident_file, "pg_ident.conf.sample");
 	set_input(&conf_file, "postgresql.conf.sample");
@@ -2602,13 +2564,11 @@ setup_data_file_paths(void)
 				"VERSION=%s\n"
 				"PGDATA=%s\nshare_path=%s\nPGPATH=%s\n"
 				"POSTGRES_SUPERUSERNAME=%s\nPOSTGRES_BKI=%s\n"
-				"POSTGRES_DESCR=%s\nPOSTGRES_SHDESCR=%s\n"
 				"POSTGRESQL_CONF_SAMPLE=%s\n"
 				"PG_HBA_SAMPLE=%s\nPG_IDENT_SAMPLE=%s\n",
 				PG_VERSION,
 				pg_data, share_path, bin_path,
 				username, bki_file,
-				desc_file, shdesc_file,
 				conf_file,
 				hba_file, ident_file);
 		if (show_setting)
@@ -2616,8 +2576,6 @@ setup_data_file_paths(void)
 	}
 
 	check_input(bki_file);
-	check_input(desc_file);
-	check_input(shdesc_file);
 	check_input(hba_file);
 	check_input(ident_file);
 	check_input(conf_file);
-- 
2.22.0

#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: John Naylor (#1)
Re: remove separate postgres.(sh)description files

On 31/12/2019 02:08, John Naylor wrote:

I'm guessing the initial data for pg_(sh)description is output into
separate files because it was too difficult for the traditional shell
script to maintain enough state to do otherwise.

Yeah, I guess so. The roots of postgres.description goes all the way
back to 1997, when not only genbki was a shell script, but also initdb.

With Perl, it's just as easy to assemble the data into the same
format as the rest of the catalogs and then let the generic code path
output it into postgres.bki. The attached patch does that and
simplifies the catalog makefile and initdb.c.

Nice cleanup! Looks like we didn't have any mention of the
postgres.(sh)decription files in the docs, so no doc updates needed.
Grepping around, there are a few stray references to
postgres.description still:

$ git grep -r -I postgres.shdescript .
src/backend/catalog/.gitignore:/postgres.shdescription
src/backend/catalog/Makefile:# postgres.bki, postgres.description,
postgres.shdescription,
src/tools/msvc/clean.bat:if %DIST%==1 if exist
src\backend\catalog\postgres.shdescription del /q
src\backend\catalog\postgres.shdescription

Barring objections, I'll remove those too, and commit this.

- Heikki

#3Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: Heikki Linnakangas (#2)
Re: remove separate postgres.(sh)description files

On Wed, Jan 08, 2020 at 02:33:23PM +0200, Heikki Linnakangas wrote:

On 31/12/2019 02:08, John Naylor wrote:

I'm guessing the initial data for pg_(sh)description is output into
separate files because it was too difficult for the traditional shell
script to maintain enough state to do otherwise.

Yeah, I guess so. The roots of postgres.description goes all the way
back to 1997, when not only genbki was a shell script, but also
initdb.

With Perl, it's just as easy to assemble the data into the same
format as the rest of the catalogs and then let the generic code path
output it into postgres.bki. The attached patch does that and
simplifies the catalog makefile and initdb.c.

Nice cleanup! Looks like we didn't have any mention of the
postgres.(sh)decription files in the docs, so no doc updates needed.
Grepping around, there are a few stray references to
postgres.description still:

$ git grep -r -I postgres.shdescript .
src/backend/catalog/.gitignore:/postgres.shdescription
src/backend/catalog/Makefile:# postgres.bki, postgres.description,
postgres.shdescription,
src/tools/msvc/clean.bat:if %DIST%==1 if exist
src\backend\catalog\postgres.shdescription del /q
src\backend\catalog\postgres.shdescription

Barring objections, I'll remove those too, and commit this.

+1 from me. Let's remove these small RFC patches out of the way.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Tomas Vondra (#3)
Re: remove separate postgres.(sh)description files

On 16/01/2020 23:39, Tomas Vondra wrote:

On Wed, Jan 08, 2020 at 02:33:23PM +0200, Heikki Linnakangas wrote:

On 31/12/2019 02:08, John Naylor wrote:

I'm guessing the initial data for pg_(sh)description is output into
separate files because it was too difficult for the traditional shell
script to maintain enough state to do otherwise.

Yeah, I guess so. The roots of postgres.description goes all the way
back to 1997, when not only genbki was a shell script, but also
initdb.

With Perl, it's just as easy to assemble the data into the same
format as the rest of the catalogs and then let the generic code path
output it into postgres.bki. The attached patch does that and
simplifies the catalog makefile and initdb.c.

Nice cleanup! Looks like we didn't have any mention of the
postgres.(sh)decription files in the docs, so no doc updates needed.
Grepping around, there are a few stray references to
postgres.description still:

$ git grep -r -I postgres.shdescript .
src/backend/catalog/.gitignore:/postgres.shdescription
src/backend/catalog/Makefile:# postgres.bki, postgres.description,e0ed6817c0..7aaefadaac
postgres.shdescription,
src/tools/msvc/clean.bat:if %DIST%==1 if exist
src\backend\catalog\postgres.shdescription del /q
src\backend\catalog\postgres.shdescription

Barring objections, I'll remove those too, and commit this.

+1 from me. Let's remove these small RFC patches out of the way.

Pushed, thanks!

- Heikki

#5John Naylor
john.naylor@2ndquadrant.com
In reply to: Heikki Linnakangas (#4)
Re: remove separate postgres.(sh)description files

On Sun, Jan 19, 2020 at 7:58 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On Wed, Jan 08, 2020 at 02:33:23PM +0200, Heikki Linnakangas wrote:

Grepping around, there are a few stray references to
postgres.description still:

$ git grep -r -I postgres.shdescript .
src/backend/catalog/.gitignore:/postgres.shdescription
src/backend/catalog/Makefile:# postgres.bki, postgres.description,e0ed6817c0..7aaefadaac
postgres.shdescription,
src/tools/msvc/clean.bat:if %DIST%==1 if exist
src\backend\catalog\postgres.shdescription del /q
src\backend\catalog\postgres.shdescription

Pushed, thanks!

Thanks for taking care of those loose ends -- that was a bit sloppy of me.

--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services