MySQL, hoe kan dit beter?

[Verwijderd]

Beste Tweakers,

Om mijn website te optimaliseren ben ik allereerst eens gaan kijken in mijn index-pagina, hier heb ik deze code:

code:

if(mysql_result(mysql_query("SELECT COUNT(id) FROM `coke_werknemers` WHERE `crimineel`='".$_SESSION['login']."' AND accepted='ja'"), 0) > 0)
{    
$lol  = mysql_query("SELECT `fabriek_id` FROM `coke_werknemers` WHERE crimineel='".$_SESSION['login']."' LIMIT 0,1");
$ifab = mysql_fetch_assoc($lol);
 $kope = mysql_query("SELECT `id` FROM `cokefabriek` WHERE id='".addslashes($ifab['fabriek_id'])."' LIMIT 0,1") or die(mysql_error());
$i = mysql_fetch_assoc($kope);
echo '<li><a href="./?p=cokefabriek-buy&id='.$i['id'].'">'.$menu['cokefabriek'].'</a></li>';
 }

Op de rijen id & fabriek_id in tabel coke_werknemers staan een index, en in de tabel cokefabriek staat ook een index.

Wat zou ik hier eventueel nog aan kunnen verbeteren?
Andere suggesties hoor ik ook graag!

Mvg
Bulletstar

[ Voor 11% gewijzigd door Verwijderd op 04-08-2008 22:24 ]

[_Apache_]

Waar heb je last van? Verkeerde results, trage query? Waar wil je precies heen?

Anders gebruik je een explain om te kijken wat het langste duur en kan zien wat je kan bijslijpen.

[Verwijderd]

Je zou mysql_result, mysql_query en mysql_fetch_assoc kunnen verplaatsen naar een database adapter class. Je zou die class fatsoenlijke foutafhandelijng kunnen laten doen (want dat doe je nu niet), je kunt eventueel veel makkelijker profilen, en je code wordt er een stuk schoner van.

Verder kun je van de eerste twee queries er makkelijk eentje schrappen.

[Verwijderd]

Verwijderd schreef op maandag 04 augustus 2008 @ 22:28:
Je zou mysql_result, mysql_query en mysql_fetch_assoc kunnen verplaatsen naar een database adapter class. Je zou die class fatsoenlijke foutafhandelijng kunnen laten doen (want dat doe je nu niet), je kunt eventueel veel makkelijker profilen, en je code wordt er een stuk schoner van.

Verder kun je van de eerste twee queries er makkelijk eentje schrappen.

Nou het werkt prima, alleen op de index pagina worden al enkele noodzakelijke querys uitgevoerd (user gegevens ophalen en dat soort zaken, fouten zijn er niet)

Hoe zouden die in een kunnen dan? Aangezien ik geen verstand van een 'join' heb

[Noork]

Verwijderd schreef op maandag 04 augustus 2008 @ 22:30:
[...]
Hoe zouden die in een kunnen dan? Aangezien ik geen verstand van een 'join' heb

Ergens een basiscursusje sql kopen? Of een pdf'je ergens van internet trekken.

Je kan ook een programma zoals Navicat gebruiken om je queries mee te bouwen. Maar dan nog is enige kennis vooraf wel handig, aangezien je moet kiezen tussen verschillende typen joins.

[ Voor 14% gewijzigd door Noork op 04-08-2008 22:34 ]

[Verwijderd]

Ik heb er zelf wel naar gekeken, inner join, left join etc.
Alleen krijg ik niet de goede query, ik zal zometeen een voorbeeld maken wellicht dat iemand direct al ziet wat ik fout doe.

[Mei]

Ik zou eens even gaan Googlen/Wikien naar info over SQL injections. verder is het misschien handig een centrale plek aan te maken voor al je DB-gerelateerde spullen, waar automatisch SQL injection tegengegaan wordt en je error reporting op een nettere manier dan met die(mysql_error()) wordt afgehandeld (bijvoorbeeld door hem aan een centrale lijst/log met berichten toe te voegen).

[Verwijderd]

