Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of PostgreSQL extension versioning #129

Closed
keithf4 opened this issue Aug 20, 2021 · 9 comments · Fixed by #163
Closed

Better handling of PostgreSQL extension versioning #129

keithf4 opened this issue Aug 20, 2021 · 9 comments · Fixed by #163

Comments

@keithf4
Copy link
Contributor

keithf4 commented Aug 20, 2021

SUMMARY

Better support PostgreSQL extension version handling to support arbitrary naming and downgrades

ISSUE TYPE

Feature Request

COMPONENT NAME

postgresql_ext module

ADDITIONAL INFORMATION

Our organization is still running Ansible 2.9 and we haven't made the full jump to collections and using Galaxy yet. So when testing the installation of PostgreSQL with PostGIS, we ran into this issue with the unpackaged extension version causing issues.

ansible-collections/community.general#1099

We saw that it has been fixed in the community.postgresql collection, but seeing how it was fixed is still a bit concerning for the long term. It is still trying to do a version comparison and, as far as PostgreSQL extensions are concerned, you cannot compare versions like this. This is because the version value and ordering is completely arbitrary, can be any valid alphanumeric value that PG can accept, and there really is no concept of one version being "greater" than another.

How PostgreSQL determines which version is installed and how you can upgrade from one to another is controlled entirely by the update files that an extension author provides. You give one version value followed by another and that provides to PG a valid update path. You can upgrade from one extension version to another, or even downgrade, as long as all the relevant update path files have been provided. For example, for pg_partman, you could literally go from all the way back in 0.1.0 to the current 4.5.1 because there is an unbroken chain (pending any compatiblity issues mentioned in the changelogs of course).

https://github.com/pgpartman/pg_partman/tree/master/updates

[...]
-rw-rw-r-- 1 keith keith  94381 Dec 23  2020 pg_partman--4.3.0--4.3.1.sql
-rw-rw-r-- 1 keith keith 131036 May 11  2020 pg_partman--4.3.1--4.4.0.sql
-rw-rw-r-- 1 keith keith 189468 May  3 11:28 pg_partman--4.4.0--4.5.0.sql
-rw-rw-r-- 1 keith keith 188926 May  3 11:28 pg_partman--4.4.1--4.5.0.sql
-rw-rw-r-- 1 keith keith  60613 May 11 14:13 pg_partman--4.5.0--4.5.1.sql
-rw-rw-r-- 1 keith keith  78361 Aug  2 19:10 pg_partman--4.5.1--4.6.0.sql
[...]

As far as PG is concerned, it's not technically an upgrade or a downgrade. It's just following the shortest path from the existing version to the target. You can see this from within PostgreSQL as well

select * from pg_extension_update_paths('pg_partman') where source = '2.3.4' and target = '3.0.0';
 source | target |                                    path                                     
--------+--------+-----------------------------------------------------------------------------
 2.3.4  | 3.0.0  | 2.3.4--2.4.0--2.4.1--2.5.0--2.5.1--2.6.0--2.6.1--2.6.2--2.6.3--2.6.4--3.0.0

If a valid path does not exist, PostgreSQL will return NULL

select * from pg_extension_update_paths('pg_partman') where source = '2.3.4' and target = '1.0.0';
 source | target |  path  
--------+--------+--------
 2.3.4  | 1.0.0  | «NULL»

This leads me to my recommendation for how this should actually be handled. Instead of trying to compare versions to find the latest or to ensure that the user's given version is "greater", it should just check and see if a valid update path exists. This would allow the postgresql_ext module to accept any extension versioning scheme that PG itself supports and also indirectly support downgrading (if the extension author provides that path).

I'd be happy to try and work on this update for the module if the community is interested. Please let me know.

@Andersson007
Copy link
Collaborator

@keithf4 thanks for reporting the issue! I'll try to review the proposal today later or tomorrow.

@Andersson007
Copy link
Collaborator

@keithf4 the suggestion sounds reasonable to me.
If you know how the version handling can be improved without introducing breaking changes (i.e. current users will get nothing unexpected after the next release), please go ahead. Thanks!

@keithf4
Copy link
Contributor Author

keithf4 commented Aug 26, 2021

Great! Thank you for the feedback. Wanted to make sure there was interest in this before going ahead with the work.

Have a few other projects to work through at the moment, but as soon as I have a chance I will take a look at this.

@Andersson007
Copy link
Collaborator

Thanks!

@Andersson007
Copy link
Collaborator

@keithf4 don't forget to update the main branch in your fork before starting working on this because there were related changes merged #138

@klando
Copy link

klando commented Oct 24, 2021

@keithf4 any plan to update the PR in the near future ?

@keithf4
Copy link
Contributor Author

keithf4 commented Oct 29, 2021

Sorry for the delay. Actually started working on it this week. I think I have the logic worked out for how I would like it to work. Just taking some time to figure out how to apply it here to the ansible module.

@keithf4
Copy link
Contributor Author

keithf4 commented Nov 15, 2021

PR has been submitted. Still working on updating the internal docs around the function changes. Could also use some help with adding some unit testing if anyone with more experience in doing that with ansible has some time.

@Andersson007
Copy link
Collaborator

@keithf4 thank you much for working on this! Sorry for the later reply, was on PTO. I'll try to take a look as soon as i can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants