Custom database functions

Jonathon_Turel · July 15, 2019, 4:01pm

Hello,

I’m currently reviewing a very nice change which adds support for Debian errata in Katello[1]. There’s one aspect of the PR that I’m looking for feedback on which is the inclusion of a postgresql function that compares versions of Debian packages[2].

The problem is that Rails uses db/schema.rb to migrate the test database but ActiveRecord does not place this new function into schema.rb

One potential workaround is to change Foreman’s config/application.rb to include this setting: config.active_record.schema_format = :sql This will use pg_dump to create a .sql file containing actual SQL queries to create the schema which will reflect the custom function. I’m not sure if this has any negative implications for us. Beyond that I don’t really see a good way to go about this.

Any thoughts?

[1] https://github.com/Katello/katello/pull/7961
[2] https://github.com/Katello/katello/pull/7961/files#diff-8ea9be1576c2fa2ee791f4949f23018b

lzap · July 19, 2019, 1:31pm

My question is do we ever load schema besides during unit testing?

TimoGoebel · July 19, 2019, 2:35pm

Tests usually run (locally) on sqlite, not on postgres. How do you imagine this in an environment without Katello?

Jonathon_Turel · July 19, 2019, 2:44pm

My question is do we ever load schema besides during unit testing?

That is a good extension of my question

m-bucher · July 23, 2019, 11:01am

I am currently looking at https://github.com/teoljungberg/fx which adds SQL-function and -trigger handling to rails.
Has anybody got any experience with that?

Not sure if this would solve the problem with the tests using :schema.

m-bucher · July 23, 2019, 11:39am

