MySQL query optimisation?!

[cuigri]

I have a question guys...
I developed a webmaster-affiliate site. The query below is for selecting a banner to be displayed. I know joining 4 tables isn't that good, but I really couldn't get all infos in one table. Anyway I need some1 to tell me if I can optimise it somehow...
For mysql sake I think some conditions in the query can be put in php like:

 if( !($qid = db_query( "SELECT things FROM tables WHERE join_conditions" ))
or !($banner = mysql_fetch_object( $qid ))
or !$banner->active or !$banner->deleted )

{
header( "Location: bad_banner.jpg" );
exit;
}

SELECT banners.*, programs.cpm as pcpm, affprog.cpm AS apcpm, programs.name AS pname, users.flags AS aff_flags
FROM banners, programs, affprog, users
WHERE users.id = '".$aid."' AND banners.id = '".$bid."' AND banners.uid = '".$uid."' AND banners.pid = '".$pid."' AND banners.active = '1' AND programs.id = '".$pid."' AND affprog.uid = '".$uid."' AND affprog.pid = '".$pid."' AND affprog.aid = '".$aid."' AND affprog.deleted = '0' AND affprog.active = '".$PROGRAM_ACCEPTED."' AND programs.active = '".$PROGRAM_ACCEPTED."' AND programs.state = '1'"

 Thnx for any suggestion/idea!

[Weedpacket]

1) Only select the fields you need instead of selecting all of them with * (for example, you don't need to select banners.active because you know it's going to be equal to '1').

2) Are you using the appropriate types for your fields?

3) Do you have some suitable indexes defined?

4) Joining tables is not a bad thing: it's the whole point of relational databases. Are your tables properly normalised?

5) If you want to make it easier to read, consider writing it on more than one line.

6) What does db_query() do? If it's part of an abstraction layer, why do you then use mysql_fetch_object() instead of the layer's function?

7) Why do you check the value of $banner->active? It's going to be '1', isn't it? That's one of the conditions you're selecting on.

[cuigri]

Thank you for responding!

Originally posted by Weedpacket
1) Only select the fields you need instead of selecting all of them with * (for example, you don't need to select banners.active because you know it's going to be equal to '1').

Yes you right here! Thnx for noticing this...

2) Are you using the appropriate types for your fields?

3) Do you have some suitable indexes defined?

I have an agent that optimises tables once per day...

Actually table structure is this:

CREATE TABLE `banners` (
  `id` int(11) NOT NULL auto_increment,
  `uid` int(11) NOT NULL default '0',
  `pid` int(11) NOT NULL default '0',
  `aid` int(11) NOT NULL default '0',
  `url` varchar(250) default NULL,
  `title` varchar(50) default NULL,
  `clics` int(11) NOT NULL default '0',
  `displays` int(11) NOT NULL default '0',
  `cpmdispl` int(11) NOT NULL default '0',
  `cdate` datetime NOT NULL default '0000-00-00 00:00:00',
  `type` tinyint(4) NOT NULL default '0',
  `img_type` tinyint(2) NOT NULL default '0',
  `behaviour` tinyint(2) NOT NULL default '0',
  `location` varchar(255) default NULL,
  `txt` text,
  `height` mediumint(9) NOT NULL default '0',
  `width` mediumint(9) NOT NULL default '0',
  `active` tinyint(1) NOT NULL default '1',
  `last_display` datetime NOT NULL default '0000-00-00 00:00:00',
  PRIMARY KEY  (`id`),
  KEY `id` (`id`),
  KEY `uid` (`uid`),
  KEY `pid` (`pid`),
  KEY `aid` (`aid`),
  KEY `height` (`height`),
  KEY `width` (`width`)
) TYPE=MyISAM;

4) Joining tables is not a bad thing: it's the whole point of relational databases. Are your tables properly normalised?
5) If you want to make it easier to read, consider writing it on more than one line.

I format my code to be readable, but I don't know how this wysiwyg parses the code when displaying it...

   if( !isset( $bid ) or !$bid
    or !isset( $uid ) or !$uid
    or !isset( $aid ) or !$aid
    or !isset( $pid ) or !$pid
    or !($qid = db_query( "SELECT banners.*, programs.cpm as pcpm, affprog.cpm AS apcpm, programs.name AS pname, users.flags AS aff_flags ".
                          " FROM banners, programs, affprog, users ".
                          " WHERE users.id = '".$aid."' AND banners.id = '".$bid."' AND banners.uid = '".$uid."' AND banners.pid = '".$pid."' AND ".
                          " banners.active = '1' AND programs.id = '".$pid."' AND affprog.uid = '".$uid."' AND ".
                          " affprog.pid = '".$pid."' AND affprog.aid = '".$aid."' AND affprog.deleted = '0' AND ".
                          " affprog.active = '".$PROGRAM_ACCEPTED."' AND programs.active = '".$PROGRAM_ACCEPTED."' AND ".
                          " programs.state = '1'" ))
    or !($banner = mysql_fetch_object( $qid )) )
   {
       // put a vid banner here
       if( isset( $btype ) )
       ....
   }

6) What does db_query() do? If it's part of an abstraction layer, why do you then use mysql_fetch_object() instead of the layer's function?

I don't use a class for mysql connection, as a simple function library is enough for me...

7) Why do you check the value of $banner->active? It's going to be '1', isn't it? That's one of the conditions you're selecting on.

