-
Notifications
You must be signed in to change notification settings - Fork 10
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
Update extension release compatibility lookup #178
Conversation
- Change the query for finding extension release compatibility to use `like` instead of `=` - Update the `extension_release_compatibility` join condition to use a `like` match - Related to #128
@@ -45,7 +45,7 @@ SELECT DISTINCT ON(extension_release.extension_id) | |||
extension_release | |||
JOIN extension ON extension.id = extension_release.extension_id | |||
LEFT JOIN platform_extension ON extension_release.id = platform_extension.extension_release_id | |||
LEFT JOIN extension_release_compatibility ON extension_release_compatibility.extension_release_id = extension_release.id AND extension_release_compatibility.quarkus_core_version = :quarkusCore | |||
LEFT JOIN extension_release_compatibility ON extension_release_compatibility.extension_release_id = extension_release.id AND :quarkusCore like extension_release_compatibility.quarkus_core_version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend to test the queries with the production database as there's a good chance it was using an index before and won't be able to after this change.
What's the format of extension_release_compatibility.quarkus_core_version
and what do you expect it to be after this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format would be 3.0.0.%
. I'd expect that versions marked in https://github.com/quarkusio/quarkus-extension-catalog/blob/main/extensions/quarkiverse-quinoa.yaml#L7 could use this new format instead of being maintained manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might miss something but I thought you were a lot more permissive for compatibility versions and that it was the point of the thing below with upper bounds and lower bounds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the upper and lower bound checks are needed to make sure 2.x extensions are not assumed to be compatible with 3.x by default.
This is to declare exceptions to that rule.
I'm still not 100% convinced this is the best solution though, this is just a prototype to gather some feedback 😉
@gsmet since you optimized queries with success, is this PR still ok? I think we really need such a feature (this way or another) |
Closing and trying out another approach |
like
instead of=
extension_release_compatibility
join condition to use alike
matchExtensionReleaseCompatibility
#128