Bug 1249219

+++ This bug was initially created as a clone of Bug #1218768 +++

Description of problem:

1.) On Satellite 5.7, sync rhel-x86_64-server-6 and jbappplatform-6-x86_64-server-6-rpm.

2.) Register and subscribe a RHEL 6 server to these channels.

3.) Install glassfish-jsf12-eap6.

# yum install glassfish-jsf12-eap6
[...]
# rpm -q glassfish-jsf12-eap6
glassfish-jsf12-eap6-1.2.15-8.b01_redhat_12.1.ep6.el6.noarch




4.) See older errata show as available for this system in the Satellite web UI, even though yum shows no updates available. From the system profile's Software > Errata tab:

RHSA-2014:0563 	Low: Red Hat JBoss Enterprise Application Platform 6.2.3 update 	N/A 	5/27/14
RHSA-2013:1786 	Low: Red Hat JBoss Enterprise Application Platform 6.2.0 update 	N/A 	12/4/13

In the database:

rhnschema=# select snc.server_id, e.advisory, pn.name as package_name, pe.epoch, pe.version, pe.release, pa.label as arch, c.label as channel from rhnserverneededcache snc, rhnerrata e, rhnpackage p, rhnpackagename pn, rhnpackageevr pe, rhnpackagearch pa, rhnchannel c where snc.server_id=1000014307 and snc.errata_id=e.id and snc.package_id=p.id and p.name_id=pn.id and p.package_arch_id=pa.id and p.evr_id=pe.id and snc.channel_id=c.id;

 server_id  |     advisory     |     package_name     | epoch | version |          release          |  arch  |               channel               
------------+------------------+----------------------+-------+---------+---------------------------+--------+-------------------------------------
 1000014307 | RHSA-2013:1786-1 | glassfish-jsf12-eap6 | 0     | 1.2_15  | 5.b01_redhat_8.1.ep6.el6  | noarch | jbappplatform-6-x86_64-server-6-rpm
 1000014307 | RHSA-2014:0563-1 | glassfish-jsf12-eap6 | 0     | 1.2_15  | 7.b01_redhat_11.1.ep6.el6 | noarch | jbappplatform-6-x86_64-server-6-rpm


Version-Release number of selected component (if applicable):

spacewalk-taskomatic-2.3.8-102.el6sat.noarch
satellite-schema-5.7.0.14-1.el6sat.noarch

How reproducible:

100%

Steps to Reproduce:

See steps above.

Actual results:

Older versions of glassfish-jsf12-eap6 show as available for update on system profile in Satellite web UI.

Expected results:

Older versions not shown as available for update.

Additional info:

The update_needed_cache procedure determines whether an available package is newer than the one installed on the system by using max() and the '<' operator to compare the EVR between two package records.


    create or replace function update_needed_cache(
        server_id_in in numeric
        ) returns void as $$
    begin
