3
\$\begingroup\$

This query is serving me a home page for users where a user can see all posts by him/her and his/her friends. This is an excellent query without any errors but I only wish to make it shorter if possible!

select U.fname, U.lname, P.postid, P.status, P.date, U.usernick, P.visible
from posts as P natural join users as U where
  U.visible=1 and P.visible=1
  and
  (P.userid='{$_SESSION['user_id']}' or P.userid in
    (select distinct U.userid from users as U inner join friends as F where 
      (U.userid=F.userid and F.friend='{$_SESSION['user_id']}' and F.status=1) or
      (U.userid=F.friend and F.userid='{$_SESSION['user_id']}' and F.status=1)))
  or
  (P.streamid='{$_SESSION['user_id']}' or P.streamid in
    (select distinct U.userid from users as U inner join friends as F where 
      (U.userid=F.userid and F.friend='{$_SESSION['user_id']}' and F.status=1) or
      (U.userid=F.friend and F.userid='{$_SESSION['user_id']}' and F.status=1)))
ORDER BY P.date DESC

CREATE  TABLE IF NOT EXISTS `users` (
  `userid` INT NOT NULL AUTO_INCREMENT ,
  `fname` VARCHAR(45) NOT NULL ,
  `lname` VARCHAR(45) NOT NULL ,
  `usernick` VARCHAR(45) NOT NULL ,
  `useremail` VARCHAR(45) NOT NULL ,
  `visible` INT(1) NOT NULL DEFAULT 1     //visible shows id is disables or not
 )

CREATE  TABLE IF NOT EXISTS `posts` (
  `postid` INT NOT NULL AUTO_INCREMENT ,   //Simple postid index
  `userid` INT NOT NULL ,                  //userid of poster
  `streamid` INT NOT NULL ,                // post is posted on his wall
  `status` VARCHAR(5000) NOT NULL ,        //the post that has been posted
  `date` TIMESTAMP NOT NULL DEFAULT now(), //post was posted on
  `visible` INT(1) NOT NULL DEFAULT 1 )    //post is deleted or not

CREATE  TABLE IF NOT EXISTS `network`.`friends` (
  `userid` INT NOT NULL ,                   //user who sent friend req
  `friend` INT NOT NULL ,                   //to whom friend req has been sent
  `status` INT(1) NOT NULL ,                //0 -> above both are not friends : 1->                   
                                            //both above are friends
  `date` TIMESTAMP NOT NULL DEFAULT now() , //
  `visible` INT(1) NOT NULL DEFAULT 1 )     //both are friends still
\$\endgroup\$
  • \$\begingroup\$ Do you want the code to be shorter or the query to execute faster? \$\endgroup\$ – Quentin Pradet May 7 '13 at 15:43
  • \$\begingroup\$ My ultimate goal is faster query. What i thought, lesser the number of operation/compare, lesser the time. \$\endgroup\$ – Mercurial May 7 '13 at 15:52
  • \$\begingroup\$ Not really. Denormalization, indexes, foreign keys, this is what you need. It could make the query simpler but will make other code more complex (eg. code that writes into the database). \$\endgroup\$ – Quentin Pradet May 7 '13 at 15:59
  • \$\begingroup\$ all the things you mentioned are well maintained in my database. I was worried about this long query. I thought this many operations and comparisions are slowing this down. Can you please take a look and confirm that it is well enough. \$\endgroup\$ – Mercurial May 7 '13 at 16:07
3
\$\begingroup\$

It looks like you are performing the same query against the Friends table twice. If you need that query, I would split it off and run that query first, passing the result to your main query as a parameter. But as you'll see at the end, I don't think that is necessary.

Also, you are using surrogate keys (yay!). When you say "natural join" I think you are joining on userid AND visible (because they have common names between the two tables). This is a waste, and also an error if you want posts that are visible and invisible. Be specific about your join:

 from posts P join users U on P.userid = U.userid

