0

I am trying to get a variable in a MySQL query.

$filter = $_GET['fltr'];
require_once('core/dbconnect.php');
if($filter)
{
    $limit = 'LIMIT '. $filter;
}
$lastonline = mysql_query("SELECT * FROM logins ORDER BY time DESC GROUP BY username '$limit'");

Now the problem is somewhere where I put '$limit' in the query. That doesn't work, but what is the proper way to do this?

It almost works okay now as I get a result only not the absolute correct one. I changed the code to this:

$filter = $_GET['fltr'];
require_once('core/dbconnect.php');
if($filter)
{
$limit = 'LIMIT '. (int) $filter;
}
$lastonline = mysql_query("SELECT * FROM logins GROUP BY username ORDER BY time DESC {$limit}");

As you can see I had to change GROUP BY and ORDER BY around as that doesnt work. I did put it in that order for a reason, as now it groups by username first but doesnt take the last login out anymore.

Anyone that knows a solution for this last issue in this query?

Thanks for all of your help in advance!

Improve this question
3
  • 4
    You need space between keyoword LIMIT and $filter Commented Jun 16, 2013 at 1:21
  • 1
    warning : sql injection still exist in world i mean your code is vulnerable to sql injection you need to escape all request properly or better use the pdo(prepared statement) ...see this stackoverflow.com/q/12859942/1723893 Commented Jun 16, 2013 at 1:23
  • 1
    what you tried to debug your code and tried to find reason ? Commented Jun 16, 2013 at 1:26

4 Answers 4

Reset to default
5

A few things:

  1. You need a space between LIMIT and the number;
  2. You need to fix your sql injection problem, for example by casting the user-sent variable to int;
  3. You need to get rid of the quotes in your sql statement around the LIMIT statement;
  4. You need to group before your order so you need to switch these two.

So the result would be:

if($filter)
{
    $limit = 'LIMIT '. (int) $filter;
}
$lastonline = mysql_query("SELECT * FROM logins GROUP BY username ORDER BY time DESC {$limit}");

And you should switch to PDO or mysqli and prepared statements as the mysql_* functions are deprecated.

Edit: To expand on the 4th point, according to the manual:

In general, clauses used must be given in exactly the order shown in the syntax description. For example, a HAVING clause must come after any GROUP BY clause and before any ORDER BY clause.

So ORDER BY comes after GROUP BY.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

+1 for addressing the issue with example code and suggesting switch to better, more supported SQL interfaces.
Works (sort of) okay now and thanks for warning me about the MySQL injection. There is only one problem.. I did ORDER BY before GROUP by for a reason. It had to take the last logins out then group by username as now it groups by username but doesnt take the last login anymore.
@ManouHH You could try something like SELECT *, MAX(time) FROM ....
1

why its not working because your query is like 

SELECT * FROM logins ORDER BY time DESC GROUP BY username 'LIMIT4'

while it must be like 

SELECT * FROM logins ORDER BY time DESC GROUP BY username LIMIT 4

why you are getting like this ? you are getting this because of the line below 

$limit = 'LIMIT'. $filter;

so instead try

require_once('core/dbconnect.php');
if(isset($_GET['fltr']))
{
  $lastonline = mysql_query("SELECT * FROM logins ORDER BY time DESC GROUP BY username LIMIT " . mysql_real_escape_string($limit));
}

and more mysql_* function are deprecated use PDO or Mysqli instead and imo use PDO instead

Good Read

Why shouldn't I use mysql_* functions in PHP?

Warning

Your code is vulnerable to sql injection you need to escape/sanitize all request properly

Improve this answer

Comments

0

Add the space as arma suggested, and consider setting $limit = ''; before the if ($filter) block (not strictly necessary, but good practice).

If this is just pseudocode, you can ignore the rest of this, but you really should consider sterilizing the $filter variable before blindly inserting it into a DB query, since mysql_query doesn't handle escaping and such. In this specific instance, `$filter = (int)$_GET['fltr']; should work just fine. Consider checking out the PHP MySQL PDO Object as well.

Improve this answer

Comments

0

First escape $filter , then put a space between limit and $filter . Like "LIMIT ".$filter

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.