Re: WIP: a way forward on bootstrap data

John Naylor <jcnaylor@gmail.com> writes:
> [ v11-bootstrap-data-conversion.tar.gz ]

I've done a round of review work on this, focusing on the Makefile
infrastructure.  I found a bunch of problems with parallel builds and
VPATH builds, which are addressed in the attached incremental patch.

The parallel-build issues are a bit of a mess really: it's surprising
we've not had problems there earlier.  For instance, catalog/Makefile has

postgres.description: postgres.bki ;
postgres.shdescription: postgres.bki ;
schemapg.h: postgres.bki ;

However, genbki.pl doesn't make any particular guarantee that postgres.bki
will be written sooner than its other output files, which means that in
principle make might think it needs to rebuild these other files on every
subsequent check.  That was somewhat harmless given the empty update rule,
but it's not really the right thing.  Your patch extended this to make
all the generated headers dependent on postgres.bki, and those are
definitely written before postgres.bki, meaning make *will* think they
are out of date.  Worse, it'll also think the symlinks to them are out
of date.  So I was running into problems with different parallel make
sub-tasks removing and recreating the symlinks.  VPATH builds didn't
work well either, because out-of-date-ness ties into whether make will
accept a file in the source dir as a valid replacement target.

I resolved this mess by setting up a couple of stamp files, which is
a technique we also use elsewhere.  bki-stamp is a single file
representing the action of having run genbki.pl, and header-stamp
likewise represents the action of having made the symlinks to the
generated headers.  By depending on those rather than individual
files, we avoid questions of exactly what the timestamps on the
individual output files are.

In the attached, I've also done some more work on the README file
and cleaned up a few other little things.

I've not really looked at the MSVC build code at all.  Personally,
I'm willing to just commit this (when the time comes) and let the
buildfarm see if the MSVC code works ... but if anyone else wants
to check that part beforehand, please do.

I also have not spent much time yet looking at the end-product .h and .dat
files.  I did note a bit of distressing inconsistency in the formatting of
the catalog struct declarations, some of which predates this patch but it
seems like you've introduced more.  I think what we ought to standardize
on is a format similar to this in pg_opclass.h:

CATALOG(pg_opclass,2616)
{
    /* index access method opclass is for */
    Oid         opcmethod       BKI_LOOKUP(pg_am);

    /* name of this opclass */
    NameData    opcname;

    /* namespace of this opclass */
    Oid         opcnamespace    BKI_DEFAULT(PGNSP);

    /* opclass owner */
    Oid         opcowner        BKI_DEFAULT(PGUID);

The former convention used in some places, of field descriptions in
same-line comments, clearly won't work anymore if we're sticking
BKI_DEFAULT annotations there.  I also don't like the format used in, eg,
pg_aggregate.h of putting field descriptions in a separate comment block
before the struct proper.  Bitter experience has shown that there are a
lot of people on this project who won't update comments that are more than
about two lines away from the code they change; so the style in
pg_aggregate.h is just inviting maintenance oversights.

I've got mixed feelings about the whitespace lines between fields.  They
seem like they are mostly bulking up the code and we could do without 'em.
On the other hand, pgindent will insist on putting one before any
multi-line field comment, and so that would create inconsistent formatting
if we don't use 'em elsewhere.  Thoughts?

Speaking of pgindent, those prettily aligned BKI annotations are a waste
of effort, because when pgindent gets done with the code it will look
like

    regproc        aggfnoid;
    char        aggkind BKI_DEFAULT(n);
    int16        aggnumdirectargs BKI_DEFAULT(0);
    regproc        aggtransfn BKI_LOOKUP(pg_proc);
    regproc        aggfinalfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggcombinefn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggserialfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggdeserialfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggmtransfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggminvtransfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
    regproc        aggmfinalfn BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);

I'm not sure if there's anything much to be done about this.  BKI_DEFAULT
isn't so bad, but additional annotations get unreadable fast.  Maybe
BKI_LOOKUP was a bad idea after all, and we should just invent more
Oid-equivalent typedef names.