That was an example thinking that removing some conditions from mysql query and adding them at php level would make mysql's life easier with running this query...

I saw a mrtg report on net running on a mysql server that had 60k queries/sec for few months. This php module is the most accessed module on the site bcuz each affiliate that subscribes a webmaster program will use a javascript script that will call banner.php module. So this query will be run many times on mysql.
Some guys said that joining that many tables in a script that is run very often is not a good thing for mysql server which may crash bcuz of that. And I was wondering if it's that bad I done it like this...

Thnx again for your patience...

[bubblenut]

If you're using MySQL study the output from explain being run on your query. If you're using another database I'm sure it has a similar function but I'm afraid I wouldn't know what it is.

[cuigri]

Thnx for advice

[Weedpacket]

Originally posted by cuigri
That was an example thinking that removing some conditions from mysql query and adding them at php level would make mysql's life easier with running this query...

It might make MySQL's life easier, but it would mean transferring more data between it and PHP, and then PHP has to filter it. Almost certainly more expensive in the long run because PHP can't optimise the filtering at query time.

Couple of things: numeric types don't need quotes around their values - I don't know what MySQL does in that case, but maybe it has to stop and convert the string '1' into the integer 1. Does MySQL have a boolean type?

If I were writing the query for legibility I'd probably do something like

$query="Select
	b.*, p.cpm as pcpm, a.cpm as apcpm, p.name as pname, u.flags as aff_flags
from
	banners b, programs p, affprog a, users u
where
	b.active=1 and b.id=".$bid." and 	b.uid=".$uid." and	b.pid=".$pid." and

p.id=".$pid." and
p.state=1 and
p.active=".$PROGRAM_ACCEPTED." and

a.active=".$PROGRAM_ACCEPTED." and
a.deleted=0 and

a.pid=".$pid." and a.uid=".$uid." and a.aid=".$aid." and

u.id=".$aid;

Just spacing things out more, and reordering the filter conditions to make the linkages more explicit. Which leads me to ask: do you need to join the banners table to the affprog table by both pid and uid? Not knowing what your schema is, but if two of those tables have a 1:1 relationship then combining them into one table would be an idea. In fact, the same analysis goes for each individual pair of fields: if each different value of one field corresponds to precisely one possible value of the other field - and vice versa - then they ought to be in the same table.

[cuigri]

Originally posted by Weedpacket
It might make MySQL's life easier, but it would mean transferring more data between it and PHP, and then PHP has to filter it. Almost certainly more expensive in the long run because PHP can't optimise the filtering at query time.

So it is better to let all conditions in mysql. Didn't tought transferring data from mysql to php would take more time than parsing queries with more conditions... I mean I use this script very often. It should stand for 1000 reqests per second. So I want to optimise the script/query for speed, not necesary for resources. (altough this is important too)

Couple of things: numeric types don't need quotes around their values - I don't know what MySQL does in that case, but maybe it has to stop and convert the string '1' into the integer 1. Does MySQL have a boolean type?

Yes I know numeric types doesn't need quotes, but it a matter of security. We can take as example a query that makes an update that looks like this:

"UPDATE users SET age = ".$age." WHERE id = ".$CUSER->id

Where $age is a variable that user has to input in the web interface. This can look ok, but in fact it has flaws: User can input in the interface at age field something like "18 AND credits = 10000" which obvious will not update only age 🙂. Well it's not a good example, cuz I think the form will be validated anyway. And not by javascript which you can skip. I always think that bugs can come out bcuz of things I didn't think could happen. So I make this kind of things. Sometimes maybe "testing NULL pointer too much".

Just spacing things out more, and reordering the filter conditions to make the linkages more explicit. Which leads me to ask: do you need to join the banners table to the affprog table by both pid and uid? Not knowing what your schema is, but if two of those tables have a 1:1 relationship then combining them into one table would be an idea. In fact, the same analysis goes for each individual pair of fields: if each different value of one field corresponds to precisely one possible value of the other field - and vice versa - then they ought to be in the same table.

Why do I use pid AND uid to link tables: This is more a security thing... You see when an affiliate takes the javascript code to put into his page the script will have some parameters like banner id, affiliate id, webmaster id and webmaster program id. These are all numbers which can be guessed. So I couldn't put just banner id (bid) and affiliate id (aid) in it cuz it would be too simple. If some1 wants to guess a banner they will have to guess all the combination which are 4 integer numbers.
And there is not a 1:1 relationship bcuz we have affiliates, webmasters, webmasters programs, affiliate programs and banners.
Webmasters can create as many programs they want. (webmaster programs - that's programs table)
Webmaster programs can have as many banners as they want. (banners table)
An affiliate can join as many webmaster programs and which ones he wants. (that's an affiliate program - affprog table)
Affiliate chooses which banners from webmaster's program wants to use. And for that banner he chosen I give him the javascript code.
That's why I made the join from 4 tables...
I thought about having some info in one table like each banner to have too affiliate program details in their enreg, but thinking this site will grow this will mean lots of space waste. Anyway I must take this in consideration tho.

Thnx for your patience.

[jonny1]

Just a thought about what cuigri said for database security
Could use something like this before the sql query

if (!preg_match ("/^[0-9]/", $age)||strlen($age) >3")
{
echo "please provide your age";
exit;
}

Would ensure the age was only an integer and not longer than 99.