From a6417078c4140e51cfd717448430f274b449d687 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 12 Mar 2019 10:50:48 -0400 Subject: [PATCH] Create a script that can renumber manually-assigned OIDs. This commit adds a Perl script renumber_oids.pl, which can reassign a range of manually-assigned OIDs to someplace else by modifying OID fields of the catalog *.dat files and OID-assigning macros in the catalog *.h files. Up to now, we've encouraged new patches that need manually-assigned OIDs to use OIDs just above the range of existing OIDs. Predictably, this leads to patches stepping on each others' toes, as whichever one gets committed first creates an OID conflict that other patch author(s) have to resolve manually. With the availability of renumber_oids.pl, we can eliminate a lot of this hassle. The new project policy, therefore, is: * Encourage new patches to use high OIDs (the documentation suggests choosing a block of OIDs at random in 8000..9999). * After feature freeze in each development cycle, run renumber_oids.pl to move all such OIDs down to lower numbers, thus freeing the high OID range for the next development cycle. This plan should greatly reduce the risk of OID collisions between concurrently-developed patches. Also, if such a collision happens anyway, we have the option to resolve it without much effort by doing an off-schedule OID renumbering to get the first-committed patch out of the way. Or a patch author could use renumber_oids.pl to change their patch's assignments without much pain. This approach does put a premium on not hard-wiring any OID values in places where renumber_oids.pl and genbki.pl can't fix them. Project practice in that respect seems to be pretty good already, but a follow-on patch will sand down some rough edges. John Naylor and Tom Lane, per an idea of Peter Geoghegan's Discussion: https://postgr.es/m/CAH2-WzmMTGMcPuph4OvsO7Ykut0AOCF_i-=eaochT0dd2BN9CQ@mail.gmail.com --- doc/src/sgml/bki.sgml | 40 +++- src/backend/catalog/Catalog.pm | 2 + src/include/catalog/indexing.h | 3 + src/include/catalog/renumber_oids.pl | 291 +++++++++++++++++++++++++++ src/include/catalog/toasting.h | 3 + src/tools/RELEASE_CHANGES | 16 +- 6 files changed, 350 insertions(+), 5 deletions(-) create mode 100755 src/include/catalog/renumber_oids.pl diff --git a/doc/src/sgml/bki.sgml b/doc/src/sgml/bki.sgml index 3c5830bc8a..5f80f9ed24 100644 --- a/doc/src/sgml/bki.sgml +++ b/doc/src/sgml/bki.sgml @@ -388,15 +388,49 @@ to see which ones do not appear. You can also use the duplicate_oids script to check for mistakes. (genbki.pl will assign OIDs for any rows that - didn't get one hand-assigned to them and also detect duplicate OIDs - at compile time.) + didn't get one hand-assigned to them, and it will also detect duplicate + OIDs at compile time.) + + + + When choosing OIDs for a patch that is not expected to be committed + immediately, best practice is to use a group of more-or-less + consecutive OIDs starting with some random choice in the range + 8000—9999. This minimizes the risk of OID collisions with other + patches being developed concurrently. To keep the 8000—9999 + range free for development purposes, after a patch has been committed + to the master git repository its OIDs should be renumbered into + available space below that range. Typically, this will be done + near the end of each development cycle, moving all OIDs consumed by + patches committed in that cycle at the same time. The script + renumber_oids.pl can be used for this purpose. + If an uncommitted patch is found to have OID conflicts with some + recently-committed patch, renumber_oids.pl may + also be useful for recovering from that situation. + + + + Because of this convention of possibly renumbering OIDs assigned by + patches, the OIDs assigned by a patch should not be considered stable + until the patch has been included in an official release. We do not + change manually-assigned object OIDs once released, however, as that + would create assorted compatibility problems. The OID counter starts at 10000 at the beginning of a bootstrap run. If a row from a source other than postgres.bki is inserted into a table that requires OIDs, then it will receive an - OID of 10000 or above. + OID of 10000 or above. For example, objects created while running + the information_schema.sql script receive such + OIDs. + + + + OIDs assigned during normal database operation are constrained to be + 16384 or higher. This leaves the range 10000—16383 available + for OIDs assigned automatically during bootstrap. These OIDs are not + considered stable, and may change from one installation to another. diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm index 3bf308fe3b..368b1dea3e 100644 --- a/src/backend/catalog/Catalog.pm +++ b/src/backend/catalog/Catalog.pm @@ -87,6 +87,8 @@ sub ParseHeader } # Push the data into the appropriate data structure. + # Caution: when adding new recognized OID-defining macros, + # also update src/include/catalog/renumber_oids.pl. if (/^DECLARE_TOAST\(\s*(\w+),\s*(\d+),\s*(\d+)\)/) { push @{ $catalog{toasting} }, diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index 833fad1f6a..f253613650 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -4,6 +4,9 @@ * This file provides some definitions to support indexing * on system catalogs * + * Caution: all #define's with numeric values in this file had better be + * object OIDs, else renumber_oids.pl might change them inappropriately. + * * * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California diff --git a/src/include/catalog/renumber_oids.pl b/src/include/catalog/renumber_oids.pl new file mode 100755 index 0000000000..8a07340ba1 --- /dev/null +++ b/src/include/catalog/renumber_oids.pl @@ -0,0 +1,291 @@ +#!/usr/bin/perl +#---------------------------------------------------------------------- +# +# renumber_oids.pl +# Perl script that shifts a range of OIDs in the Postgres catalog data +# to a different range, skipping any OIDs that are already in use. +# +# Note: This does not reformat the .dat files, so you may want +# to run reformat_dat_file.pl afterwards. +# +# Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group +# Portions Copyright (c) 1994, Regents of the University of California +# +# src/include/catalog/renumber_oids.pl +# +#---------------------------------------------------------------------- + +use strict; +use warnings; + +use FindBin; +use Getopt::Long; + +# Must run in src/include/catalog +chdir $FindBin::RealBin or die "could not cd to $FindBin::RealBin: $!\n"; + +use lib "$FindBin::RealBin/../../backend/catalog/"; +use Catalog; + +# We'll need this number. +my $FirstGenbkiObjectId = + Catalog::FindDefinedSymbol('access/transam.h', '..', 'FirstGenbkiObjectId'); + +# Process command line switches. +my $output_path = ''; +my $first_mapped_oid = 0; +my $last_mapped_oid = $FirstGenbkiObjectId - 1; +my $target_oid = 0; + +GetOptions( + 'output=s' => \$output_path, + 'first-mapped-oid=i' => \$first_mapped_oid, + 'last-mapped-oid=i' => \$last_mapped_oid, + 'target-oid=i' => \$target_oid) || usage(); + +# Sanity check arguments. +die "Unexpected non-switch arguments.\n" if @ARGV; +die "--first-mapped-oid must be specified.\n" + if $first_mapped_oid <= 0; +die "Empty mapped OID range.\n" + if $last_mapped_oid < $first_mapped_oid; +die "--target-oid must be specified.\n" + if $target_oid <= 0; +die "--target-oid must not be within mapped OID range.\n" + if $target_oid >= $first_mapped_oid && $target_oid <= $last_mapped_oid; + +# Make sure output_path ends in a slash. +if ($output_path ne '' && substr($output_path, -1) ne '/') +{ + $output_path .= '/'; +} + +# Collect all the existing assigned OIDs (including those to be remapped). +my @header_files = (glob("pg_*.h"), qw(indexing.h toasting.h)); +my $oids = Catalog::FindAllOidsFromHeaders(@header_files); + +# Hash-ify the existing OIDs for convenient lookup. +my %oidhash; +@oidhash{@$oids} = undef; + +# Select new OIDs for existing OIDs in the mapped range. +# We do this first so that we preserve the ordering of the mapped OIDs +# (for reproducibility's sake), and so that if we fail due to running out +# of OID room, that happens before we've overwritten any files. +my %maphash; +my $next_oid = $target_oid; + +for ( + my $mapped_oid = $first_mapped_oid; + $mapped_oid <= $last_mapped_oid; + $mapped_oid++) +{ + next if !exists $oidhash{$mapped_oid}; + $next_oid++ + while ( + exists $oidhash{$next_oid} + || ( $next_oid >= $first_mapped_oid + && $next_oid <= $last_mapped_oid)); + die "Reached FirstGenbkiObjectId before assigning all OIDs.\n" + if $next_oid >= $FirstGenbkiObjectId; + $maphash{$mapped_oid} = $next_oid; + $next_oid++; +} + +die "There are no OIDs in the mapped range.\n" if $next_oid == $target_oid; + +# Read each .h file and write out modified data. +foreach my $input_file (@header_files) +{ + $input_file =~ /(\w+)\.h$/ + or die "Input file $input_file needs to be a .h file.\n"; + my $catname = $1; + + # Ignore generated *_d.h files. + next if $catname =~ /_d$/; + + open(my $ifd, '<', $input_file) || die "$input_file: $!"; + + # Write output files to specified directory. + # Use a .tmp suffix, then rename into place, in case we're overwriting. + my $output_file = "$output_path$catname.h"; + my $tmp_output_file = "$output_file.tmp"; + open my $ofd, '>', $tmp_output_file + or die "can't open $tmp_output_file: $!"; + my $changed = 0; + + # Scan the input file. + while (<$ifd>) + { + my $line = $_; + + # Check for OID-defining macros that Catalog::ParseHeader knows about, + # and update OIDs as needed. + if ($line =~ m/^(DECLARE_TOAST\(\s*\w+,\s*)(\d+)(,\s*)(\d+)\)/) + { + my $oid2 = $2; + my $oid4 = $4; + if (exists $maphash{$oid2}) + { + $oid2 = $maphash{$oid2}; + my $repl = $1 . $oid2 . $3 . $oid4 . ")"; + $line =~ s/^DECLARE_TOAST\(\s*\w+,\s*\d+,\s*\d+\)/$repl/; + $changed = 1; + } + if (exists $maphash{$oid4}) + { + $oid4 = $maphash{$oid4}; + my $repl = $1 . $oid2 . $3 . $oid4 . ")"; + $line =~ s/^DECLARE_TOAST\(\s*\w+,\s*\d+,\s*\d+\)/$repl/; + $changed = 1; + } + } + elsif ( + $line =~ m/^(DECLARE_(UNIQUE_)?INDEX\(\s*\w+,\s*)(\d+)(,\s*.+)\)/) + { + if (exists $maphash{$3}) + { + my $repl = $1 . $maphash{$3} . $4 . ")"; + $line =~ + s/^DECLARE_(UNIQUE_)?INDEX\(\s*\w+,\s*\d+,\s*.+\)/$repl/; + $changed = 1; + } + } + elsif ($line =~ m/^CATALOG\((\w+),(\d+),(\w+)\)/) + { + if (exists $maphash{$2}) + { + my $repl = + "CATALOG(" . $1 . "," . $maphash{$2} . "," . $3 . ")"; + $line =~ s/^CATALOG\(\w+,\d+,\w+\)/$repl/; + $changed = 1; + } + + if ($line =~ m/BKI_ROWTYPE_OID\((\d+),(\w+)\)/) + { + if (exists $maphash{$1}) + { + my $repl = + "BKI_ROWTYPE_OID(" . $maphash{$1} . "," . $2 . ")"; + $line =~ s/BKI_ROWTYPE_OID\(\d+,\w+\)/$repl/; + $changed = 1; + } + } + } + + # In indexing.h and toasting.h only, check for #define SYM nnnn, + # and replace if within mapped range. + elsif ($line =~ m/^(\s*#\s*define\s+\w+\s+)(\d+)\b/) + { + if (($catname eq 'indexing' || $catname eq 'toasting') + && exists $maphash{$2}) + { + my $repl = $1 . $maphash{$2}; + $line =~ s/^\s*#\s*define\s+\w+\s+\d+\b/$repl/; + $changed = 1; + } + } + + print $ofd $line; + } + + close $ifd; + close $ofd; + + # Avoid updating files if we didn't change them. + if ($changed || $output_path ne '') + { + rename $tmp_output_file, $output_file + or die "can't rename $tmp_output_file to $output_file: $!"; + } + else + { + unlink $tmp_output_file + or die "can't unlink $tmp_output_file: $!"; + } +} + +# Likewise, read each .dat file and write out modified data. +foreach my $input_file (glob("pg_*.dat")) +{ + $input_file =~ /(\w+)\.dat$/ + or die "Input file $input_file needs to be a .dat file.\n"; + my $catname = $1; + + open(my $ifd, '<', $input_file) || die "$input_file: $!"; + + # Write output files to specified directory. + # Use a .tmp suffix, then rename into place, in case we're overwriting. + my $output_file = "$output_path$catname.dat"; + my $tmp_output_file = "$output_file.tmp"; + open my $ofd, '>', $tmp_output_file + or die "can't open $tmp_output_file: $!"; + my $changed = 0; + + # Scan the input file. + while (<$ifd>) + { + my $line = $_; + + # Check for oid => 'nnnn', and replace if within mapped range. + if ($line =~ m/\b(oid\s*=>\s*)'(\d+)'/) + { + if (exists $maphash{$2}) + { + my $repl = $1 . "'" . $maphash{$2} . "'"; + $line =~ s/\boid\s*=>\s*'\d+'/$repl/; + $changed = 1; + } + } + + # Likewise for array_type_oid. + if ($line =~ m/\b(array_type_oid\s*=>\s*)'(\d+)'/) + { + if (exists $maphash{$2}) + { + my $repl = $1 . "'" . $maphash{$2} . "'"; + $line =~ s/\barray_type_oid\s*=>\s*'\d+'/$repl/; + $changed = 1; + } + } + + print $ofd $line; + } + + close $ifd; + close $ofd; + + # Avoid updating files if we didn't change them. + if ($changed || $output_path ne '') + { + rename $tmp_output_file, $output_file + or die "can't rename $tmp_output_file to $output_file: $!"; + } + else + { + unlink $tmp_output_file + or die "can't unlink $tmp_output_file: $!"; + } +} + +sub usage +{ + my $last = $FirstGenbkiObjectId - 1; + die <