Why do you have a status column in Friends table and what is the difference between status and visible? Couldn't you just delete the Friend record if a user no longer wants that friend? The reason I'm asking is that you could limit the size of this table and at the same time add a unique constraint on (userid, friendid) so that you test for uniqueness on insert and don't have to in the query. Ensuring uniqueness takes time. If you aren't already getting unique results, then your first choice should be to restructure your table and your query so that you do.

UNIQUE KEY `friendship_unique` (`userid`,`friend`),

You may have to use InnoDB instead of MyISAM in order for that to work. MyISAM will accept it and silently ignore it because MyISAM does not support foreign keys or most constraints. For this reason, InnoDB may be faster for joins. With big tables I think it is. The one down side of using InnoDB vs. MyISAM is that the minimum time for inserting into an InnoDB table is about 5ms where MyISAM inserts are less than 1ms. In practice, this is only an issue if you are storing log files or session information in the database that needs to be updated with every request. You can always turn a single table from InnoDB to MyISAM later if you need the insert speed more than you need query speed, but that should be the exception rather than the rule.

You are making a lot of non-standard type choices for your column definitions. Why are you using int instead of bigint for your ID column? Why are you using int instead of bit for your boolean columns? Why aren't you using UTF-8? Why no keys? Are you using MyISAM tables because you don't want to pay to distribute this database, or is it just on a web server where it's free to use InnoDB? I think something like the following would be much more standard:

