⚡ Optimize removeClientVendas query performance#5
Conversation
Eliminated N+1 query issue in removeClientVendas by gathering all ids and performing a single batch delete with where_in(). Reduces database query count significantly. Co-authored-by: cezargf <25113573+cezargf@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request optimizes the removeClientVendas method in Clientes_model.php by refactoring individual database deletions inside a loop into bulk deletions using where_in. The reviewer suggested wrapping the deletion operations in a database transaction to ensure data integrity and recommended using array_column() for a more concise way to extract IDs from the input array.
| if (!empty($vendas_ids)) { | ||
| $this->db->where_in('vendas_id', $vendas_ids); | ||
| $this->db->delete('itens_de_vendas'); | ||
|
|
||
| $this->db->where('idVendas', $v->idVendas); | ||
| $this->db->where_in('idVendas', $vendas_ids); | ||
| $this->db->delete('vendas'); | ||
| } |
There was a problem hiding this comment.
To ensure data integrity, these deletion operations should be wrapped in a database transaction. This guarantees that either both deletions succeed or neither does, preventing a state where items are deleted but the sale record remains (or vice versa) if an error occurs during execution.
if (!empty($vendas_ids)) {
$this->db->trans_start();
$this->db->where_in('vendas_id', $vendas_ids);
$this->db->delete('itens_de_vendas');
$this->db->where_in('idVendas', $vendas_ids);
$this->db->delete('vendas');
$this->db->trans_complete();
if ($this->db->trans_status() === false) {
return false;
}
}| $vendas_ids = []; | ||
| foreach ($vendas as $v) { | ||
| $this->db->where('vendas_id', $v->idVendas); | ||
| $vendas_ids[] = $v->idVendas; | ||
| } |
💡 What: Replaced the
foreachloop that performed individualDELETEqueries inClientes_model::removeClientVendaswith an approach that collects all target IDs and executes bulkDELETEqueries using CodeIgniter's$this->db->where_in().🎯 Why: The previous implementation suffered from an N+1 query problem, executing 2 queries for every item in the
$vendasarray. This resulted in significant performance overhead for large datasets due to excessive database round trips.📊 Measured Improvement:
PR created automatically by Jules for task 18252179729418527296 started by @cezargf