The attached is just one incremental patch on top of your v11 series.
I couldn't think of an easy way to migrate the changes back into the
most relevant diffs of your series, so I didn't try.

            regards, tom lane

diff --git a/src/Makefile b/src/Makefile
index febbced..433d00b 100644
*** a/src/Makefile
--- b/src/Makefile
*************** SUBDIRS = \
*** 37,42 ****
--- 37,47 ----

  $(recurse)

+ # Update the commonly used headers before building the subdirectories;
+ # otherwise, in a parallel build, several different sub-jobs will try to
+ # remake them concurrently
+ $(SUBDIRS:%=all-%-recurse): | submake-generated-headers
+
  install: install-local

  install-local: installdirs-local
diff --git a/src/backend/Makefile b/src/backend/Makefile
index ef00b38..775f7a3 100644
*** a/src/backend/Makefile
--- b/src/backend/Makefile
*************** utils/probes.h: utils/probes.d
*** 150,156 ****

  # run this unconditionally to avoid needing to know its dependencies here:
  submake-catalog-headers:
!     $(MAKE) -C catalog distprep builddir-headers

  .PHONY: submake-catalog-headers

--- 150,156 ----

  # run this unconditionally to avoid needing to know its dependencies here:
  submake-catalog-headers:
!     $(MAKE) -C catalog distprep generated-header-symlinks

  .PHONY: submake-catalog-headers

*************** endif
*** 299,312 ****
  ##########################################################################

  clean:
!     rm -f $(LOCALOBJS) postgres$(X) $(POSTGRES_IMP) \
!         $(top_builddir)/src/include/parser/gram.h \
!         $(top_builddir)/src/include/catalog/pg_*_d.h \
!         $(top_builddir)/src/include/catalog/schemapg.h \
!         $(top_builddir)/src/include/storage/lwlocknames.h \
!         $(top_builddir)/src/include/utils/fmgroids.h \
!         $(top_builddir)/src/include/utils/fmgrprotos.h \
!         $(top_builddir)/src/include/utils/probes.h
  ifeq ($(PORTNAME), cygwin)
      rm -f postgres.dll libpostgres.a
  endif
--- 299,305 ----
  ##########################################################################

  clean:
!     rm -f $(LOCALOBJS) postgres$(X) $(POSTGRES_IMP)
  ifeq ($(PORTNAME), cygwin)
      rm -f postgres.dll libpostgres.a
  endif
*************** distclean: clean
*** 318,333 ****
      rm -f port/tas.s port/dynloader.c port/pg_sema.c port/pg_shmem.c

  maintainer-clean: distclean
      rm -f bootstrap/bootparse.c \
            bootstrap/bootscanner.c \
            parser/gram.c \
            parser/gram.h \
            parser/scan.c \
-           catalog/pg_*_d.h \
-           catalog/schemapg.h \
-           catalog/postgres.bki \
-           catalog/postgres.description \
-           catalog/postgres.shdescription \
            replication/repl_gram.c \
            replication/repl_scanner.c \
            replication/syncrep_gram.c \
--- 311,322 ----
      rm -f port/tas.s port/dynloader.c port/pg_sema.c port/pg_shmem.c

  maintainer-clean: distclean
+     $(MAKE) -C catalog $@
      rm -f bootstrap/bootparse.c \
            bootstrap/bootscanner.c \
            parser/gram.c \
            parser/gram.h \
            parser/scan.c \
            replication/repl_gram.c \
            replication/repl_scanner.c \
            replication/syncrep_gram.c \
diff --git a/src/backend/catalog/.gitignore b/src/backend/catalog/.gitignore
index 1044a1c..9abe91d 100644
*** a/src/backend/catalog/.gitignore
--- b/src/backend/catalog/.gitignore
***************
*** 3,5 ****
--- 3,6 ----
  /postgres.shdescription
  /schemapg.h
  /pg_*_d.h
+ /bki-stamp
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 6fe5566..39dae86 100644
*** a/src/backend/catalog/Catalog.pm
--- b/src/backend/catalog/Catalog.pm
*************** sub ParseHeader
*** 201,207 ****
  #
  # The parameter $preserve_formatting needs to be set for callers that want
  # to work with non-data lines in the data files, such as comments and blank
