Re: Why does this code/query hang & time out ?

* Jos Juffermans <jjuffermans@chellomedia.com>: > So far, I have found a few mistakes in your SQL: > > $query .= " OR variant LIKE > '%".$_GET['search']."%'"; > Why do you have backtick-quotes around variant? Know that backticks are used > to call system commands. You also use these backticks in the ORDER BY lines. > My guess is the backticks are causing your problems. Backticks can be used in MySQL to surround database, table, and fieldnames to ensure that MySQL does not attempt to parse them as reserved keywords. This would not cause a problem. > $query .= " AND make.makeID='".$_GET['make']."'"; > ... > $query .= " AND make.makeID<>0"; > If make.makeID is a number field, why is the value in quotes for the cases > where $_GET['make'] is set? Same with socket.socketID a few lines down. While it doesn't need to be quoted, again, in MySQL, this doesn't make a difference. Typically, you should quote the values just to be safe (this can also help prevent SQL injection). > Unrelated but very important, putting form variables (either GET or POST) > directly into a query is dangerous. If I create a copy of your form and fill > $_GET['search'] with eg: a%';DELETE * FROM model;SELECT * FROM model WHERE > something LIKE '%a > > Your query will be like this: SELECT ...... FROM .... WHERE .... AND > model.modelName LIKE '%a%';DELETE * FROM model;SELECT * FROM model WHERE > something LIKE '%a OR VARIANT .... > > You can't trust incoming variables. Absolutely. The OP should be filtering all user input *before* attempting to place it in the DB. To the OP: One reason the query may be running slow is lack of indexing. Index any fields you're selecting against (i.e. the fields referenced in your WHERE clause). > -----Original Message----- > From: -{ Rene Brehmer }- [mailto:steelrodent@metalbunny.net] > Sent: 31 March 2005 23:43 > To: php-db@lists.php.net > Subject: [PHP-DB] Why does this code/query hang & time out ? > > > Hi gang > > My CPU database (http://metalbunny.net/computers/cpudb.php) - still a work > in progress - used to be in 1 table, but for several reasons I've decided > it's better to split the data into multiple related tables. Obviously this > means I have to rewrite the query tool for it, and that's where my problem > lies. I've included the code I'm working with below, it's a little > different than the one for the above URL, as it uses my new faster > templates and relies more on the database than the old code did. > > All the DB connect stuff is in the template, and I use MySQL. The new > version isn't available online, it's only on my local development server. > I'm pretty sure it's simply a coding problem, but for the life of me I > can't find anything that looks wrong ... but then I've been staring at it > for hours... > > My problem came after I tried making it possible to pick 'all' as a search > option in make & model, and now, nomatter whether it's set to all or not, > and nomatter what's in the search field, the code stalls and hangs ... and > in the last tries, Firefox ended up closing down ... > > I tried putting athlon in the search box, and just leave everything on > default, and the generated query looks like this: > > SELECT > make.makeID,makeName,model.modelID,modelName,fsb2,socket.socketID,socketName > ,cpuID,variant,clock,multi,fsb,l1,l2,l3,vcore,vcache > FROM cpu_maker AS make,cpu_model AS model,cpu_socket AS socket,cpu_cpus AS > cpu WHERE make.makeID=model.makeID AND socket.socketID=model.socketID AND > cpu.modelID=model.modelID AND model.modelName LIKE '%athlon%' OR > variant > LIKE '%athlon%' AND make.makeID<>0 AND socket.socketID<>0 > > Leaving the search box empty produces no result - it's an unintended > leftover from the old code that I haven't found a good way to get around > yet. > > The code I'm working on looks like this (beware, it's rather long): > > <?php > // load dependencies > require('../include/sql.php'); > > // set data for template > $section = 'tools'; > $style2 = 'cputables.css'; > $title = 'CPU Database'; > $menu = true; > > // begin to build query string > $query = 'none'; > $basequery = 'SELECT > make.makeID,makeName,model.modelID,modelName,fsb2,socket.socketID,socketName > ,cpuID,variant,clock,multi,fsb,l1,l2,l3,vcore,vcache > FROM cpu_maker AS make,cpu_model AS model,cpu_socket AS > socket,cpu_cpus AS cpu > WHERE make.makeID=model.makeID AND > socket.socketID=model.socketID AND cpu.modelID=model.modelID'; > > // part 1, search parameters > if (! empty($_GET['search'])) { > $query = $basequery; > $setorder = true; > > $query .= " AND model.modelName LIKE > '%".$_GET['search']."%'"; > $query .= " OR variant LIKE > '%".$_GET['search']."%'"; > > if ($_GET['make'] != 'all') { > $query .= " AND make.makeID='".$_GET['make']."'"; > } else { > $query .= " AND make.makeID<>0"; > } > > if ($_GET['socket'] != 'all') { > $query .= " AND > socket.socketID='".$_GET['socket']."'"; > } else { > $query .= " AND socket.socketID<>0"; > } > > $linkquery = > substr($_SERVER['QUERY_STRING'],0,strpos($_SERVER['QUERY_STRING'], > '&order')); > } > > // part 2, sort order > if ($setorder) { > switch ($_GET['order']) { > case 'socket': > $query .= ' ORDER BY socketName'; > break; > case 'vcache': > $query .= ' ORDER BY vcache'; > break; > case 'vcore': > $query .= ' ORDER BY vcore'; > break; > case 'l2': > $query .= ' ORDER BY l2'; > break; > case 'l1': > $query .= ' ORDER BY l1'; > break; > case 'fsb': > $query .= ' ORDER BY fsb'; > break; > case 'multi': > $query .= ' ORDER BY multi'; > break; > case 'clock': > $query .= ' ORDER BY clock'; > break; > case 'variant': > $query .= ' ORDER BY variant'; > break; > case 'model': > $query .= ' ORDER BY modelName'; > break; > case 'make': > default: > $query .= ' ORDER BY makeName'; > break; > } > // part 2b, asc/desc > switch ($_GET['ad']) { > case 'd': > $query .= ' DESC'; > break; > case 'a': > default: > $query .= ' ASC'; > break; > } > } > > // include template > require('../include/temptop2.php'); > > if ($query != 'none') { > $result = mysql_query($query) or die('Unable to do > > query<br>'.mysql_error().'<br><br>'.$query.'<br>'); > $numrows = mysql_num_rows($result); > if (! isset($numrows)) { > $numrows = 0; > } > } > ?> > <p align="left" class="txt">This is a beta version of this database. > As > such some minor errors in the operation > can be expected, and all features may not work equally reliable. The > detailed information for when you click makes or model have not yet been > made for all CPUs, > but this work is underway. The database is not entirely up to date, or > complete, but as time permits and information becomes available, > it will be updated with the missing information.<br> > <br> > If you find any errors, or would like some additional functionality, plz > use the <a href="../contact.php" class="link">contact form</a> > to message > me.</p> > > <div align="center"> > <form action="<?php echo($_SERVER['PHP_SELF']); ?>" > method="get"> > <table width="560" cellpadding="0" cellspacing="0" > border="0"> > <tr> > <td width="13" height="30" align="left"><img > src="../images/menu_hdn_left.gif" width="13" height="30" > border="0"></td> > <td align="center" valign="middle" > background="../images/menu_hdn_back.gif" class="menu_hdn">Search for > CPUs...</td> > <td width="13" height="30" align="left"><img > src="../images/menu_hdn_right.gif" width="13" height="30" > border="0"></td> > </tr><tr> > <td colspan="3" height="5"><img > src="../images/bspace.gif" width="5" > height="5" border="0"></td> > </tr><tr> > <td height="10" align="right"><img > src="../images/silver_boxcrn_tl.gif" width="10" height="10" > border="0"></td> > <td height="10" > background="../images/silver_box_top.gif"><img > src="../images/bspace.gif" width="2" height="10" > border="0"></td> > <td width="10" height="10"><img > src="../images/silver_boxcrn_tr.gif" > width="10" height="10" border="0"></td> > </tr><tr> > <td width="10" align="right" > background="../images/silver_box_left.gif" class="menu_boxleft"><img > src="../images/bspace.gif" width="2" height="10" > border="0"></td> > <td align="left" background="../images/backgrnd.gif"> > <table border="0" cellpadding="0" cellspacing="0"> > <tr> > <td align="center" valign="top"> > <table border="0" cellpadding="0" > cellspacing="5"><table > border="0" cellpadding="2" cellspacing="0"> > <tr> > <td align="left"><span > class="smlwhtbld">Search > for:</span><br> > <input type="Text" name="search" > size="48"<?php if (! > empty($_GET['search'])) { echo(' > value="'.$_GET['search'].'"'); } ?>></td> > <td valign="bottom"><button > type="submit">Search</button></td> > </tr> > </table> > </td> > </tr><tr> > <td align="center" valign="top"> > <table border="0" cellpadding="0" > cellspacing="5"><table > border="0" cellpadding="2" cellspacing="0"> > <tr> > <td align="left"><span > class="smlwhtbld">Make:</span><br> > <select name="make"> > <option value="all">All > <?php > $itemquery = "SELECT makeID,makeName > FROM cpu_maker > ORDER BY makeName > ASC"; > $itemresult = mysql_query($itemquery) or die('Unable > to get items<br>'.mysql_error()); > > while ($item = mysql_fetch_array($itemresult)) { > echo('<option > value="'.$item['makeID'].'"'); > if ($_GET['make'] == $item['makeID']) > echo(' selected'); > echo('>'.$item['makeName']."\n"); > } > ?> > </select></td> > <td align="left"><span > class="smlwhtbld">Socket/slot:</span><br> > <select name="socket"> > <option value="all">All > <?php > $itemquery = "SELECT socketID,socketName > FROM cpu_socket > ORDER BY socketName > ASC"; > $itemresult = mysql_query($itemquery) or die('Unable > to get items<br>'.mysql_error()); > > while ($item = mysql_fetch_array($itemresult)) { > echo('<option > value="'.$item['socketID'].'"'); > if ($_GET['socket'] == $item['socketID']) > echo(' selected'); > echo('>'.$item['socketName']."\n"); > } > ?> > </select></td> > <td align="left"><span class="smlwhtbld">Sort > by:</span><br> > <select name="order"> > <option value="make">Make > <option value="model">Model > <option value="variant">Variant > <option value="type">Type > <option value="clock">Clock > <option value="multi">Multiplier > <option value="fsb">FSB > <option value="l1">L1 cache > <option value="l2">L2 cache > <option value="vcore">Vcore > <option value="vcache">Vcache > <option value="socket">Socket > </select></td> > <td align="left"><span > class="smlwhtbld">Asc./Desc.:</span><br> > <select name="ad"> > <option value="a">Ascending > <option value="d">Descending > </select></td> > </tr> > </table> > </td> > </tr><tr> > <td align="center" valign="top"><span > class="txtredbld">NOTE! > Due to the large amount of records in the database, > you cannot perform any searches with an empty search field. > You must enter atleast 1 character (letter or number) in the search > box.</span></td> > </tr> > </table> > </td> > <td width="10" background="../images/silver_box_right.gif" > class="menu_boxright"><img src="../images/bspace.gif" > width="2" height="10" > border="0"></td> > </tr><tr> > <td height="10" align="right"><img > src="../images/silver_boxcrn_bl.gif" width="10" height="10" > border="0"></td> > <td height="10" > background="../images/silver_box_bottom.gif"><img > src="../images/bspace.gif" width="2" height="10" > border="0"></td> > <td width="10" height="10"><img > src="../images/silver_boxcrn_br.gif" > width="10" height="10" border="0"></td> > </tr> > </table> > </form> > </div> > > <p align="left" class="txt">Click the headings to change the sort > order.<br> > All CPU makes and models are clickable and will reveal extra information > about these when clicked.</p> > > <div align="center"> > <?php > if ($query != 'none') { > echo('<span class="txt">'); > if ($numrows > 0) { > echo('Found '.$numrows.' matches to your query<br>'); > } > else { > echo('Could not find any matches to your query<br>'); > } > // echo('Query run: '.$query."</span><br><br>\n"); > } > ?> > <table border="0" cellpadding="2" cellspacing="0"> > <tr> > <td align="center" valign="middle" > class="tblhd1"><a > href="cpudb.php?<?php echo($linkquery); ?>&order=make" title="Sort by > Make" > class="tblhd">Make</a></td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=model" title="Sort by > > Model" class="tblhd">Model</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=variant" title="Sort > by > Variant" class="tblhd">Variant</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=clock" title="Sort by > > Clock" class="tblhd">Clock<br>(MHz)</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=multi" title="Sort by > > Multiplier" class="tblhd">Multip.</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=fsb" title="Sort by > FSB" > class="tblhd">FSB<br>(MHz)</td> > <td align="center" valign="middle" > class="tblhd">FSBx</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=l1" title="Sort by > L1" > class="tblhd">L1<br>(kB)</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=l2" title="Sort by > L2" > class="tblhd">L2<br>(kB)</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=l2" title="Sort by > L3" > class="tblhd">L3<br>(kB)</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=vcore" title="Sort by > > Vcore" class="tblhd">Vcore</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=vcache" title="Sort > by > Vcache" class="tblhd">Vcache</td> > <td align="center" valign="middle" class="tblhd"><a > > href="cpudb.php?<?php echo($linkquery); ?>&order=socket" title="Sort > by > Socket/Slot" class="tblhd">Socket /<br>Slot</td> > </tr> > <?php > // output data > if ($query != 'none' && $numrows > 0) { > while($cpu = mysql_fetch_array($result)) { > $makeName = htmlentities($cpu['makeName'],ENT_QUOTES); > $modelName = htmlentities($cpu['modelName'],ENT_QUOTES); > > echo('<tr>'); > echo('<td align="left" class="make"><a > href="cpu_data.php?make='.$cpu['makeID'].'" title="Show info > about > '.$makeName.'" > class="tbllink">'.$makeName.'</a></td>'); > echo('<td align="left" class="model"><a > > href=cpu_data.php?make='.$cpu['makeID'].'&model='.$cpu['modelID'].'" > > title="Show detailed info about '.$makeName.' '.$modelName.'" > class="tbllink">'.$modelName.'</a></td>'); > echo('<td align="left" class="variant">'); > if (empty($cpu['variant'])) { > echo('&nbsp;'); > } else { > echo(htmlentities($cpu['variant'],ENT_QUOTES)); > } > echo('</td>'); > echo('<td align="center" > > class="tbltxt">'.number_format($cpu['clock']).'</td>'); > echo('<td align="center" > class="tbltxt">'.$cpu['multi'].'&times;</td>'); > echo('<td align="center" > class="tbltxt">'.$cpu['fsb'].'</td>'); > echo('<td align="center" > class="tbltxt">&times;'.$cpu['fsb2'].'</td>'); > echo('<td align="center" class="tbltxt">'); > if (empty($cpu['l1'])) { > echo('&nbsp;'); > } else { > echo(number_format($cpu['l1'])); > } > echo('</td>'); > echo('<td align="center" class="tbltxt">'); > if (empty($cpu['l2'])) { > echo('&nbsp;'); > } else { > echo(number_format($cpu['l2'])); > } > echo('</td>'); > echo('<td align="center" class="tbltxt">'); > if (empty($cpu['l3'])) { > echo('&nbsp;'); > } else { > echo(number_format($cpu['l3'])); > } > echo('</td>'); > echo('<td align="center" class="tbltxt">'); > if (empty($cpu['vcore']) || $cpu['vcore'] == 0) { > echo('?'); > } else { > echo(number_format($cpu['vcore'],2).'</td>'); > } > echo('<td align="center" class="tbltxt">'); > if (empty($cpu['vcache'])) { > echo('N/A'); > } else { > echo(number_format($cpu['vcache'],2)); > } > echo('</td>'); > echo('<td align="center" > > class="tbltxt">'.htmlentities($cpu['socketName'],ENT_QUOTES).'</td>'); > echo("<tr>\n"); > } > } else { > echo('<tr>'); > echo('<td colspan="13" class="txt">No data for current > query</td>'."\n"); > echo("<tr>\n"); > } > ?> > </tr> > </table> > </div> > > <?php > mysql_close(); > > include('../include/tempbottom2.php'); > ?> > > Table structure in the database is pretty simple: > > cpu_cpus - cpuID = primary key, modelID = foreign key > cpu_model - modelID = primary key, makeID,socketID = foreign keys > cpu_socket - socketID = primary key > cpu_make - makeID = primary key > > The code worked fine until I turned the dropdown boxes back on ... but I > just can't see how they cause the code to screw up ... the query looks fine > to me ... > > Hope some of you can/will help... > > > TIA > > Rene > -- Matthew Weier O'Phinney | WEBSITES: Webmaster and IT Specialist | http://www.garden.org National Gardening Association | http://www.kidsgardening.com 802-863-5251 x156 | http://nationalgardenmonth.org mailto:matthew@garden.org | http://vermontbotanical.org