agora inbox for [email protected]
help / color / mirror / Atom feedRe: Extracting cross-version-upgrade knowledge from buildfarm client
27+ messages / 4 participants
[nested] [flat]
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
@ 2023-01-15 16:01 Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-15 16:01 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
Andrew Dunstan <[email protected]> writes:
> On 2023-01-14 Sa 15:06, Tom Lane wrote:
>> Here's version 2, incorporating your suggestions and with some
>> further work to make it handle 9.2 fully.
> This looks pretty good to me.
Great! I'll work on making back-branch versions.
> I'll probably change this line
> my $adjust_cmds = adjust_database_contents($oversion, %dbnames);
> so it's only called if the old and new versions are different. Is there
> any case where a repo shouldn't be upgradeable to its own version
> without adjustment?
Makes sense. I'd keep the check for $oversion eq 'HEAD' in the
subroutines, but that's mostly just to protect the version
conversion code below it.
Another thing I was just thinking about was not bothering to run
"diff" if the fixed dump strings are equal in-memory. You could
take that even further and not write out the fixed files at all,
but that seems like a bad idea for debuggability of the adjustment
subroutines. However, I don't see why we need to write an
empty diff file, nor parse it.
One other question before I continue --- do the adjustment
subroutines need to worry about Windows newlines in the strings?
It's not clear to me whether Perl will automatically make "\n"
in a pattern match "\r\n", or whether it's not a problem because
something upstream will have stripped \r's.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-15 23:12 ` Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-15 23:12 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
On 2023-01-15 Su 11:01, Tom Lane wrote:
> Another thing I was just thinking about was not bothering to run
> "diff" if the fixed dump strings are equal in-memory. You could
> take that even further and not write out the fixed files at all,
> but that seems like a bad idea for debuggability of the adjustment
> subroutines. However, I don't see why we need to write an
> empty diff file, nor parse it.
Yeah, that makes sense.
> One other question before I continue --- do the adjustment
> subroutines need to worry about Windows newlines in the strings?
> It's not clear to me whether Perl will automatically make "\n"
> in a pattern match "\r\n", or whether it's not a problem because
> something upstream will have stripped \r's.
>
>
I don't think we need to worry about them, but I will have a closer
look. Those replacement lines are very difficult to read. I think use of
extended regexes and some multi-part replacements would help. I'll have
a go at that tomorrow.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-15 23:37 ` Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-15 23:37 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
Andrew Dunstan <[email protected]> writes:
> Those replacement lines are very difficult to read. I think use of
> extended regexes and some multi-part replacements would help. I'll have
> a go at that tomorrow.
Yeah, after I wrote that code I remembered about \Q ... \E, which would
eliminate the need for most of the backslashes and probably make things
better that way. I didn't get around to improving it yet though, so
feel free to have a go.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-16 19:08 ` Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-16 19:08 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
On 2023-01-15 Su 18:37, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> Those replacement lines are very difficult to read. I think use of
>> extended regexes and some multi-part replacements would help. I'll have
>> a go at that tomorrow.
> Yeah, after I wrote that code I remembered about \Q ... \E, which would
> eliminate the need for most of the backslashes and probably make things
> better that way. I didn't get around to improving it yet though, so
> feel free to have a go.
>
>
OK, here's my version. It tests clean against all of crake's dump files
back to 9.2.
To some extent it's a matter of taste, but I hate very long regex lines
- it makes it very hard to see what's actually changing, so I broke up
most of those.
Given that we are looking at newlines in some places I decided that
after all it was important to convert CRLF to LF.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Attachments:
[text/x-patch] adjustupgrade-3.patch (16.1K, ../../[email protected]/2-adjustupgrade-3.patch)
download | inline diff:
diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
new file mode 100644
index 0000000000..2134afd64e
--- /dev/null
+++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
@@ -0,0 +1,550 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+=pod
+
+=head1 NAME
+
+PostgreSQL::Test::AdjustUpgrade - helper module for cross-version upgrade tests
+
+=head1 SYNOPSIS
+
+ use PostgreSQL::Test::AdjustUpgrade;
+
+ # Build commands to adjust contents of old-version database before dumping
+ $statements = adjust_database_contents($old_version, %dbnames);
+
+ # Adjust contents of old pg_dumpall output file to match newer version
+ $dump = adjust_old_dumpfile($old_version, $dump);
+
+ # Adjust contents of new pg_dumpall output file to match older version
+ $dump = adjust_new_dumpfile($old_version, $dump);
+
+=head1 DESCRIPTION
+
+C<PostgreSQL::Test::AdjustUpgrade> encapsulates various hacks needed to
+compare the results of cross-version upgrade tests.
+
+=cut
+
+package PostgreSQL::Test::AdjustUpgrade;
+
+use strict;
+use warnings;
+
+use Exporter 'import';
+use PostgreSQL::Version;
+
+our @EXPORT = qw(
+ adjust_database_contents
+ adjust_old_dumpfile
+ adjust_new_dumpfile
+);
+
+=pod
+
+=head1 ROUTINES
+
+=over
+
+=item $statements = adjust_database_contents($old_version, %dbnames)
+
+Generate SQL commands to perform any changes to an old-version installation
+that are needed before we can pg_upgrade it into the current PostgreSQL
+version.
+
+Typically this involves dropping or adjusting no-longer-supported objects.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from. This can be a branch
+name such as 'HEAD' or 'REL_11_STABLE', but it can also be any string
+that PostgreSQL::Version accepts.
+
+=item C<dbnames>: Hash of database names present in the old installation.
+
+=back
+
+Returns a reference to a hash, wherein the keys are database names and the
+values are arrayrefs to lists of statements to be run in those databases.
+
+=cut
+
+sub adjust_database_contents
+{
+ my ($old_version, %dbnames) = @_;
+ my $result = {};
+
+ # nothing to do for non-cross-version tests
+ return $result if $old_version eq 'HEAD';
+
+ # convert branch name to numeric form
+ $old_version =~ s/REL_?(\d+(?:_\d+)?)_STABLE/$1/;
+ $old_version =~ s/_/./;
+ $old_version = PostgreSQL::Version->new($old_version);
+
+ # remove dbs of modules known to cause pg_upgrade to fail
+ # anything not builtin and incompatible should clean up its own db
+ foreach my $bad_module ('test_ddl_deparse', 'tsearch2')
+ {
+ if ($dbnames{"contrib_regression_$bad_module"})
+ {
+ _add_st($result, 'postgres',
+ "drop database contrib_regression_$bad_module");
+ delete($dbnames{"contrib_regression_$bad_module"});
+ }
+ }
+
+ # avoid version number issues with test_ext7
+ if ($dbnames{contrib_regression_test_extensions})
+ {
+ _add_st(
+ $result,
+ 'contrib_regression_test_extensions',
+ 'drop extension if exists test_ext7');
+ }
+
+ # stuff not supported from release 16
+ if ($old_version >= 12 && $old_version < 16)
+ {
+ # Can't upgrade aclitem in user tables from pre 16 to 16+.
+ _add_st($result, 'regression',
+ 'alter table public.tab_core_types drop column aclitem');
+ # Can't handle child tables with locally-generated columns.
+ _add_st(
+ $result, 'regression',
+ 'drop table public.gtest_normal_child',
+ 'drop table public.gtest_normal_child2');
+ }
+
+ # stuff not supported from release 14
+ if ($old_version < 14)
+ {
+ # postfix operators (some don't exist in very old versions)
+ _add_st(
+ $result,
+ 'regression',
+ 'drop operator #@# (bigint,NONE)',
+ 'drop operator #%# (bigint,NONE)',
+ 'drop operator if exists !=- (bigint,NONE)',
+ 'drop operator if exists #@%# (bigint,NONE)');
+
+ # get rid of dblink's dependencies on regress.so
+ my $regrdb =
+ $old_version le '9.4'
+ ? 'contrib_regression'
+ : 'contrib_regression_dblink';
+
+ if ($dbnames{$regrdb})
+ {
+ _add_st(
+ $result, $regrdb,
+ 'drop function if exists public.putenv(text)',
+ 'drop function if exists public.wait_pid(integer)');
+ }
+ }
+
+ # user table OIDs are gone from release 12 on
+ if ($old_version < 12)
+ {
+ my $nooid_stmt = q{
+ DO $stmt$
+ DECLARE
+ rec text;
+ BEGIN
+ FOR rec in
+ select oid::regclass::text
+ from pg_class
+ where relname !~ '^pg_'
+ and relhasoids
+ and relkind in ('r','m')
+ order by 1
+ LOOP
+ execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
+ RAISE NOTICE 'removing oids from table %', rec;
+ END LOOP;
+ END; $stmt$;
+ };
+
+ foreach my $oiddb ('regression', 'contrib_regression_btree_gist')
+ {
+ next unless $dbnames{$oiddb};
+ _add_st($result, $oiddb, $nooid_stmt);
+ }
+
+ # this table had OIDs too, but we'll just drop it
+ if ($old_version >= 10 && $dbnames{'contrib_regression_postgres_fdw'})
+ {
+ _add_st(
+ $result,
+ 'contrib_regression_postgres_fdw',
+ 'drop foreign table ft_pg_type');
+ }
+ }
+
+ # abstime+friends are gone from release 12 on; but these tables
+ # might or might not be present depending on regression test vintage
+ if ($old_version < 12)
+ {
+ _add_st($result, 'regression',
+ 'drop table if exists abstime_tbl, reltime_tbl, tinterval_tbl');
+ }
+
+ # some regression functions gone from release 11 on
+ if ($old_version < 11)
+ {
+ _add_st(
+ $result, 'regression',
+ 'drop function if exists public.boxarea(box)',
+ 'drop function if exists public.funny_dup17()');
+ }
+
+ # version-0 C functions are no longer supported
+ if ($old_version < 10)
+ {
+ _add_st($result, 'regression',
+ 'drop function oldstyle_length(integer, text)');
+ }
+
+ if ($old_version lt '9.5')
+ {
+ # cope with changes of underlying functions
+ _add_st(
+ $result,
+ 'regression',
+ 'drop operator @#@ (NONE, bigint)',
+ 'CREATE OPERATOR @#@ ('
+ . 'PROCEDURE = factorial, RIGHTARG = bigint )',
+ 'drop aggregate public.array_cat_accum(anyarray)',
+ 'CREATE AGGREGATE array_larger_accum (anyarray) ' . ' ( '
+ . ' sfunc = array_larger, '
+ . ' stype = anyarray, '
+ . ' initcond = $${}$$ ' . ' ) ');
+
+ # "=>" is no longer valid as an operator name
+ _add_st($result, 'regression',
+ 'drop operator if exists public.=> (bigint, NONE)');
+ }
+
+ return $result;
+}
+
+# Internal subroutine to add statement(s) to the list for the given db.
+sub _add_st
+{
+ my ($result, $db, @st) = @_;
+
+ $result->{$db} ||= [];
+ push(@{ $result->{$db} }, @st);
+}
+
+=pod
+
+=item adjust_old_dumpfile($old_version, $dump)
+
+Edit a dump output file, taken from the adjusted old-version installation
+by current-version C<pg_dumpall -s>, so that it will match the results of
+C<pg_dumpall -s> on the pg_upgrade'd installation.
+
+Typically this involves coping with cosmetic differences in the output
+of backend subroutines used by pg_dump.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from. This can be a branch
+name such as 'HEAD' or 'REL_11_STABLE', but it can also be any string
+that PostgreSQL::Version accepts.
+
+=item C<dump>: Contents of dump file
+
+=back
+
+Returns the modified dump text.
+
+=cut
+
+sub adjust_old_dumpfile
+{
+ my ($old_version, $dump) = @_;
+
+ # nothing to do for non-cross-version tests
+ return $dump if $old_version eq 'HEAD';
+
+ # convert branch name to numeric form
+ $old_version =~ s/REL_?(\d+(?:_\d+)?)_STABLE/$1/;
+ $old_version =~ s/_/./;
+ $old_version = PostgreSQL::Version->new($old_version);
+
+ # use Unix newlines
+
+ $dump =~ s/\r\n/\n/g;
+
+ # Version comments will certainly not match.
+ $dump =~ s/^-- Dumped from database version.*\n//mg;
+
+ if ($old_version >= 14 && $old_version < 16)
+ {
+ # Fix up some privilege-set discrepancies.
+ $dump =~
+ s {^REVOKE SELECT,INSERT,REFERENCES,DELETE,TRIGGER,TRUNCATE,UPDATE ON TABLE}
+ {REVOKE ALL ON TABLE}mg;
+ $dump =~
+ s {^(GRANT SELECT,INSERT,REFERENCES,TRIGGER,TRUNCATE),UPDATE ON TABLE}
+ {$1,MAINTAIN,UPDATE ON TABLE}mg;
+ }
+
+ if ($old_version < 14)
+ {
+ # Remove mentions of extended hash functions.
+ $dump =~ s {(^\s+OPERATOR\s1\s=\((?:integer,integer|text,text)\))\s,\n
+ \s+FUNCTION\s2\s.*?public.part_hash.*?;}
+ {$1;}mxg;
+ }
+
+ # Change trigger definitions to say ... EXECUTE FUNCTION ...
+ if ($old_version < 12)
+ {
+ # would like to use lookbehind here but perl complains
+ # so do it this way
+ $dump =~ s/
+ (^CREATE\sTRIGGER\s.*?)
+ \sEXECUTE\sPROCEDURE
+ /$1 EXECUTE FUNCTION/mgx;
+ }
+
+ if ($old_version lt '9.6')
+ {
+ # adjust some places where we don't print so many parens anymore
+
+ my $prefix =
+ "'New York'\tnew & york | big & apple | nyc\t'new' & 'york'\t";
+ my $orig = "( 'new' & 'york' | 'big' & 'appl' ) | 'nyc'";
+ my $repl = "'new' & 'york' | 'big' & 'appl' | 'nyc'";
+ $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
+
+ $prefix = "'Sanct Peter'\tPeterburg | peter | 'Sanct Peterburg'\t'sanct' & 'peter'\t";
+ $orig = "( 'peterburg' | 'peter' ) | 'sanct' & 'peterburg'";
+ $repl = "'peterburg' | 'peter' | 'sanct' & 'peterburg'";
+ $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
+ }
+
+ if ($old_version lt '9.5')
+ {
+ # adjust some places where we don't print so many parens anymore
+
+ my $prefix = "CONSTRAINT (?:sequence|copy)_con CHECK [(][(]";
+ my $orig = "((x > 3) AND (y <> 'check failed'::text))";
+ my $repl = "(x > 3) AND (y <> 'check failed'::text)";
+ $dump =~ s/($prefix)\Q$orig\E/$1$repl/mg;
+
+ $prefix = "CONSTRAINT insert_con CHECK";
+ $orig = "((x >= 3) AND (y <> 'check failed'::text))";
+ $repl = "(x >= 3) AND (y <> 'check failed'::text)";
+ $dump =~ s/($prefix.*?)\Q$orig\E/$1$repl/mg;
+
+ $orig = "DEFAULT ((-1) * currval('public.insert_seq'::regclass))";
+ $repl = "DEFAULT ('-1'::integer * currval('public.insert_seq'::regclass))";
+ $dump =~ s/\Q$orig\E/$repl/mg;
+
+ my $expr =
+ "(rsl.sl_color = rsh.slcolor) AND (rsl.sl_len_cm >= rsh.slminlen_cm)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+
+ $expr = "(rule_and_refint_t3.id3a = new.id3a) AND (rule_and_refint_t3.id3b = new.id3b)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+
+ $expr = "(rule_and_refint_t3_1.id3a = new.id3a) AND (rule_and_refint_t3_1.id3b = new.id3b)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+ }
+
+ if ($old_version lt '9.3')
+ {
+ # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
+ # To cope, reduce all whitespace sequences within them to one space.
+ # This must be done on both old and new dumps.
+ $dump = _mash_view_whitespace($dump);
+
+ # _mash_view_whitespace doesn't handle multi-command rules;
+ # rather than trying to fix that, just hack the exceptions manually.
+
+ my $prefix = "CREATE RULE rtest_sys_del AS ON DELETE TO public.rtest_system DO (DELETE FROM public.rtest_interface WHERE (rtest_interface.sysname = old.sysname);";
+ my $line2 = " DELETE FROM public.rtest_admin";
+ my $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
+ $dump =~ s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
+
+ $prefix = "CREATE RULE rtest_sys_upd AS ON UPDATE TO public.rtest_system DO (UPDATE public.rtest_interface SET sysname = new.sysname WHERE (rtest_interface.sysname = old.sysname);";
+ $line2 = " UPDATE public.rtest_admin SET sysname = new.sysname";
+ $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
+ $dump =~ s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
+
+ # and there's one place where pre-9.3 uses less aliases than we do now
+
+ # add the alias to the subquery
+ $dump =~ s/(^CREATE\sRULE\srule_and_refint_t3_ins\s.*?)
+ (SELECT\s1\sFROM\spublic\.rule_and_refint_t3)
+ (\sWHERE)
+ /$1$2 rule_and_refint_t3_1$3/mx;
+
+ # and use it in the subquery (g flag doesn't work here)
+ for (1..3)
+ {
+ $dump =~ s/(^CREATE\sRULE\srule_and_refint_t3_ins.*?)
+ rule_and_refint_t3\.id3([abc])\s=\snew
+ (.*?DO\sINSTEAD)
+ /$1rule_and_refint_t3_1.id3$2 = new$3/mx;
+ }
+
+ # Also fix old use of NATURAL JOIN syntax
+ $dump =~
+ s {NATURAL JOIN public\.credit_card r}
+ {JOIN public.credit_card r USING (cid)}mg;
+ $dump =~
+ s {NATURAL JOIN public\.credit_usage r}
+ {JOIN public.credit_usage r USING (cid)}mg;
+ }
+
+ # Suppress blank lines, as some places in pg_dump emit more or fewer.
+ $dump =~ s/\n\n+/\n/g;
+
+ return $dump;
+}
+
+# Internal subroutine to mangle whitespace within view/rule commands.
+# Any consecutive sequence of whitespace is reduced to one space.
+sub _mash_view_whitespace
+{
+ my ($dump) = @_;
+
+ foreach my $leader ('CREATE VIEW', 'CREATE RULE')
+ {
+ my @splitchunks = split $leader, $dump;
+
+ $dump = shift(@splitchunks);
+ foreach my $chunk (@splitchunks)
+ {
+ my @thischunks = split /;/, $chunk, 2;
+ my $stmt = shift(@thischunks);
+
+ # now $stmt is just the body of the CREATE VIEW/RULE
+ $stmt =~ s/\s+/ /sg;
+ # we also need to smash these forms for sub-selects and rules
+ $stmt =~ s/\( SELECT/(SELECT/g;
+ $stmt =~ s/\( INSERT/(INSERT/g;
+ $stmt =~ s/\( UPDATE/(UPDATE/g;
+ $stmt =~ s/\( DELETE/(DELETE/g;
+
+ $dump .= $leader . $stmt . ';' . $thischunks[0];
+ }
+ }
+ return $dump;
+}
+
+=pod
+
+=item adjust_new_dumpfile($old_version, $dump)
+
+Edit a dump output file, taken from the pg_upgrade'd installation
+by current-version C<pg_dumpall -s>, so that it will match the old
+dump output file as adjusted by C<adjust_old_dumpfile>.
+
+Typically this involves deleting data not present in the old installation.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from. This can be a branch
+name such as 'HEAD' or 'REL_11_STABLE', but it can also be any string
+that PostgreSQL::Version accepts.
+
+=item C<dump>: Contents of dump file
+
+=back
+
+Returns the modified dump text.
+
+=cut
+
+sub adjust_new_dumpfile
+{
+ my ($old_version, $dump) = @_;
+
+ # nothing to do for non-cross-version tests
+ return $dump if $old_version eq 'HEAD';
+
+ # convert branch name to numeric form
+ $old_version =~ s/REL_?(\d+(?:_\d+)?)_STABLE/$1/;
+ $old_version =~ s/_/./;
+ $old_version = PostgreSQL::Version->new($old_version);
+
+ # use Unix newlines
+
+ $dump =~ s/\r\n/\n/g;
+
+ # Version comments will certainly not match.
+ $dump =~ s/^-- Dumped from database version.*\n//mg;
+
+ if ($old_version < 14)
+ {
+ # Suppress noise-word uses of IN in CREATE/ALTER PROCEDURE.
+ $dump =~ s/^(CREATE PROCEDURE .*?)\(IN /$1(/mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?)\(IN /$1(/mg;
+ $dump =~ s/^(CREATE PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(CREATE PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?), IN /$1, /mg;
+
+ # Remove SUBSCRIPT clauses in CREATE TYPE.
+ $dump =~ s/^\s+SUBSCRIPT = raw_array_subscript_handler,\n//mg;
+
+ # Remove multirange_type_name clauses in CREATE TYPE AS RANGE.
+ $dump =~ s {,\n\s+multirange_type_name = .*?(,?)$} {$1}mg;
+
+ # Remove mentions of extended hash functions.
+ my $line1 =
+ "ALTER OPERATOR FAMILY public[.]part_test_.*?_ops USING hash ADD";
+ my $line2 = "FUNCTION 2 .*? public\.part_hash.*?[(].*?[)];";
+ $dump =~ s {^$line1\n\s+$line2} {}mg;
+ }
+
+ # pre-v12 dumps will not say anything about default_table_access_method.
+ if ($old_version < 12)
+ {
+ $dump =~ s/^SET default_table_access_method = heap;\n//mg;
+ }
+
+ # dumps from pre-9.6 dblink may include redundant ACL settings
+ if ($old_version lt '9.6')
+ {
+ my $comment =
+ "-- Name: FUNCTION dblink_connect_u[(].*?[)]; Type: ACL; Schema: public; Owner: .*";
+ my $sql = "REVOKE ALL ON FUNCTION public[.]dblink_connect_u[(].*?[)] FROM PUBLIC;";
+ $dump =~
+ s/^--\n$comment\n--\n+$sql\n+--\n$comment\n--\n+$sql\n//mg;
+ }
+
+ if ($old_version lt '9.3')
+ {
+ # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
+ # To cope, reduce all whitespace sequences within them to one space.
+ # This must be done on both old and new dumps.
+ $dump = _mash_view_whitespace($dump);
+ }
+
+ # Suppress blank lines, as some places in pg_dump emit more or fewer.
+ $dump =~ s/\n\n+/\n/g;
+
+ return $dump;
+}
+
+=pod
+
+=back
+
+=cut
+
+1;
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-16 19:34 ` Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-16 19:34 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
Andrew Dunstan <[email protected]> writes:
> OK, here's my version. It tests clean against all of crake's dump files
> back to 9.2.
> To some extent it's a matter of taste, but I hate very long regex lines
> - it makes it very hard to see what's actually changing, so I broke up
> most of those.
I don't mind breaking things up, but I'm not terribly excited about
making the patterns looser, as you've done in some places like
if ($old_version < 14)
{
# Remove mentions of extended hash functions.
- $dump =~
- s/^(\s+OPERATOR 1 =\(integer,integer\)) ,\n\s+FUNCTION 2 \(integer, integer\) public\.part_hashint4_noop\(integer,bigint\);/$1;/mg;
- $dump =~
- s/^(\s+OPERATOR 1 =\(text,text\)) ,\n\s+FUNCTION 2 \(text, text\) public\.part_hashtext_length\(text,bigint\);/$1;/mg;
+ $dump =~ s {(^\s+OPERATOR\s1\s=\((?:integer,integer|text,text)\))\s,\n
+ \s+FUNCTION\s2\s.*?public.part_hash.*?;}
+ {$1;}mxg;
}
I don't think that's any easier to read, and it risks masking
diffs that we'd wish to know about.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-16 20:59 ` Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-16 20:59 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
On 2023-01-16 Mo 14:34, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> OK, here's my version. It tests clean against all of crake's dump files
>> back to 9.2.
>> To some extent it's a matter of taste, but I hate very long regex lines
>> - it makes it very hard to see what's actually changing, so I broke up
>> most of those.
> I don't mind breaking things up, but I'm not terribly excited about
> making the patterns looser, as you've done in some places like
>
> if ($old_version < 14)
> {
> # Remove mentions of extended hash functions.
> - $dump =~
> - s/^(\s+OPERATOR 1 =\(integer,integer\)) ,\n\s+FUNCTION 2 \(integer, integer\) public\.part_hashint4_noop\(integer,bigint\);/$1;/mg;
> - $dump =~
> - s/^(\s+OPERATOR 1 =\(text,text\)) ,\n\s+FUNCTION 2 \(text, text\) public\.part_hashtext_length\(text,bigint\);/$1;/mg;
> + $dump =~ s {(^\s+OPERATOR\s1\s=\((?:integer,integer|text,text)\))\s,\n
> + \s+FUNCTION\s2\s.*?public.part_hash.*?;}
> + {$1;}mxg;
> }
>
> I don't think that's any easier to read, and it risks masking
> diffs that we'd wish to know about.
OK, I'll make another pass and tighten things up.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-16 21:00 ` Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-16 21:00 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
Andrew Dunstan <[email protected]> writes:
> On 2023-01-16 Mo 14:34, Tom Lane wrote:
>> I don't think that's any easier to read, and it risks masking
>> diffs that we'd wish to know about.
> OK, I'll make another pass and tighten things up.
Don't sweat it, I'm just working the bugs out of a new version.
I'll have something to post shortly, I hope.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-16 21:48 ` Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 03:32 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Michael Paquier <[email protected]>
0 siblings, 2 replies; 27+ messages in thread
From: Tom Lane @ 2023-01-16 21:48 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
OK, here's a v4:
* It works with 002_pg_upgrade.pl now. The only substantive change
I had to make for that was to define the $old_version arguments as
being always PostgreSQL::Version objects not strings, because
otherwise I got complaints like
Argument "HEAD" isn't numeric in numeric comparison (<=>) at /home/postgres/pgsql/src/bin/pg_upgrade/../../../src/test/perl/PostgreSQL/Version.pm line 130.
So now TestUpgradeXversion.pm is responsible for performing that
conversion, and also for not doing any conversions on HEAD (which
Andrew wanted anyway).
* I improved pg_upgrade's TESTING directions after figuring out how
to get it to work for contrib modules.
* Incorporated (most of) Andrew's stylistic improvements.
* Simplified TestUpgradeXversion.pm's use of diff, as discussed.
I think we're about ready to go, except for cutting down
AdjustUpgrade.pm to make versions to put in the back branches.
I'm slightly tempted to back-patch 002_pg_upgrade.pl so that there
is an in-tree way to verify back-branch AdjustUpgrade.pm files.
On the other hand, it's hard to believe that testing that in
HEAD won't be sufficient; I doubt the back-branch copies will
need to change much.
regards, tom lane
Attachments:
[text/x-diff] adjustupgrade-4.patch (23.9K, ../../[email protected]/2-adjustupgrade-4.patch)
download | inline diff:
diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 98286231d7..81a4324a76 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -10,31 +10,14 @@ This will run the TAP tests to run pg_upgrade, performing an upgrade
from the version in this source tree to a new instance of the same
version.
-Testing an upgrade from a different version requires a dump to set up
-the contents of this instance, with its set of binaries. The following
-variables are available to control the test (see DETAILS below about
-the creation of the dump):
+Testing an upgrade from a different PG version is also possible, and
+provides a more thorough test that pg_upgrade does what it's meant for.
+This requires both a source tree and an installed tree for the old
+version, as well as a dump file to set up the instance to be upgraded.
+The following environment variables must be set to enable this testing:
export olddump=...somewhere/dump.sql (old version's dump)
export oldinstall=...otherversion/ (old version's install base path)
-
-"filter_rules" is a variable that can be used to specify a file with custom
-filtering rules applied before comparing the dumps of the PostgreSQL
-instances near the end of the tests, in the shape of regular expressions
-valid for perl. This is useful to enforce certain validation cases where
-pg_dump could create inconsistent outputs across major versions.
-For example:
-
- # Remove all CREATE POLICY statements
- s/^CREATE\sPOLICY.*//mgx
- # Replace REFRESH with DROP for materialized views
- s/^REFRESH\s(MATERIALIZED\sVIEW)/DROP $1/mgx
-
-Lines beginning with '#' and empty lines are ignored. One rule can be
-defined per line.
-
-Finally, the tests can be done by running
-
- make check
+See DETAILS below for more information about creation of the dump.
You can also test the different transfer modes (--copy, --link,
--clone) by setting the environment variable PG_TEST_PG_UPGRADE_MODE
@@ -52,22 +35,32 @@ The most effective way to test pg_upgrade, aside from testing on user
data, is by upgrading the PostgreSQL regression database.
This testing process first requires the creation of a valid regression
-database dump that can be then used for $olddump. Such files contain
+database dump that can then be used for $olddump. Such files contain
most database features and are specific to each major version of Postgres.
Here are the steps needed to create a dump file:
1) Create and populate the regression database in the old cluster.
This database can be created by running 'make installcheck' from
- src/test/regress using its source code tree.
+ src/test/regress in the old version's source code tree.
-2) Use pg_dumpall to dump out the contents of the instance, including the
- regression database, in the shape of a SQL file. This requires the *old*
- cluster's pg_dumpall so as the dump created is compatible with the
- version of the cluster it is dumped into.
+ If you like, you can also populate regression databases for one or
+ more contrib modules by running 'make installcheck USE_MODULE_DB=1'
+ in their directories. (USE_MODULE_DB is essential so that the
+ pg_upgrade test script will understand which database is which.)
-Once the dump is created, it can be repeatedly used with $olddump and
-`make check`, that automates the dump of the old database, its upgrade,
-the dump out of the new database and the comparison of the dumps between
-the old and new databases. The contents of the dumps can also be manually
-compared.
+2) Use pg_dumpall to dump out the contents of the instance, including the
+ regression database(s), into a SQL file. Use the *old* version's
+ pg_dumpall so that the dump created is compatible with that version.
+
+Once the dump file is created, it can be used repeatedly. Set $olddump
+to point to the dump file and run 'make check' or 'make installcheck'
+in the new version's src/bin/pg_upgrade directory. (If you included any
+contrib databases in the old dump, you must use 'make installcheck' and
+ensure that the corresponding contrib modules have been installed in
+the new version's installation tree.) This will build a temporary cluster
+using the old installation's executables, populate it from the dump file,
+and then try to pg_upgrade it to the new version. Success is reported
+if pg_dumpall output matches between the pre-upgrade and post-upgrade
+databases. In case of trouble, manually comparing those dump files may
+help to isolate the problem.
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index c066fd7d93..62a8fa9d8b 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -10,6 +10,7 @@ use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::AdjustUpgrade;
use Test::More;
# Can be changed to test the other modes.
@@ -37,37 +38,16 @@ sub generate_db
# This returns the path to the filtered dump.
sub filter_dump
{
- my ($node, $dump_file) = @_;
+ my ($is_old, $old_version, $dump_file) = @_;
my $dump_contents = slurp_file($dump_file);
- # Remove the comments.
- $dump_contents =~ s/^\-\-.*//mgx;
- # Remove empty lines.
- $dump_contents =~ s/^\n//mgx;
-
- # Apply custom filtering rules, if any.
- if (defined($ENV{filter_rules}))
+ if ($is_old)
{
- my $filter_file = $ENV{filter_rules};
- die "no file with custom filter rules found!" unless -e $filter_file;
-
- open my $filter_handle, '<', $filter_file
- or die "could not open $filter_file";
- while (<$filter_handle>)
- {
- my $filter_line = $_;
-
- # Skip comments and empty lines
- next if ($filter_line =~ /^#/);
- next if ($filter_line =~ /^\s*$/);
-
- # Apply lines with filters.
- note "Applying custom rule $filter_line to $dump_file";
- my $filter = "\$dump_contents =~ $filter_line";
- ## no critic (ProhibitStringyEval)
- eval $filter;
- }
- close $filter_handle;
+ $dump_contents = adjust_old_dumpfile($old_version, $dump_contents);
+ }
+ else
+ {
+ $dump_contents = adjust_new_dumpfile($old_version, $dump_contents);
}
my $dump_file_filtered = "${dump_file}_filtered";
@@ -83,7 +63,7 @@ sub filter_dump
# that gets upgraded. Before running the upgrade, a logical dump of the
# old cluster is taken, and a second logical dump of the new one is taken
# after the upgrade. The upgrade test passes if there are no differences
-# in these two dumps.
+# (after filtering) in these two dumps.
# Testing upgrades with an older version of PostgreSQL requires setting up
# two environment variables, as of:
@@ -198,15 +178,29 @@ my $oldbindir = $oldnode->config_data('--bindir');
# only if different major versions are used for the dump.
if (defined($ENV{oldinstall}))
{
- # Note that upgrade_adapt.sql and psql from the new version are used,
- # to cope with an upgrade to this version.
- $newnode->command_ok(
- [
- 'psql', '-X',
- '-f', "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
- '-d', $oldnode->connstr('regression'),
- ],
- 'ran adapt script');
+ # Consult AdjustUpgrade to find out what we need to do.
+ my $dbnames =
+ $oldnode->safe_psql('postgres', qq(SELECT datname FROM pg_database));
+ my %dbnames;
+ do { $dbnames{$_} = 1; }
+ foreach split /\s+/s, $dbnames;
+ my $adjust_cmds =
+ adjust_database_contents($oldnode->pg_version, %dbnames);
+
+ foreach my $updb (keys %$adjust_cmds)
+ {
+ my $upcmds = join(";\n", @{ $adjust_cmds->{$updb} });
+
+ # For simplicity, use the newer version's psql to issue the commands.
+ $newnode->command_ok(
+ [
+ 'psql', '-X',
+ '-v', 'ON_ERROR_STOP=1',
+ '-c', $upcmds,
+ '-d', $oldnode->connstr($updb),
+ ],
+ "ran version adaptation commands for database $updb");
+ }
}
# Take a dump before performing the upgrade as a base comparison. Note
@@ -359,8 +353,8 @@ my $dump1_filtered = $dump1_file;
my $dump2_filtered = $dump2_file;
if ($oldnode->pg_version != $newnode->pg_version)
{
- $dump1_filtered = filter_dump($oldnode, $dump1_file);
- $dump2_filtered = filter_dump($newnode, $dump2_file);
+ $dump1_filtered = filter_dump(1, $oldnode->pg_version, $dump1_file);
+ $dump2_filtered = filter_dump(0, $oldnode->pg_version, $dump2_file);
}
# Compare the two dumps, there should be no differences.
@@ -371,7 +365,7 @@ is($compare_res, 0, 'old and new dumps match after pg_upgrade');
if ($compare_res != 0)
{
my ($stdout, $stderr) =
- run_command([ 'diff', $dump1_filtered, $dump2_filtered ]);
+ run_command([ 'diff', '-u', $dump1_filtered, $dump2_filtered ]);
print "=== diff of $dump1_filtered and $dump2_filtered\n";
print "=== stdout ===\n";
print $stdout;
diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
new file mode 100644
index 0000000000..7b4a19be2a
--- /dev/null
+++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
@@ -0,0 +1,524 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+=pod
+
+=head1 NAME
+
+PostgreSQL::Test::AdjustUpgrade - helper module for cross-version upgrade tests
+
+=head1 SYNOPSIS
+
+ use PostgreSQL::Test::AdjustUpgrade;
+
+ # Build commands to adjust contents of old-version database before dumping
+ $statements = adjust_database_contents($old_version, %dbnames);
+
+ # Adjust contents of old pg_dumpall output file to match newer version
+ $dump = adjust_old_dumpfile($old_version, $dump);
+
+ # Adjust contents of new pg_dumpall output file to match older version
+ $dump = adjust_new_dumpfile($old_version, $dump);
+
+=head1 DESCRIPTION
+
+C<PostgreSQL::Test::AdjustUpgrade> encapsulates various hacks needed to
+compare the results of cross-version upgrade tests.
+
+=cut
+
+package PostgreSQL::Test::AdjustUpgrade;
+
+use strict;
+use warnings;
+
+use Exporter 'import';
+use PostgreSQL::Version;
+
+our @EXPORT = qw(
+ adjust_database_contents
+ adjust_old_dumpfile
+ adjust_new_dumpfile
+);
+
+=pod
+
+=head1 ROUTINES
+
+=over
+
+=item $statements = adjust_database_contents($old_version, %dbnames)
+
+Generate SQL commands to perform any changes to an old-version installation
+that are needed before we can pg_upgrade it into the current PostgreSQL
+version.
+
+Typically this involves dropping or adjusting no-longer-supported objects.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from, represented as a
+PostgreSQL::Version object.
+
+=item C<dbnames>: Hash of database names present in the old installation.
+
+=back
+
+Returns a reference to a hash, wherein the keys are database names and the
+values are arrayrefs to lists of statements to be run in those databases.
+
+=cut
+
+sub adjust_database_contents
+{
+ my ($old_version, %dbnames) = @_;
+ my $result = {};
+
+ # remove dbs of modules known to cause pg_upgrade to fail
+ # anything not builtin and incompatible should clean up its own db
+ foreach my $bad_module ('test_ddl_deparse', 'tsearch2')
+ {
+ if ($dbnames{"contrib_regression_$bad_module"})
+ {
+ _add_st($result, 'postgres',
+ "drop database contrib_regression_$bad_module");
+ delete($dbnames{"contrib_regression_$bad_module"});
+ }
+ }
+
+ # avoid version number issues with test_ext7
+ if ($dbnames{contrib_regression_test_extensions})
+ {
+ _add_st(
+ $result,
+ 'contrib_regression_test_extensions',
+ 'drop extension if exists test_ext7');
+ }
+
+ # stuff not supported from release 16
+ if ($old_version >= 12 && $old_version < 16)
+ {
+ # Can't upgrade aclitem in user tables from pre 16 to 16+.
+ _add_st($result, 'regression',
+ 'alter table public.tab_core_types drop column aclitem');
+ # Can't handle child tables with locally-generated columns.
+ _add_st(
+ $result, 'regression',
+ 'drop table public.gtest_normal_child',
+ 'drop table public.gtest_normal_child2');
+ }
+
+ # stuff not supported from release 14
+ if ($old_version < 14)
+ {
+ # postfix operators (some don't exist in very old versions)
+ _add_st(
+ $result,
+ 'regression',
+ 'drop operator #@# (bigint,NONE)',
+ 'drop operator #%# (bigint,NONE)',
+ 'drop operator if exists !=- (bigint,NONE)',
+ 'drop operator if exists #@%# (bigint,NONE)');
+
+ # get rid of dblink's dependencies on regress.so
+ my $regrdb =
+ $old_version le '9.4'
+ ? 'contrib_regression'
+ : 'contrib_regression_dblink';
+
+ if ($dbnames{$regrdb})
+ {
+ _add_st(
+ $result, $regrdb,
+ 'drop function if exists public.putenv(text)',
+ 'drop function if exists public.wait_pid(integer)');
+ }
+ }
+
+ # user table OIDs are gone from release 12 on
+ if ($old_version < 12)
+ {
+ my $nooid_stmt = q{
+ DO $stmt$
+ DECLARE
+ rec text;
+ BEGIN
+ FOR rec in
+ select oid::regclass::text
+ from pg_class
+ where relname !~ '^pg_'
+ and relhasoids
+ and relkind in ('r','m')
+ order by 1
+ LOOP
+ execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
+ RAISE NOTICE 'removing oids from table %', rec;
+ END LOOP;
+ END; $stmt$;
+ };
+
+ foreach my $oiddb ('regression', 'contrib_regression_btree_gist')
+ {
+ next unless $dbnames{$oiddb};
+ _add_st($result, $oiddb, $nooid_stmt);
+ }
+
+ # this table had OIDs too, but we'll just drop it
+ if ($old_version >= 10 && $dbnames{'contrib_regression_postgres_fdw'})
+ {
+ _add_st(
+ $result,
+ 'contrib_regression_postgres_fdw',
+ 'drop foreign table ft_pg_type');
+ }
+ }
+
+ # abstime+friends are gone from release 12 on; but these tables
+ # might or might not be present depending on regression test vintage
+ if ($old_version < 12)
+ {
+ _add_st($result, 'regression',
+ 'drop table if exists abstime_tbl, reltime_tbl, tinterval_tbl');
+ }
+
+ # some regression functions gone from release 11 on
+ if ($old_version < 11)
+ {
+ _add_st(
+ $result, 'regression',
+ 'drop function if exists public.boxarea(box)',
+ 'drop function if exists public.funny_dup17()');
+ }
+
+ # version-0 C functions are no longer supported
+ if ($old_version < 10)
+ {
+ _add_st($result, 'regression',
+ 'drop function oldstyle_length(integer, text)');
+ }
+
+ if ($old_version lt '9.5')
+ {
+ # cope with changes of underlying functions
+ _add_st(
+ $result,
+ 'regression',
+ 'drop operator @#@ (NONE, bigint)',
+ 'CREATE OPERATOR @#@ ('
+ . 'PROCEDURE = factorial, RIGHTARG = bigint )',
+ 'drop aggregate public.array_cat_accum(anyarray)',
+ 'CREATE AGGREGATE array_larger_accum (anyarray) ' . ' ( '
+ . ' sfunc = array_larger, '
+ . ' stype = anyarray, '
+ . ' initcond = $${}$$ ' . ' ) ');
+
+ # "=>" is no longer valid as an operator name
+ _add_st($result, 'regression',
+ 'drop operator if exists public.=> (bigint, NONE)');
+ }
+
+ return $result;
+}
+
+# Internal subroutine to add statement(s) to the list for the given db.
+sub _add_st
+{
+ my ($result, $db, @st) = @_;
+
+ $result->{$db} ||= [];
+ push(@{ $result->{$db} }, @st);
+}
+
+=pod
+
+=item adjust_old_dumpfile($old_version, $dump)
+
+Edit a dump output file, taken from the adjusted old-version installation
+by current-version C<pg_dumpall -s>, so that it will match the results of
+C<pg_dumpall -s> on the pg_upgrade'd installation.
+
+Typically this involves coping with cosmetic differences in the output
+of backend subroutines used by pg_dump.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from, represented as a
+PostgreSQL::Version object.
+
+=item C<dump>: Contents of dump file
+
+=back
+
+Returns the modified dump text.
+
+=cut
+
+sub adjust_old_dumpfile
+{
+ my ($old_version, $dump) = @_;
+
+ # use Unix newlines
+ $dump =~ s/\r\n/\n/g;
+
+ # Version comments will certainly not match.
+ $dump =~ s/^-- Dumped from database version.*\n//mg;
+
+ if ($old_version >= 14 && $old_version < 16)
+ {
+ # Fix up some privilege-set discrepancies.
+ $dump =~
+ s {^REVOKE SELECT,INSERT,REFERENCES,DELETE,TRIGGER,TRUNCATE,UPDATE ON TABLE}
+ {REVOKE ALL ON TABLE}mg;
+ $dump =~
+ s {^(GRANT SELECT,INSERT,REFERENCES,TRIGGER,TRUNCATE),UPDATE ON TABLE}
+ {$1,MAINTAIN,UPDATE ON TABLE}mg;
+ }
+
+ if ($old_version < 14)
+ {
+ # Remove mentions of extended hash functions.
+ $dump =~ s {^(\s+OPERATOR\s1\s=\(integer,integer\))\s,\n
+ \s+FUNCTION\s2\s\(integer,\sinteger\)\spublic\.part_hashint4_noop\(integer,bigint\);}
+ {$1;}mxg;
+ $dump =~ s {^(\s+OPERATOR\s1\s=\(text,text\))\s,\n
+ \s+FUNCTION\s2\s\(text,\stext\)\spublic\.part_hashtext_length\(text,bigint\);}
+ {$1;}mxg;
+ }
+
+ # Change trigger definitions to say ... EXECUTE FUNCTION ...
+ if ($old_version < 12)
+ {
+ # would like to use lookbehind here but perl complains
+ # so do it this way
+ $dump =~ s/
+ (^CREATE\sTRIGGER\s.*?)
+ \sEXECUTE\sPROCEDURE
+ /$1 EXECUTE FUNCTION/mgx;
+ }
+
+ if ($old_version lt '9.6')
+ {
+ # adjust some places where we don't print so many parens anymore
+
+ my $prefix =
+ "'New York'\tnew & york | big & apple | nyc\t'new' & 'york'\t";
+ my $orig = "( 'new' & 'york' | 'big' & 'appl' ) | 'nyc'";
+ my $repl = "'new' & 'york' | 'big' & 'appl' | 'nyc'";
+ $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
+
+ $prefix =
+ "'Sanct Peter'\tPeterburg | peter | 'Sanct Peterburg'\t'sanct' & 'peter'\t";
+ $orig = "( 'peterburg' | 'peter' ) | 'sanct' & 'peterburg'";
+ $repl = "'peterburg' | 'peter' | 'sanct' & 'peterburg'";
+ $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
+ }
+
+ if ($old_version lt '9.5')
+ {
+ # adjust some places where we don't print so many parens anymore
+
+ my $prefix = "CONSTRAINT (?:sequence|copy)_con CHECK [(][(]";
+ my $orig = "((x > 3) AND (y <> 'check failed'::text))";
+ my $repl = "(x > 3) AND (y <> 'check failed'::text)";
+ $dump =~ s/($prefix)\Q$orig\E/$1$repl/mg;
+
+ $prefix = "CONSTRAINT insert_con CHECK [(][(]";
+ $orig = "((x >= 3) AND (y <> 'check failed'::text))";
+ $repl = "(x >= 3) AND (y <> 'check failed'::text)";
+ $dump =~ s/($prefix)\Q$orig\E/$1$repl/mg;
+
+ $orig = "DEFAULT ((-1) * currval('public.insert_seq'::regclass))";
+ $repl =
+ "DEFAULT ('-1'::integer * currval('public.insert_seq'::regclass))";
+ $dump =~ s/\Q$orig\E/$repl/mg;
+
+ my $expr =
+ "(rsl.sl_color = rsh.slcolor) AND (rsl.sl_len_cm >= rsh.slminlen_cm)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+
+ $expr =
+ "(rule_and_refint_t3.id3a = new.id3a) AND (rule_and_refint_t3.id3b = new.id3b)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+
+ $expr =
+ "(rule_and_refint_t3_1.id3a = new.id3a) AND (rule_and_refint_t3_1.id3b = new.id3b)";
+ $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
+ }
+
+ if ($old_version lt '9.3')
+ {
+ # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
+ # To cope, reduce all whitespace sequences within them to one space.
+ # This must be done on both old and new dumps.
+ $dump = _mash_view_whitespace($dump);
+
+ # _mash_view_whitespace doesn't handle multi-command rules;
+ # rather than trying to fix that, just hack the exceptions manually.
+
+ my $prefix =
+ "CREATE RULE rtest_sys_del AS ON DELETE TO public.rtest_system DO (DELETE FROM public.rtest_interface WHERE (rtest_interface.sysname = old.sysname);";
+ my $line2 = " DELETE FROM public.rtest_admin";
+ my $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
+ $dump =~
+ s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
+
+ $prefix =
+ "CREATE RULE rtest_sys_upd AS ON UPDATE TO public.rtest_system DO (UPDATE public.rtest_interface SET sysname = new.sysname WHERE (rtest_interface.sysname = old.sysname);";
+ $line2 = " UPDATE public.rtest_admin SET sysname = new.sysname";
+ $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
+ $dump =~
+ s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
+
+ # and there's one place where pre-9.3 uses a different table alias
+ $dump =~ s {^(CREATE\sRULE\srule_and_refint_t3_ins\sAS\s
+ ON\sINSERT\sTO\spublic\.rule_and_refint_t3\s
+ WHERE\s\(EXISTS\s\(SELECT\s1\sFROM\spublic\.rule_and_refint_t3)\s
+ (WHERE\s\(\(rule_and_refint_t3)
+ (\.id3a\s=\snew\.id3a\)\sAND\s\(rule_and_refint_t3)
+ (\.id3b\s=\snew\.id3b\)\sAND\s\(rule_and_refint_t3)}
+ {$1 rule_and_refint_t3_1 $2_1$3_1$4_1}mx;
+
+ # Also fix old use of NATURAL JOIN syntax
+ $dump =~ s {NATURAL JOIN public\.credit_card r}
+ {JOIN public.credit_card r USING (cid)}mg;
+ $dump =~ s {NATURAL JOIN public\.credit_usage r}
+ {JOIN public.credit_usage r USING (cid)}mg;
+ }
+
+ # Suppress blank lines, as some places in pg_dump emit more or fewer.
+ $dump =~ s/\n\n+/\n/g;
+
+ return $dump;
+}
+
+# Internal subroutine to mangle whitespace within view/rule commands.
+# Any consecutive sequence of whitespace is reduced to one space.
+sub _mash_view_whitespace
+{
+ my ($dump) = @_;
+
+ foreach my $leader ('CREATE VIEW', 'CREATE RULE')
+ {
+ my @splitchunks = split $leader, $dump;
+
+ $dump = shift(@splitchunks);
+ foreach my $chunk (@splitchunks)
+ {
+ my @thischunks = split /;/, $chunk, 2;
+ my $stmt = shift(@thischunks);
+
+ # now $stmt is just the body of the CREATE VIEW/RULE
+ $stmt =~ s/\s+/ /sg;
+ # we also need to smash these forms for sub-selects and rules
+ $stmt =~ s/\( SELECT/(SELECT/g;
+ $stmt =~ s/\( INSERT/(INSERT/g;
+ $stmt =~ s/\( UPDATE/(UPDATE/g;
+ $stmt =~ s/\( DELETE/(DELETE/g;
+
+ $dump .= $leader . $stmt . ';' . $thischunks[0];
+ }
+ }
+ return $dump;
+}
+
+=pod
+
+=item adjust_new_dumpfile($old_version, $dump)
+
+Edit a dump output file, taken from the pg_upgrade'd installation
+by current-version C<pg_dumpall -s>, so that it will match the old
+dump output file as adjusted by C<adjust_old_dumpfile>.
+
+Typically this involves deleting data not present in the old installation.
+
+Arguments:
+
+=over
+
+=item C<old_version>: Branch we are upgrading from, represented as a
+PostgreSQL::Version object.
+
+=item C<dump>: Contents of dump file
+
+=back
+
+Returns the modified dump text.
+
+=cut
+
+sub adjust_new_dumpfile
+{
+ my ($old_version, $dump) = @_;
+
+ # use Unix newlines
+ $dump =~ s/\r\n/\n/g;
+
+ # Version comments will certainly not match.
+ $dump =~ s/^-- Dumped from database version.*\n//mg;
+
+ if ($old_version < 14)
+ {
+ # Suppress noise-word uses of IN in CREATE/ALTER PROCEDURE.
+ $dump =~ s/^(CREATE PROCEDURE .*?)\(IN /$1(/mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?)\(IN /$1(/mg;
+ $dump =~ s/^(CREATE PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(CREATE PROCEDURE .*?), IN /$1, /mg;
+ $dump =~ s/^(ALTER PROCEDURE .*?), IN /$1, /mg;
+
+ # Remove SUBSCRIPT clauses in CREATE TYPE.
+ $dump =~ s/^\s+SUBSCRIPT = raw_array_subscript_handler,\n//mg;
+
+ # Remove multirange_type_name clauses in CREATE TYPE AS RANGE.
+ $dump =~ s {,\n\s+multirange_type_name = .*?(,?)$} {$1}mg;
+
+ # Remove mentions of extended hash functions.
+ $dump =~
+ s {^ALTER\sOPERATOR\sFAMILY\spublic\.part_test_int4_ops\sUSING\shash\sADD\n
+ \s+FUNCTION\s2\s\(integer,\sinteger\)\spublic\.part_hashint4_noop\(integer,bigint\);} {}mxg;
+ $dump =~
+ s {^ALTER\sOPERATOR\sFAMILY\spublic\.part_test_text_ops\sUSING\shash\sADD\n
+ \s+FUNCTION\s2\s\(text,\stext\)\spublic\.part_hashtext_length\(text,bigint\);} {}mxg;
+ }
+
+ # pre-v12 dumps will not say anything about default_table_access_method.
+ if ($old_version < 12)
+ {
+ $dump =~ s/^SET default_table_access_method = heap;\n//mg;
+ }
+
+ # dumps from pre-9.6 dblink may include redundant ACL settings
+ if ($old_version lt '9.6')
+ {
+ my $comment =
+ "-- Name: FUNCTION dblink_connect_u\(.*?\); Type: ACL; Schema: public; Owner: .*";
+ my $sql =
+ "REVOKE ALL ON FUNCTION public\.dblink_connect_u\(.*?\) FROM PUBLIC;";
+ $dump =~ s/^--\n$comment\n--\n+$sql\n+//mg;
+ }
+
+ if ($old_version lt '9.3')
+ {
+ # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
+ # To cope, reduce all whitespace sequences within them to one space.
+ # This must be done on both old and new dumps.
+ $dump = _mash_view_whitespace($dump);
+ }
+
+ # Suppress blank lines, as some places in pg_dump emit more or fewer.
+ $dump =~ s/\n\n+/\n/g;
+
+ return $dump;
+}
+
+=pod
+
+=back
+
+=cut
+
+1;
[text/x-diff] xversion-4.patch (11.3K, ../../[email protected]/3-xversion-4.patch)
download | inline diff:
diff -pudr client-code-REL_16.orig/PGBuild/Modules/TestUpgradeXversion.pm client-code-REL_16/PGBuild/Modules/TestUpgradeXversion.pm
--- client-code-REL_16.orig/PGBuild/Modules/TestUpgradeXversion.pm 2023-01-13 12:20:51.000000000 -0500
+++ client-code-REL_16/PGBuild/Modules/TestUpgradeXversion.pm 2023-01-16 15:01:22.502366802 -0500
@@ -323,31 +323,6 @@ sub save_for_testing
return if $?;
}
- if ($this_branch ne 'HEAD' && $this_branch le 'REL9_4_STABLE')
- {
- my $opsql = 'drop operator if exists public.=> (bigint, NONE)';
-
- # syntax is illegal in 9.5 and later, and it shouldn't
- # be possible for it to exist there anyway.
- # quoting the operator can also fail, so it's left unquoted.
- run_psql("$installdir/bin/psql", "-e", $opsql, "regression",
- "$upgrade_loc/fix.log", 1);
- return if $?;
- }
-
- # remove dbs of modules known to cause pg_upgrade to fail
- # anything not builtin and incompatible should clean up its own db
- # e.g. jsonb_set_lax
-
- foreach my $bad_module ("test_ddl_deparse")
- {
- my $dsql = "drop database if exists contrib_regression_$bad_module";
-
- run_psql("$installdir/bin/psql", "-e", $dsql,
- "postgres", "$upgrade_loc/fix.log", 1);
- return if $?;
- }
-
# use a different logfile here to get around windows sharing issue
system( qq{"$installdir/bin/pg_ctl" -D "$installdir/data-C" -w stop }
. qq{>> "$upgrade_loc/ctl2.log" 2>&1});
@@ -375,6 +350,21 @@ sub test_upgrade ## no critic (Subrou
print time_str(), "checking upgrade from $oversion to $this_branch ...\n"
if $verbose;
+ # load helper module from source tree
+ unshift(@INC, "$self->{pgsql}/src/test/perl");
+ require PostgreSQL::Test::AdjustUpgrade;
+ PostgreSQL::Test::AdjustUpgrade->import;
+ shift(@INC);
+
+ # if $oversion isn't HEAD, convert it into a PostgreSQL::Version object
+ my $old_version = $oversion;
+ if ($old_version ne 'HEAD')
+ {
+ $old_version =~ s/REL_?(\d+(?:_\d+)?)_STABLE/$1/;
+ $old_version =~ s/_/./;
+ $old_version = PostgreSQL::Version->new($old_version);
+ }
+
rmtree "$other_branch/inst/$upgrade_test";
copydir(
"$other_branch/inst/data-C",
@@ -414,6 +404,7 @@ sub test_upgrade ## no critic (Subrou
return if $?;
+ # collect names of databases present in old installation.
my $sql = 'select datname from pg_database';
run_psql("psql", "-A -t", $sql, "postgres",
@@ -425,186 +416,22 @@ sub test_upgrade ## no critic (Subrou
do { s/\r$//; $dbnames{$_} = 1; }
foreach @dbnames;
- if ($this_branch gt 'REL9_6_STABLE' || $this_branch eq 'HEAD')
- {
- run_psql(
- "$other_branch/inst/bin/psql", "-e",
- "drop database if exists contrib_regression_tsearch2", "postgres",
- "$upgrade_loc/$oversion-copy.log", 1
- );
- return if $?;
-
- run_psql(
- "$other_branch/inst/bin/psql",
- "-e",
- "drop function if exists oldstyle_length(integer, text)",
- "regression",
- "$upgrade_loc/$oversion-copy.log",
- 1
- );
- return if $?;
- }
-
- # some regression functions gone from release 11 on
- if ( ($this_branch ge 'REL_11_STABLE' || $this_branch eq 'HEAD')
- && ($oversion lt 'REL_11_STABLE' && $oversion ne 'HEAD'))
- {
- my $missing_funcs = q{drop function if exists public.boxarea(box);
- drop function if exists public.funny_dup17();
- };
- $missing_funcs =~ s/\n//g;
-
- run_psql("$other_branch/inst/bin/psql", "-e", $missing_funcs,
- "regression", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
- }
-
- # avoid version number issues with test_ext7
- if ($dbnames{contrib_regression_test_extensions})
- {
- my $noext7 = "drop extension if exists test_ext7";
- run_psql(
- "$other_branch/inst/bin/psql", "-e", $noext7,
- "contrib_regression_test_extensions",
- "$upgrade_loc/$oversion-copy.log", 1
- );
- return if $?;
- }
-
- # user table OIDS and abstime+friends are gone from release 12 on
- if ( ($this_branch gt 'REL_11_STABLE' || $this_branch eq 'HEAD')
- && ($oversion le 'REL_11_STABLE' && $oversion ne 'HEAD'))
- {
- my $nooid_stmt = q{
- DO $stmt$
- DECLARE
- rec text;
- BEGIN
- FOR rec in
- select oid::regclass::text
- from pg_class
- where relname !~ '^pg_'
- and relhasoids
- and relkind in ('r','m')
- order by 1
- LOOP
- execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
- RAISE NOTICE 'removing oids from table %', rec;
- END LOOP;
- END; $stmt$;
- };
- foreach my $oiddb ("regression", "contrib_regression_btree_gist")
- {
- next unless $dbnames{$oiddb};
- run_psql("$other_branch/inst/bin/psql", "-e", $nooid_stmt,
- "$oiddb", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
- }
-
- if ( $oversion ge 'REL_10_STABLE'
- && $dbnames{'contrib_regression_postgres_fdw'})
- {
- run_psql(
- "$other_branch/inst/bin/psql",
- "-e",
- "drop foreign table if exists ft_pg_type",
- "contrib_regression_postgres_fdw",
- "$upgrade_loc/$oversion-copy.log",
- 1
- );
- return if $?;
- }
-
- if ($oversion lt 'REL9_3_STABLE')
- {
- run_psql(
- "$other_branch/inst/bin/psql",
- "-e",
- "drop table if exists abstime_tbl, reltime_tbl, tinterval_tbl",
- "regression",
- "$upgrade_loc/$oversion-copy.log",
- 1
- );
- return if $?;
- }
- }
-
- # stuff not supported from release 14
- if ( ($this_branch gt 'REL_13_STABLE' || $this_branch eq 'HEAD')
- && ($oversion le 'REL_13_STABLE' && $oversion ne 'HEAD'))
+ if ($oversion ne $this_branch)
{
- my $prstmt = join(';',
- 'drop operator if exists #@# (bigint,NONE)',
- 'drop operator if exists #%# (bigint,NONE)',
- 'drop operator if exists !=- (bigint,NONE)',
- 'drop operator if exists #@%# (bigint,NONE)');
-
- run_psql("$other_branch/inst/bin/psql", "-e", $prstmt,
- "regression", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
-
- $prstmt = "drop function if exists public.putenv(text)";
-
- my $regrdb =
- $oversion le "REL9_4_STABLE"
- ? "contrib_regression"
- : "contrib_regression_dblink";
-
- if ($dbnames{$regrdb})
- {
- run_psql("$other_branch/inst/bin/psql", "-e", $prstmt,
- "$regrdb", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
- }
+ # obtain and execute commands needed to make old database upgradable.
+ my $adjust_cmds = adjust_database_contents($old_version, %dbnames);
- if ($oversion le 'REL9_4_STABLE')
+ foreach my $updb (keys %$adjust_cmds)
{
- # this is fixed in 9.5 and later
- $prstmt = join(';',
- 'drop operator @#@ (NONE, bigint)',
- 'CREATE OPERATOR @#@ ('
- . 'PROCEDURE = factorial, '
- . 'RIGHTARG = bigint )');
- run_psql("$other_branch/inst/bin/psql", "-e", $prstmt,
- "regression", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
- }
+ my $upcmds = join(";\n", @{ $adjust_cmds->{$updb} });
- if ($oversion le 'REL9_4_STABLE')
- {
- # this is fixed in 9.5 and later
- $prstmt = join(';',
- 'drop aggregate if exists public.array_cat_accum(anyarray)',
- 'CREATE AGGREGATE array_larger_accum (anyarray) ' . ' ( '
- . ' sfunc = array_larger, '
- . ' stype = anyarray, '
- . ' initcond = $${}$$ '
- . ' ) ');
- run_psql("$other_branch/inst/bin/psql", "-e", $prstmt,
- "regression", "$upgrade_loc/$oversion-copy.log", 1);
+ run_psql("$other_branch/inst/bin/psql", "-e -v ON_ERROR_STOP=1",
+ $upcmds, $updb, "$upgrade_loc/$oversion-fix.log", 1);
return if $?;
}
}
- # stuff not supported from release 16
- if ( ($this_branch gt 'REL_15_STABLE' || $this_branch eq 'HEAD')
- && ($oversion le 'REL_15_STABLE' && $oversion ne 'HEAD'))
- {
- # Can't upgrade aclitem in user tables from pre 16 to 16+.
- # Also can't handle child tables with newly-generated columns.
- my $prstmt = join(
- ';',
- 'alter table if exists public.tab_core_types
- drop column if exists aclitem',
- 'drop table if exists public.gtest_normal_child',
- 'drop table if exists public.gtest_normal_child2'
- );
-
- run_psql("$other_branch/inst/bin/psql", "-e", $prstmt,
- "regression", "$upgrade_loc/$oversion-copy.log", 1);
- return if $?;
- }
-
+ # perform a dump from the old database for comparison purposes.
my $extra_digits = "";
if ( $oversion ne 'HEAD'
@@ -793,65 +620,40 @@ sub test_upgrade ## no critic (Subrou
return if $?;
}
- foreach my $dump ("$upgrade_loc/origin-$oversion.sql",
- "$upgrade_loc/converted-$oversion-to-$this_branch.sql")
- {
- # Change trigger definitions to say ... EXECUTE FUNCTION ...
+ # Slurp the pg_dump output files, and filter them if not same version.
+ my $olddumpfile = "$upgrade_loc/origin-$oversion.sql";
+ my $olddump = file_contents($olddumpfile);
- my $contents = file_contents($dump);
+ $olddump = adjust_old_dumpfile($old_version, $olddump)
+ if ($oversion ne $this_branch);
- # would like to use lookbehind here but perl complains
- # so do it this way
- $contents =~ s/
- (^CREATE\sTRIGGER\s.*?)
- \sEXECUTE\sPROCEDURE
- /$1 EXECUTE FUNCTION/mgx;
- open(my $dh, '>', "$dump.fixed") || die "opening $dump.fixed";
- print $dh $contents;
- close($dh);
- }
+ my $newdumpfile = "$upgrade_loc/converted-$oversion-to-$this_branch.sql";
+ my $newdump = file_contents($newdumpfile);
- system( qq{diff -I "^\$" -I "SET default_table_access_method = heap;" }
- . qq{ -I "^SET default_toast_compression = 'pglz';\$" -I "^-- " }
- . qq{-u "$upgrade_loc/origin-$oversion.sql.fixed" }
- . qq{"$upgrade_loc/converted-$oversion-to-$this_branch.sql.fixed" }
- . qq{> "$upgrade_loc/dumpdiff-$oversion" 2>&1});
+ $newdump = adjust_new_dumpfile($old_version, $newdump)
+ if ($oversion ne $this_branch);
- # diff exits with status 1 if files differ
- return if $? >> 8 > 1;
+ # Always write out the filtered files, to aid in diagnosing filter bugs.
+ open(my $odh, '>', "$olddumpfile.fixed")
+ || die "opening $olddumpfile.fixed: $!";
+ print $odh $olddump;
+ close($odh);
+ open(my $ndh, '>', "$newdumpfile.fixed")
+ || die "opening $newdumpfile.fixed: $!";
+ print $ndh $newdump;
+ close($ndh);
- open(my $diffile, '<', "$upgrade_loc/dumpdiff-$oversion")
- || die "opening $upgrade_loc/dumpdiff-$oversion: $!";
- my $difflines = 0;
- while (<$diffile>)
+ # Are the results the same?
+ if ($olddump ne $newdump)
{
- $difflines++ if /^[+-]/;
- }
- close($diffile);
-
- # If the versions match we require that there be no diff lines.
- # In the past we have seen a handful of diffs from reordering of
- # large object output, but that appears to have disppeared.
- # If the versions don't match we heuristically allow more lines of diffs
- # based on observed differences. For versions from 9.6 on, that's
- # not very many lines, though.
+ # Trouble, so run diff to show the problem.
+ system( qq{diff -u "$olddumpfile.fixed" "$newdumpfile.fixed" }
+ . qq{> "$upgrade_loc/dumpdiff-$oversion" 2>&1});
- if (
- ($oversion eq $this_branch && $difflines == 0)
- || ( $oversion ne $this_branch
- && $oversion ge 'REL9_6_STABLE'
- && $difflines < 90)
- || ( $oversion ne $this_branch
- && $oversion lt 'REL9_6_STABLE'
- && $difflines < 700)
- )
- {
- return 1;
- }
- else
- {
return;
}
+
+ return 1;
}
sub installcheck
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-16 23:11 ` Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-16 23:11 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
I wrote:
> I think we're about ready to go, except for cutting down
> AdjustUpgrade.pm to make versions to put in the back branches.
Hmmm ... so upon trying to test in the back branches, I soon
discovered that PostgreSQL/Version.pm isn't there before v15.
I don't see a good reason why we couldn't back-patch it, though.
Any objections?
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-17 00:46 ` Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-17 00:46 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
On 2023-01-16 Mo 18:11, Tom Lane wrote:
> I wrote:
>> I think we're about ready to go, except for cutting down
>> AdjustUpgrade.pm to make versions to put in the back branches.
> Hmmm ... so upon trying to test in the back branches, I soon
> discovered that PostgreSQL/Version.pm isn't there before v15.
>
> I don't see a good reason why we couldn't back-patch it, though.
> Any objections?
>
>
No, that seems perfectly reasonable.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-17 02:58 ` Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-17 02:58 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
I've pushed the per-branch AdjustUpgrade.pm files and tested by performing
a fresh round of buildfarm runs with the patched TestUpgradeXversion.pm
file. I think we're in good shape with this project.
I dunno if we want to stretch buildfarm owners' patience with yet
another BF client release right now. On the other hand, I'm antsy
to see if we can un-revert 1b4d280ea after doing a little more
work in AdjustUpgrade.pm.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-17 13:35 ` Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 21:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 2 replies; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-17 13:35 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
On 2023-01-16 Mo 21:58, Tom Lane wrote:
> I've pushed the per-branch AdjustUpgrade.pm files and tested by performing
> a fresh round of buildfarm runs with the patched TestUpgradeXversion.pm
> file. I think we're in good shape with this project.
>
> I dunno if we want to stretch buildfarm owners' patience with yet
> another BF client release right now. On the other hand, I'm antsy
> to see if we can un-revert 1b4d280ea after doing a little more
> work in AdjustUpgrade.pm.
>
>
It looks like the only animals doing the cross version tests crake,
drongo and fairywren. These are all mine, so I don't think we need to do
a new release for this.
I think the next step is to push the buildfarm client changes, and
update those three animals to use it, and make sure nothing breaks. I'll
go and do those things now. Then you should be able to try your unrevert.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-17 15:18 ` Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-17 15:18 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
Andrew Dunstan <[email protected]> writes:
> On 2023-01-16 Mo 21:58, Tom Lane wrote:
>> I dunno if we want to stretch buildfarm owners' patience with yet
>> another BF client release right now. On the other hand, I'm antsy
>> to see if we can un-revert 1b4d280ea after doing a little more
>> work in AdjustUpgrade.pm.
> It looks like the only animals doing the cross version tests crake,
> drongo and fairywren. These are all mine, so I don't think we need to do
> a new release for this.
copperhead, kittiwake, snapper, and tadarida were running them
until fairly recently.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-17 16:04 ` Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-17 16:04 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
On 2023-01-17 Tu 10:18, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> On 2023-01-16 Mo 21:58, Tom Lane wrote:
>>> I dunno if we want to stretch buildfarm owners' patience with yet
>>> another BF client release right now. On the other hand, I'm antsy
>>> to see if we can un-revert 1b4d280ea after doing a little more
>>> work in AdjustUpgrade.pm.
>> It looks like the only animals doing the cross version tests crake,
>> drongo and fairywren. These are all mine, so I don't think we need to do
>> a new release for this.
> copperhead, kittiwake, snapper, and tadarida were running them
> until fairly recently.
>
>
Ah, yes, true, I didn't look far enough back.
The new file can be downloaded from
<https://raw.githubusercontent.com/PGBuildFarm/client-code/75efff0fbd70ca89b097593824911ab6ccbd258f/P...;
- it's a dropin replacement.
FYI crake has just passed the test with flying colours.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-17 16:30 ` Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tom Lane @ 2023-01-17 16:30 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
Andrew Dunstan <[email protected]> writes:
> FYI crake has just passed the test with flying colours.
Cool. I await the Windows machines' results with interest.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-18 12:36 ` Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-18 12:36 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
On 2023-01-17 Tu 11:30, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> FYI crake has just passed the test with flying colours.
> Cool. I await the Windows machines' results with interest.
fairwren and drongo are clean except for fairywren upgrading 9.6 to 11.
This appears to be a longstanding issue that the fuzz processing was
causing us to ignore. See for example
<https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=fairywren&dt=2022-09-01%2018%3A27%...;
It's somewhat interesting that this doesn't appear to be an issue with
the MSVC builds on drongo. And it disappears when upgrading to release
12 or later where we use the extra-float-digits=0 hack.
I propose to add this to just the release 11 AdjustUpgrade.pm:
# float4 values in this table on Msys can have precision differences
# in representation between old and new versions
if ($old_version < 10 && $dbnames{contrib_regression_btree_gist} &&
$^O eq 'msys')
{
_add_st($result, 'contrib_regression_btree_gist',
'drop table if exists float4tmp');
}
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-18 15:33 ` Tom Lane <[email protected]>
2023-01-18 19:32 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-19 21:49 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
0 siblings, 2 replies; 27+ messages in thread
From: Tom Lane @ 2023-01-18 15:33 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
Andrew Dunstan <[email protected]> writes:
> fairwren and drongo are clean except for fairywren upgrading 9.6 to 11.
> This appears to be a longstanding issue that the fuzz processing was
> causing us to ignore. See for example
> <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=fairywren&dt=2022-09-01%2018%3A27%...;
Interesting. I suspected that removing the fuzz allowance would teach
us some things we hadn't known about.
> I propose to add this to just the release 11 AdjustUpgrade.pm:
> # float4 values in this table on Msys can have precision differences
> # in representation between old and new versions
> if ($old_version < 10 && $dbnames{contrib_regression_btree_gist} &&
> $^O eq 'msys')
> {
> _add_st($result, 'contrib_regression_btree_gist',
> 'drop table if exists float4tmp');
> }
Seems reasonable (but I wonder if you don't need "$old_version < 11").
A nicer answer would be to apply --extra-float-digits=0 across the
board, but pre-v12 pg_dump lacks that switch.
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-18 19:32 ` Tom Lane <[email protected]>
2023-01-18 21:05 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-18 21:14 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
1 sibling, 2 replies; 27+ messages in thread
From: Tom Lane @ 2023-01-18 19:32 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
One more thing before we move on from this topic. I'd been testing
modified versions of the AdjustUpgrade.pm logic by building from a
--from-source source tree, which seemed way easier than dealing
with a private git repo. As it stands, TestUpgradeXversion.pm
refuses to run under $from_source, but I just diked that check out
and it seemed to work fine for my purposes. Now, that's going to be
a regular need going forward, so I'd like to not need a hacked version
of the BF client code to do it.
Also, your committed version of TestUpgradeXversion.pm breaks that
use-case because you did
- unshift(@INC, "$self->{pgsql}/src/test/perl");
+ unshift(@INC, "$self->{buildroot}/$this_branch/pgsql/src/test/perl");
which AFAICS is an empty directory in a $from_source run.
I suppose that the reason for not running under $from_source is to
avoid corrupting the saved installations with unofficial versions.
However, couldn't we skip the "save" step and still run the upgrade
tests against whatever we have saved? (Maybe skip the same-version
test, as it's not quite reflecting any real case then.)
Here's a quick draft patch showing what I have in mind. There may
well be a better way to deal with the wheres-the-source issue than
what is in hunk 2. Also, I didn't reindent the unchanged code in
sub installcheck, and I didn't add anything about skipping
same-version tests.
regards, tom lane
Attachments:
[text/x-diff] support-from-source-in-xversion-testing.patch (2.0K, ../../[email protected]/2-support-from-source-in-xversion-testing.patch)
download | inline diff:
diff --git a/PGBuild/Modules/TestUpgradeXversion.pm b/PGBuild/Modules/TestUpgradeXversion.pm
index a784686..408432d 100644
--- a/PGBuild/Modules/TestUpgradeXversion.pm
+++ b/PGBuild/Modules/TestUpgradeXversion.pm
@@ -51,8 +51,6 @@ sub setup
my $conf = shift; # ref to the whole config object
my $pgsql = shift; # postgres build dir
- return if $from_source;
-
return if $branch !~ /^(?:HEAD|REL_?\d+(?:_\d+)?_STABLE)$/;
my $animal = $conf->{animal};
@@ -351,7 +349,16 @@ sub test_upgrade ## no critic (Subroutines::ProhibitManyArgs)
if $verbose;
# load helper module from source tree
- unshift(@INC, "$self->{buildroot}/$this_branch/pgsql/src/test/perl");
+ my $source_tree;
+ if ($from_source)
+ {
+ $source_tree = $self->{pgsql};
+ }
+ else
+ {
+ $source_tree = "$self->{buildroot}/$this_branch/pgsql";
+ }
+ unshift(@INC, "$source_tree/src/test/perl");
require PostgreSQL::Test::AdjustUpgrade;
PostgreSQL::Test::AdjustUpgrade->import;
shift(@INC);
@@ -694,6 +701,11 @@ sub installcheck
my $upgrade_loc = "$upgrade_install_root/$this_branch";
my $installdir = "$upgrade_loc/inst";
+ # Don't save in a $from_source build: we want the saved trees always
+ # to correspond to branch tips of the animal's standard repo. We can
+ # perform upgrade tests against previously-saved trees, though.
+ if (!$from_source)
+ {
# for saving we need an exclusive lock.
get_lock($self, $this_branch, 1);
@@ -716,6 +728,7 @@ sub installcheck
if ($verbose > 1);
send_result('XversionUpgradeSave', $status, \@saveout) if $status;
$steps_completed .= " XVersionUpgradeSave";
+ }
# in saveonly mode our work is done
return if $ENV{PG_UPGRADE_SAVE_ONLY};
@@ -744,7 +757,7 @@ sub installcheck
# other branch from being removed or changed under us.
get_lock($self, $oversion, 0);
- $status =
+ my $status =
test_upgrade($self, $save_env, $this_branch, $upgrade_install_root,
$dport, $install_loc, $other_branch) ? 0 : 1;
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 19:32 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-18 21:05 ` Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-18 21:05 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
On 2023-01-18 We 14:32, Tom Lane wrote:
> One more thing before we move on from this topic. I'd been testing
> modified versions of the AdjustUpgrade.pm logic by building from a
> --from-source source tree, which seemed way easier than dealing
> with a private git repo. As it stands, TestUpgradeXversion.pm
> refuses to run under $from_source, but I just diked that check out
> and it seemed to work fine for my purposes. Now, that's going to be
> a regular need going forward, so I'd like to not need a hacked version
> of the BF client code to do it.
>
> Also, your committed version of TestUpgradeXversion.pm breaks that
> use-case because you did
>
> - unshift(@INC, "$self->{pgsql}/src/test/perl");
> + unshift(@INC, "$self->{buildroot}/$this_branch/pgsql/src/test/perl");
>
> which AFAICS is an empty directory in a $from_source run.
>
> I suppose that the reason for not running under $from_source is to
> avoid corrupting the saved installations with unofficial versions.
> However, couldn't we skip the "save" step and still run the upgrade
> tests against whatever we have saved? (Maybe skip the same-version
> test, as it's not quite reflecting any real case then.)
>
> Here's a quick draft patch showing what I have in mind. There may
> well be a better way to deal with the wheres-the-source issue than
> what is in hunk 2. Also, I didn't reindent the unchanged code in
> sub installcheck, and I didn't add anything about skipping
> same-version tests.
No that won't work if we're using vpath builds (which was why I changed
it from what you had). $self->{pgsql} is always the build directory.
Something like this should do it:
my $source_tree = $from_source || "$self->{buildroot}/$this_branch/pgsql";
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 19:32 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-18 21:14 ` Tom Lane <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Tom Lane @ 2023-01-18 21:14 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
Andrew Dunstan <[email protected]> writes:
> On 2023-01-18 We 14:32, Tom Lane wrote:
>> I suppose that the reason for not running under $from_source is to
>> avoid corrupting the saved installations with unofficial versions.
>> However, couldn't we skip the "save" step and still run the upgrade
>> tests against whatever we have saved? (Maybe skip the same-version
>> test, as it's not quite reflecting any real case then.)
> Something like this should do it:
> my $source_tree = $from_source || "$self->{buildroot}/$this_branch/pgsql";
Ah, I didn't understand that $from_source is a path not just a bool.
What do you think about the above questions? Is this $from_source
exclusion for the reason I guessed, or some other one?
regards, tom lane
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 16:04 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-18 12:36 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-19 21:49 ` Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Andrew Dunstan @ 2023-01-19 21:49 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]; [email protected]
On 2023-01-18 We 10:33, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> fairwren and drongo are clean except for fairywren upgrading 9.6 to 11.
>> This appears to be a longstanding issue that the fuzz processing was
>> causing us to ignore. See for example
>> <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=fairywren&dt=2022-09-01%2018%3A27%...;
> Interesting. I suspected that removing the fuzz allowance would teach
> us some things we hadn't known about.
>
>> I propose to add this to just the release 11 AdjustUpgrade.pm:
>> # float4 values in this table on Msys can have precision differences
>> # in representation between old and new versions
>> if ($old_version < 10 && $dbnames{contrib_regression_btree_gist} &&
>> $^O eq 'msys')
>> {
>> _add_st($result, 'contrib_regression_btree_gist',
>> 'drop table if exists float4tmp');
>> }
> Seems reasonable (but I wonder if you don't need "$old_version < 11").
> A nicer answer would be to apply --extra-float-digits=0 across the
> board, but pre-v12 pg_dump lacks that switch.
>
>
It turns out this was due to the fact that fairywren's setup changed
some time after the EOL of 9.6. I have rebuilt 9.6 and earlier
backbranches and there should now be no need for this adjustment.
There is still a Windows issue with MSVC builds <= 9.4 that I'm trying
to track down.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 23:11 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 00:46 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-17 13:35 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
@ 2023-01-17 21:12 ` Tom Lane <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Tom Lane @ 2023-01-17 21:12 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]
Andrew Dunstan <[email protected]> writes:
> On 2023-01-16 Mo 21:58, Tom Lane wrote:
>> I dunno if we want to stretch buildfarm owners' patience with yet
>> another BF client release right now. On the other hand, I'm antsy
>> to see if we can un-revert 1b4d280ea after doing a little more
>> work in AdjustUpgrade.pm.
> I think the next step is to push the buildfarm client changes, and
> update those three animals to use it, and make sure nothing breaks. I'll
> go and do those things now. Then you should be able to try your unrevert.
It looks like unrevert will require ~130 lines in AdjustUpgrade.pm,
which is not great but not awful either. I think this is ready to
go once you've vetted your remaining buildfarm animals.
regards, tom lane
Attachments:
[text/x-diff] fix-AdjustUpgrade-for-1b4d280ea.patch (5.9K, ../../[email protected]/2-fix-AdjustUpgrade-for-1b4d280ea.patch)
download | inline diff:
diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
index 7cf4ced392..5bed1d6839 100644
--- a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
+++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
@@ -268,6 +268,12 @@ sub adjust_old_dumpfile
# Version comments will certainly not match.
$dump =~ s/^-- Dumped from database version.*\n//mg;
+ if ($old_version < 16)
+ {
+ # Fix up some view queries that no longer require table-qualification.
+ $dump = _mash_view_qualifiers($dump);
+ }
+
if ($old_version >= 14 && $old_version < 16)
{
# Fix up some privilege-set discrepancies.
@@ -396,6 +402,133 @@ sub adjust_old_dumpfile
return $dump;
}
+
+# Data for _mash_view_qualifiers
+my @_unused_view_qualifiers = (
+ # Present at least since 9.2
+ { obj => 'VIEW public.trigger_test_view', qual => 'trigger_test' },
+ { obj => 'VIEW public.domview', qual => 'domtab' },
+ { obj => 'VIEW public.my_property_normal', qual => 'customer' },
+ { obj => 'VIEW public.my_property_secure', qual => 'customer' },
+ { obj => 'VIEW public.pfield_v1', qual => 'pf' },
+ { obj => 'VIEW public.rtest_v1', qual => 'rtest_t1' },
+ { obj => 'VIEW public.rtest_vview1', qual => 'x' },
+ { obj => 'VIEW public.rtest_vview2', qual => 'rtest_view1' },
+ { obj => 'VIEW public.rtest_vview3', qual => 'x' },
+ { obj => 'VIEW public.rtest_vview5', qual => 'rtest_view1' },
+ { obj => 'VIEW public.shoelace_obsolete', qual => 'shoelace' },
+ { obj => 'VIEW public.shoelace_candelete', qual => 'shoelace_obsolete' },
+ { obj => 'VIEW public.toyemp', qual => 'emp' },
+ { obj => 'VIEW public.xmlview4', qual => 'emp' },
+ # Since 9.3 (some of these were removed in 9.6)
+ { obj => 'VIEW public.tv', qual => 't' },
+ { obj => 'MATERIALIZED VIEW mvschema.tvm', qual => 'tv' },
+ { obj => 'VIEW public.tvv', qual => 'tv' },
+ { obj => 'MATERIALIZED VIEW public.tvvm', qual => 'tvv' },
+ { obj => 'VIEW public.tvvmv', qual => 'tvvm' },
+ { obj => 'MATERIALIZED VIEW public.bb', qual => 'tvvmv' },
+ { obj => 'VIEW public.nums', qual => 'nums' },
+ { obj => 'VIEW public.sums_1_100', qual => 't' },
+ { obj => 'MATERIALIZED VIEW public.tm', qual => 't' },
+ { obj => 'MATERIALIZED VIEW public.tmm', qual => 'tm' },
+ { obj => 'MATERIALIZED VIEW public.tvmm', qual => 'tvm' },
+ # Since 9.4
+ {
+ obj => 'MATERIALIZED VIEW public.citext_matview',
+ qual => 'citext_table'
+ },
+ {
+ obj => 'OR REPLACE VIEW public.key_dependent_view',
+ qual => 'view_base_table'
+ },
+ {
+ obj => 'OR REPLACE VIEW public.key_dependent_view_no_cols',
+ qual => 'view_base_table'
+ },
+ # Since 9.5
+ {
+ obj => 'VIEW public.dummy_seclabel_view1',
+ qual => 'dummy_seclabel_tbl2'
+ },
+ { obj => 'VIEW public.vv', qual => 'test_tablesample' },
+ { obj => 'VIEW public.test_tablesample_v1', qual => 'test_tablesample' },
+ { obj => 'VIEW public.test_tablesample_v2', qual => 'test_tablesample' },
+ # Since 9.6
+ {
+ obj => 'MATERIALIZED VIEW public.test_pg_dump_mv1',
+ qual => 'test_pg_dump_t1'
+ },
+ { obj => 'VIEW public.test_pg_dump_v1', qual => 'test_pg_dump_t1' },
+ { obj => 'VIEW public.mvtest_tv', qual => 'mvtest_t' },
+ {
+ obj => 'MATERIALIZED VIEW mvtest_mvschema.mvtest_tvm',
+ qual => 'mvtest_tv'
+ },
+ { obj => 'VIEW public.mvtest_tvv', qual => 'mvtest_tv' },
+ { obj => 'MATERIALIZED VIEW public.mvtest_tvvm', qual => 'mvtest_tvv' },
+ { obj => 'VIEW public.mvtest_tvvmv', qual => 'mvtest_tvvm' },
+ { obj => 'MATERIALIZED VIEW public.mvtest_bb', qual => 'mvtest_tvvmv' },
+ { obj => 'MATERIALIZED VIEW public.mvtest_tm', qual => 'mvtest_t' },
+ { obj => 'MATERIALIZED VIEW public.mvtest_tmm', qual => 'mvtest_tm' },
+ { obj => 'MATERIALIZED VIEW public.mvtest_tvmm', qual => 'mvtest_tvm' },
+ # Since 10 (some removed in 12)
+ { obj => 'VIEW public.itestv10', qual => 'itest10' },
+ { obj => 'VIEW public.itestv11', qual => 'itest11' },
+ { obj => 'VIEW public.xmltableview2', qual => '"xmltable"' },
+ # Since 12
+ {
+ obj => 'MATERIALIZED VIEW public.tableam_tblmv_heap2',
+ qual => 'tableam_tbl_heap2'
+ },
+ # Since 13
+ { obj => 'VIEW public.limit_thousand_v_1', qual => 'onek' },
+ { obj => 'VIEW public.limit_thousand_v_2', qual => 'onek' },
+ { obj => 'VIEW public.limit_thousand_v_3', qual => 'onek' },
+ { obj => 'VIEW public.limit_thousand_v_4', qual => 'onek' });
+
+# Internal subroutine to remove no-longer-used table qualifiers from
+# CREATE [MATERIALIZED] VIEW commands. See list of targeted views above.
+sub _mash_view_qualifiers
+{
+ my ($dump) = @_;
+
+ for my $uvq (@_unused_view_qualifiers)
+ {
+ my $leader = "CREATE $uvq->{obj} ";
+ my $qualifier = $uvq->{qual};
+ # Note: we loop because there are presently some cases where the same
+ # view name appears in multiple databases. Fortunately, the same
+ # qualifier removal applies or is harmless for each instance ... but
+ # we might want to rename some things to avoid assuming that.
+ my @splitchunks = split $leader, $dump;
+ $dump = shift(@splitchunks);
+ foreach my $chunk (@splitchunks)
+ {
+ my @thischunks = split /;/, $chunk, 2;
+ my $stmt = shift(@thischunks);
+ my $ostmt = $stmt;
+
+ # now $stmt is just the body of the CREATE [MATERIALIZED] VIEW
+ $stmt =~ s/$qualifier\.//g;
+
+ $dump .= $leader . $stmt . ';' . $thischunks[0];
+ }
+ }
+
+ # Further hack a few cases where not all occurrences of the qualifier
+ # should be removed.
+ $dump =~ s {^(CREATE VIEW public\.rtest_vview1 .*?)(a\)\)\);)}
+ {$1x.$2}ms;
+ $dump =~ s {^(CREATE VIEW public\.rtest_vview3 .*?)(a\)\)\);)}
+ {$1x.$2}ms;
+ $dump =~
+ s {^(CREATE VIEW public\.shoelace_obsolete .*?)(sl_color\)\)\)\);)}
+ {$1shoelace.$2}ms;
+
+ return $dump;
+}
+
+
# Internal subroutine to mangle whitespace within view/rule commands.
# Any consecutive sequence of whitespace is reduced to one space.
sub _mash_view_whitespace
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: Extracting cross-version-upgrade knowledge from buildfarm client
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 19:08 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 20:59 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-16 21:48 ` Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
@ 2023-01-17 03:32 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 27+ messages in thread
From: Michael Paquier @ 2023-01-17 03:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; [email protected]
On Mon, Jan 16, 2023 at 04:48:28PM -0500, Tom Lane wrote:
> I'm slightly tempted to back-patch 002_pg_upgrade.pl so that there
> is an in-tree way to verify back-branch AdjustUpgrade.pm files.
> On the other hand, it's hard to believe that testing that in
> HEAD won't be sufficient; I doubt the back-branch copies will
> need to change much.
Backpatching 002_pg_upgrade.pl requires a bit more than the test:
there is one compatibility gotcha as of dc57366. I did not backpatch
it because nobody has complained about it until I found out about it,
but the test would require it.
By the way, thanks for your work on this stuff :)
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca4301bb8a9..5776ae84f4d 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -569,10 +672,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -709,7 +816,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -722,7 +829,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -759,13 +866,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -775,7 +889,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -786,7 +901,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 368c570a0f4..8d0dd40ba6d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7d7e1d2744c..52513fcdc90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -565,10 +668,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -705,7 +812,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -718,7 +825,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -755,13 +862,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -771,7 +885,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -782,7 +897,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f1122453738..29daab7aeb8 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca4301bb8a9..5776ae84f4d 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -569,10 +672,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -709,7 +816,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -722,7 +829,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -759,13 +866,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -775,7 +889,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -786,7 +901,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 368c570a0f4..8d0dd40ba6d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7d7e1d2744c..52513fcdc90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -565,10 +668,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -705,7 +812,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -718,7 +825,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -755,13 +862,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -771,7 +885,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -782,7 +897,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f1122453738..29daab7aeb8 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
end of thread, other threads:[~2024-03-25 22:31 UTC | newest]
Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-15 16:01 Re: Extracting cross-version-upgrade knowledge from buildfarm client Tom Lane <[email protected]>
2023-01-15 23:12 ` Andrew Dunstan <[email protected]>
2023-01-15 23:37 ` Tom Lane <[email protected]>
2023-01-16 19:08 ` Andrew Dunstan <[email protected]>
2023-01-16 19:34 ` Tom Lane <[email protected]>
2023-01-16 20:59 ` Andrew Dunstan <[email protected]>
2023-01-16 21:00 ` Tom Lane <[email protected]>
2023-01-16 21:48 ` Tom Lane <[email protected]>
2023-01-16 23:11 ` Tom Lane <[email protected]>
2023-01-17 00:46 ` Andrew Dunstan <[email protected]>
2023-01-17 02:58 ` Tom Lane <[email protected]>
2023-01-17 13:35 ` Andrew Dunstan <[email protected]>
2023-01-17 15:18 ` Tom Lane <[email protected]>
2023-01-17 16:04 ` Andrew Dunstan <[email protected]>
2023-01-17 16:30 ` Tom Lane <[email protected]>
2023-01-18 12:36 ` Andrew Dunstan <[email protected]>
2023-01-18 15:33 ` Tom Lane <[email protected]>
2023-01-18 19:32 ` Tom Lane <[email protected]>
2023-01-18 21:05 ` Andrew Dunstan <[email protected]>
2023-01-18 21:14 ` Tom Lane <[email protected]>
2023-01-19 21:49 ` Andrew Dunstan <[email protected]>
2023-01-17 21:12 ` Tom Lane <[email protected]>
2023-01-17 03:32 ` Michael Paquier <[email protected]>
2024-03-25 22:31 [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox