Horton

Re: [HACKERS] Custom compression methods (buildfarm xupgrade)

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, David Steele <david@pgmasters.net>, Ildus Kurbangaliev <i.kurbangaliev@gmail.com>, Dmitry Dolgov <9erthalion6@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-28T23:14:33Z
Lists: pgsql-hackers 
On Sun, Mar 28, 2021 at 04:48:29PM -0400, Andrew Dunstan wrote:
> Nothing is hidden here - the diffs are reported, see for example
> <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2021-03-28%2015%3A37%3A07&stg=xversion-upgrade-REL9_4_STABLE-HEAD>
> What we're comparing here is target pg_dumpall against the original
> source vs target pg_dumpall against the upgraded source.

The command being run is:

https://github.com/PGBuildFarm/client-code/blob/master/PGBuild/Modules/TestUpgradeXversion.pm#L610
        system( "diff -I '^-- ' -u $upgrade_loc/origin-$oversion.sql "
                  . "$upgrade_loc/converted-$oversion-to-$this_branch.sql "
                  . "> $upgrade_loc/dumpdiff-$oversion 2>&1");
...
	my $difflines = `wc -l < $upgrade_loc/dumpdiff-$oversion`;

where -I means: --ignore-matching-lines=RE

I think wc -l should actually be grep -c '^[-+]'
otherwise context lines count for as much as diff lines.
You could write that with diff -U0 |wc -l, except the context is useful to
humans.

With some more effort, the number of lines of diff can be very small, allowing
a smaller fudge factor.  

For upgrade from v10:
time make -C src/bin/pg_upgrade check oldsrc=`pwd`/10 oldbindir=`pwd`/10/tmp_install/usr/local/pgsql/bin

$ diff -u src/bin/pg_upgrade/tmp_check/dump1.sql src/bin/pg_upgrade/tmp_check/dump2.sql |wc -l
622

Without context:
$ diff -u src/bin/pg_upgrade/tmp_check/dump1.sql src/bin/pg_upgrade/tmp_check/dump2.sql |grep -c '^[-+]'
142

Without comments:
$ diff -I '^-- ' -u src/bin/pg_upgrade/tmp_check/dump1.sql src/bin/pg_upgrade/tmp_check/dump2.sql |grep -c '^[-+]'
130

Without SET default stuff:
diff -I '^$' -I "SET default_table_access_method = heap;" -I "^SET default_toast_compression = 'pglz';$" -I '^-- ' -u /home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump1.sql /home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump2.sql |less |grep -c '^[-+]'
117

Without trigger function call noise:
diff -I "^CREATE TRIGGER [_[:alnum:]]\+ .* FOR EACH \(ROW\|STATEMENT\) EXECUTE \(PROCEDURE\|FUNCTION\)" -I '^$' -I "SET default_table_access_method = heap;" -I "^SET default_toast_compression = 'pglz';$" -I '^-- ' -u /home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump1.sql /home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump2.sql |grep -c '^[-+]'
11

Maybe it's important not to totally ignore that, and instead perhaps clean up
the known/accepted changes like s/FUNCTION/PROCEDURE/:

</home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump2.sql sed '/^CREATE TRIGGER/s/FUNCTION/PROCEDURE/' |diff -I '^$' -I "SET default_table_access_method = heap;" -I "^SET default_toast_compression = 'pglz';$" -I '^-- ' -u /home/pryzbyj/src/postgres/src/bin/pg_upgrade/tmp_check/dump1.sql - |grep -c '^[-+]'
11

It seems weird that we don't quote "heap" but we quote tablespaces and not
toast compression methods.

-- 
Justin

Commits

  1. docs: Update TOAST storage docs for configurable compression.

  2. Further tweaking of pg_dump's handling of default_toast_compression.

  3. Fix interaction of TOAST compression with expression indexes.

  4. Tidy up more loose ends related to configurable TOAST compression.

  5. Short-circuit slice requests that are for more than the object's size.

  6. Mostly-cosmetic adjustments of TOAST-related macros.

  7. Remove useless configure probe for <lz4/lz4.h>.

  8. Error on invalid TOAST compression in CREATE or ALTER TABLE.

  9. docs: Fix omissions related to configurable TOAST compression.

  10. More code cleanup for configurable TOAST compression.

  11. Bring configure support for LZ4 up to snuff.

  12. Make compression.sql regression test independent of default.

  13. Use valid compression method in brin_form_tuple

  14. Fix up pg_dump's handling of per-attribute compression options.

  15. Allow configurable LZ4 TOAST compression.

  16. Fix inconsistencies in the code

  17. Mostly-cosmetic improvements in memory chunk header alignment coding.

  18. Allow numeric to use a more compact, 2-byte header in many cases.