Opened 6 months ago

Closed 6 months ago

#5194 closed defect (fixed)

SELECT postgis_extensions_upgrade() fails with pgextwlist

Reported by: robe Owned by: robe
Priority: blocker Milestone: PostGIS 3.3.0
Component: build/upgrade/install Version: master
Keywords: Cc:

Description

I installed the pgextwlist extension, which is commonly used (or similar setup) for PostGIS cloud providers, and I think the change we put in for postgis_extensions_upgrade has to be refined or removed.

When I have pgextwlist extension - https://github.com/dimitri/pgextwlist installed and run CREATE EXTENSION as a non-super user (but owner of a db).

 SELECT postgis_extensions_upgrade();

I get the error:

ERROR:  permission denied for table pg_extension
CONTEXT:  SQL statement "UPDATE pg_catalog.pg_extension SET extversion = 'ANY' WHERE extname = 'postgis';ALTER EXTENSION postgis UPDATE TO "3.3.0dev""
PL/pgSQL function postgis_extensions_upgrade() line 90 at EXECUTE

I thought this is only ever run if there is no upgrade path, it shouldn't be run for any other situation.

In this particular case, to test that, I did this:

ALTER EXTENSION postgis UPDATE VERSION "3.3.0devnext";

Now there is a path going from 3.3.0devnext back to 3.3.0dev, so this should have not needed to do any ANY update of catalogs

SELECT postgis_extensions_upgrade();

and yet it still errors with:

NOTICE:  Updating extension postgis 3.3.0devnext
ERROR:  permission denied for table pg_extension
CONTEXT:  SQL statement "UPDATE pg_catalog.pg_extension SET extversion = 'ANY' WHERE extname = 'postgis';ALTER EXTENSION postgis UPDATE TO "3.3.0dev""
PL/pgSQL function postgis_extensions_upgrade() line 90 at EXECUTE
testpgtwlist=> ALTER EXTENSION postgis UPDATE TO "3.3.0dev";

It should recognize there is a path, and only resort to try to do ANY if there is no PATH.

I am able to do this just fine:

ALTER EXTENSION postgis UPDATE TO "3.3.0dev";

If it's just an issue with dev releases, can we still fix it. It's hard to test dev in these kind of environments if it doesn't follow the "If there is a path use it"

Change History (15)

comment:1 by robe, 6 months ago

Component: postgisbuild/upgrade/install
Owner: changed from pramsey to strk

comment:2 by strk, 6 months ago