! # lines. If a caller just wants consume the data, leave it unset.
  sub ParseData
  {
      my ($input_file, $schema, $preserve_formatting) = @_;
--- 201,207 ----
  #
  # The parameter $preserve_formatting needs to be set for callers that want
  # to work with non-data lines in the data files, such as comments and blank
! # lines. If a caller just wants to consume the data, leave it unset.
  sub ParseData
  {
      my ($input_file, $schema, $preserve_formatting) = @_;
*************** sub ParseData
*** 299,304 ****
--- 299,305 ----

  # Fill in default values of a record using the given schema. It's the
  # caller's responsibility to specify other values beforehand.
+ # If we fail to fill all fields, return a nonempty error message.
  sub AddDefaultValues
  {
      my ($row, $schema) = @_;
*************** sub FindDefinedSymbolFromData
*** 391,398 ****
          {
              return $row->{oid};
          }
-         die "no definition found for $symbol\n";
      }
  }

  1;
--- 392,399 ----
          {
              return $row->{oid};
          }
      }
+     die "no definition found for $symbol\n";
  }

  1;
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index 9b87f85..17213d4 100644
*** a/src/backend/catalog/Makefile
--- b/src/backend/catalog/Makefile
*************** CATALOG_HEADERS := \
*** 44,68 ****
      pg_default_acl.h pg_init_privs.h pg_seclabel.h pg_shseclabel.h \
      pg_collation.h pg_partitioned_table.h pg_range.h pg_transform.h \
      pg_sequence.h pg_publication.h pg_publication_rel.h pg_subscription.h \
!     pg_subscription_rel.h \

  GENERATED_HEADERS := $(CATALOG_HEADERS:%.h=%_d.h) schemapg.h

- distprep: $(BKIFILES) $(GENERATED_HEADERS)
-
- .PHONY: builddir-headers
-
- builddir-headers: $(addprefix $(top_builddir)/src/include/catalog/, $(GENERATED_HEADERS))
-
- all: distprep builddir-headers
-
  # In the list of headers used to assemble postgres.bki, indexing.h needs
  # be last, and toasting.h just before it.
