Bug 998424

This error is actually a result of a problem in the version comparison sql.

select rpm.rpmstrcmp('0', 'alpha9.8') from dual;

should return -1 (meaning 0 is smaller than alpha9.0. Instead it was returning 1, because the comparing function looked at the first arg, saw that it was entirely numeric, and then assuming that they were both entirely numeric and doing an incorrect comparison.

"But wait!" you say, "There is no '0' version of that rpm, all versions of the rpm should have been alphanumeric instead of merely numeric!" Yes, but the max function for evr_t type objects starts with ('0','0','0') as the initial conditions, leading us to run into this problem.

I suspect that this is a problem with any packages with an alphanumeric version. This incorrect version comparison problem ballooned up to the package not being correctly included in rhnServerNeededCache, which *a lot* of things drive off of, including taskomatic's errata cache task and the sql for generating the list of updates available for your server in the webui.

I would not be surprised if a lot of customer problems were due to this. They would all be of the class "yum tells me I have an updated version of X ready to install but Satellite says I'm up-to-date".

Committing to Spacewalk master:
94d95c3334ef62ade78c850a955a217d9a7df052

I just noticed comment 0 said this only happens on pgsql, not oracle. I think that makes sense, the max function is only defined specifically for evr_t objects in pgsql, oracle uses a more generic max function. So even though the version compare function would have been wrong in both places, we would only really trip over it in pgsql. Oracle would only have a problem if there were actually two packages, the installed package being "package-1-1" and the "newest" package being "package-beta1-1". Usually version strings go the other way, so I think we wouldn't really run into this on Oracle. I think.

Also need this commit because I named a file wrong:
c093eb247eb1267ab558780241c66ab240920449

> This error is actually a result of a problem in the version comparison sql.
> 
> select rpm.rpmstrcmp('0', 'alpha9.8') from dual;
> 
> should return -1 (meaning 0 is smaller than alpha9.0. Instead it was
> returning 1, because the comparing function looked at the first arg, saw
> that it was entirely numeric, and then assuming that they were both entirely
> numeric and doing an incorrect comparison.

This is actually not true. Version 0 is newer (higher) version than alpha9.8 both in version and release.

$ rpmdev-vercmp 1-0 1-alpha9.8
0:1-0 is newer
$ rpmdev-vercmp 0-1 alpha9.8-1
0:0-1 is newer

There's no bug in rpm.rpmstrcmp() but the issue is in initcond of max(evr_t) definition:

 create aggregate max (
  sfunc=evr_t_larger,
  basetype=evr_t,
  stype=evr_t,
  initcond='(0,0,0)'
 );

evr_t(0,0,0) is not the lowest evr_t member(!) so e.g. 

 select max(x.ver) from (select evr_t('0','0.alpha9.8','0') as ver from dual) x;
 -------
 (0,0,0)

which is clearly nonsense. Correct initcond for max should be (NULL, NULL, NULL) which can be simply omitted in the definition, i.e. 

 create aggregate max (
  sfunc=evr_t_larger,
  basetype=evr_t,
  stype=evr_t
 );

(In reply to Michael Mráka from comment #4)
> > This error is actually a result of a problem in the version comparison sql.
> > 
> > select rpm.rpmstrcmp('0', 'alpha9.8') from dual;
> > 
> > should return -1 (meaning 0 is smaller than alpha9.0. Instead it was
> > returning 1, because the comparing function looked at the first arg, saw
> > that it was entirely numeric, and then assuming that they were both entirely
> > numeric and doing an incorrect comparison.
> 
> This is actually not true. Version 0 is newer (higher) version than alpha9.8
> both in version and release.
> 
> $ rpmdev-vercmp 1-0 1-alpha9.8
> 0:1-0 is newer
> $ rpmdev-vercmp 0-1 alpha9.8-1
> 0:0-1 is newer

I disagree. We cannot make assumptions about the word "alpha" or "beta" as being higher than or lower than version 0. The only thing we can do is apply a consistent set of rules:
1) if both versions are numeric we will compare them numerically
2) if at least one version is not numeric we will compare them alphanumerically

Not this problem does not only apply to the "0" version, the existing code returns the first package version as being larger *any* time the first compared package version is entirely numeric and the second compared package version is not. Right now the function is argument-order dependent, so to use your own example:
> $ rpmdev-vercmp 1-0 1-alpha9.8
> 0:1-0 is newer
> $ rpmdev-vercmp 1-alpha9.8 1-0
> 1-alpha9.8 is newer
This is clearly not correct behavior, and is what my original patch fixes. 

Removing the init condition from the max function is fine but unnecessary once we have a corrected vercmp method.

It turns out that Michael is right and my assertion that our version compare function is argument-order-dependant is not correct. It looks like it is, but then there's a complicated if tree at the end that correctly does the right thing in each case. I was assuming things that were not true about how it should work.

So what we *really* should be doing is implementing the version comparison found here: https://fedoraproject.org/wiki/Archive:Tools/RPM/VersionComparison?rd=Tools/RPM/VersionComparison
*Not* implementing our own set of consistent rules. The version compare method apparently does this already, so really the only problem here is that the postgres implementation of max() for evr_t objects uses an incorrect initial state, as Michael said. I will revert my previous commits and fix this the right way now.

The upgrade script will be a little complicated because we'd need to drop and create the aggregate max function and there are a couple of views that depend on it.

Reverting:
85e77f54adb9f83d628c6d7873ad6f17e1876515
87045dea1317ac9d209d43aa1bca997c6f63c3cc

Committing correct patch (removing the init condition as in comment 5):
c52d9433111063915f447e3175dcdf4463354f3e

Satellite 5.6 has been released. This bug was tracked under the release.  

This bug was either VERIFIED or RELEASE_PENDING (re-verified prior shortly
before release). 

Moving to CLOSED CURRENT_RELEASE. 

Text from Upgrade Erratum follows:

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHEA-2013-1395.html