Update: fx gem seems to have problems, with functions that need parameters (https://github.com/teoljungberg/fx/issues/7) they can be applied fine, but droping them is currently not possible

Jonathon_Turel · July 23, 2019, 6:39pm

It looks like there’s a fix in f(x) to support dropping of functions w/ parameters however it requires postgresql 10 : https://github.com/teoljungberg/fx/commit/53f8b3fb651e7574f98f6ff3685361a13de0981c

lzap · July 24, 2019, 2:01pm

Maybe a dumb question but is it possible to take debian version and convert it into (human unreadable, machine friendly) format that sorts out correctly? Numbers could be represented as fixed-length hex numbers, strings as strings and special characters as different characters or short strings which follow sorting behavior. It does not matter if this form would be 3x longer because this could take advantage of RDBM indices and I would expect this to be faster than full table scan which you always need to do for custom function. For presentation purposes, a copy of the original NVRE would be saved.

Jonathon_Turel · July 24, 2019, 3:35pm

I quite like that. Not sure I can comment on whether there are pitfalls there, though. @m-bucher what do you think about @lzap’s suggestion? Perhaps the errata service could emit those searchable values.

m-bucher · July 24, 2019, 4:20pm

Interesting idea, I am pretty sure it’s possible, but I need to think about it. @x9c4 you originally implemented the db-functions, what is your opinion?

I do not think the errata-service should supply the converted versions. We would need the conversion-service/-method in Katello nevertheless, because the package-versions received from the hosts need to be converted as well. It might be better to keep it in one place.

The conversion adds complexity and maybe redundant data (if we would also decide to save the plain version). That could be regarded as negative, though I am not sure whether using db-functions is worse .

Just for reference, here is the definition of dpkg-versions: https://www.debian.org/doc/debian-policy/ch-controlfields.html#version

Justin_Sherrill · July 24, 2019, 4:55pm

We do something very similar for rpms:

github.com

Katello/katello/blob/master/app/lib/katello/util/package.rb#L157-L174


  # Converts a package version to a sortable string
  #
  # See the Fedora docs for what chars are accepted
  # https://fedoraproject.org/wiki/Archive:Tools/RPM/VersionComparison
  #
  # See Pulp's documentation for more info about this algorithm
  # http://pulp-rpm-dev-guide.readthedocs.org/en/latest/sort-index.html
  #
  # @param version [String] a package version (e.g. "3.9")
  # @return [String] a string that can be sorted (e.g. "01-3.01-9")
  def self.sortable_version(version)
    return "" if version.blank?
    pieces = version.scan(/([A-Za-z]+|\d+)/).flatten.map do |chunk|
      chunk =~ /\d+/ ? "#{"%02d" % chunk.length}-#{chunk}" : "$#{chunk}"
    end
    pieces.join(".")
  end
end

which we use to sort rpms by. It seems like a hack to me and db
functions are the better solution, but are so uncommon in a rails app
that i’m okay with it.

Justin

lzap · July 25, 2019, 6:45am

I like that, however I am not sure if this catches all special cases. Quick look at rpm -qa on my Fedora 30 reveals few candidates which are worth being added to the unit test to verify:

containers-common-0.1.37-0.gite079f9d.fc30.x86_64
rubygem-asciidoctor-pdf-1.5.0-0.9.alpha.16.fc30.noarch
texlive-index-svn24099.4.1beta-25.fc30.noarch
libedit-devel-3.1-27.20190324cvs.fc30.x86_64
google-noto-fonts-common-20181223-2.fc30.noarch
dpkg-1.18.25-8.fc30.x86_64
tbb-2019.6-1.fc30.x86_64
pocl-1.2-4.20190221gita0b083a1b47a738.fc30.x86_64

But these all can be fixed. Probably it is worth extracting this into some kind of service so debian could provide its own implementation.

Justin_Sherrill · July 25, 2019, 1:11pm

Yeah, i’ve always suspected it didn’t fully cover all cases, this came
from the pulp team and is what they use for applicability. Their use
case needs to be a lot more accurate than ours (simple list sorting)

I’ll open an issue to look into those test cases. Thanks lzap!

Justin

x9c4 · August 7, 2019, 11:48am

In Debian’s versioning system, the character “~” has a special meaning, as it evaluates to being prior to anything else, including the end of string. I do not see how this can be mapped into a lexical ordering.

For example, the versions should be ordered as follows:
0.99 < 1.0~~ < 1.0~ < 1.0~x9c4 < 1.0 < 1.0-4~x9c4 < 1.0-4

x9c4 · September 16, 2019, 2:52pm

There is also a software project to provide a debversion field in postgres. Not sure, whether foreman can supply the dependencies (libapt-pkg-dev), but it would serve to completely offload the responsibility.

x9c4 · January 7, 2020, 3:18pm

I started to package postgresql-debversion in

A few questions come up:

Should this package be built with the postgres scl foreman is using?
Can this package be shared by pulp and katello?
Is foreman-packaging the right place for this extension, because pulp should be able to use it independently?

m-bucher · May 12, 2020, 8:29pm

Bringing this thread back to life…
The postgresql-extension is now available through the foreman-repos.
I created a draft-PR for using it in katello: https://github.com/Katello/katello/pull/8708

However, the extension has to be ‘enabled’ by executing the following in the database-console:

CREATE EXTENSION debversion;

We probably do not want to do this within the migration-script as this will probably result in the same problems we had with the pgsql-functions.

Justin_Sherrill · May 12, 2020, 8:37pm

We’ve actually solved this as part of our evr extension:

github.com

Katello/katello/blob/master/db/migrate/20200213184848_create_evr_type.rb#L7


require 'fx'


class CreateEvrType < ActiveRecord::Migration[5.2]
  def up
    unless connection.adapter_name.downcase.include?('sqlite')


      enable_extension "evr"


      add_column :katello_rpms, :evr, :evr_t
      add_column :katello_installed_packages, :evr, :evr_t


      create_trigger :evr_insert_trigger_katello_rpms, on: :katello_rpms
      create_trigger :evr_update_trigger_katello_rpms, on: :katello_rpms
      create_trigger :evr_insert_trigger_katello_installed_packages, on: :katello_installed_packages
      create_trigger :evr_update_trigger_katello_installed_packages, on: :katello_installed_packages


      execute <<-SQL

m-bucher · May 13, 2020, 9:38am

Thanks for the tip.

Another concern I have is, how to make sure the extension’s rpm is installed.
Do we need a n additional Requires: in katello.spec?
And if so, what should be in there, because the extensions rpm is currently named rh-postgresql12-postgresql-debversion

Justin_Sherrill · May 13, 2020, 12:53pm

We handled it in the puppet-katello module (since some users may run an
external database):

It also has to be installed on jenkins slaves:

Also don’t forget puppet-katello-devel:

Justin