patternphpMinor
Building an SQL query string
Viewed 0 times
sqlquerystringbuilding
Problem
How can I write this PHP code better? It puts together an SQL query string from user input. The task is to return search results based on one or more text fields. The user can determine if partial matches are allowed.
$compop = $allowpartial ? "LIKE" : "="; // use LIKE for wildcard matching
$any = $allowpartial ? "%" : ""; // SQL uses % instead of * as wildcard
$namecondstr = ($name === "") ? "TRUE" : ("Name $compop '$any$name$any'");
$citycondstr = ($city === "") ? "TRUE" : ("City $compop '$any$city$any'");
$itemnocondstr = ($itemno === "") ? "TRUE" : ("ItemNo $compop '$any$itemno$any'");
$ordernocondstr = ($orderno === "") ? "TRUE" : ("OrderNo $compop '$any$orderno$any'");
$serialcondstr = ($serial === "") ? "TRUE" : ("Serial $compop '$any$serial$any'");
$sortstr = ($name !== "") ? "Name" :
(($city !== "") ? "City" :
(($itemno !== "") ? "ItemNo" :
(($orderno !== "") ? "OrderNo" :
"Serial")));
$query = "SELECT * From Licenses
LEFT JOIN Items
ON Licenses.LicenseID = Items.LicenseID
WHERE $namecondstr
AND $citycondstr
AND $itemnocondstr
AND $ordernocondstr
AND $serialcondstr
ORDER BY $sortstr, Licenses.LicenseID";Solution
Use Prepared Statements
Your code is vulnerable to SQL injection attacks. Use PDO or mysqli prepared statements to avoid this. See this answer for how to use PDO for this. If you are using mysql_* you should know that it is already in the deprecation process.
Miscellaneous
Your code is vulnerable to SQL injection attacks. Use PDO or mysqli prepared statements to avoid this. See this answer for how to use PDO for this. If you are using mysql_* you should know that it is already in the deprecation process.
Miscellaneous
- Consider using empty to check for empty strings (see comment from Corbin below).
- Personally I would use an if else rather than have two identical ternary conditions.
Context
StackExchange Code Review Q#19128, answer score: 6
Revisions (0)
No revisions yet.