Обсуждение: Installation of regress.so?
Hi all,
While diving into a transformation of the tests of pg_upgrade to TAP,
I am getting annoyed by the fact that regress.so is needed if you
upgrade an older instance that holds the regression objects from the
main regression test suite. The buildfarm code is using a trick to
copy regress.so from the source code tree of the old instance into its
installation. See in PGBuild/Modules/TestUpgradeXversion.pm:
# at some stage we stopped installing regress.so
copy "$self->{pgsql}/src/test/regress/regress.so",
"$installdir/lib/postgresql/regress.so"
unless (-e "$installdir/lib/postgresql/regress.so");
This creates a hard dependency with the source code of the old
instance if attempting to create an old instance based on a dump,
which is what the buildfarm does, and something that I'd like to get
support for in the TAP tests of pg_upgrade in the tree.
Could it be possible to install regress.so at least in the same
location as pg_regress? This would still require the test to either
move regress.so into a location from where the backend could load the
library, but at least the library could be accessible without a
dependency to the source tree of the old instance upgrading from. To
make that really usable, this would require a backpatch, though..
Thoughts?
--
Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes:
> Could it be possible to install regress.so at least in the same
> location as pg_regress?
I don't think this is a great idea. Aside from the fact that
we'd be littering the install tree with a .so of no use to end
users, I'm failing to see how it really gets you anywhere unless
you want to further require regress.so from back versions to be
loadable into the current server.
regards, tom lane
On 5/19/21 10:24 PM, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> Could it be possible to install regress.so at least in the same >> location as pg_regress? > I don't think this is a great idea. Aside from the fact that > we'd be littering the install tree with a .so of no use to end > users, I'm failing to see how it really gets you anywhere unless > you want to further require regress.so from back versions to be > loadable into the current server. > > We certainly shouldn't want that. But we do need it for the target unless we wipe out everything in the source that refers to it. However, a given installation can be a source in one test and a target in another - currently we test upgrade to every live version from every known version less than or equal to that version (currently crake knows about versions down to 9.2, but it could easily be taught more). I do agree that we should not install regress.so by default. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> On 5/19/21 10:24 PM, Tom Lane wrote:
>> Michael Paquier <michael@paquier.xyz> writes:
>>> Could it be possible to install regress.so at least in the same
>>> location as pg_regress?
>> I don't think this is a great idea. ...
> I do agree that we should not install regress.so by default.
I'd be okay with it being some sort of non-default option.
regards, tom lane
On Thu, May 20, 2021 at 09:30:37AM -0400, Tom Lane wrote: > I'd be okay with it being some sort of non-default option. Okay. It would be possible to control that with an environment variable. However I am wondering if it would not be more user-friendly for automated environments if we had a configure switch to control when things related to the tests are installed or not. Say a --with-test-install? -- Michael
Вложения
On 5/20/21 7:58 PM, Michael Paquier wrote: > On Thu, May 20, 2021 at 09:30:37AM -0400, Tom Lane wrote: >> I'd be okay with it being some sort of non-default option. > Okay. It would be possible to control that with an environment > variable. However I am wondering if it would not be more user-friendly > for automated environments if we had a configure switch to control > when things related to the tests are installed or not. Say a > --with-test-install? That seems a bit tortured. Why should you have to make the decision at configure time? ISTM all you need is an extra make target that will install it for you, or a make variable that controls it in the existing install target. e.g. make INSTALL_REGRESS_LIB=1 install cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> That seems a bit tortured. Why should you have to make the decision at
> configure time? ISTM all you need is an extra make target that will
> install it for you, or a make variable that controls it in the existing
> install target.
Works fine for Unix, but what about Windows?
regards, tom lane
On 5/21/21 9:25 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> That seems a bit tortured. Why should you have to make the decision at >> configure time? ISTM all you need is an extra make target that will >> install it for you, or a make variable that controls it in the existing >> install target. > Works fine for Unix, but what about Windows? > > Good point. One item on my TODO is to make the cross version test module work in Windows ... currently it's Unix only. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 5/21/21 9:46 AM, Andrew Dunstan wrote: > On 5/21/21 9:25 AM, Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >>> That seems a bit tortured. Why should you have to make the decision at >>> configure time? ISTM all you need is an extra make target that will >>> install it for you, or a make variable that controls it in the existing >>> install target. >> Works fine for Unix, but what about Windows? >> >> > > > Good point. One item on my TODO is to make the cross version test module > work in Windows ... currently it's Unix only. > > On further investigation there doesn't actually seem to be anything to do here: the MSVC install script installs everything, including regress.dll, and so does the EDB installer. Maybe we need to look at modifying that, but there don't seem to have been any complaints over an extended period. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Hi,
On 2021-05-20 09:16:50 -0400, Andrew Dunstan wrote:
> We certainly shouldn't want that. But we do need it for the target
> unless we wipe out everything in the source that refers to it.
Is there a reason not to go for the wipe? I don't think the type of
functions we have in regress.so are necessarily ones we'd even expect to
work in the next version?
Here's references to explicit files I see after an installcheck:
SELECT oid::regproc, prosrc, probin FROM pg_proc WHERE probin IS NOT NULL AND probin NOT LIKE '$libdir%';
┌───────────────────────────┬───────────────────────────┬────────────────────────────────────────────────────────────────────────────┐
│ oid │ prosrc │ probin
│
├───────────────────────────┼───────────────────────────┼────────────────────────────────────────────────────────────────────────────┤
│ check_primary_key │ check_primary_key │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/refint.so │
│ check_foreign_key │ check_foreign_key │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/refint.so │
│ autoinc │ autoinc │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/autoinc.so│
│ trigger_return_old │ trigger_return_old │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ ttdummy │ ttdummy │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ set_ttdummy │ set_ttdummy │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ make_tuple_indirect │ make_tuple_indirect │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ test_atomic_ops │ test_atomic_ops │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ test_fdw_handler │ test_fdw_handler │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ test_support_func │ test_support_func │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ test_opclass_options_func │ test_opclass_options_func │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ test_enc_conversion │ test_enc_conversion │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ binary_coercible │ binary_coercible │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ widget_in │ widget_in │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ widget_out │ widget_out │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ int44in │ int44in │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ int44out │ int44out │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ pt_in_widget │ pt_in_widget │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ overpaid │ overpaid │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ interpt_pp │ interpt_pp │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
│ reverse_name │ reverse_name │
/home/andres/build/postgres/dev-optimize/vpath/src/test/regress/regress.so│
└───────────────────────────┴───────────────────────────┴────────────────────────────────────────────────────────────────────────────┘
(21 rows)
Testing the pg_upgrade path for these doesn't seem to add meaningful
coverage, and several seem likely to cause problems across versions?
Greetings,
Andres Freund
On 5/21/21 5:43 PM, Andres Freund wrote: > Hi, > > On 2021-05-20 09:16:50 -0400, Andrew Dunstan wrote: >> We certainly shouldn't want that. But we do need it for the target >> unless we wipe out everything in the source that refers to it. > Is there a reason not to go for the wipe? I don't think the type of > functions we have in regress.so are necessarily ones we'd even expect to > work in the next version? > > Here's references to explicit files I see after an installcheck: > [...] > Testing the pg_upgrade path for these doesn't seem to add meaningful > coverage, and several seem likely to cause problems across versions? > Possibly. My approach generally has been to upgrade as much as possible, only removing things known to have issues. However, this discussion does raise some deeper points. The first is that while we test that pg_upgrade passes we don't actually test that everything is still working. So for example if an SQL function in a loaded module changed signature from one version to another we might never discover it. So one area that needs development is some post-upgrade tests. Second, we are treating the regression databases as a suitable base for testing pg_upgrade. But they aren't designed for that, they are designed for completely different purposes, and we're really just using them out of laziness because they are something we happen to have on hand. Maybe we should develop a suitable purpose-designed upgrade database for testing. There are things that we have found in the past that caused issues we didn't detect because they weren't covered in the upgraded databases. Both of these seem like possibly good Summer of Code or intern projects. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> Second, we are treating the regression databases as a suitable base for
> testing pg_upgrade. But they aren't designed for that, they are designed
> for completely different purposes, and we're really just using them out
> of laziness because they are something we happen to have on hand.
Yup, no doubt about that.
> Maybe we should develop a suitable purpose-designed upgrade database for
> testing. There are things that we have found in the past that caused
> issues we didn't detect because they weren't covered in the upgraded
> databases.
I'm not sure what a "purpose-designed upgrade database" would look like,
though. However, it does seem like the setup would only need to create a
bunch of objects (maybe some of them involving a create/alter sequence),
which means it could run a great deal faster than the current method of
running the core regression tests. And we could stop worrying about
whether the core tests leave an adequate set of objects behind. So yeah,
I'm on board with this if we can find someone who wants to do the
gruntwork.
regards, tom lane