Thread: pg_upgrade test for binary compatibility of core data types
I'm finally returning to this 14 month old thread: (was: Re: BUG #16045: vacuum_db crash and illegal memory alloc after pg_upgrade from PG11 to PG12) On Tue, Oct 15, 2019 at 09:07:25AM +0200, Tomas Vondra wrote: > On Mon, Oct 14, 2019 at 11:41:18PM -0500, Justin Pryzby wrote: > > > > Perhaps it'd be worth creating a test for on-disk format ? > > > > Like a table with a column for each core type, which is either SELECTed from > > after pg_upgrade, or pg_dump output compared before and after. > > IMO that would be useful - we now have a couple of these checks for > different data types (line, unknown, sql_identifier), with a couple of > combinations each. And I've been looking if we do similar pg_upgrade > tests, but I haven't found anything. I mean, we do pg_upgrade the > cluster used for regression tests, but here we need to test a number of > cases that are meant to abort the pg_upgrade. So we'd need a number of > pg_upgrade runs, to test that. I meant to notice if the binary format is accidentally changed again, which was what happened here: 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. I added a table to the regression tests so it's processed by pg_upgrade tests, run like: | time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin I checked that if I cherry-pick 0002 to v11, and comment out old_11_check_for_sql_identifier_data_type_usage(), then pg_upgrade/test.sh detects the original problem: pg_dump: error: Error message from server: ERROR: invalid memory alloc request size 18446744073709551613 I understand the buildfarm has its own cross-version-upgrade test, which I think would catch this on its own. These all seem to complicate use of pg_upgrade/test.sh, so 0001 is needed to allow testing upgrade from older releases. e78900afd217fa3eaa77c51e23a94c1466af421c Create by default sql/ and expected/ for output directory in pg_regress 40b132c1afbb4b1494aa8e48cc35ec98d2b90777 In the pg_upgrade test suite, don't write to src/test/regress. fc49e24fa69a15efacd5b8958115ed9c43c48f9a Make WAL segment size configurable at initdb time. c37b3d08ca6873f9d4eaf24c72a90a550970cbb8 Allow group access on PGDATA da9b580d89903fee871cf54845ffa2b26bda2e11 Refactor dir/file permissions -- Justin
Attachment
On Sun, Dec 06, 2020 at 12:02:48PM -0600, Justin Pryzby wrote: > I meant to notice if the binary format is accidentally changed again, which was > what happened here: > 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. > > I added a table to the regression tests so it's processed by pg_upgrade tests, > run like: > > | time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin Per cfbot, this avoids testing ::xml (support for which may not be enabled) And also now tests oid types. I think the per-version hacks should be grouped by logical change, rather than by version. Which I've started doing here. -- Justin
Attachment
On Wed, Dec 16, 2020 at 11:22:23AM -0600, Justin Pryzby wrote: > On Sun, Dec 06, 2020 at 12:02:48PM -0600, Justin Pryzby wrote: > > I meant to notice if the binary format is accidentally changed again, which was > > what happened here: > > 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. > > > > I added a table to the regression tests so it's processed by pg_upgrade tests, > > run like: > > > > | time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin > > Per cfbot, this avoids testing ::xml (support for which may not be enabled) > And also now tests oid types. > > I think the per-version hacks should be grouped by logical change, rather than > by version. Which I've started doing here. rebased on 6df7a9698bb036610c1e8c6d375e1be38cb26d5f -- Justin
Attachment
On 2020-12-27 20:07, Justin Pryzby wrote: > On Wed, Dec 16, 2020 at 11:22:23AM -0600, Justin Pryzby wrote: >> On Sun, Dec 06, 2020 at 12:02:48PM -0600, Justin Pryzby wrote: >>> I meant to notice if the binary format is accidentally changed again, which was >>> what happened here: >>> 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. >>> >>> I added a table to the regression tests so it's processed by pg_upgrade tests, >>> run like: >>> >>> | time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin >> >> Per cfbot, this avoids testing ::xml (support for which may not be enabled) >> And also now tests oid types. >> >> I think the per-version hacks should be grouped by logical change, rather than >> by version. Which I've started doing here. > > rebased on 6df7a9698bb036610c1e8c6d375e1be38cb26d5f I think these patches could use some in-place documentation of what they are trying to achieve and how they do it. The required information is spread over a lengthy thread. No one wants to read that. Add commit messages to the patches.
On Mon, Jan 11, 2021 at 03:28:08PM +0100, Peter Eisentraut wrote: > On 2020-12-27 20:07, Justin Pryzby wrote: > > rebased on 6df7a9698bb036610c1e8c6d375e1be38cb26d5f > > I think these patches could use some in-place documentation of what they are > trying to achieve and how they do it. The required information is spread > over a lengthy thread. No one wants to read that. Add commit messages to > the patches. Oh, I see that now, and agree that you need to explain each item with a comment. pg_upgrade is doing some odd things, so documenting everything it does is a big win. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On Mon, Jan 11, 2021 at 03:28:08PM +0100, Peter Eisentraut wrote: > On 2020-12-27 20:07, Justin Pryzby wrote: > > On Wed, Dec 16, 2020 at 11:22:23AM -0600, Justin Pryzby wrote: > > > On Sun, Dec 06, 2020 at 12:02:48PM -0600, Justin Pryzby wrote: > > > > I meant to notice if the binary format is accidentally changed again, which was > > > > what happened here: > > > > 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. > > > > > > > > I added a table to the regression tests so it's processed by pg_upgrade tests, > > > > run like: > > > > > > > > | time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin > > > > > > Per cfbot, this avoids testing ::xml (support for which may not be enabled) > > > And also now tests oid types. > > > > > > I think the per-version hacks should be grouped by logical change, rather than > > > by version. Which I've started doing here. > > > > rebased on 6df7a9698bb036610c1e8c6d375e1be38cb26d5f > > I think these patches could use some in-place documentation of what they are > trying to achieve and how they do it. The required information is spread > over a lengthy thread. No one wants to read that. Add commit messages to > the patches. 0001 patch fixes pg_upgrade/test.sh, which was disfunctional. Portions of the first patch were independently handled by commits 52202bb39, fa744697c, 091866724. So this is rebased on those. I guess updating this script should be a part of a beta-checklist somewhere, since I guess nobody will want to backpatch changes for testing older releases. 0002 allows detecting the information_schema problem that was introduced at: 7c15cef86 Base information_schema.sql_identifier domain on name, not varchar. +-- Create a table with different data types, to exercise binary compatibility +-- during pg_upgrade test If binary compatibility is changed I expect this will error, crash, at least return wrong data, and thereby fail tests. -- Justin On Sun, Dec 06, 2020 at 12:02:48PM -0600, Justin Pryzby wrote: > I checked that if I cherry-pick 0002 to v11, and comment out > old_11_check_for_sql_identifier_data_type_usage(), then pg_upgrade/test.sh > detects the original problem: > pg_dump: error: Error message from server: ERROR: invalid memory alloc request size 18446744073709551613 > > I understand the buildfarm has its own cross-version-upgrade test, which I > think would catch this on its own. > > These all seem to complicate use of pg_upgrade/test.sh, so 0001 is needed to > allow testing upgrade from older releases. > > e78900afd217fa3eaa77c51e23a94c1466af421c Create by default sql/ and expected/ for output directory in pg_regress > 40b132c1afbb4b1494aa8e48cc35ec98d2b90777 In the pg_upgrade test suite, don't write to src/test/regress. > fc49e24fa69a15efacd5b8958115ed9c43c48f9a Make WAL segment size configurable at initdb time. > c37b3d08ca6873f9d4eaf24c72a90a550970cbb8 Allow group access on PGDATA > da9b580d89903fee871cf54845ffa2b26bda2e11 Refactor dir/file permissions
Attachment
On Mon, Jan 11, 2021 at 10:13:52PM -0600, Justin Pryzby wrote: > On Mon, Jan 11, 2021 at 03:28:08PM +0100, Peter Eisentraut wrote: > > I think these patches could use some in-place documentation of what they are > > trying to achieve and how they do it. The required information is spread > > over a lengthy thread. No one wants to read that. Add commit messages to > > the patches. > > 0001 patch fixes pg_upgrade/test.sh, which was disfunctional. > Portions of the first patch were independently handled by commits 52202bb39, > fa744697c, 091866724. So this is rebased on those. > I guess updating this script should be a part of a beta-checklist somewhere, > since I guess nobody will want to backpatch changes for testing older releases. Uh, what exactly is missing from the beta checklist? I read the patch and commit message but don't understand it. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On Tue, Jan 12, 2021 at 12:15:59PM -0500, Bruce Momjian wrote: > On Mon, Jan 11, 2021 at 10:13:52PM -0600, Justin Pryzby wrote: > > On Mon, Jan 11, 2021 at 03:28:08PM +0100, Peter Eisentraut wrote: > > > I think these patches could use some in-place documentation of what they are > > > trying to achieve and how they do it. The required information is spread > > > over a lengthy thread. No one wants to read that. Add commit messages to > > > the patches. > > > > 0001 patch fixes pg_upgrade/test.sh, which was disfunctional. > > Portions of the first patch were independently handled by commits 52202bb39, > > fa744697c, 091866724. So this is rebased on those. > > I guess updating this script should be a part of a beta-checklist somewhere, > > since I guess nobody will want to backpatch changes for testing older releases. > > Uh, what exactly is missing from the beta checklist? I read the patch > and commit message but don't understand it. Did you try to use test.sh to upgrade from a prior release ? Evidently it's frequently forgotten, as evidenced by all the "deferred maintenance" I had to do to allow testing the main patch (currently 0003). See also: commit 5bab1985dfc25eecf4b098145789955c0b246160 Author: Tom Lane <tgl@sss.pgh.pa.us> Date: Thu Jun 8 13:48:27 2017 -0400 Fix bit-rot in pg_upgrade's test.sh, and improve documentation. Doing a cross-version upgrade test with test.sh evidently hasn't been tested since circa 9.2, because the script lacked case branches for old-version servers newer than 9.1. Future-proof that a bit, and clean up breakage induced by our recent drop of V0 function call protocol (namely that oldstyle_length() isn't in the regression suite anymore). -- Justin
On Tue, Jan 12, 2021 at 11:27:53AM -0600, Justin Pryzby wrote: > On Tue, Jan 12, 2021 at 12:15:59PM -0500, Bruce Momjian wrote: > > Uh, what exactly is missing from the beta checklist? I read the patch > > and commit message but don't understand it. > > Did you try to use test.sh to upgrade from a prior release ? > > Evidently it's frequently forgotten, as evidenced by all the "deferred > maintenance" I had to do to allow testing the main patch (currently 0003). > > See also: > > commit 5bab1985dfc25eecf4b098145789955c0b246160 > Author: Tom Lane <tgl@sss.pgh.pa.us> > Date: Thu Jun 8 13:48:27 2017 -0400 > > Fix bit-rot in pg_upgrade's test.sh, and improve documentation. > > Doing a cross-version upgrade test with test.sh evidently hasn't been > tested since circa 9.2, because the script lacked case branches for > old-version servers newer than 9.1. Future-proof that a bit, and > clean up breakage induced by our recent drop of V0 function call > protocol (namely that oldstyle_length() isn't in the regression > suite anymore). Oh, that is odd. I thought that was regularly run. I have my own test infrastructure that I run for every major release so I never have run the built-in one, except for make check-world. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On 1/12/21 12:53 PM, Bruce Momjian wrote: > On Tue, Jan 12, 2021 at 11:27:53AM -0600, Justin Pryzby wrote: >> On Tue, Jan 12, 2021 at 12:15:59PM -0500, Bruce Momjian wrote: >>> Uh, what exactly is missing from the beta checklist? I read the patch >>> and commit message but don't understand it. >> Did you try to use test.sh to upgrade from a prior release ? >> >> Evidently it's frequently forgotten, as evidenced by all the "deferred >> maintenance" I had to do to allow testing the main patch (currently 0003). >> >> See also: >> >> commit 5bab1985dfc25eecf4b098145789955c0b246160 >> Author: Tom Lane <tgl@sss.pgh.pa.us> >> Date: Thu Jun 8 13:48:27 2017 -0400 >> >> Fix bit-rot in pg_upgrade's test.sh, and improve documentation. >> >> Doing a cross-version upgrade test with test.sh evidently hasn't been >> tested since circa 9.2, because the script lacked case branches for >> old-version servers newer than 9.1. Future-proof that a bit, and >> clean up breakage induced by our recent drop of V0 function call >> protocol (namely that oldstyle_length() isn't in the regression >> suite anymore). > Oh, that is odd. I thought that was regularly run. I have my own test > infrastructure that I run for every major release so I never have run > the built-in one, except for make check-world. > Cross version pg_upgrade is tested regularly in the buildfarm, but not using test.sh. Instead it uses the saved data repository from a previous run of the buildfarm client for the source branch, and tries to upgrade that to the target branch. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 2021-01-12 22:44, Andrew Dunstan wrote: > Cross version pg_upgrade is tested regularly in the buildfarm, but not > using test.sh. Instead it uses the saved data repository from a previous > run of the buildfarm client for the source branch, and tries to upgrade > that to the target branch. Does it maintain a set of fixups similar to what is in test.sh? Are those two sets the same?
On Fri, 2021-04-30 at 13:33 -0500, Justin Pryzby wrote: > On Sat, Mar 06, 2021 at 03:01:43PM -0500, Tom Lane wrote: > > v4-0001 mostly teaches test.sh about specific changes that have to be > > made to historic versions of the regression database to allow them > > to be reloaded into current servers. As already discussed, this is > > really duplicative of knowledge that's been embedded into the buildfarm > > client over time. It'd be better if we could refactor that so that > > the buildfarm shares a common database of these actions with test.sh. > > And said database ought to be in our git tree, so committers could > > fix problems without having to get Andrew involved every time. > > I think this could be represented as a psql script, at least in > > versions that have psql \if (but that came in in v10, so maybe > > we're there already). > > I started this. I don't know if it's compatible with the buildfarm client, but > I think any issues maybe can be avoided by using "IF EXISTS". I'm going to try pulling this into a psql script today and see how far I get. > > But I'm not sure I believe > > that query. It's got hard-wired assumptions about which typtype values > > need to be covered. Why is it okay to exclude range and multirange? > > Are we sure that all composites are okay to exclude? Likewise, the > > restriction to pg_catalog and information_schema schemas seems likely to > > bite us someday. There are some very random exclusions based on name > > patterns, which seem unsafe (let's list the specific type OIDs), and > > again the nearby comments don't match the code. But the biggest issue > > is that this can only cover core datatypes, not any contrib stuff. > > I changed to use regtype/OIDs, included range/multirange and stopped including > only pg_catalog/information_schema. But didn't yet handle composites. Per cfbot, this test needs to be taught about the new pg_brin_bloom_summary and pg_brin_minmax_multi_summary types. --Jacob
On Fri, 2021-07-16 at 16:21 +0000, Jacob Champion wrote: > On Fri, 2021-04-30 at 13:33 -0500, Justin Pryzby wrote: > > On Sat, Mar 06, 2021 at 03:01:43PM -0500, Tom Lane wrote: > > > v4-0001 mostly teaches test.sh about specific changes that have to be > > > made to historic versions of the regression database to allow them > > > to be reloaded into current servers. As already discussed, this is > > > really duplicative of knowledge that's been embedded into the buildfarm > > > client over time. It'd be better if we could refactor that so that > > > the buildfarm shares a common database of these actions with test.sh. > > > And said database ought to be in our git tree, so committers could > > > fix problems without having to get Andrew involved every time. > > > I think this could be represented as a psql script, at least in > > > versions that have psql \if (but that came in in v10, so maybe > > > we're there already). > > > > I started this. I don't know if it's compatible with the buildfarm client, but > > I think any issues maybe can be avoided by using "IF EXISTS". > > I'm going to try pulling this into a psql script today and see how far > I get. I completely misread this exchange -- you already did this in 0004. Sorry for the noise. --Jacob
On Fri, 2021-04-30 at 13:33 -0500, Justin Pryzby wrote: > On Sat, Mar 06, 2021 at 03:01:43PM -0500, Tom Lane wrote: > > v4-0001 mostly teaches test.sh about specific changes that have to be > > made to historic versions of the regression database to allow them > > to be reloaded into current servers. As already discussed, this is > > really duplicative of knowledge that's been embedded into the buildfarm > > client over time. It'd be better if we could refactor that so that > > the buildfarm shares a common database of these actions with test.sh. > > And said database ought to be in our git tree, so committers could > > fix problems without having to get Andrew involved every time. > > I think this could be represented as a psql script, at least in > > versions that have psql \if (but that came in in v10, so maybe > > we're there already). > > I started this. I don't know if it's compatible with the buildfarm client, but > I think any issues maybe can be avoided by using "IF EXISTS". Here are the differences I see on a first pass (without putting too much thought into how significant the differences are). Buildfarm code I'm comparing against is at [1]. - Both versions drop @#@ and array_cat_accum, but the buildfarm additionally replaces them with a new operator and aggregate, respectively. - The buildfarm's dropping of table OIDs is probably more resilient, since it loops over pg_class looking for relhasoids. - The buildfarm handles (or drops) several contrib databases in addition to the core regression DB. - The psql script drops the first_el_agg_any aggregate and a `TRANSFORM FOR integer`; I don't see any corresponding code in the buildfarm. - Some version ranges are different between the two. For example, abstime_/reltime_/tinterval_tbl are dropped by the buildfarm if the old version is < 9.3, while the psql script drops them for old versions <= 10. - The buildfarm drops the public.=> operator for much older versions of Postgres. I assume we don't need that here. - The buildfarm adjusts pg_proc for the location of regress.so; I see there's a commented placeholder for this at the end of the psql script but it's not yet implemented. As an aside, I think the "fromv10" naming scheme for the "old version <= 10" condition is unintuitive. If the old version is e.g. 9.6, we're not upgrading "from 10". --Jacob [1] https://github.com/PGBuildFarm/client-code/blob/main/PGBuild/Modules/TestUpgradeXversion.pm
Jacob Champion <pchampion@vmware.com> writes: > On Fri, 2021-04-30 at 13:33 -0500, Justin Pryzby wrote: >> I started this. I don't know if it's compatible with the buildfarm client, but >> I think any issues maybe can be avoided by using "IF EXISTS". > Here are the differences I see on a first pass (without putting too > much thought into how significant the differences are). Buildfarm code > I'm comparing against is at [1]. I switched the CF entry for this to "Waiting on Author". It's been failing in the cfbot for a couple of months, and Jacob's provided some review-ish comments here, so I think there's plenty of reason to deem the ball to be in Justin's court. regards, tom lane
On 9/11/21 8:51 PM, Justin Pryzby wrote: > > @Andrew: did you have any comment on this part ? > > |Subject: buildfarm xversion diff > |Forking https://www.postgresql.org/message-id/20210328231433.GI15100@telsasoft.com > | > |I gave suggestion how to reduce the "lines of diff" metric almost to nothing, > |allowing a very small "fudge factor", and which I think makes this a pretty > |good metric rather than a passable one. > Somehow I missed that. Looks like some good suggestions. I'll experiment. (Note: we can't assume the presence of sed, especially on Windows). cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 9/12/21 2:41 PM, Andrew Dunstan wrote: > On 9/11/21 8:51 PM, Justin Pryzby wrote: >> @Andrew: did you have any comment on this part ? >> >> |Subject: buildfarm xversion diff >> |Forking https://www.postgresql.org/message-id/20210328231433.GI15100@telsasoft.com >> | >> |I gave suggestion how to reduce the "lines of diff" metric almost to nothing, >> |allowing a very small "fudge factor", and which I think makes this a pretty >> |good metric rather than a passable one. >> > Somehow I missed that. Looks like some good suggestions. I'll > experiment. (Note: we can't assume the presence of sed, especially on > Windows). > > I tried with the attached patch on crake, which tests back as far as 9.2. Here are the diff counts from HEAD: andrew@emma:HEAD $ grep -c '^[+-]' dumpdiff-REL9_* dumpdiff-REL_1* dumpdiff-HEAD dumpdiff-REL9_2_STABLE:514 dumpdiff-REL9_3_STABLE:169 dumpdiff-REL9_4_STABLE:185 dumpdiff-REL9_5_STABLE:221 dumpdiff-REL9_6_STABLE:11 dumpdiff-REL_10_STABLE:11 dumpdiff-REL_11_STABLE:73 dumpdiff-REL_12_STABLE:73 dumpdiff-REL_13_STABLE:73 dumpdiff-REL_14_STABLE:0 dumpdiff-HEAD:0 I've also attached those non-empty dumpdiff files for information, since they are quite small. There is still work to do, but this is promising. Next step: try it on Windows. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
On 9/13/21 9:20 AM, Andrew Dunstan wrote: > On 9/12/21 2:41 PM, Andrew Dunstan wrote: >> On 9/11/21 8:51 PM, Justin Pryzby wrote: >>> @Andrew: did you have any comment on this part ? >>> >>> |Subject: buildfarm xversion diff >>> |Forking https://www.postgresql.org/message-id/20210328231433.GI15100@telsasoft.com >>> | >>> |I gave suggestion how to reduce the "lines of diff" metric almost to nothing, >>> |allowing a very small "fudge factor", and which I think makes this a pretty >>> |good metric rather than a passable one. >>> >> Somehow I missed that. Looks like some good suggestions. I'll >> experiment. (Note: we can't assume the presence of sed, especially on >> Windows). >> >> > I tried with the attached patch on crake, which tests back as far as > 9.2. Here are the diff counts from HEAD: > > > andrew@emma:HEAD $ grep -c '^[+-]' dumpdiff-REL9_* dumpdiff-REL_1* > dumpdiff-HEAD > dumpdiff-REL9_2_STABLE:514 > dumpdiff-REL9_3_STABLE:169 > dumpdiff-REL9_4_STABLE:185 > dumpdiff-REL9_5_STABLE:221 > dumpdiff-REL9_6_STABLE:11 > dumpdiff-REL_10_STABLE:11 > dumpdiff-REL_11_STABLE:73 > dumpdiff-REL_12_STABLE:73 > dumpdiff-REL_13_STABLE:73 > dumpdiff-REL_14_STABLE:0 > dumpdiff-HEAD:0 > > > I've also attached those non-empty dumpdiff files for information, since > they are quite small. > > > There is still work to do, but this is promising. Next step: try it on > Windows. > > It appears to do the right thing on Windows. yay! We probably need to get smarter about the heuristics, though, e.g. by taking into account the buildfarm options and the platform. It would also help a lot if we could make vcregress.pl honor USE_MODULE_DB. That's on my TODO list, but it just got a lot higher priority. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 9/15/21 3:28 PM, Andrew Dunstan wrote: > On 9/13/21 9:20 AM, Andrew Dunstan wrote: >> On 9/12/21 2:41 PM, Andrew Dunstan wrote: >>> On 9/11/21 8:51 PM, Justin Pryzby wrote: >>>> @Andrew: did you have any comment on this part ? >>>> >>>> |Subject: buildfarm xversion diff >>>> |Forking https://www.postgresql.org/message-id/20210328231433.GI15100@telsasoft.com >>>> | >>>> |I gave suggestion how to reduce the "lines of diff" metric almost to nothing, >>>> |allowing a very small "fudge factor", and which I think makes this a pretty >>>> |good metric rather than a passable one. >>>> >>> Somehow I missed that. Looks like some good suggestions. I'll >>> experiment. (Note: we can't assume the presence of sed, especially on >>> Windows). >>> >>> >> I tried with the attached patch on crake, which tests back as far as >> 9.2. Here are the diff counts from HEAD: >> >> >> andrew@emma:HEAD $ grep -c '^[+-]' dumpdiff-REL9_* dumpdiff-REL_1* >> dumpdiff-HEAD >> dumpdiff-REL9_2_STABLE:514 >> dumpdiff-REL9_3_STABLE:169 >> dumpdiff-REL9_4_STABLE:185 >> dumpdiff-REL9_5_STABLE:221 >> dumpdiff-REL9_6_STABLE:11 >> dumpdiff-REL_10_STABLE:11 >> dumpdiff-REL_11_STABLE:73 >> dumpdiff-REL_12_STABLE:73 >> dumpdiff-REL_13_STABLE:73 >> dumpdiff-REL_14_STABLE:0 >> dumpdiff-HEAD:0 >> >> >> I've also attached those non-empty dumpdiff files for information, since >> they are quite small. >> >> >> There is still work to do, but this is promising. Next step: try it on >> Windows. >> >> > It appears to do the right thing on Windows. yay! > > > We probably need to get smarter about the heuristics, though, e.g. by > taking into account the buildfarm options and the platform. It would > also help a lot if we could make vcregress.pl honor USE_MODULE_DB. > That's on my TODO list, but it just got a lot higher priority. > > Here's what I've committed: <https://github.com/PGBuildFarm/client-code/commit/6317d82c0e897a29dabd57ed8159d13920401f96> cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 11/17/21 02:01, Michael Paquier wrote: > > The oldest version tested by the buildfarm is 9.2, so we could ignore > this part I guess? > > Andrew, what do you think about this part? Based on my read of this > thread, there is an agreement that this approach makes the buildfarm > code more manageable so as committers would not need to patch the > buildfarm code if their test fail. I agree with this conclusion, but > I wanted to double-check with you first. This would need a backpatch > down to 10 so as we could clean up a maximum of code in > TestUpgradeXversion.pm without waiting for an extra 5 years. Please > note that I am fine to send a patch for the buildfarm client. > > In general I'm in agreement with the direction here. If we can have a script that applies to back branches to make them suitable for upgrade testing instead of embedding this in the buildfarm client, so much the better. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Michael Paquier <michael@paquier.xyz> writes: > Okay. I have worked on 0001 to add the table to check after the > binary compatibilities and applied it. Something funny about that on prion: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2021-11-18%2001%3A55%3A38 @@ -747,6 +747,8 @@ '{(2020-01-02 03:04:05, 2021-02-03 06:07:08)}'::tstzmultirange, arrayrange(ARRAY[1,2], ARRAY[2,1]), arraymultirange(arrayrange(ARRAY[1,2], ARRAY[2,1])); +ERROR: unrecognized key word: "ec2" +HINT: ACL key word must be "group" or "user". -- Sanity check on the previous table, checking that all core types are -- included in this table. SELECT oid, typname, typtype, typelem, typarray, typarray Not sure what's going on there. regards, tom lane