! POSTGRES_BKI_SRCS := $(addprefix $(top_srcdir)/src/include/catalog/, $(CATALOG_HEADERS) toasting.h indexing.h)

  POSTGRES_BKI_DATA = $(addprefix $(top_srcdir)/src/include/catalog/,\
      pg_aggregate.dat pg_am.dat pg_amop.dat pg_amproc.dat pg_authid.dat \
!     pg_cast.dat pg_class.dat pg_collation.dat pg_database.dat pg_language.dat \
      pg_namespace.dat pg_opclass.dat pg_operator.dat pg_opfamily.dat \
      pg_pltemplate.dat pg_proc.dat pg_range.dat pg_tablespace.dat \
      pg_ts_config.dat pg_ts_config_map.dat pg_ts_dict.dat pg_ts_parser.dat \
--- 44,64 ----
      pg_default_acl.h pg_init_privs.h pg_seclabel.h pg_shseclabel.h \
      pg_collation.h pg_partitioned_table.h pg_range.h pg_transform.h \
      pg_sequence.h pg_publication.h pg_publication_rel.h pg_subscription.h \
!     pg_subscription_rel.h

  GENERATED_HEADERS := $(CATALOG_HEADERS:%.h=%_d.h) schemapg.h

  # In the list of headers used to assemble postgres.bki, indexing.h needs
  # be last, and toasting.h just before it.
! POSTGRES_BKI_SRCS := $(addprefix $(top_srcdir)/src/include/catalog/,\
!     $(CATALOG_HEADERS) toasting.h indexing.h \
!     )

+ # The .dat files we need can just be listed alphabetically.
  POSTGRES_BKI_DATA = $(addprefix $(top_srcdir)/src/include/catalog/,\
      pg_aggregate.dat pg_am.dat pg_amop.dat pg_amproc.dat pg_authid.dat \
!     pg_cast.dat pg_class.dat pg_collation.dat \
!     pg_database.dat pg_language.dat \
      pg_namespace.dat pg_opclass.dat pg_operator.dat pg_opfamily.dat \
      pg_pltemplate.dat pg_proc.dat pg_range.dat pg_tablespace.dat \
      pg_ts_config.dat pg_ts_config_map.dat pg_ts_dict.dat pg_ts_parser.dat \
*************** POSTGRES_BKI_DATA = $(addprefix $(top_sr
*** 72,104 ****
  # location of Catalog.pm
  catalogdir = $(top_srcdir)/src/backend/catalog

! # see explanation in ../parser/Makefile
! postgres.description: postgres.bki ;
! postgres.shdescription: postgres.bki ;
! $(GENERATED_HEADERS): postgres.bki ;

! # Technically, this should depend on Makefile.global, but then
! # postgres.bki would need to be rebuilt after every configure run,
! # even in distribution tarballs.  So this is cheating a bit, but it
! # will achieve the goal of updating the version number when it
! # changes.
! postgres.bki: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS) $(POSTGRES_BKI_DATA) $(top_srcdir)/configure
$(top_srcdir)/src/include/catalog/duplicate_oids
      cd $(top_srcdir)/src/include/catalog && $(PERL) ./duplicate_oids
      $(PERL) -I $(catalogdir) $< --set-version=$(MAJORVERSION) $(POSTGRES_BKI_SRCS)

! # see explanation in src/backend/Makefile
! $(top_builddir)/src/include/catalog/%_d.h: %_d.h
!     prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
!       cd '$(dir $@)' && rm -f $(notdir $@) && \
!       $(LN_S) "$$prereqdir/$(notdir $<)" .
!
! $(top_builddir)/src/include/catalog/schemapg.h: schemapg.h
      prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
!       cd '$(dir $@)' && rm -f $(notdir $@) && \
!       $(LN_S) "$$prereqdir/$(notdir $<)" .

  .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'
--- 68,105 ----
  # location of Catalog.pm
  catalogdir = $(top_srcdir)/src/backend/catalog

! all: distprep generated-header-symlinks

! distprep: bki-stamp
!
! .PHONY: generated-header-symlinks
!
! generated-header-symlinks: $(top_builddir)/src/include/catalog/header-stamp
!
! # Technically, this should depend on Makefile.global which supplies
! # $(MAJORVERSION); but then postgres.bki would need to be rebuilt after every
! # configure run, even in distribution tarballs.  So depending on configure.in
! # instead is cheating a bit, but it will achieve the goal of updating the
! # version number when it changes.
! bki-stamp: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS) $(POSTGRES_BKI_DATA) $(top_srcdir)/configure.in
$(top_srcdir)/src/include/catalog/duplicate_oids
      cd $(top_srcdir)/src/include/catalog && $(PERL) ./duplicate_oids
      $(PERL) -I $(catalogdir) $< --set-version=$(MAJORVERSION) $(POSTGRES_BKI_SRCS)
+     touch $@

! # The generated headers must all be symlinked into builddir/src/include/,
! # using absolute links for the reasons explained in src/backend/Makefile.
! # We use header-stamp to record that we've done this because the symlinks
! # themselves may appear older than bki-stamp.
! $(top_builddir)/src/include/catalog/header-stamp: bki-stamp
      prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
!     cd '$(dir $@)' && for file in $(GENERATED_HEADERS); do \
!       rm -f $$file && $(LN_S) "$$prereqdir/$$file" . ; \
!     done
!     touch $@

+ # Note: installation of generated headers is handled elsewhere
  .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'
*************** installdirs:
*** 113,121 ****
  uninstall-data:
      rm -f $(addprefix '$(DESTDIR)$(datadir)'/, $(BKIFILES) system_views.sql information_schema.sql sql_features.txt)

! # postgres.bki, postgres.description, postgres.shdescription, and the generated headers
! # are in the distribution tarball, so they are not cleaned here.
  clean:

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

! # postgres.bki, postgres.description, postgres.shdescription,
! # and the generated headers are in the distribution tarball,
! # so they are not cleaned here.
  clean:

  maintainer-clean: clean
!     rm -f bki-stamp $(BKIFILES) $(GENERATED_HEADERS)
diff --git a/src/backend/catalog/README b/src/backend/catalog/README
index 447fce6..4aa2adb 100644
*** a/src/backend/catalog/README
--- b/src/backend/catalog/README
*************** src/backend/catalog/README
*** 3,20 ****
  System Catalog
  ==============

! This directory contains .c files that manipulate the system catalogs;
! src/include/catalog contains the .h files that define the structure
! of the system catalogs.

! When the compile-time script genbki.pl executes, it parses the .h files
  and .dat files in order to generate the postgres.* files.  These are then
  used as input to initdb (which is just a wrapper around postgres
  running single-user in bootstrapping mode) in order to generate the
  initial (template) system catalog relation files.

! backend/utils/Gen_fmgrtab.pl uses the same mechanism to genarate .c and
! .h files used by the function manager.

  -----------------------------------------------------------------

--- 3,27 ----
  System Catalog
  ==============

! This directory contains .c files that manipulate the system catalogs.
! src/include/catalog contains the pg_XXX.h files that define the structure
! of the system catalogs, as well as pg_XXX.dat files that define the
! initial contents of the catalogs.

! When the compile-time script genbki.pl executes, it parses the pg_XXX.h files
  and .dat files in order to generate the postgres.* files.  These are then
  used as input to initdb (which is just a wrapper around postgres
  running single-user in bootstrapping mode) in order to generate the
  initial (template) system catalog relation files.

! genbki.pl also produces some generated header files that are used in
! compiling the C code.  These include a pg_XXX_d.h file corresponding
! to each pg_XXX.h catalog header file, which contains #define's extracted
! from the corresponding .dat file as well as some automatically-generated
! symbols.
!
! backend/utils/Gen_fmgrtab.pl uses the same catalog-data-reading code
! to generate .c and .h files used by the function manager.

  -----------------------------------------------------------------

*************** The data file format and bootstrap data
*** 22,33 ****

  - As far as the bootstrap code is concerned, it is very important
  that the insert statements in postgres.bki be properly formatted
! (e.g., no broken lines, proper use of white-space and _null_).  The
! scripts are line-oriented and break easily.  In addition, the only
! documentation on the proper format for them is the code in the
! bootstrap/ directory.  Fortunately, the source bootstrap data is much
! more tolerant with respect to formatting, but it still pays to be
! careful when adding new data.

  - The .dat files contain Perl data structure literals that are simply
  eval'd to produce in-memory data structures.  As such, the code reading
--- 29,39 ----

  - As far as the bootstrap code is concerned, it is very important
  that the insert statements in postgres.bki be properly formatted
! (e.g., no broken lines, proper use of white-space and _null_).
! In addition, the only documentation on the proper format for them
! is the code in the bootstrap/ directory.  Fortunately, the source
! bootstrap data is much more tolerant with respect to formatting,
! but it still pays to be careful when adding new data.

  - The .dat files contain Perl data structure literals that are simply
  eval'd to produce in-memory data structures.  As such, the code reading
*************** demonstrate the key features:
*** 43,54 ****

  # a comment
  { oid => '1', oid_symbol => 'TemplateDbOid', shdescr => 'default template',
!   datname => 'Berkely\'s DB', datcollate => '"LC_COLLATE"', datacl => '_null_' },

  ]

! -The layout is: open bracket, one or more sets of curly brackets containing
! comma-separated key-value pairs, close bracket.
  -All values are single-quoted.
  -Single quotes within values must be escaped.
  -If a value is a macro to be expanded by initdb.c, it must also have double-
--- 49,61 ----

  # a comment
  { oid => '1', oid_symbol => 'TemplateDbOid', shdescr => 'default template',
!   datname => 'Berkeley\'s DB', datcollate => '"LC_COLLATE"',
!   datacl => '_null_' },

  ]

