0

I have two variables like this

$date1 = $_POST['f_date1'];
$date2 = $_POST['f_date2'];

is this the correct way of putting it inside?

$sql = "SELECT location, COUNT(*) as Referrals,
        SUM(CASE WHEN leadstatus = 'Hired' THEN 1 ELSE 0 END) as Hired,
        SUM(CASE WHEN leadstatus = 'Failed' THEN 1 ELSE 0 END) as Failed
        FROM vtiger_leadscf
        LEFT JOIN vtiger_leaddetails ON vtiger_leadscf.leadid = vtiger_leaddetails.leadid
        WHERE location > '' AND (date_table BETWEEN '$date1' AND '$date2')
        GROUP BY location 
        ORDER BY Referrals DESC";
4
  • Should be like this : AND (date_table BETWEEN '".$date1."' AND '".$date2."') Commented May 29, 2015 at 3:42
  • 2
    Only if you want to get hacked. Your code is vulnerable to SQL injection. You need to use prepared statements. Commented May 29, 2015 at 3:42
  • @RubahMalam that's just wrong. The two versions do the same thing. Commented May 29, 2015 at 3:43
  • Yes, @EdCottrell was right. Your code is easy to inject. Commented May 29, 2015 at 3:43

2 Answers 2

1

The way you do this depends on what interface you are using to mysql.

If you are using the (old and deprecated) mysql_* interface (you shouldn't be), then at a minimum, before you use your variables you need to escape them using mysql_real_escape_string().

eg:

$date1 = mysql_real_escape_string($_POST['f_date1']);
$date2 = mysql_real_escape_string($_POST['f_date2']);

After which yes your query construction is fine (for this method, which you shouldn't use).

Ideally, you need to be using either PDO or mysqli, both of which support prepared statements. This example will be PDO, just because.

$pdo = new PDO('mysql:host=localhost;dbname=whatever', 'username', 'password');

$stmt = $pdo->prepare("SELECT location, COUNT(*) as Referrals,
        SUM(CASE WHEN leadstatus = 'Hired' THEN 1 ELSE 0 END) as Hired,
        SUM(CASE WHEN leadstatus = 'Failed' THEN 1 ELSE 0 END) as Failed
        FROM vtiger_leadscf
        LEFT JOIN vtiger_leaddetails ON vtiger_leadscf.leadid = vtiger_leaddetails.leadid
        WHERE location > '' AND (date_table BETWEEN :startDate AND :endDate)
        GROUP BY location 
        ORDER BY Referrals DESC");

$stmt->execute(array(
  'startDate' => $date1,
  'endDate' => $date2
));

Note inside the query the use of :startDate and :endDate. those are placeholders which get filled by the associative array passed to $stmt->execute. Prepared statements are preferred as they prevent the nastiness that can occur when you simply concatenate unsanitised values into the query (look up: sql injection).

The mysqli_ interface is more similar to the deprecated mysql_ interface, but it also supports prepared statements.

mysqli method:

$mysqli = new mysqli('localhost', 'username', 'password', 'db');

$stmt = $mysqli->prepare("SELECT location, COUNT(*) as Referrals,
            SUM(CASE WHEN leadstatus = 'Hired' THEN 1 ELSE 0 END) as Hired,
            SUM(CASE WHEN leadstatus = 'Failed' THEN 1 ELSE 0 END) as Failed
            FROM vtiger_leadscf
            LEFT JOIN vtiger_leaddetails ON vtiger_leadscf.leadid = vtiger_leaddetails.leadid
            WHERE location > '' AND (date_table BETWEEN ? AND ?)
            GROUP BY location 
            ORDER BY Referrals DESC");

$stmt->bind_param("ss", $date1, $date2);

$stmt->execute();

Note the key difference there is the use of ? as the placeholder (pdo also supports this, i simply prefer named placeholders), and the way the variables are bound. The "ss" specifies the 'type' of the value being bound.

My personal preference is for PDO, purely because I prefer its execute call with the array parameter.

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

3 Comments

syntax is the same when i am using mysqli?
it's a little different. is that what you are using?
@marse okay there you go, updated slightly with mysqli
0

This way:

$sql = "SELECT location, COUNT(*) as Referrals,
        SUM(CASE WHEN leadstatus = 'Hired' THEN 1 ELSE 0 END) as Hired,
        SUM(CASE WHEN leadstatus = 'Failed' THEN 1 ELSE 0 END) as Failed
        FROM vtiger_leadscf
        LEFT JOIN vtiger_leaddetails ON vtiger_leadscf.leadid = vtiger_leaddetails.leadid
        WHERE location > '' AND (date_table BETWEEN '" . $date1 . "' AND '" . $date2 . "')
        GROUP BY location 
        ORDER BY Referrals DESC";

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.