I am building a store. When a user decides to make a purchase I need to store it in the database, but since the purchase might be of more than 1 item I'm wondering which way is the best to insert multiple rows, here are my 3 options.
I use a database object that combines PDO with PDOStatement, which is why I don't define $stmt
anywhere.
The input is:
$input = array(
'user_id' => 15,
'food_id' => 2,
'quantity' => 15
);
The table I'm inserting into:
CREATE TABLE `users_foods` (
`user_id` int(10) unsigned NOT NULL,
`food_id` int(10) unsigned NOT NULL,
KEY `user_id` (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8
Option one
$insert_data = array();
while($input['quantity']){
$insert_data[] = array($input['user_id'], $input['food_id']);
$input['quantity']--;
}
$db->insert('users_foods', array('user_id', 'food_id'), $insert_data);
Respectively used functions are
public function insert($table, array $columns, array $values){
$valArr = array();
$data = array();
$id = 0;
if(!empty($values)){
if(is_array($values[0])){
foreach($values as $k => $v){
foreach($v as $key => $val){
$data[':i'.$id] = $val;
$v[$key] = ':i'.$id;
$id++;
}
$valArr[] = '('.join(',', $v).')';
}
}else{
foreach($values as $key => $val){
$data[':i'.$id] = $val;
$v[$key] = ':i'.$id;
$id++;
}
$valArr[] = '('.join(',', $v).')';
}
}
return $this->prepare('INSERT INTO '.$table.' ('.join(',', $columns).') VALUES '.join(',', $valArr))->execute($data);
}
private function parseWhereClause(array $where, $iteration = 0, $id = 0, $sd = null){
$sql = array();
$data = array();
if(count($where) == 3 && !is_array(reset($where))){
if(is_array($where[2])){
foreach($where[2] as $expr){
$sql[] = ':w'.$iteration.$id;
$data[':w'.$iteration.$id++] = $expr;
}
switch(strtolower($where[1])){
case 'in': $sql = $where[0] . ' ' . $where[1] . ' ('.join(',', $sql).')'; break;
case 'between': $sql = $where[0] . ' ' . $where[1] . ' '.join(' AND ', $sql); break;
}
}else{
$sql = $where[0] . ' ' . $where[1] . ' :w'.$iteration.$id;
$data[':w'.$iteration.$id] = $where[2];
}
}else{
if(!empty($where)){
$id = 0;
foreach($where as $c => $expressions){
$recur = $this->parseWhereClause($expressions, ++$iteration, $id++, $c);
$sql[] = $recur['sql'];
$data += $recur['data'];
}
$sql = '('.join(' '.$sd.' ', $sql).')';
}else{
$sql = '1';
}
}
return array(
'sql' => $sql,
'data' => $data
);
}
This method will only run one large query.
Option two
$db->prepare('INSERT INTO users_foods (user_id, food_id) VALUES (?, ?)');
while($input['quantity']){
$db->execute(array($input['user_id'], $input['food_id']));
$input['quantity']--;
}
not quite sure how this prepare-execute thing works under the hood though, but I believe that this will run 15 queries (because quantity is 15 in the example).
Option three
MySQL
CREATE PROCEDURE `buy_food`(IN `user_id` INT UNSIGNED, IN `food_id` INT UNSIGNED, IN `quantity` INT UNSIGNED)
BEGIN
WHILE quantity > 0 DO
INSERT INTO users_foods (`user_id`, `food_id`) VALUES (user_id, food_id);
SET quantity = quantity - 1;
END WHILE;
END
PHP
$db->prepare('CALL buy_food(?, ?, ?)')->execute(array(
$input['user_id'],
$input['food_id'],
$input['quantity']
));
Which one do you think is the best way to go and why?
2 Answers 2
It is a maxim in computer science that there are really "only three numbers": zero, one, and many. You have a case of many, and you should treat it as such. Otherwise, someday a customer might want to buy a hundred of some food item, and performance will be poor.
Therefore, I recommend...
Option four
Instead of having the table represent current supply levels, keep a transaction log, where the quantity
column stores purchases as positive values and consumption as negative values.
INSERT INTO user_food_purchase (user_id, food_id, quantity) VALUES (?, ?, ?);
Then, adjust the rest of your application to accommodate the schema change. For compatibility, you could define a view:
CREATE VIEW user_food AS
SELECT user_id, food_id, SUM(quantity) AS quantity
FROM user_food_purchase
GROUP BY user_id, food_id;
-
\$\begingroup\$ I'm not quite sure I follow :/ \$\endgroup\$php_nub_qq– php_nub_qq2014年09月22日 19:23:08 +00:00Commented Sep 22, 2014 at 19:23
-
7\$\begingroup\$ Insert just one row per transaction. (If you withdraw 47ドル from your bank account, the bank doesn't record that as 47 one-dollar withdrawal records, does it?) \$\endgroup\$200_success– 200_success2014年09月22日 19:24:56 +00:00Commented Sep 22, 2014 at 19:24
-
\$\begingroup\$ I totally agree, this is the best way to go. \$\endgroup\$Simon Forsberg– Simon Forsberg2014年09月22日 19:26:52 +00:00Commented Sep 22, 2014 at 19:26
-
\$\begingroup\$ Now I see. It used to be like this but then handling quantity became really sloppy. For example when a user does something with food with id 2, I have to update the table and reduce quantity, then if it's 0 I have to delete the row and this is just not giving me the feeling of clean coding. As opposed to now - 1 row = 1 in quantity, when used I simply delete with limit 1 and I'm done, all flows easily. I think the example with the bank is a little misleading \$\endgroup\$php_nub_qq– php_nub_qq2014年09月22日 19:30:26 +00:00Commented Sep 22, 2014 at 19:30
-
\$\begingroup\$ And the same with inserting, I have to do REPLACE instead of INSERT which would delete the entire row and create it again which is needless operations in the long run and a slow down in performance because HDD operations.. \$\endgroup\$php_nub_qq– php_nub_qq2014年09月22日 19:34:28 +00:00Commented Sep 22, 2014 at 19:34
I am building a store. When a user decides to make a purchase I need to store it in the database but since the purchase might be of more than 1 item I'm wondering which way is the best to insert multiple rows, here are my 3 options.
Who said you had to insert multiple rows?
If I buy 10 apples, that's one row: user_id = 42, food_id = 103, quantity = 10
.
Option one
You are creating a method to produce a very long SQL statement. This makes it very very very hard to follow. Avoid such methods! It will make it very confusing for Mr. Maintainer to understand what it actually does. Additionally, you are not using parametrized SQL correctly (which would be even more complicated, although possible, to use there). Just because you prepare a statement doesn't make it safe.
A concatenated prepared SQL statement is just as dangerous as a non-prepared one. It is the parametrization that can make it safe(r). It is hard to tell for me if this version is really vulnerable to SQL, because it is hard to keep track of all the concatenations. Either way, this is the worst in my opinion.
Option one and two
If quantity
is negative, these loops will never end.
Option three
Sure, this one would work fine as it is now I guess. However, this is not right as Option four (introduced by 200_success) is much better. And in my opinion, it would be overkill to use a SQL procedure for this (others might disagree).
I think currently, option two is the best, but it is doing the wrong thing.
Loop through all the kinds of items I am buying instead of looping over all the 10 apples.
It is much easier (requires less computing power and less work by the database) to loop over an array of (apples, pears, bananas)
(3 times) than to loop 10 times for apples, 7 times for pears, and 30 times for bananas. (I like bananas, OK? And I do plan to share some...)
-
\$\begingroup\$ I'm not quite an expert on databases, or anything really, but isn't that violating database normalization rules :? \$\endgroup\$php_nub_qq– php_nub_qq2014年09月22日 19:49:56 +00:00Commented Sep 22, 2014 at 19:49
-
4\$\begingroup\$ Your current schema violates database normalization even more. Suppose that a user has five apples, represented by five identical rows in the table. After consuming one apple, you would have to pick exactly one of the five identical rows representing apples to be deleted, which is an unusual operation to do. \$\endgroup\$200_success– 200_success2014年09月22日 20:11:07 +00:00Commented Sep 22, 2014 at 20:11
-
\$\begingroup\$ @php_nub_qq Actually, I would say that when you can have multiple identical rows in your database, that is violating database normalization. \$\endgroup\$Simon Forsberg– Simon Forsberg2014年09月22日 20:14:24 +00:00Commented Sep 22, 2014 at 20:14
Explore related questions
See similar questions with these tags.
$db
is an instance of a wrapper aroundPDO
: Get rid of it: You're calling$db->prepare
, which suggests it's an instance ofPDO
, but you're also calling$db->execute
, which is a method ofPDOStatement
. That's just awful. Aslo: aPDOStatement
has a methodcloseCursor
, meant to be used in case of multipleINSERT
queries \$\endgroup\$$db
is in the PS. Why would you suggest that is "awful". I think it is pretty handy, saving me a lot of lines. I can always callgetStmt()
and return thePDOStatement
object when I need it. \$\endgroup\$PDO
and fetching results throughPDOStatement
) violates the SRP, it also makes it hard to re-use a statement safely (I could be fetching results from X, processing them, and inserting them in Y, requiring at least 2 statements). You can never reliably callcloseCursor
or be sure the statement is not referenced anywhere. \$\endgroup\$