! -The overall file layout is: open bracket, one or more sets of curly brackets
! containing comma-separated key-value pairs, close bracket.
  -All values are single-quoted.
  -Single quotes within values must be escaped.
  -If a value is a macro to be expanded by initdb.c, it must also have double-
*************** the catalog's data file, and use the #de
*** 91,100 ****
  the actual numeric value of any OID in C code is considered very bad form.
  Direct references to pg_proc OIDs are common enough that there's a special
  mechanism to create the necessary #define's automatically: see
! backend/utils/Gen_fmgrtab.pl.  We also have standard conventions for setting
! up #define's for the pg_class OIDs of system catalogs and indexes.  For all
! the other system catalogs, you have to manually create any #define's you
! need.

  - If you need to find a valid OID for a new predefined tuple, use the
  script src/include/catalog/unused_oids.  It generates inclusive ranges of
--- 98,109 ----
  the actual numeric value of any OID in C code is considered very bad form.
  Direct references to pg_proc OIDs are common enough that there's a special
  mechanism to create the necessary #define's automatically: see
! backend/utils/Gen_fmgrtab.pl.  Similarly (but, for historical reasons, not
! done the same way), there's an automatic method for creating #define's
! for pg_type OIDs.  We also have standard conventions for setting up #define's
! for the pg_class OIDs of system catalogs and indexes.  For all the other
! system catalogs, you have to manually specify any #define's you need, via
! oid_symbol entries.

  - If you need to find a valid OID for a new predefined tuple, use the
  script src/include/catalog/unused_oids.  It generates inclusive ranges of
