All users were logged out of Bugzilla on October 13th, 2018

VersionCheck does not correctly sort versions

VERIFIED FIXED in 1.1

Status

--
major
VERIFIED FIXED
14 years ago
3 years ago

People

(Reporter: Bugzilla-alanjstrBugs, Assigned: morgamic)

Tracking

Dependency tree / graph

Details

(Reporter)

Description

14 years ago
We lost the versioncompare function
http://lxr.mozilla.org/mozilla/source/webtools/update/update/VersionCheck.php#94

It is used because in the mozilla world, 0.10 > 0.9
(Reporter)

Updated

14 years ago
Hardware: PC → All
Target Milestone: 1.0 → 1.1

Comment 1

14 years ago
Per IRC discussion, here is a suggested approach in case one is needed.  =)

At about http://lxr.mozilla.org/update1.0/source/update/VersionCheck.php#159
take out checks for version.minappversion and version.version and also the limit 1

Then drop in the procedure from
http://lxr.mozilla.org/mozilla/source/webtools/update/update/VersionCheck.php#268
to line 294 (with possible optimizations) and also the vercmp function at lines
94-106
(Assignee)

Comment 2

14 years ago
I think the correct way to fix this would be to take a look at how .10 > .9 in
the first place.

The source of this problem comes from a lack of constraints placed on internal
versioning.  Because of this, an arithmetic impossibility occurs in versioning
across the board.

A more proper way to fix this might be:
    Separate display versions from application logic.
    Properly use and validate internal versions via a key or index that is
mathematically acceptable.

Advantages:
    Not using PHP to simulate what MySQL should be doing with a properly ordered
result set.
    Reduce load caused by unnecessary data transfer and subsequent string parsing.
    Reduce time complexity from N to 1...
    Would be fairly easy to implement in the database, and would be a one-time
fix to the workflow.

Disadvantages:
    Would require some manipulation of current production database for ".10"
versions.
    Possible negative impacts on some extensions ... ?
(Reporter)

Updated

14 years ago
Depends on: 255803, 271269, 283803
(Reporter)

Comment 3

14 years ago
We could use a user-defined mysql function like ORDER BY parseInts(version_strings)

parseInts would understand A.B.C.D and convert it into something that natively
sorts correctly like fixed width strings 0000A.0000B.0000C.0000D.

Comment 4

14 years ago
Using a user-defined function, you'd still have to parse every version number. 
Even if it's compiled in C, it may not be much faster than the PHP code and will
add possibly unneeded complexity.  Another thing, you'd have to pad enough so
that no subversion number is longer than the padding.  I guess you could safely
do 255 if the versions are being stored as VARCHARs in the first place.

Comment 5

13 years ago
*** Bug 300679 has been marked as a duplicate of this bug. ***

Comment 6

13 years ago
What if we convert all versions to be in the form x.x.x.x (by adding 0's as
necessary), then use MySQL's INET_ATON and INET_NTOA functions? The INET_ATON
function converts it to a 4 byte number, and the INET_NTOA converts it back.
This is a good way to handle IP addresses in MySQL, and as long as all the
versions are in the x.x.x.x format, it'll be sorted correctly. Another
limitation would be that you can't have a version number greater than 255.
(Reporter)

Updated

13 years ago
Assignee: Bugzilla-alanjstrBugs → morgamic

Comment 7

13 years ago
*** Bug 312133 has been marked as a duplicate of this bug. ***

Comment 8

13 years ago
*** Bug 322069 has been marked as a duplicate of this bug. ***

Comment 9

13 years ago
Here is a possible working PHP solution. I'm sure this could be converted into a MySQL user function:
-----
<html>
<body>
<pre>
<?php
function versionCompare($a, $b)
{
	if ($a{0} == '.')
		$a = '0'.$a;
	if ($b{0} == '.')
		$b = '0'.$b;

	$aa = explode('.', $a);
	$ba = explode('.', $b);
	
	while (count($aa) < count($ba))
		$aa[] = 0;
	while (count($ba) < count($aa))
		$ba[] = 0;
		
	for ($i = 0; $i < count($aa); $i++) {
		$diff = intval($aa[$i]) - intval($ba[$i]);
		if ($diff != 0)
			break;
	}

	return $diff;
}

function test($a, $b, $expect)
{
	$rc = versionCompare($a, $b);
	printf(
		"%s a=%s b=%s expect=%d result=%d\n",
		$rc == $expect ? 'OK' : 'NO',
		$a, $b, $expect, $rc
	);
}

// Compare strings of equal length.
test('1', '1', 0);
test('1', '2', -1);
test('2', '1', 1);
test('1.1', '1.1', 0);
test('1.1', '1.2', -1);
test('1.2', '1.1', 1);
test('1.2.2', '1.2.2', 0);
test('1.2.2', '1.2.3', -1);
test('1.2.3', '1.2.2', 1);

// Compare more unusual cases.
test('1', '1.1', -1);
test('2.001', '2.1', 0);
test('3.1', '1.2.2', 2);
test('.10', '0.1', 9);

?>
</pre>
</body>
</html>
----

Alternatively, simply compare the versions by-submission-date instead of by-version. This assumes that previous versions cannot be replaced once submitted. Using by-submission-date should allow for numerical testing in MySQL IF the submission date is a timestamp.
(Assignee)

Comment 10

13 years ago
What about going based on vID, which an auto-incrementing primary key?  This might solve the problem.  What do you think?
Status: NEW → ASSIGNED
(Reporter)

Comment 11

13 years ago
(In reply to comment #10)
> What about going based on vID, which an auto-incrementing primary key?  This
> might solve the problem.  What do you think?

It would be faster than doing it by date.

The only problem would be edge cases, where someone maintains an older version.  For example, what if their 0.9.x branch is for Firefox 1.0.x and their 0.10.x branch is for Firefox 1.5.  If they made any update for the former, it would sort at the top.

But that's an edge case and worth the risk I say.
(Reporter)

Comment 12

13 years ago
*** Bug 333578 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 13

13 years ago
This was eventually fixed by sorting by version.vID.  See bug 334747.

Granted, the C++ alrorithm isn't used (and probably shouldn't be, because of performance, IMO).

An eventual fix would be generating all possible results as static RDFs and removing the need for a dynamic script here.  That would be a win-win because it would remove the scalability requirement and let us mimic the vercomp stuff in the browser.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.