Verwijderd schreef op maandag 04 augustus 2008 @ 22:30:

Nou het werkt prima, alleen op de index pagina worden al enkele noodzakelijke querys uitgevoerd (user gegevens ophalen en dat soort zaken, fouten zijn er niet)

Het werkt misschien prima, maar je vroeg hoe het beter kan.

Hoe zouden die in een kunnen dan? Aangezien ik geen verstand van een 'join' heb

Je hebt geen join nodig.
De eerste query kun je veranderen in:

SQL:

SELECT `fabriek_id` FROM `coke_werknemers`
WHERE `crimineel`='blah' AND `accepted`='ja'
LIMIT 0,1

De COUNT kun je weglaten. Als deze query een resultaat heeft is het goed, anders niet en moet het blok worden overgeslagen.

Overigens zie ik niet in wat je met de 3e query wilt bereiken. Je selecteert het id waar de id gelijk is aan $ifab['fabriek_id'], maar de laatste is al bekend. Als je meer informatie uit de cokefabriek tabel wilt halen, kun je misschien beter wel een join gebruiken, sowieso heb je hier maar 1 query nodig. Voor joins: zie de MySQL manual.

[ Voor 18% gewijzigd door Verwijderd op 04-08-2008 22:40 ]

[Verwijderd]

Ah je hebt helemaal gelijk ja, ik dacht direct al te moeilijk!

[Gomez12]

Gewoon eventjes een tussendoor vraagje, wat doet die addslashes op regel 5 daar?

Als het gaat om input die je niet vertrouwt zijn er handiger en betere functies ( vb. mysql_real_escape ), maar als het puur om een id gaat ( met een integer type in de dbase ) dan zou ik het wel vertrouwen en weglaten.

Op het moment dat je het gaat joinen doe je deze check toch niet meer...

[Verwijderd]

Die zit daar omdat sommige bijvoorbeeld heten: Sjonnie's Sjop

[Gomez12]

Aaah, een string als id gebruiken...

Nou ja, ik zou er niet voor kiezen en menig dbase zal trager zijn met char / varchar / text lookups dan int lookups...

Nieuwsgierig vraagje, waarom noem je dit een fabriek_id? Ik zou er gewoon voor kiezen om een aparte tabel te hebben : fabrieken en daarin heeft elke fabriek een numerieke id en een naam, zodat de dbase intern met numerieke id's werkt en de strings alleen voor de representatie zijn, niet voor de logica tussen de tabellen...
En helemaal als je iets id noemt verwacht ik gewoon een numeriek iets. Ik zou er niet zo snel een string bij verwachten...

[Mei]

Verwijderd schreef op maandag 04 augustus 2008 @ 22:42:
Die zit daar omdat sommige bijvoorbeeld heten: Sjonnie's Sjop

Da's dus tegen SQL injection. Zoals de bovenstaande poster al zei zijn daar veel betere methodes voor. Drupal, bijvoorbeeld, heeft db_query(), waar je met behulp van placeholders je dynamische data aan een query doorgeeft. Deze functie roept _db_query_callback() aan, die voor de input filtering zorgt.

Booleans sla je trouwens op als 0 en 1, niet als 'ja' en 'nee'. Met 0 en 1 kan je bijvoorbeeld heel gemakkelijk booleaanse vergelijkingen uitvoeren. De strings die jij gebruikt moet je eerst nog omzetten.

[ Voor 16% gewijzigd door Mei op 04-08-2008 22:53 . Reden: Even linkjes gefixt. Vergeten dat GoT bbcode gebruikt en geen HTML O-) ]

[Verwijderd]

Mei schreef op maandag 04 augustus 2008 @ 22:50:
[...]


Da's dus tegen SQL injection. Zoals de bovenstaande poster al zei zijn daar veel betere methodes voor. Drupal, bijvoorbeeld, heeft <a href="http://api.drupal.org/api/function/db_query">db_query()</a>, waar je met behulp van placeholders je dynamische data aan een query doorgeeft. Deze functie roept <a href="http://api.drupal.org/api/function/_db_query_callback">_db_query_callback()</a> aan, die voor de input filtering zorgt.