*************** script src/include/catalog/unused_oids.
*** 102,111 ****
  not been allocated yet).  Currently, OIDs 1-9999 are reserved for manual
  assignment; the unused_oids script simply looks through the include/catalog
  headers and .dat files to see which ones do not appear.
! (As of Postgres 8.1, it also looks at CATALOG and DECLARE_INDEX lines.)
! You can use the duplicate_oids script to check for mistakes.  This script
! is also run at compile time, and will stop the build if a duplicate is
! found.

  - The OID counter starts at 10000 at bootstrap.  If a catalog row is in a
  table that requires OIDs, but no OID was preassigned by an "OID =" clause,
--- 111,119 ----
  not been allocated yet).  Currently, OIDs 1-9999 are reserved for manual
  assignment; the unused_oids script simply looks through the include/catalog
  headers and .dat files to see which ones do not appear.
! You can also use the duplicate_oids script to check for mistakes.
! (This script is run automatically at compile time, and will stop the build
! if a duplicate is found.)

  - The OID counter starts at 10000 at bootstrap.  If a catalog row is in a
  table that requires OIDs, but no OID was preassigned by an "OID =" clause,
*************** the tables are in place, and toasting.h
*** 134,144 ****
  (or at least after all the tables that need toast tables).  There are
  reputedly some other order dependencies in the .bki list, too.

! -Client code should not include the catalog headers directly.  Instead, it
! should include the corresponding generated pg_*_d.h header.  If you want
! macros or other code in the catalog headers to be visible to clients, use
! the undefined macro EXPOSE_TO_CLIENT_CODE to instruct genbki.pl to copy
! that section to the pg_*_d.h header.

  -----------------------------------------------------------------

--- 142,155 ----
  (or at least after all the tables that need toast tables).  There are
  reputedly some other order dependencies in the .bki list, too.

! - Frontend code should not include any pg_XXX.h header directly, as these
! files may contain C code that won't compile outside the backend.  Instead,
! frontend code may include the corresponding generated pg_*_d.h header, which
! will contain OID #defines and any other data that might be of use on the
! client side.  If you want macros or other code in a catalog header to be
! visible to frontend code, write "#ifdef EXPOSE_TO_CLIENT_CODE" ... "#endif"
! around that section to instruct genbki.pl to copy that section to the
! pg_*_d.h header.

  -----------------------------------------------------------------

*************** piece of code will likely perform "typet
*** 155,163 ****
  random errors or even segmentation violations.  Hence, do NOT insert
  catalog tuples that contain NULL attributes except in their
  variable-length portions!  (The bootstrapping code is fairly good about
! marking NOT NULL each of the columns that can legally be referenced via
! C struct declarations ... but those markings won't be enforced against
! insert commands, so you must get it right in the data files.)

  - Modification of the catalogs must be performed with the proper
  updating of catalog indexes!  That is, most catalogs have indexes
