Hi Erica, On Wed, Mar 10, 2021 at 11:14:52AM +0800, Erica Zhang wrote: > Hi Julien, > Thanks a lot for the quick review. Please see my answer below in blue. > Attached is the new patch.
Thanks! >> The upgrade scripts are already tested as postgres will install 1.4 and >> perform >> all upgrades to reach the default version. > Thanks for pointing that the upgrades paths are covered by upgrade scripts > tests. Since I don't need to test the upgrade, I will test the installation > of different versions directly, any concern? I think you should keep your previous approach. The result will be the same but it will consume less resources for that which is always good. >> +SELECT * FROM pg_available_extensions WHERE name = 'pg_stat_statements' and >> installed_version = '1.4'; >> >> >> What is this supposed to test? All those tests will break every time >> we change >> the default version, which will add maintenance efforts. It could be >> good to >> have one test breaking when changing the version to remind us to add a test >> for >> the new version, but not more. > Here I just want to verify that "installed" version is the expected version. > But we do have the issue as you mentioned which will add maintenance efforts. > > So I prefer to keep one test as now which can remind us to add a new version. > As for others, just to check the count(*) to make sure installation is > success. > Such as SELECT count(*) FROM pg_available_extensions WHERE name = > 'pg_stat_statements' and installed_version = '1.4'; What do you think? How about tweaking your previous query so only the last execution fails when pg_stat_statements default version is updated? Something like: SELECT installed_version = default_version, installed_version FROM pg_available_extensions WHERE name = 'pg_stat_statements'; This way the same query can be reused for both older versions and current version. Also, can you register your patch for the next commitfest at https://commitfest.postgresql.org/33/, to make sure it won't be forgotten?
