Closed Bug 292138 Opened 19 years ago Closed 18 years ago

VersionCheck does not correctly sort versions

Categories

(addons.mozilla.org Graveyard :: Public Pages, defect)

Product:
addons.mozilla.org Graveyard
Component:
Public Pages
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: Bugzilla-alanjstrBugs, Assigned: morgamic)

References

Details

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
Hardware: PC → All
Target Milestone: 1.0 → 1.1
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
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 ... ?
Depends on: 255803, 271269, 283803
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.
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.
*** Bug 300679 has been marked as a duplicate of this bug. ***
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.
Assignee: Bugzilla-alanjstrBugs → morgamic
*** Bug 312133 has been marked as a duplicate of this bug. ***
*** Bug 322069 has been marked as a duplicate of this bug. ***
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.
What about going based on vID, which an auto-incrementing primary key?  This might solve the problem.  What do you think?
Status: NEW → ASSIGNED
(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.
*** Bug 333578 has been marked as a duplicate of this bug. ***
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
Closed: 18 years ago
Resolution: --- → FIXED
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.