Only doing ANY if a path does not exist can surely be done but defeats the purpose of using ANY there, which is AVOIDING to install those many upgrade paths. There are at the moment 79 upgradeable_versions which multiplied by the number of extensions we are installing (6) means 474 upgrade path files created on the system (slighly more as I belive there'd be also the "next" ones). That's the problem we're trying to solve with that system catalog update.

Alternative proposal would be (which would only work from now ON, meaning we'll still need to keep the 474 upgrade paths for older releases):

  • Always install a current—ANY
  • Never add the versions released with <version>—ANY to upgradeable-path
  • Have postgis_extension_upgrade UPDATE TO ANY; UPDATE;

How does it sound ?

comment:3 by strk, 6 months ago

Well I may add:

  • For upgradeable-paths always install <version>—ANY

This is to support upgrade from version which did not ship <version>—ANY themselves, turning upgradeable_paths.mk into a list of postgis version which did not install that file (meaning it should STOP growing as soon as we install that file for each and every version of postgis.

The file, btw, would be EMPTY.

comment:4 by strk, 6 months ago

See #5092 for the rationale behind using ANY

comment:5 by strk, 6 months ago

The use of ANY was illustrated as SOLUTION1 in https://trac.osgeo.org/postgis/wiki/PostGISExtensionPaths — I've now marked that solution as failing, with reference to this ticket

comment:6 by gdt, 6 months ago

Can you clarify the permission issue, as I don't really follow "cloud"? Is it exactly "someone who is db owner of FOO, but not superuser, and has postgis installed in FOO, cannot upgrade postgis"? And that they were able to install, and would be able to upgrade if some particular thing were not true?

Are we working around "user doesn't have the documented/expected permissions for postgis in the first place"? Something else?

in reply to:  6 comment:7 by robe, 6 months ago

Replying to gdt:

Can you clarify the permission issue, as I don't really follow "cloud"? Is it exactly "someone who is db owner of FOO, but not superuser, and has postgis installed in FOO, cannot upgrade postgis"? And that they were able to install, and would be able to upgrade if some particular thing were not true?

Are we working around "user doesn't have the documented/expected permissions for postgis in the first place"? Something else?

The way many cloud DbaaS work, they never give super user rights to any user. So they have kind of a pseudo super user that is allowed to run CREATE EXTENSION and ALTER EXTENSION.

While these commands are in place, the user is briefly elevated to super user status. Kind of like sudo.

However our postgis_extensions_upgrade() runs in the context of the regular user, so can only do things like updating system catalogs when it is run by a real super user, or is running within the CREATE EXTENSION and ALTER EXTENSION commands we call in that function.

Sandro introduced a commit to update the pg_extension table, to mark the postgis and family of extensions to ANY (essentially forcing the ALTER EXTENSION to run the ANY—3.3.0.sql instead of one of the symlinks (or copies of files). It was an attempt to not have to ship useless files.

That would have been fine, if it only ran if it could not find an extension upgrade path. But as it stands, I think it breaks all cloud installs. Given that a good number of users are now deploying on DbaaS, this is going to break for a lot of people.

I think it's still useful to have it there and run, if it can't find the install path, as it would save some people who decide to delete all these files, and perhaps later we can even have an option in config not to install them especially for users who run their own system. It should however not do that if it can find the install — there are functions to query for if the path is available or not.

comment:8 by strk, 6 months ago

I'm working on an alternative setup in which rather than messing the catalog tables to set our version to 'ANY' we install 0-bytes upgrade paths from any supported version to ANY, which does the SAME thing as the catalog update but using ALTER EXTENSION UPDATE instead of UPDATE pg_extension. It will still be good to try the direct catalog update in case those <version>--ANY upgrade files are missing for any reason (package managers end up dropping them or users want to avoid having all those files around). The work in progress is in an upgrade-paths branch in official repository

in reply to:  8 ; comment:9 by robe, 6 months ago

Replying to strk:

I'm working on an alternative setup in which rather than messing the catalog tables to set our version to 'ANY' we install 0-bytes upgrade paths from any supported version to ANY, which does the SAME thing as the catalog update but using ALTER EXTENSION UPDATE instead of UPDATE pg_extension. It will still be good to try the direct catalog update in case those <version>--ANY upgrade files are missing for any reason (package managers end up dropping them or users want to avoid having all those files around). The work in progress is in an upgrade-paths branch in official repository

Doesn't that still suffer from the issue of, if a user uninstalls a version before they upgrade, they are missing that

<my current version>—ANY.sql

script and since the new install won't provide that, they are still stuck? or is that where your make target comes into play, to install these missing files?

comment:10 by strk, 6 months ago

Here's the pull request with the code I described in my previous comment: https://git.osgeo.org/gitea/postgis/postgis/pulls/104

We'll still be installing the upgrade paths for OLDER versions, so this change does NOT help with the problem of filling up the disk (#5092) but should fix the pgextwlist problem (as soon as I also change postgis_extensions_upgrade() to not mess with system catalogs)

comment:11 by strk, 6 months ago

it would be helpful to install and configure pgextwlist as part of our CI runs (using autoload?). That way we can tell when the problem is fixed.

in reply to:  9 comment:12 by robe, 6 months ago

Replying to robe:

Replying to strk:

I'm working on an alternative setup in which rather than messing the catalog tables to set our version to 'ANY' we install 0-bytes upgrade paths from any supported version to ANY, which does the SAME thing as the catalog update but using ALTER EXTENSION UPDATE instead of UPDATE pg_extension. It will still be good to try the direct catalog update in case those <version>--ANY upgrade files are missing for any reason (package managers end up dropping them or users want to avoid having all those files around). The work in progress is in an upgrade-paths branch in official repository

Doesn't that still suffer from the issue of, if a user uninstalls a version before they upgrade, they are missing that

<my current version>—ANY.sql

script and since the new install won't provide that, they are still stuck? or is that where your make target comes into play, to install these missing files?

A further note — remember when people are doing pg_upgrade, they have only the newest version of PostGIS installed in their new cluster, how are they going to upgrade with this solution?

Please can you do what I said. Keep the catalog update, it's still very useful for dev upgrades and for users, who build their own they can delete all those files. Come 3.4.0 we can refine this more.

If you are not going to do it, then I have to do it myself, and you are more familiar with the code.

comment:13 by robe, 6 months ago

I've worked up a patch I'm testing. For super users they'll follow the same mode, none super users (which is usually only DbaaS users), they'll follow the standard upgrade path.

comment:14 by robe, 6 months ago

Owner: changed from strk to robe

comment:15 by Regina Obe <lr@…>, 6 months ago

Resolution: fixed
Status: newclosed

In 2f08c4f/git:

1) Only use ANY option if superuser installing
2) Don't allow downgrade
3) Schema qual changes
Closes #5194 for PostGIS 3.3.0
Closes #5202 for PostGIS 3.3.0

Note: See TracTickets for help on using tickets.