CREATE TABLE IF NOT EXISTS `user` (
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT
  `fname` VARCHAR(45) NOT NULL ,
  `lname` VARCHAR(45) NOT NULL ,
  `usernick` VARCHAR(45) NOT NULL ,
  `useremail` VARCHAR(45) NOT NULL ,
  `is_deleted` bit(1) NOT NULL DEFAULT b'0' COMMENT 'True if record is logically deleted.',
  PRIMARY KEY (`id`),
  UNIQUE KEY `id` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

CREATE TABLE IF NOT EXISTS `friend` (
  `id` bigint(20) unsigned NOT NULL
  `friend_id` bigint(20) unsigned NOT NULL
  `date` TIMESTAMP NOT NULL DEFAULT now() , //
  `is_deleted` bit(1) NOT NULL DEFAULT b'0' COMMENT 'True if record is logically deleted.',
  PRIMARY KEY (`id`),
  UNIQUE KEY `id` (`id`)
  UNIQUE KEY `user_friend_unique` (`id`,`friend_id`),
  KEY `friend_fk_user` (`id`),
  KEY `friend_fk_friend` (`friend_id`),
  CONSTRAINT `friend_fk_user` FOREIGN KEY (`id`) REFERENCES `user` (`id`) ON DELETE CASCADE,
  CONSTRAINT `friend_fk_friend` FOREIGN KEY (`friend_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8

OK, now you have clearly told MySQL to use InnoDB, to make your surrogate key unique, index on it, use it as a foreign key, and to delete records from the friend table when either of the corresponding user records are deleted. This ensures a much cleaner database - fewer records means faster queries.

Hmm... I also think you can restructure your query. Here are your friends posts:

select F.fname, F.lname, P.postid, P.status, P.date, U.usernick, P.is_deleted
  from friend F join post P on p.user_id = F.id
  where F.id = '{$_SESSION['user_id']}'

Here are your posts:

select U.fname, U.lname, P.postid, P.status, P.date, U.usernick, P.is_deleted
  from post P join user U on P.user_id = U.id
  where P.user_id = '{$_SESSION['user_id']}'

You should be able to do something like that without making any of the other changes I suggested.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You are performing the same query against the Friends table twice True, Noted. When you say "natural join" I think you are joining on userid AND visible (because they have common names between the two tables). This is a waste, and also an error if you want posts that are visible and invisible. Be specific about your join. Yes, that's my intention. I only want want visible posts from visible users so that ok part. \$\endgroup\$ – Mercurial May 8 '13 at 14:27
  • \$\begingroup\$ Why do you have a status column in Friends table and what is the difference between status and visible?... friend? The reason I'm asking is that you could limit the size of this table and at the same time add a unique constraint on (userid, friendid) True, adding visible to friend table is unnecessary. But adding unique column to prevent checking while insertion ... will help! :D. But after depending on Unique tuple while inserting tuple (1,2) i am totally not checking for (2,1). I must rely on PHP and other queries for this query to not come. (Sorry, My English) \$\endgroup\$ – Mercurial May 8 '13 at 14:30
  • \$\begingroup\$ Myisam VS InnoDB I didn't knew the query insertion part. I'll look into that. Thanks. lot of non-standard type choices for your column definitions. I was little confused on this, thanks for guiding. And i didn't posted my keys part in question i already have it. And for last part: If i use two queries, then how do i display results by concatenating them on basis of date!? Also, i am using infinite scroll for wall (Ajax). Thanks for giving me your time. \$\endgroup\$ – Mercurial May 8 '13 at 14:30
  • \$\begingroup\$ Well, i guess i am clear enough to mark this as solved. Thank you for your help. \$\endgroup\$ – Mercurial May 8 '13 at 14:32
  • \$\begingroup\$ I'm sad to see such a great answer with so few upvotes. \$\endgroup\$ – Quentin Pradet May 14 '13 at 11:51
1
\$\begingroup\$

Looking at your original query I think you might have issues with the precedence of the AND and OR parts of the WHERE clause.

However I agree with the above comments on the duplicate subselects, but not sure whether keeping those will still be the most efficient solution (might well depend on your data), as I can't really see a clean way of eliminating them.

First suggestion would be to split off the join to one specifying the userid columns, and then doing joins to the 2 subselects. The sub selects are both the same but the columns they join on are different. This might mean the join(s) are more efficient.

SELECT U.fname, U.lname, P.postid, P.status, P.date, U.usernick, P.visible
FROM posts P 
INNER JOIN users U 
ON P.userid = U.userid
AND U.visible = 1 
AND P.visible = 1
LEFT OUTER JOIN (
    SELECT distinct U.userid 
    FROM users as U 
    INNER JOIN friends as F 
    ON U.userid=F.userid
    AND F.status=1
    WHERE F.friend='{$_SESSION['user_id']}' or F.userid='{$_SESSION['user_id']}' ) Sub1 ON Sub1.userid = P.userid
LEFT OUTER JOIN (
    SELECT distinct U.userid 
    FROM users as U 
    INNER JOIN friends as F 
    ON U.userid=F.userid
    AND F.status=1
    WHERE F.friend='{$_SESSION['user_id']}' or F.userid='{$_SESSION['user_id']}' ) Sub2 ON Sub2.userid = P.streamid
WHERE P.userid='{$_SESSION['user_id']}' 
OR Sub1.userid IS NOT NULL
OR P.streamid='{$_SESSION['user_id']}' 
OR Sub2.userid IS NOT NULL
ORDER BY P.date DESC

2nd suggestion would be to do the join once but with an OR in the ON clause. 2 down sides of this is possible inefficiency of the join due to the OR, and also that this can possibly result in 2 matches, hence the need to use a DISTINCT to eliminate these duplicates.

SELECT DISTINCT U.fname, U.lname, P.postid, P.status, P.date, U.usernick, P.visible
FROM posts P 
INNER JOIN users U 
ON P.userid = U.userid
AND U.visible = 1 
AND P.visible = 1
LEFT OUTER JOIN (
    SELECT distinct U.userid 
    FROM users as U 
    INNER JOIN friends as F 
    ON U.userid=F.userid
    AND F.status=1
    WHERE F.friend='{$_SESSION['user_id']}' or F.userid='{$_SESSION['user_id']}' ) Sub1 ON Sub1.userid = P.userid OR Sub1.userid = P.streamid
WHERE P.userid='{$_SESSION['user_id']}' 
OR P.streamid='{$_SESSION['user_id']}' 
OR Sub1.userid IS NOT NULL
ORDER BY P.date DESC
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.