Variable in mysql query, grouping and ordering

Question

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!

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 http://stackoverflow.com/q/12859942/1723893 — NullPoiиteя, Jun 16 '13 at 01:23
what you tried to debug your code and tried to find reason ? — NullPoiиteя, Jun 16 '13 at 01:26

jeroen · Accepted Answer · 2013-06-16T01:45:52.700

5

A few things:

You need a space between LIMIT and the number;
You need to fix your sql injection problem, for example by casting the user-sent variable to int;
You need to get rid of the quotes in your sql statement around the LIMIT statement;
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.

edited Jun 16 '13 at 01:45

answered Jun 16 '13 at 01:27

jeroen

91,079
21
114
132

1

+1 for addressing the issue with example code and suggesting switch to better, more supported SQL interfaces. – Dave Chen Jun 16 '13 at 01:32
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 Jun 16 '13 at 01:58
@ManouHH You could try something like `SELECT *, MAX(time) FROM ...`. – jeroen Jun 16 '13 at 02:09

score 1 · Answer 2 · edited May 23 '17 at 11:50

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

score 0 · Answer 3 · answered Jun 16 '13 at 01:26

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.

score 0 · Answer 4 · answered Jun 16 '13 at 01:26

0

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

answered Jun 16 '13 at 01:26

AnuragD

327
3
15

Variable in mysql query, grouping and ordering

4 Answers4