[aur-dev] [PATCH 1/5] Submission process code refactor
We had a ton of duplicate code shared between the insert and update cases. Do a refactor so we can pull this stuff out below the if/else block and only need it there once, saving some headaches. Signed-off-by: Dan McGee <dan@archlinux.org> --- web/html/pkgsubmit.php | 96 +++++++++++++++++------------------------------ 1 files changed, 35 insertions(+), 61 deletions(-) diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 05cc866..131bf37 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -303,11 +303,12 @@ if ($_COOKIE["AURSID"]): # This is an overwrite of an existing package, the database ID # needs to be preserved so that any votes are retained. However, # PackageDepends and PackageSources can be purged. + $packageID = $pdata["ID"]; # Flush out old data that will be replaced with new data - $q = "DELETE FROM PackageDepends WHERE PackageID = " . $pdata["ID"]; + $q = "DELETE FROM PackageDepends WHERE PackageID = " . $packageID; db_query($q, $dbh); - $q = "DELETE FROM PackageSources WHERE PackageID = " . $pdata["ID"]; + $q = "DELETE FROM PackageSources WHERE PackageID = " . $packageID; db_query($q, $dbh); # If the package was a dummy, undummy it @@ -315,7 +316,7 @@ if ($_COOKIE["AURSID"]): $q = sprintf( "UPDATE Packages SET DummyPkg = 0, SubmitterUID = %d, MaintainerUID = %d, SubmittedTS = UNIX_TIMESTAMP() WHERE ID = %d", $uid, $uid, - $pdata["ID"]); + $packageID); db_query($q, $dbh); } @@ -324,7 +325,7 @@ if ($_COOKIE["AURSID"]): if ($_POST['category'] > 1) { $q = sprintf( "UPDATE Packages SET CategoryID = %d WHERE ID = %d", mysql_real_escape_string($_REQUEST['category']), - $pdata["ID"]); + $packageID); db_query($q, $dbh); } @@ -338,41 +339,10 @@ if ($_COOKIE["AURSID"]): mysql_real_escape_string($new_pkgbuild['pkgdesc']), mysql_real_escape_string($new_pkgbuild['url']), uid_from_sid($_COOKIE["AURSID"]), - $pdata["ID"]); + $packageID); db_query($q, $dbh); - # Update package depends - $depends = explode(" ", $new_pkgbuild['depends']); - foreach ($depends as $dep) { - $q = "INSERT INTO PackageDepends (PackageID, DepPkgID, DepCondition) VALUES ("; - $deppkgname = preg_replace("/(<|<=|=|>=|>).*/", "", $dep); - $depcondition = str_replace($deppkgname, "", $dep); - - if ($deppkgname == "#") { - break; - } - - $deppkgid = create_dummy($deppkgname, $_COOKIE['AURSID']); - $q .= $pdata["ID"] . ", " . $deppkgid . ", '" . mysql_real_escape_string($depcondition) . "')"; - - db_query($q, $dbh); - } - - # Insert sources - $sources = explode(" ", $new_pkgbuild['source']); - foreach ($sources as $src) { - if ($src != "" ) { - $q = "INSERT INTO PackageSources (PackageID, Source) VALUES ("; - $q .= $pdata["ID"] . ", '" . mysql_real_escape_string($src) . "')"; - db_query($q, $dbh); - } - } - - if ($pdata["MaintainerUID"] === NULL) pkg_notify(account_from_sid($_COOKIE["AURSID"]), array($pdata["ID"])); - - header('Location: packages.php?ID=' . $pdata['ID']); - } else { $uid = uid_from_sid($_COOKIE["AURSID"]); @@ -388,41 +358,45 @@ if ($_COOKIE["AURSID"]): $uid, $uid); - $result = db_query($q, $dbh); + db_query($q, $dbh); $packageID = mysql_insert_id($dbh); - # Update package depends - $depends = explode(" ", $new_pkgbuild['depends']); - foreach ($depends as $dep) { - $q = "INSERT INTO PackageDepends (PackageID, DepPkgID, DepCondition) VALUES ("; - $deppkgname = preg_replace("/(<|<=|=|>=|>).*/", "", $dep); - $depcondition = str_replace($deppkgname, "", $dep); - - if ($deppkgname == "#") { - break; - } + } - $deppkgid = create_dummy($deppkgname, $_COOKIE['AURSID']); - $q .= $packageID . ", " . $deppkgid . ", '" . mysql_real_escape_string($depcondition) . "')"; + # Update package depends + $depends = explode(" ", $new_pkgbuild['depends']); + foreach ($depends as $dep) { + $q = "INSERT INTO PackageDepends (PackageID, DepPkgID, DepCondition) VALUES ("; + $deppkgname = preg_replace("/(<|<=|=|>=|>).*/", "", $dep); + $depcondition = str_replace($deppkgname, "", $dep); - db_query($q, $dbh); + if ($deppkgname == "#") { + break; } - # Insert sources - $sources = explode(" ", $new_pkgbuild['source']); - foreach ($sources as $src) { - if ($src != "" ) { - $q = "INSERT INTO PackageSources (PackageID, Source) VALUES ("; - $q .= $packageID . ", '" . mysql_real_escape_string($src) . "')"; - db_query($q, $dbh); - } - } + $deppkgid = create_dummy($deppkgname, $_COOKIE['AURSID']); + $q .= $packageID . ", " . $deppkgid . ", '" . mysql_real_escape_string($depcondition) . "')"; - pkg_notify(account_from_sid($_COOKIE["AURSID"]), array($packageID)); + db_query($q, $dbh); + } - header('Location: packages.php?ID=' . $packageID); + # Insert sources + $sources = explode(" ", $new_pkgbuild['source']); + foreach ($sources as $src) { + if ($src != "" ) { + $q = "INSERT INTO PackageSources (PackageID, Source) VALUES ("; + $q .= $packageID . ", '" . mysql_real_escape_string($src) . "')"; + db_query($q, $dbh); + } + } + # If we just created this package, or it was an orphan and we + # auto-adopted, add submitting user to the notification list. + if (!$pdata || $pdata["MaintainerUID"] === NULL) { + pkg_notify(account_from_sid($_COOKIE["AURSID"]), array($packageID)); } + + header('Location: packages.php?ID=' . $packageID); } chdir($cwd); -- 1.7.4.2
Set it equal to the SubmittedTS field, which will be our indication the package is new when we show the logo on the front page of the AUR. This results in the ability to remove the use of the unindexable GREATEST() function from the AUR code everywhere we had to use it before to handle the 0 timestamp case. Note that there is no race condition here in calling UNIX_TIMESTAMP() twice- it always returns the time at the beginning of statment execution: mysql> select unix_timestamp(), sleep(2), unix_timestamp(); +------------------+----------+------------------+ | unix_timestamp() | sleep(2) | unix_timestamp() | +------------------+----------+------------------+ | 1300851746 | 0 | 1300851746 | +------------------+----------+------------------+ 1 row in set (2.00 sec) Signed-off-by: Dan McGee <dan@archlinux.org> --- UPGRADING | 7 +++++++ web/html/pkgsubmit.php | 2 +- web/lib/pkgfuncs.inc | 2 +- web/lib/stats.inc | 4 ++-- web/template/stats/updates_table.php | 7 ++----- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/UPGRADING b/UPGRADING index 5335d89..4743b8e 100644 --- a/UPGRADING +++ b/UPGRADING @@ -1,6 +1,13 @@ Upgrading ========= +From 1.8.1 to X.X.X +------------------- + +1. Update the modified package timestamp for new packages. + +UPDATE Packages SET ModifiedTS = SubmittedTS WHERE ModifiedTS = 0; + From 1.8.0 to 1.8.1 ------------------- diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 131bf37..3eb60c5 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -347,7 +347,7 @@ if ($_COOKIE["AURSID"]): $uid = uid_from_sid($_COOKIE["AURSID"]); # This is a brand new package - $q = sprintf("INSERT INTO Packages (Name, License, Version, CategoryID, Description, URL, SubmittedTS, SubmitterUID, MaintainerUID) VALUES ('%s', '%s', '%s-%s', %d, '%s', '%s', UNIX_TIMESTAMP(), %d, %d)", + $q = sprintf("INSERT INTO Packages (Name, License, Version, CategoryID, Description, URL, SubmittedTS, ModifiedTS, SubmitterUID, MaintainerUID) VALUES ('%s', '%s', '%s-%s', %d, '%s', '%s', UNIX_TIMESTAMP(), UNIX_TIMESTAMP(), %d, %d)", mysql_real_escape_string($new_pkgbuild['pkgname']), mysql_real_escape_string($new_pkgbuild['license']), mysql_real_escape_string($new_pkgbuild['pkgver']), diff --git a/web/lib/pkgfuncs.inc b/web/lib/pkgfuncs.inc index b38c7da..699a3a9 100644 --- a/web/lib/pkgfuncs.inc +++ b/web/lib/pkgfuncs.inc @@ -525,7 +525,7 @@ function pkg_search_page($SID="") { $q_sort = "ORDER BY Maintainer ".$order.", Name ASC "; break; case 'a': - $q_sort = "ORDER BY GREATEST(SubmittedTS,ModifiedTS) ".$order.", Name ASC "; + $q_sort = "ORDER BY ModifiedTS ".$order.", Name ASC "; break; default: break; diff --git a/web/lib/stats.inc b/web/lib/stats.inc index f42e417..f924fb5 100644 --- a/web/lib/stats.inc +++ b/web/lib/stats.inc @@ -36,7 +36,7 @@ function updates_table($dbh) global $apc_prefix, $apc_ttl; $key = $apc_prefix . 'recent_updates'; if(!(EXTENSION_LOADED_APC && ($newest_packages = apc_fetch($key)))) { - $q = 'SELECT * FROM Packages WHERE DummyPkg != 1 ORDER BY GREATEST(SubmittedTS,ModifiedTS) DESC LIMIT 0 , 10'; + $q = 'SELECT * FROM Packages WHERE DummyPkg != 1 ORDER BY ModifiedTS DESC LIMIT 0 , 10'; $result = db_query($q, $dbh); $newest_packages = new ArrayObject(); @@ -84,7 +84,7 @@ function general_stats_table($dbh) $tu_count = db_cache_value($q, $dbh, $apc_prefix . 'tu_count'); $targstamp = intval(strtotime("-7 days")); - $q = "SELECT count(*) from Packages WHERE (Packages.SubmittedTS >= $targstamp OR Packages.ModifiedTS >= $targstamp)"; + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $targstamp"; $update_count = db_cache_value($q, $dbh, $apc_prefix . 'update_count'); include('stats/general_stats_table.php'); diff --git a/web/template/stats/updates_table.php b/web/template/stats/updates_table.php index b9f6d6d..a8cdf5a 100644 --- a/web/template/stats/updates_table.php +++ b/web/template/stats/updates_table.php @@ -20,12 +20,10 @@ $mod_int = intval($row["ModifiedTS"]); $sub_int = intval($row["SubmittedTS"]); -if ($mod_int != 0): - $modstring = gmdate("r", $mod_int); -elseif ($sub_int != 0): +if ($mod_int == $sub_int): $modstring = '<img src="images/new.gif" alt="New!" /> ' . gmdate("r", $sub_int); else: - $modstring = '(unknown)'; + $modstring = gmdate("r", $mod_int); endif; ?> @@ -36,4 +34,3 @@ endif; <?php endforeach; ?> </table> - -- 1.7.4.2
It might be worth while to add an index on ModifiedTS as well. That would make sorting by ModifiedTS really fast for generating the rss and recent packages lists (as well as sorting aur results by date). Granted, adding another index would make insertion or updates a tad slower, but I am not sure *how much* slower. Dan, do you have any thoughts on adding an index for ModifiedTS? It might be a nice trade off with the removal of the DummyPkg stuff (and related dummypkg index). On Wed, Mar 30, 2011 at 6:48 PM, Dan McGee <dan@archlinux.org> wrote:
Set it equal to the SubmittedTS field, which will be our indication the package is new when we show the logo on the front page of the AUR.
This results in the ability to remove the use of the unindexable GREATEST() function from the AUR code everywhere we had to use it before to handle the 0 timestamp case.
Note that there is no race condition here in calling UNIX_TIMESTAMP() twice- it always returns the time at the beginning of statment execution:
mysql> select unix_timestamp(), sleep(2), unix_timestamp(); +------------------+----------+------------------+ | unix_timestamp() | sleep(2) | unix_timestamp() | +------------------+----------+------------------+ | 1300851746 | 0 | 1300851746 | +------------------+----------+------------------+ 1 row in set (2.00 sec)
Signed-off-by: Dan McGee <dan@archlinux.org> --- UPGRADING | 7 +++++++ web/html/pkgsubmit.php | 2 +- web/lib/pkgfuncs.inc | 2 +- web/lib/stats.inc | 4 ++-- web/template/stats/updates_table.php | 7 ++----- 5 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/UPGRADING b/UPGRADING index 5335d89..4743b8e 100644 --- a/UPGRADING +++ b/UPGRADING @@ -1,6 +1,13 @@ Upgrading =========
+From 1.8.1 to X.X.X +------------------- + +1. Update the modified package timestamp for new packages. + +UPDATE Packages SET ModifiedTS = SubmittedTS WHERE ModifiedTS = 0; + From 1.8.0 to 1.8.1 -------------------
diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 131bf37..3eb60c5 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -347,7 +347,7 @@ if ($_COOKIE["AURSID"]): $uid = uid_from_sid($_COOKIE["AURSID"]);
# This is a brand new package - $q = sprintf("INSERT INTO Packages (Name, License, Version, CategoryID, Description, URL, SubmittedTS, SubmitterUID, MaintainerUID) VALUES ('%s', '%s', '%s-%s', %d, '%s', '%s', UNIX_TIMESTAMP(), %d, %d)", + $q = sprintf("INSERT INTO Packages (Name, License, Version, CategoryID, Description, URL, SubmittedTS, ModifiedTS, SubmitterUID, MaintainerUID) VALUES ('%s', '%s', '%s-%s', %d, '%s', '%s', UNIX_TIMESTAMP(), UNIX_TIMESTAMP(), %d, %d)", mysql_real_escape_string($new_pkgbuild['pkgname']), mysql_real_escape_string($new_pkgbuild['license']), mysql_real_escape_string($new_pkgbuild['pkgver']), diff --git a/web/lib/pkgfuncs.inc b/web/lib/pkgfuncs.inc index b38c7da..699a3a9 100644 --- a/web/lib/pkgfuncs.inc +++ b/web/lib/pkgfuncs.inc @@ -525,7 +525,7 @@ function pkg_search_page($SID="") { $q_sort = "ORDER BY Maintainer ".$order.", Name ASC "; break; case 'a': - $q_sort = "ORDER BY GREATEST(SubmittedTS,ModifiedTS) ".$order.", Name ASC "; + $q_sort = "ORDER BY ModifiedTS ".$order.", Name ASC "; break; default: break; diff --git a/web/lib/stats.inc b/web/lib/stats.inc index f42e417..f924fb5 100644 --- a/web/lib/stats.inc +++ b/web/lib/stats.inc @@ -36,7 +36,7 @@ function updates_table($dbh) global $apc_prefix, $apc_ttl; $key = $apc_prefix . 'recent_updates'; if(!(EXTENSION_LOADED_APC && ($newest_packages = apc_fetch($key)))) { - $q = 'SELECT * FROM Packages WHERE DummyPkg != 1 ORDER BY GREATEST(SubmittedTS,ModifiedTS) DESC LIMIT 0 , 10'; + $q = 'SELECT * FROM Packages WHERE DummyPkg != 1 ORDER BY ModifiedTS DESC LIMIT 0 , 10'; $result = db_query($q, $dbh);
$newest_packages = new ArrayObject(); @@ -84,7 +84,7 @@ function general_stats_table($dbh) $tu_count = db_cache_value($q, $dbh, $apc_prefix . 'tu_count');
$targstamp = intval(strtotime("-7 days")); - $q = "SELECT count(*) from Packages WHERE (Packages.SubmittedTS >= $targstamp OR Packages.ModifiedTS >= $targstamp)"; + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $targstamp"; $update_count = db_cache_value($q, $dbh, $apc_prefix . 'update_count');
include('stats/general_stats_table.php'); diff --git a/web/template/stats/updates_table.php b/web/template/stats/updates_table.php index b9f6d6d..a8cdf5a 100644 --- a/web/template/stats/updates_table.php +++ b/web/template/stats/updates_table.php @@ -20,12 +20,10 @@ $mod_int = intval($row["ModifiedTS"]); $sub_int = intval($row["SubmittedTS"]);
-if ($mod_int != 0): - $modstring = gmdate("r", $mod_int); -elseif ($sub_int != 0): +if ($mod_int == $sub_int): $modstring = '<img src="images/new.gif" alt="New!" /> ' . gmdate("r", $sub_int); else: - $modstring = '(unknown)'; + $modstring = gmdate("r", $mod_int); endif; ?>
@@ -36,4 +34,3 @@ endif; <?php endforeach; ?>
</table> - -- 1.7.4.2
On Wed, Mar 30, 2011 at 9:13 PM, elij <elij.mx@gmail.com> wrote:
It might be worth while to add an index on ModifiedTS as well. That would make sorting by ModifiedTS really fast for generating the rss and recent packages lists (as well as sorting aur results by date).
Granted, adding another index would make insertion or updates a tad slower, but I am not sure *how much* slower.
Dan, do you have any thoughts on adding an index for ModifiedTS? It might be a nice trade off with the removal of the DummyPkg stuff (and related dummypkg index).
That conversation we had on IRC the other day was in direct relation to this- for the package list, we don't use the indexes at all because our query isn't exactly how MySQL likes it. The only way I could convince it to use a index was remove most of the joins and drop the ORDER BY condition down to one column. The recent packages list is cached too, so although it would be used there we don't run that query more than once every 5 minutes anyway. -Dan
On Wed, Mar 30, 2011 at 7:19 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Wed, Mar 30, 2011 at 9:13 PM, elij <elij.mx@gmail.com> wrote:
It might be worth while to add an index on ModifiedTS as well. That would make sorting by ModifiedTS really fast for generating the rss and recent packages lists (as well as sorting aur results by date).
Granted, adding another index would make insertion or updates a tad slower, but I am not sure *how much* slower.
Dan, do you have any thoughts on adding an index for ModifiedTS? It might be a nice trade off with the removal of the DummyPkg stuff (and related dummypkg index).
That conversation we had on IRC the other day was in direct relation to this- for the package list, we don't use the indexes at all because our query isn't exactly how MySQL likes it. The only way I could convince it to use a index was remove most of the joins and drop the ORDER BY condition down to one column.
The recent packages list is cached too, so although it would be used there we don't run that query more than once every 5 minutes anyway.
I was writing a part of the spew backend that was setting the last modified http header to handle if-modified-since behavior, for generating the rss/atom feed in realtime instead of periodic. I used MAX on the ModifiedTS field to get the latest updated package's ModifiedTS, and used that as the http last modified value. The query planner was showing full table scans to find said max value (most recent update for any package). Adding an index on ModifiedTS of course made the query planner then show that it was using an index. However, the table isn't _that_ large, and a full table scan may even fit inside the query cache...so it may or may not be worthwhile. I had forgotten that the AUR caches the package feed for 5 minutes too. So yeah, adding that index probably wouldn't make much sense. :)
Instead, we just store dependencies directly in the PackageDepends table. Since we don't use this info anywhere besides the package details page, there is little value in precalculating what is in the AUR vs. what is not. An upgrade path is provided via several SQL statements in the UPGRADING document. There should be no user-visible change from this, but the DB schema gets a bit more sane and we no longer have loads of junk packages in our tables that are never shown to the end user. This should also help the MySQL query planner in several cases as we no longer have to be careful to exclude dummy packages on every query. Signed-off-by: Dan McGee <dan@archlinux.org> --- UPGRADING | 12 ++++++++ support/schema/aur-schema.sql | 5 +-- support/scripts/newpackage-notify | 2 +- web/html/pkgsubmit.php | 17 ++-------- web/html/rss.php | 1 - web/lib/aur.inc | 3 +- web/lib/aurjson.class.php | 4 +- web/lib/pkgfuncs.inc | 56 +++++++----------------------------- web/lib/stats.inc | 4 +- web/template/pkg_details.php | 21 ++++--------- 10 files changed, 41 insertions(+), 84 deletions(-) diff --git a/UPGRADING b/UPGRADING index 4743b8e..e6590f0 100644 --- a/UPGRADING +++ b/UPGRADING @@ -8,6 +8,18 @@ From 1.8.1 to X.X.X UPDATE Packages SET ModifiedTS = SubmittedTS WHERE ModifiedTS = 0; +2. Move to new method of storing package depends. + +---- +ALTER TABLE PackageDepends ADD COLUMN DepName VARCHAR(64) NOT NULL DEFAULT '' AFTER PackageID; +UPDATE PackageDepends SET DepName = (SELECT Name FROM Packages WHERE ID = DepPkgID); +ALTER TABLE PackageDepends MODIFY DepName VARCHAR(64) NOT NULL; +ALTER TABLE PackageDepends DROP FOREIGN KEY `PackageDepends_ibfk_2`; +ALTER TABLE PackageDepends DROP COLUMN DepPkgID; +DELETE FROM Packages WHERE DummyPkg = 1; +ALTER TABLE Packages DROP COLUMN DummyPkg; +---- + From 1.8.0 to 1.8.1 ------------------- diff --git a/support/schema/aur-schema.sql b/support/schema/aur-schema.sql index 8226b18..d8c8560 100644 --- a/support/schema/aur-schema.sql +++ b/support/schema/aur-schema.sql @@ -98,7 +98,6 @@ CREATE TABLE Packages ( CategoryID TINYINT UNSIGNED NOT NULL DEFAULT 1, Description VARCHAR(255) NOT NULL DEFAULT "An Arch Package", URL VARCHAR(255) NOT NULL DEFAULT "http://www.archlinux.org", - DummyPkg TINYINT UNSIGNED NOT NULL DEFAULT 0, -- 1=>dummy License VARCHAR(40) NOT NULL DEFAULT '', NumVotes INTEGER UNSIGNED NOT NULL DEFAULT 0, OutOfDateTS BIGINT UNSIGNED NULL DEFAULT NULL, @@ -109,7 +108,6 @@ CREATE TABLE Packages ( PRIMARY KEY (ID), UNIQUE (Name), INDEX (CategoryID), - INDEX (DummyPkg), INDEX (NumVotes), INDEX (SubmitterUID), INDEX (MaintainerUID), @@ -124,11 +122,10 @@ CREATE TABLE Packages ( -- CREATE TABLE PackageDepends ( PackageID INTEGER UNSIGNED NOT NULL, - DepPkgID INTEGER UNSIGNED NOT NULL, + DepName VARCHAR(64) NOT NULL, DepCondition VARCHAR(20), INDEX (PackageID), FOREIGN KEY (PackageID) REFERENCES Packages(ID) ON DELETE CASCADE, - FOREIGN KEY (DepPkgID) REFERENCES Packages(ID) ON DELETE CASCADE ) ENGINE = InnoDB; diff --git a/support/scripts/newpackage-notify b/support/scripts/newpackage-notify index 66cb45c..9afee07 100755 --- a/support/scripts/newpackage-notify +++ b/support/scripts/newpackage-notify @@ -40,7 +40,7 @@ q = dbconnection.cursor() q.execute("SELECT Packages.Name, Packages.Version, Packages.ID, " "Packages.Description, Users.Username FROM Packages, Users " - "WHERE SubmittedTS >= %d AND DummyPkg = 0 AND " + "WHERE SubmittedTS >= %d AND " "Packages.SubmitterUID = Users.ID" % starttime) packages = q.fetchall() diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 3eb60c5..1540c8a 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -311,16 +311,6 @@ if ($_COOKIE["AURSID"]): $q = "DELETE FROM PackageSources WHERE PackageID = " . $packageID; db_query($q, $dbh); - # If the package was a dummy, undummy it - if ($pdata['DummyPkg']) { - $q = sprintf( "UPDATE Packages SET DummyPkg = 0, SubmitterUID = %d, MaintainerUID = %d, SubmittedTS = UNIX_TIMESTAMP() WHERE ID = %d", - $uid, - $uid, - $packageID); - - db_query($q, $dbh); - } - # If a new category was chosen, change it to that if ($_POST['category'] > 1) { $q = sprintf( "UPDATE Packages SET CategoryID = %d WHERE ID = %d", @@ -366,7 +356,6 @@ if ($_COOKIE["AURSID"]): # Update package depends $depends = explode(" ", $new_pkgbuild['depends']); foreach ($depends as $dep) { - $q = "INSERT INTO PackageDepends (PackageID, DepPkgID, DepCondition) VALUES ("; $deppkgname = preg_replace("/(<|<=|=|>=|>).*/", "", $dep); $depcondition = str_replace($deppkgname, "", $dep); @@ -374,8 +363,10 @@ if ($_COOKIE["AURSID"]): break; } - $deppkgid = create_dummy($deppkgname, $_COOKIE['AURSID']); - $q .= $packageID . ", " . $deppkgid . ", '" . mysql_real_escape_string($depcondition) . "')"; + $q = sprintf("INSERT INTO PackageDepends (PackageID, DepName, DepCondition) VALUES (%d, '%s', '%s')", + $packageID, + mysql_real_escape_string($deppkgname), + mysql_real_escape_string($depcondition)); db_query($q, $dbh); } diff --git a/web/html/rss.php b/web/html/rss.php index c9b87ec..cb0bf40 100644 --- a/web/html/rss.php +++ b/web/html/rss.php @@ -31,7 +31,6 @@ $rss->image = $image; #Get the latest packages and add items for them $dbh = db_connect(); $q = "SELECT * FROM Packages "; -$q.= "WHERE DummyPkg != 1 "; $q.= "ORDER BY SubmittedTS DESC "; $q.= "LIMIT 0 , 20"; $result = db_query($q, $dbh); diff --git a/web/lib/aur.inc b/web/lib/aur.inc index 430666c..0e494a2 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -331,12 +331,11 @@ function html_footer($ver="") { function can_submit_pkg($name="", $sid="") { if (!$name || !$sid) {return 0;} $dbh = db_connect(); - $q = "SELECT MaintainerUID, DummyPkg "; + $q = "SELECT MaintainerUID "; $q.= "FROM Packages WHERE Name = '".mysql_real_escape_string($name)."'"; $result = db_query($q, $dbh); if (mysql_num_rows($result) == 0) {return 1;} $row = mysql_fetch_row($result); - if ($row[1] == "1") { return 1; } $my_uid = uid_from_sid($sid); if ($row[0] === NULL || $row[0] == $my_uid) { diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index 0c69281..3570909 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -110,7 +110,7 @@ class AurJSON { $keyword_string = addcslashes($keyword_string, '%_'); $query = "SELECT " . implode(',', $this->fields) . - " FROM Packages WHERE DummyPkg=0 AND " . + " FROM Packages WHERE " . " ( Name LIKE '%{$keyword_string}%' OR " . " Description LIKE '%{$keyword_string}%' )"; $result = db_query($query, $this->dbh); @@ -136,7 +136,7 @@ class AurJSON { **/ private function info($pqdata) { $base_query = "SELECT " . implode(',', $this->fields) . - " FROM Packages WHERE DummyPkg=0 AND "; + " FROM Packages WHERE "; if ( is_numeric($pqdata) ) { // just using sprintf to coerce the pqd to an int diff --git a/web/lib/pkgfuncs.inc b/web/lib/pkgfuncs.inc index 699a3a9..d9e2c13 100644 --- a/web/lib/pkgfuncs.inc +++ b/web/lib/pkgfuncs.inc @@ -95,7 +95,6 @@ function package_exists($name="") { $dbh = db_connect(); $q = "SELECT ID FROM Packages "; $q.= "WHERE Name = '".mysql_real_escape_string($name)."' "; - $q.= "AND DummyPkg = 0"; $result = db_query($q, $dbh); if (!$result) {return NULL;} $row = mysql_fetch_row($result); @@ -109,10 +108,10 @@ function package_dependencies($pkgid=0) { $pkgid = intval($pkgid); if ($pkgid > 0) { $dbh = db_connect(); - $q = "SELECT DepPkgID, Name, DummyPkg, DepCondition FROM PackageDepends, Packages "; - $q.= "WHERE PackageDepends.DepPkgID = Packages.ID "; - $q.= "AND PackageDepends.PackageID = ". $pkgid; - $q.= " ORDER BY Name"; + $q = "SELECT pd.DepName, pd.DepCondition, p.ID FROM PackageDepends pd "; + $q.= "LEFT JOIN Packages p ON pd.DepName = p.Name "; + $q.= "WHERE pd.PackageID = ". $pkgid . " "; + $q.= "ORDER BY pd.DepName"; $result = db_query($q, $dbh); if (!$result) {return array();} while ($row = mysql_fetch_row($result)) { @@ -122,15 +121,14 @@ function package_dependencies($pkgid=0) { return $deps; } -function package_required($pkgid=0) { +function package_required($name="") { $deps = array(); - $pkgid = intval($pkgid); - if ($pkgid > 0) { + if ($name != "") { $dbh = db_connect(); - $q = "SELECT PackageID, Name, DummyPkg from PackageDepends, Packages "; - $q.= "WHERE PackageDepends.PackageID = Packages.ID "; - $q.= "AND PackageDepends.DepPkgID = ". $pkgid; - $q.= " ORDER BY Name"; + $q = "SELECT p.Name, PackageID FROM PackageDepends pd "; + $q.= "JOIN Packages p ON pd.PackageID = p.ID "; + $q.= "WHERE DepName = '".mysql_real_escape_string($name)."' "; + $q.= "ORDER BY p.Name"; $result = db_query($q, $dbh); if (!$result) {return array();} while ($row = mysql_fetch_row($result)) { @@ -140,38 +138,6 @@ function package_required($pkgid=0) { return $deps; } -# create a dummy package and return it's Packages.ID if it already exists, -# return the existing ID -# -function create_dummy($pname="", $sid="") { - if ($pname && $sid) { - $uid = uid_from_sid($sid); - if (!$uid) {return NULL;} - $dbh = db_connect(); - $q = "SELECT ID FROM Packages WHERE Name = '"; - $q.= mysql_real_escape_string($pname)."'"; - $result = db_query($q, $dbh); - if (!mysql_num_rows($result)) { - # Insert the dummy - # - $q = "INSERT INTO Packages (Name, Description, URL, SubmittedTS, "; - $q.= "SubmitterUID, DummyPkg) VALUES ('"; - $q.= mysql_real_escape_string($pname)."', 'A dummy package', '/#', "; - $q.= "UNIX_TIMESTAMP(), ".$uid.", 1)"; - $result = db_query($q, $dbh); - if (!$result) { - return NULL; - } - return mysql_insert_id($dbh); - } else { - $data = mysql_fetch_row($result); - return $data[0]; - } - } - return NULL; - -} - # Return the number of comments for a specified package function package_comments_count($pkgid = 0) { $pkgid = intval($pkgid); @@ -458,7 +424,7 @@ function pkg_search_page($SID="") { $q_from_extra = ""; } - $q_where = "WHERE Packages.DummyPkg = 0 "; + $q_where = "WHERE 1 = 1 "; // TODO: possibly do string matching on category // to make request variable values more sensible if (isset($_GET["C"]) && intval($_GET["C"])) { diff --git a/web/lib/stats.inc b/web/lib/stats.inc index f924fb5..a345c40 100644 --- a/web/lib/stats.inc +++ b/web/lib/stats.inc @@ -36,7 +36,7 @@ function updates_table($dbh) global $apc_prefix, $apc_ttl; $key = $apc_prefix . 'recent_updates'; if(!(EXTENSION_LOADED_APC && ($newest_packages = apc_fetch($key)))) { - $q = 'SELECT * FROM Packages WHERE DummyPkg != 1 ORDER BY ModifiedTS DESC LIMIT 0 , 10'; + $q = 'SELECT * FROM Packages ORDER BY ModifiedTS DESC LIMIT 0 , 10'; $result = db_query($q, $dbh); $newest_packages = new ArrayObject(); @@ -74,7 +74,7 @@ function general_stats_table($dbh) { global $apc_prefix; # AUR statistics - $q = "SELECT count(*) FROM Packages WHERE DummyPkg = 0"; + $q = "SELECT count(*) FROM Packages"; $unsupported_count = db_cache_value($q, $dbh, $apc_prefix . 'unsupported_count'); $q = "SELECT count(*) from Users"; diff --git a/web/template/pkg_details.php b/web/template/pkg_details.php index 1a1e611..ef2ba73 100644 --- a/web/template/pkg_details.php +++ b/web/template/pkg_details.php @@ -101,24 +101,22 @@ $out_of_date_time = ($row["OutOfDateTS"] == 0) ? $msg : gmdate("r", intval($row[ </p> <?php - # $deps[0] = array('id','name', 'dummy'); $deps = package_dependencies($row["ID"]); - $requiredby = package_required($row["ID"]); + $requiredby = package_required($row["Name"]); if (count($deps) > 0 || count($requiredby) > 0) { echo '<p>'; } if (count($deps) > 0) { - echo "<span class='boxSoftTitle'><span class='f3'>". __("Dependencies")."</span></span>"; while (list($k, $darr) = each($deps)) { - if ($darr[2] == 0) { - # $darr[3] is the DepCondition - echo " <a href='packages.php?ID=".$darr[0]."'>".$darr[1].$darr[3]."</a>"; + # darr: (DepName, DepCondition, PackageID), where ID is NULL if it didn't exist + if (!is_null($darr[2])) { + echo " <a href='packages.php?ID=".$darr[2]."'>".$darr[0].$darr[1]."</a>"; } else { - echo " <a href='http://www.archlinux.org/packages/?q=".$darr[1]."'>".$darr[1].$darr[3]."</a>"; + echo " <a href='http://www.archlinux.org/packages/?q=".$darr[0]."'>".$darr[0].$darr[1]."</a>"; } } @@ -128,17 +126,12 @@ $out_of_date_time = ($row["OutOfDateTS"] == 0) ? $msg : gmdate("r", intval($row[ } if (count($requiredby) > 0) { - echo "<span class='boxSoftTitle'><span class='f3'>". __("Required by")."</span></span>"; while (list($k, $darr) = each($requiredby)) { - if ($darr[2] == 0) { - echo " <a href='packages.php?ID=".$darr[0]."'>".$darr[1]."</a>"; - } else { - print "<a href='http://www.archlinux.org/packages/?q=".$darr[1]."'>".$darr[1]."</a>"; - } + # darr: (PackageName, PackageID) + echo " <a href='packages.php?ID=".$darr[1]."'>".$darr[0]."</a>"; } - } if (count($deps) > 0 || count($requiredby) > 0) { -- 1.7.4.2
On Wed, Mar 30, 2011 at 08:48:09PM -0500, Dan McGee wrote:
Instead, we just store dependencies directly in the PackageDepends table. Since we don't use this info anywhere besides the package details page, there is little value in precalculating what is in the AUR vs. what is not.
An upgrade path is provided via several SQL statements in the UPGRADING document. There should be no user-visible change from this, but the DB schema gets a bit more sane and we no longer have loads of junk packages in our tables that are never shown to the end user. This should also help the MySQL query planner in several cases as we no longer have to be careful to exclude dummy packages on every query.
Signed-off-by: Dan McGee <dan@archlinux.org> --- UPGRADING | 12 ++++++++ support/schema/aur-schema.sql | 5 +-- support/scripts/newpackage-notify | 2 +- web/html/pkgsubmit.php | 17 ++-------- web/html/rss.php | 1 - web/lib/aur.inc | 3 +- web/lib/aurjson.class.php | 4 +- web/lib/pkgfuncs.inc | 56 +++++++----------------------------- web/lib/stats.inc | 4 +- web/template/pkg_details.php | 21 ++++--------- 10 files changed, 41 insertions(+), 84 deletions(-)
Just a short feedback: Those patches have already been in my working tree for a few days now. I will double-check if there are no side effects - especially with the dummy package removal - before merging and pushing tho (just to be sure there's no unexpected breakage, e.g. this one will remove all dummy package pages, like [1] - tho they're broken anyways and have probably never been used by anyone). [1] https://aur.archlinux.org/packages.php?ID=42
Signed-off-by: Dan McGee <dan@archlinux.org> --- web/template/stats/general_stats_table.php | 49 +++++++++++----------------- 1 files changed, 19 insertions(+), 30 deletions(-) diff --git a/web/template/stats/general_stats_table.php b/web/template/stats/general_stats_table.php index 01f1fec..c136111 100644 --- a/web/template/stats/general_stats_table.php +++ b/web/template/stats/general_stats_table.php @@ -1,32 +1,21 @@ <table class='boxSoft'> -<tr> -<th colspan='2' class='boxSoftTitle'> -<span class='f3'><?php print __("Statistics") ?></span> -</th> -</tr> -<tr> -<td class='boxSoft'> -<span class='f4'><?php print __("Packages in unsupported"); ?></span> -</td> -<td class='boxSoft'><span class='f4'><?php print $unsupported_count; ?></span></td> -</tr> -<tr> -<td class='boxSoft'> -<span class='f4'><?php print __("Packages added or updated in the past 7 days"); ?></span> -</td> -<td class='boxSoft'><span class='f4'><?php print $update_count; ?></span></td> -</tr> -<tr> -<td class='boxSoft'> -<span class='blue'><span class='f4'><?php print __("Registered Users"); ?></span></span> -</td> -<td class='boxSoft'><span class='f4'><?php print $user_count; ?></span></td> -</tr> -<tr> -<td class='boxSoft'> -<span class='f4'><?php print __("Trusted Users"); ?></span> -</td> -<td class='boxSoft'><span class='f4'><?php print $tu_count; ?></span></td> -</tr> + <tr> + <th colspan='2' class='boxSoftTitle'><span class='f3'><?php print __("Statistics") ?></span></th> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages in unsupported"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $unsupported_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages added or updated in the past 7 days"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $update_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Registered Users"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $user_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Trusted Users"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $tu_count; ?></span></td> + </tr> </table> - -- 1.7.4.2
Signed-off-by: Dan McGee <dan@archlinux.org> --- web/lib/stats.inc | 18 ++++++++++++++++-- web/template/stats/general_stats_table.php | 20 ++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/web/lib/stats.inc b/web/lib/stats.inc index a345c40..75b2537 100644 --- a/web/lib/stats.inc +++ b/web/lib/stats.inc @@ -36,7 +36,7 @@ function updates_table($dbh) global $apc_prefix, $apc_ttl; $key = $apc_prefix . 'recent_updates'; if(!(EXTENSION_LOADED_APC && ($newest_packages = apc_fetch($key)))) { - $q = 'SELECT * FROM Packages ORDER BY ModifiedTS DESC LIMIT 0 , 10'; + $q = 'SELECT * FROM Packages ORDER BY ModifiedTS DESC LIMIT 10'; $result = db_query($q, $dbh); $newest_packages = new ArrayObject(); @@ -77,6 +77,9 @@ function general_stats_table($dbh) $q = "SELECT count(*) FROM Packages"; $unsupported_count = db_cache_value($q, $dbh, $apc_prefix . 'unsupported_count'); + $q = "SELECT count(*) FROM Packages WHERE MaintainerUID IS NULL"; + $orphan_count = db_cache_value($q, $dbh, $apc_prefix . 'orphan_count'); + $q = "SELECT count(*) from Users"; $user_count = db_cache_value($q, $dbh, $apc_prefix . 'user_count'); @@ -84,9 +87,20 @@ function general_stats_table($dbh) $tu_count = db_cache_value($q, $dbh, $apc_prefix . 'tu_count'); $targstamp = intval(strtotime("-7 days")); - $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $targstamp"; + $yearstamp = intval(strtotime("-1 year")); + + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $targstamp AND Packages.ModifiedTS = Packages.SubmittedTS"; + $add_count = db_cache_value($q, $dbh, $apc_prefix . 'add_count'); + + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $targstamp AND Packages.ModifiedTS != Packages.SubmittedTS"; $update_count = db_cache_value($q, $dbh, $apc_prefix . 'update_count'); + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS >= $yearstamp AND Packages.ModifiedTS != Packages.SubmittedTS"; + $update_year_count = db_cache_value($q, $dbh, $apc_prefix . 'update_year_count'); + + $q = "SELECT count(*) from Packages WHERE Packages.ModifiedTS = Packages.SubmittedTS"; + $never_update_count = db_cache_value($q, $dbh, $apc_prefix . 'never_update_count'); + include('stats/general_stats_table.php'); } diff --git a/web/template/stats/general_stats_table.php b/web/template/stats/general_stats_table.php index c136111..a100dfe 100644 --- a/web/template/stats/general_stats_table.php +++ b/web/template/stats/general_stats_table.php @@ -3,14 +3,30 @@ <th colspan='2' class='boxSoftTitle'><span class='f3'><?php print __("Statistics") ?></span></th> </tr> <tr> - <td class='boxSoft'><span class='f4'><?php print __("Packages in unsupported"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print __("Packages"); ?></span></td> <td class='boxSoft'><span class='f4'><?php print $unsupported_count; ?></span></td> </tr> <tr> - <td class='boxSoft'><span class='f4'><?php print __("Packages added or updated in the past 7 days"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print __("Orphan Packages"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $orphan_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages added in the past 7 days"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $add_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages updated in the past 7 days"); ?></span></td> <td class='boxSoft'><span class='f4'><?php print $update_count; ?></span></td> </tr> <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages updated in the past year"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $update_year_count; ?></span></td> + </tr> + <tr> + <td class='boxSoft'><span class='f4'><?php print __("Packages never updated"); ?></span></td> + <td class='boxSoft'><span class='f4'><?php print $never_update_count; ?></span></td> + </tr> + <tr> <td class='boxSoft'><span class='f4'><?php print __("Registered Users"); ?></span></td> <td class='boxSoft'><span class='f4'><?php print $user_count; ?></span></td> </tr> -- 1.7.4.2
participants (4)
-
Dan McGee
-
Dan McGee
-
elij
-
Lukas Fleischer