[...]
      insert into rhnServerNeededCache
             (server_id, errata_id, package_id, channel_id)
        (select distinct sp.server_id, x.errata_id, p.id, x.channel_id
           FROM (SELECT sp_sp.server_id, sp_sp.name_id,
                        sp_sp.package_arch_id, max(sp_pe.evr) AS max_evr
[...]
           join rhnPackageEvr pe ON pe.id = p.evr_id AND sp.max_evr < pe.evr


The operator is defined using the function evr_t_gt(), which uses evr_t_compare(), which in turn uses rpm.rpmvercmp():

****
create operator > (
  leftarg = evr_t,
  rightarg = evr_t,
  procedure = evr_t_gt,
  commutator = <,
  negator = <=,
  restrict = scalargtsel,
  join = scalargtjoinsel
);

create or replace function evr_t_gt( a evr_t, b evr_t )
returns boolean as $$
begin
  return evr_t_compare( a, b ) > 0;
end;
$$ language plpgsql immutable strict;

create or replace function evr_t_compare( a evr_t, b evr_t )
returns int as $$
begin
  return rpm.vercmp(a.epoch, a.version, a.release, b.epoch, b.version, b.release);
end;
$$ language plpgsql immutable strict;
****

Confirming that the '>' operator and evr_t_compare() function give the incorrect result:

rhnschema=# select e1.evr as evr1, e2.evr as evr2, e1.evr > e2.evr as is_evr1_greater from rhnpackageevr e1, rhnpackageevr e2 where e1.id=17409 and e2.id=18033;
                 evr1                 | evr2                 | is_evr1_greater 
--------------------------------------+--------------------------------------+--
 (0,1.2.15,8.b01_redhat_12.1.ep6.el6) | (0,1.2_15,7.b01_redhat_11.1.ep6.el6) | f


rhnschema=# select e1.evr as evr1, e2.evr as evr2, evr_t_compare(e1.evr, e2.evr) as evr_t_compare from rhnpackageevr e1, rhnpackageevr e2 where e1.id=17409 and e2.id=18033;
                 evr1                 |                 evr2                 | evr_t_compare 
--------------------------------------+--------------------------------------+---------------
 (0,1.2.15,8.b01_redhat_12.1.ep6.el6) | (0,1.2_15,7.b01_redhat_11.1.ep6.el6) |            -1

Interestingly, switching the order of the two gives the same output:

rhnschema=# select e1.evr as evr1, e2.evr as evr2, e1.evr > e2.evr as is_evr1_greater from rhnpackageevr e1, rhnpackageevr e2 where e1.id=18033 and e2.id=17409;
                 evr1                 |                 evr2                 | is_evr1_greater 
--------------------------------------+--------------------------------------+-----------------
 (0,1.2_15,7.b01_redhat_11.1.ep6.el6) | (0,1.2.15,8.b01_redhat_12.1.ep6.el6) | f

rhnschema=# select e1.evr as evr1, e2.evr as evr2, evr_t_compare(e1.evr, e2.evr) as evr_t_compare from rhnpackageevr e1, rhnpackageevr e2 where e1.id=18033 and e2.id=17409;
                 evr1                 |                 evr2                 | evr_t_compare 
--------------------------------------+--------------------------------------+---------------
 (0,1.2_15,7.b01_redhat_11.1.ep6.el6) | (0,1.2.15,8.b01_redhat_12.1.ep6.el6) |            -1


So, some faulty logic is giving both evr1 < evr2 and evr2 < evr1.

rpm.vercmp() performs epoch comparison, then calls rpm.rpmstrcmp() on the versions and release numbers.

****
   create or replace FUNCTION vercmp(
        e1 VARCHAR, v1 VARCHAR, r1 VARCHAR, 
        e2 VARCHAR, v2 VARCHAR, r2 VARCHAR)
    RETURNS INTEGER as $$
    declare
        rc INTEGER;
          ep1 INTEGER;
          ep2 INTEGER;
          BEGIN
            if e1 is null or e1 = '' then
              ep1 := 0;
            else
              ep1 := e1::integer;
            end if;
            if e2 is null or e2 = '' then
              ep2 := 0;
            else
              ep2 := e2::integer;
            end if;
            -- Epochs are non-null; compare them
            if ep1 < ep2 then return -1; end if;
            if ep1 > ep2 then return 1; end if;
            rc := rpm.rpmstrcmp(v1, v2);
            if rc != 0 then return rc; end if;
           return rpm.rpmstrcmp(r1, r2);
         END;
         $$ language 'plpgsql';


    create or replace FUNCTION rpmstrcmp (string1 IN VARCHAR, string2 IN VARCHAR)
    RETURNS INTEGER as $$
    declare
        str1 VARCHAR := string1;
        str2 VARCHAR := string2;
        digits VARCHAR(10) := '0123456789';
        lc_alpha VARCHAR(27) := 'abcdefghijklmnopqrstuvwxyz';
        uc_alpha VARCHAR(27) := 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
        alpha VARCHAR(54) := lc_alpha || uc_alpha;
        one VARCHAR;
        two VARCHAR;
        isnum BOOLEAN;
    BEGIN
        if str1 is NULL or str2 is NULL
        then
            RAISE EXCEPTION 'VALUE_ERROR.';
        end if;
      
        if str1 = str2
        then
            return 0;
        end if;
        one := str1;
        two := str2;

        <<segment_loop>>
        while one <> '' and two <> ''
        loop
            declare
                segm1 VARCHAR;
                segm2 VARCHAR;
            begin
                --DBMS_OUTPUT.PUT_LINE('Params: ' || one || ',' || two);
                -- Throw out all non-alphanum characters
                while one <> '' and not rpm.isalphanum(one)
                loop
                    one := substr(one, 2);
                end loop;
                while two <> '' and not rpm.isalphanum(two)
                loop
                    two := substr(two, 2);
                end loop;
                str1 := one;
                str2 := two;
                if str1 <> '' and rpm.isdigit(str1)
                then
                    str1 := ltrim(str1, digits);
                    str2 := ltrim(str2, digits);
                    isnum := true;
                else
                    str1 := ltrim(str1, alpha);
                    str2 := ltrim(str2, alpha);
                    isnum := false;
                end if;
                if str1 <> ''
                then segm1 := substr(one, 1, length(one) - length(str1));
                else segm1 := one;
                end if;
                    
                if str2 <> ''
                then segm2 := substr(two, 1, length(two) - length(str2));
                else segm2 := two;
                end if;
                if segm1 = '' then return -1; end if; /* arbitrary */
                if segm2 = '' then
                                        if isnum then
                                                return 1;
                                        else
                                                return -1;
                                        end if;
                                end if;
                if isnum
                then
                   
                    segm1 := ltrim(segm1, '0');
                    segm2 := ltrim(segm2, '0');

                    if segm1 = '' and segm2 <> ''
                    then
                        return -1;
                    end if;
                    if segm1 <> '' and segm2 = ''
                    then
                        return 1;
                    end if;
                    if length(segm1) > length(segm2) then return 1; end if;
                    if length(segm2) > length(segm1) then return -1; end if;
                end if;
                  if segm1 < segm2 then return -1; end if;
                if segm1 > segm2 then return 1; end if;
               one := str1;
                two := str2;
            end;
        end loop segment_loop;
     
        if one = '' then return -1; end if;
        return 1;
    END ;
$$ language 'plpgsql';
****

Comparing the version strings also shows the faulty results version1 < version2 and version2 < version1:

rhnschema=# select e1.version as version1, e2.version as version2, rpm.rpmstrcmp(e1.version, e2.version) as rpmstrcmp from rhnpackageevr e1, rhnpackageevr e2 where e1.id=18033 and e2.id=17409; version1 | version2 | rpmstrcmp 
----------+----------+-----------
 1.2_15   | 1.2.15   |        -1

rhnschema=# select e1.version as version1, e2.version as version2, rpm.rpmstrcmp(e1.version, e2.version) as rpmstrcmp from rhnpackageevr e1, rhnpackageevr e2 where e1.id=17409 and e2.id=18033;
 version1 | version2 | rpmstrcmp 
----------+----------+-----------
 1.2.15   | 1.2_15   |        -1


So the faulty logic is within rpmstrcmp().

Compare to rpm/yum:

# python
>>> import rpmUtils.miscutils
>>> rpmUtils.miscutils.compareEVR(('0', '1.2.15', '8.b01_redhat_12.1.ep6.el6'), ('0', '1.2_15', '7.b01_redhat_11.1.ep6.el6'))
1
>>> rpmUtils.miscutils.compareEVR(('0', '1.2_15', '7.b01_redhat_11.1.ep6.el6'), ('0', '1.2.15', '8.b01_redhat_12.1.ep6.el6'))
-1

--- Additional comment from Tasos Papaioannou on 2015-05-05 16:21:48 EDT ---

The problem in rpmstrcmp() appears to be that only string1 = string2 can return 0. If the only difference is in a '.' instead of a '_' at one position, then the logic comes down to the very end, where the first parameter is arbitrarily chosen as less than the second parameter:

[...]
        if one = '' then return -1; end if;
        return 1;
    END ;
$$ language 'plpgsql';

A check whether both one = '' and two = '' at this point, with a return value of 0, resolves this particular issue in my testing:


    create or replace FUNCTION test_rpmstrcmp (string1 IN VARCHAR, string2 IN VARCHAR)
    RETURNS INTEGER as $$
[...]
        if one = '' and two = '' then return 0; end if;
        if one = '' then return -1; end if;
        return 1;
    END ;
$$ language 'plpgsql';


rhnschema=# select test_rpmstrcmp('1.2.15', '1.2_15');
 test_rpmstrcmp 
----------------
              0

rhnschema=# select test_rpmstrcmp('1.2_15', '1.2.15');
 test_rpmstrcmp 
----------------
              0

--- Additional comment from Tasos Papaioannou on 2015-05-28 10:28:59 EDT ---

> Is this a regression from say Sat 5.5 ?

The issue is not present in Satellite 5.5.

> Is this an Oracle vs PostgreSQL issue?

I have not tested on Oracle in Satellite 5.6 or 5.7, but both the Satellite and Spacewalk git repos show the same rpmstrcmp function in Oracle as in Satellite 5.5. The Oracle version of the function does perform the check I suggested in https://bugzilla.redhat.com/show_bug.cgi?id=1218768#c1 (see below), so this appears to be an issue only in the postgres version:

SATELLITE-5.7:

# git show c43c7764
commit c43c7764d22ca8a78fd6f446b0892b6dec5e78a8
Author: Spacewalk Devel <spacewalk-devel>
Date:   Tue Jun 17 11:18:41 2008 -0300

    Initial commit.
[...]
diff --git a/schema/rhnsat/packages/rpm.pkb b/schema/rhnsat/packages/rpm.pkb
new file mode 100644
index 0000000..0f24b38
--- /dev/null
+++ b/schema/rhnsat/packages/rpm.pkb
[...]
+        end loop segment_loop;
+        /* this catches the case where all numeric and alpha segments have */
+        /* compared identically but the segment sepparating characters were */
+        /* different */
+        if one is null and two is null then return 0; end if;
+
+        /* whichever version still has characters left over wins */
+        if one is null then return -1; end if;
+        return 1;
+    END rpmstrcmp;