--- 166,176 ----
  random errors or even segmentation violations.  Hence, do NOT insert
  catalog tuples that contain NULL attributes except in their
  variable-length portions!  (The bootstrapping code is fairly good about
! automatically marking NOT NULL each of the columns that can legally be
! referenced via C struct declarations, and it can be helped along with
! BKI_FORCE_NOT_NULL and BKI_FORCE_NULL annotations where needed.  But
! attnotnull constraints are only enforced in the executor, not against
! tuples that are generated by random C code.)

  - Modification of the catalogs must be performed with the proper
  updating of catalog indexes!  That is, most catalogs have indexes
*************** method calls to insert new or modified t
*** 167,170 ****
  also make the calls to insert the tuple into ALL of its indexes!  If
  not, the new tuple will generally be "invisible" to the system because
  most of the accesses to the catalogs in question will be through the
! associated indexes.
--- 180,185 ----
  also make the calls to insert the tuple into ALL of its indexes!  If
  not, the new tuple will generally be "invisible" to the system because
  most of the accesses to the catalogs in question will be through the
! associated indexes.  Current practice is to always use CatalogTupleInsert,
! CatalogTupleUpdate, CatalogTupleDelete, or one of their sibling functions
! when updating the system catalogs, so that this is handled automatically.
diff --git a/src/common/Makefile b/src/common/Makefile
index f28c136..80e78d7 100644
*** a/src/common/Makefile
--- b/src/common/Makefile
*************** libpgcommon_srv.a: $(OBJS_SRV)
*** 88,95 ****
  %_srv.o: %.c %.o
      $(CC) $(CFLAGS) $(subst -DFRONTEND ,, $(CPPFLAGS)) -c $< -o $@

- $(OBJS_COMMON): | submake-generated-headers
-
  $(OBJS_SRV): | submake-errcodes

  .PHONY: submake-errcodes
--- 88,93 ----
diff --git a/src/include/Makefile b/src/include/Makefile
index 7fe4d45..59e18c7 100644
*** a/src/include/Makefile
--- b/src/include/Makefile
*************** install: all installdirs
*** 54,60 ****
        chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$dir/*.h  || exit; \
      done
  ifeq ($(vpath_build),yes)
!     for file in dynloader.h catalog/schemapg.h parser/gram.h storage/lwlocknames.h utils/probes.h; do \
        cp $$file '$(DESTDIR)$(includedir_server)'/$$file || exit; \
        chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$file || exit; \
      done
--- 54,60 ----
        chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$dir/*.h  || exit; \
      done
  ifeq ($(vpath_build),yes)
!     for file in dynloader.h catalog/schemapg.h catalog/pg_*_d.h parser/gram.h storage/lwlocknames.h utils/probes.h;
do\ 
        cp $$file '$(DESTDIR)$(includedir_server)'/$$file || exit; \
        chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$file || exit; \
      done
*************** uninstall:
*** 73,79 ****


  clean:
!     rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h parser/gram.h utils/probes.h catalog/schemapg.h
catalog/pg_*_d.h

  distclean maintainer-clean: clean
      rm -f pg_config.h pg_config_ext.h pg_config_os.h dynloader.h stamp-h stamp-ext-h
--- 73,81 ----


  clean:
!     rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h
!     rm -f parser/gram.h storage/lwlocknames.h utils/probes.h
!     rm -f catalog/schemapg.h catalog/pg_*_d.h catalog/header-stamp

  distclean maintainer-clean: clean
      rm -f pg_config.h pg_config_ext.h pg_config_os.h dynloader.h stamp-h stamp-ext-h
diff --git a/src/include/catalog/.gitignore b/src/include/catalog/.gitignore
index dfd5616..6c8da54 100644
*** a/src/include/catalog/.gitignore
--- b/src/include/catalog/.gitignore
***************
*** 1,2 ****
--- 1,3 ----
  /schemapg.h
  /pg_*_d.h
+ /header-stamp