Booleans sla je trouwens op als 0 en 1, niet als 'ja' en 'nee'. Met 0 en 1 kan je bijvoorbeeld heel gemakkelijk booleaanse vergelijkingen uitvoeren. De strings die jij gebruikt moet je eerst nog omzetten.

Thnx!

[Verwijderd]

Gomez12 schreef op maandag 04 augustus 2008 @ 22:49:
Aaah, een string als id gebruiken...

Nou ja, ik zou er niet voor kiezen en menig dbase zal trager zijn met char / varchar / text lookups dan int lookups...

Nieuwsgierig vraagje, waarom noem je dit een fabriek_id? Ik zou er gewoon voor kiezen om een aparte tabel te hebben : fabrieken en daarin heeft elke fabriek een numerieke id en een naam, zodat de dbase intern met numerieke id's werkt en de strings alleen voor de representatie zijn, niet voor de logica tussen de tabellen...
En helemaal als je iets id noemt verwacht ik gewoon een numeriek iets. Ik zou er niet zo snel een string bij verwachten...

Nu ik dit zo zie denk ik eerder dat het eerst fabriek_naam was oid.. iig addslashes weggehaald

[Verwijderd]

Boolean nodig? Bit
En idd, unique primary key op een textfield. Ook niet mijn eerste keus.

[ Voor 64% gewijzigd door Verwijderd op 04-08-2008 23:48 ]

[Voutloos]

Mei schreef op maandag 04 augustus 2008 @ 22:50:
[...]
Da's dus tegen SQL injection. Zoals de bovenstaande poster al zei zijn daar veel betere methodes voor.

Of gebruik in ieder geval minsten mysql_real_escape_string() voor strings.

Verwijderd schreef op maandag 04 augustus 2008 @ 22:53:
[...]
Nu ik dit zo zie denk ik eerder dat het eerst fabriek_naam was oid.. iig addslashes weggehaald

Nee, strings moet je gewoon altijd correct escapen.

Anyway lees je dus eerst maar een paar uurtjes in voor wat betreft zo goed mogelijk passende data types, goede (synthetische) keys, joins en sql injection.

Overigens kunnen je var namen ook beter, evenals je code/sql layout. En als je enkel rijen wil tellen is count(*) beter.

Gomez12 schreef op maandag 04 augustus 2008 @ 22:41:
maar als het puur om een id gaat ( met een integer type in de dbase ) dan zou ik het wel vertrouwen en weglaten.

Dan forceer je gewoon altijd dat het een int is. Niets vertrouwen, niets weglaten. Je wil gewoon direct bij de query zien (of weten dat je db class het adhv je dynamische query kan) kunnen zien dat je goed zit. Maar goed, dit specifieke geval kan inderdaad gewoon met een te basic join query.

[ Voor 30% gewijzigd door Voutloos op 05-08-2008 08:22 ]

[Verwijderd]

Ik ga me vandaag zeker inlezen over types, keys, joins, sql injecties & indexen .

En kan iemand anders nog bevestigen dat count(*) beter is ?
Ook qua snelheid of merk je daar weinig van?

[Voutloos]

Dat staat gewoon in de manual.

quote: http://dev.mysql.com/doc/...tions.html#function_count

Returns a count of the number of non-NULL values of expr in the rows retrieved by a SELECT statement....
COUNT(*) is somewhat different in that it returns a count of the number of rows retrieved...COUNT(*) is optimized

Ja, die optimalisatie gaat niet op bij elke engine, maar je kan gewoon count(*) == aantal rows, count(expr) is aantal expressie not null onthouden.

[Verwijderd]

Bedankt!

[remmelt]

En als je dan toch aan het inlezen bent: PDO http://nl.php.net/pdo en met name het gedeelte over prepared statements alhier http://nl.php.net/manual/en/pdo.prepared-statements.php. Ik kan me de laatste keer dat ik direct de mysql_* functies heb gebruikt niet meer herinneren, ook niet voor kleine projectjes.