Query Reviews für Teams: Was in Code Reviews geprüft werden sollte

1. Warum SQL Code Reviews einen eigenen Fokus brauchen

SQL Code Review ist in vielen Teams ein blinder Fleck. PHP- und JavaScript-Code wird auf Lesbarkeit, Testbarkeit und Architektur geprüft — SQL-Abfragen in Repositories und Collections dagegen oft nur auf syntaktische Korrektheit. Das ist ein Fehler: Die teuersten Bugs in produktiven Magento-Shops entstehen nicht durch falsche PHP-Logik, sondern durch Queries, die lokal in Millisekunden laufen und unter Last Minuten brauchen.

Die folgende Checkliste enthält zehn Punkte, die bei jedem SQL Code Review geprüft werden sollten. Für jeden Punkt gibt es ein konkretes Beispiel — erst die problematische Variante, dann die korrigierte. Die meisten Punkte lassen sich mechanisch prüfen und eignen sich daher auch als Grundlage für automatisierte Code-Review-Checks.

2. Fehlende WHERE-Klausel bei UPDATE/DELETE

Ein UPDATE oder DELETE ohne WHERE-Klausel betrifft alle Zeilen der Tabelle. Das passiert seltener durch Absicht als durch Refactoring-Fehler — eine Bedingung wird auskommentiert, ein Parameter fehlt, eine Variable ist leer. Der Effekt auf dem Produktivsystem ist oft irreversibel.

-- WRONG: Updates ALL orders in the table
UPDATE sales_order SET status = 'processing';

-- CORRECT: Always include a specific WHERE condition
UPDATE sales_order
SET    status     = 'processing',
       updated_at = NOW()
WHERE  entity_id  = :entity_id
  AND  status     = 'pending';

Im Code Review ist besonders auf parametrisierte WHERE-Bedingungen zu achten: Wenn ein Parameter per Anwendungslogik leer werden kann, muss das abgefangen werden, bevor die Query ausgeführt wird. Ein leeres Array als IN-Liste sollte die Query ganz verhindern, nicht zu WHERE entity_id IN () führen.

3. SELECT * statt gezielter Spaltenauswahl

SELECT * lädt alle Spalten — auch solche, die die Anwendung nicht benötigt. Das kostet Netzwerkbandbreite, erhöht die Speichernutzung und bricht leise, wenn das Schema sich ändert und neue Spalten hinzukommen, die die Anwendungslogik nicht erwartet. In Magento-Tabellen mit VARCHAR(255)- und TEXT-Spalten kann das erhebliche Mengen unnötiger Daten bedeuten.

-- WRONG: Loads all columns including large text fields
SELECT * FROM sales_order WHERE entity_id = :id;

-- CORRECT: Select only the columns the application actually needs
SELECT
    entity_id,
    increment_id,
    status,
    grand_total,
    customer_email,
    created_at
FROM sales_order
WHERE entity_id = :id;

4. N+1-Queries in Schleifen

Das N+1-Problem entsteht, wenn eine Liste von Datensätzen geladen wird und anschließend für jeden Datensatz in einer Schleife eine weitere Query ausgeführt wird. Das Ergebnis: statt einer Query werden N+1 Queries ausgeführt, wobei N die Anzahl der geladenen Datensätze ist. Bei 100 Bestellungen mit je einer Kundenabfrage sind das 101 Queries.

-- WRONG PATTERN: N+1 queries in application code
-- Step 1: load orders
SELECT entity_id, customer_id FROM sales_order WHERE status = 'pending' LIMIT 100;

-- Step 2 (inside a loop, executed N times):
SELECT email, firstname, lastname FROM customer_entity WHERE entity_id = :customer_id;

-- CORRECT: Single query with JOIN
SELECT
    o.entity_id,
    o.increment_id,
    o.grand_total,
    o.status,
    c.email,
    c.firstname,
    c.lastname
FROM sales_order o
LEFT JOIN customer_entity c ON c.entity_id = o.customer_id
WHERE o.status = 'pending'
ORDER BY o.created_at DESC
LIMIT 100;

5. Fehlender Index auf JOIN- oder WHERE-Spalte

Ein JOIN auf eine nicht-indexierte Spalte erzwingt für jede Zeile der treibenden Tabelle einen vollständigen Scan der Join-Tabelle. Bei Tabellen mit Millionen von Zeilen ist das ein gravierendes Performance-Problem. Im Code Review sollte für jede JOIN-Bedingung und jede häufig genutzte WHERE-Spalte geprüft werden, ob ein Index vorhanden ist.

-- Check if an index exists on a column used in JOIN/WHERE
SHOW INDEX FROM sales_order_item WHERE Key_name != 'PRIMARY';

-- Missing index example: joining on a column without index
-- EXPLAIN shows "ALL" type and no key used
EXPLAIN
SELECT o.increment_id, oi.sku, oi.qty_ordered
FROM sales_order o
JOIN sales_order_item oi ON oi.order_id = o.entity_id
WHERE oi.sku = 'CUSTOM-001';

-- Add index if missing (use pt-osc on production tables)
-- ALTER TABLE sales_order_item ADD INDEX idx_sku (sku);

-- Always verify with EXPLAIN after adding the index
EXPLAIN
SELECT o.increment_id, oi.sku, oi.qty_ordered
FROM sales_order o
JOIN sales_order_item oi ON oi.order_id = o.entity_id
WHERE oi.sku = 'CUSTOM-001';

6. SARGable-Verletzung: Funktion auf indexierter Spalte

Eine Query ist SARGable (Search ARGument ABLE), wenn der MySQL-Optimizer für die WHERE-Bedingung einen Index verwenden kann. Das ist nicht möglich, wenn eine Funktion auf der indexierten Spalte aufgerufen wird — der Index gilt dann für die transformierten Werte nicht. Das häufigste Beispiel ist YEAR(created_at) = 2024 statt eines Datumsbereichs.

-- WRONG: Function on indexed column prevents index usage (non-SARGable)
SELECT entity_id FROM sales_order
WHERE YEAR(created_at) = 2024
  AND MONTH(created_at) = 10;

-- WRONG: Same problem with DATE()
SELECT entity_id FROM sales_order
WHERE DATE(created_at) = '2024-10-15';

-- CORRECT: Use a range condition — allows index range scan
SELECT entity_id, increment_id, grand_total
FROM sales_order
WHERE created_at >= '2024-10-01 00:00:00'
  AND created_at <  '2024-11-01 00:00:00';

-- CORRECT: Same for a single day
SELECT entity_id, increment_id
FROM sales_order
WHERE created_at >= '2024-10-15 00:00:00'
  AND created_at <  '2024-10-16 00:00:00';

7. Fehlendes LIMIT bei großen Ergebnismengen

Queries ohne LIMIT auf Tabellen, die theoretisch beliebig viele Zeilen zurückgeben können, sind ein latentes Risiko. Lokal mit hundert Testdatensätzen ist das kein Problem. Auf dem Produktivsystem mit Millionen von Zeilen kann eine solche Query alle Verbindungsslots blockieren. Im Code Review sollte jede Abfrage ohne LIMIT erklärt und begründet werden.

-- WRONG: Could return millions of rows without LIMIT
SELECT entity_id, increment_id, grand_total
FROM sales_order
WHERE status = 'pending';

-- CORRECT: Always add LIMIT for potentially large result sets
-- Use pagination for full data access
SELECT entity_id, increment_id, grand_total
FROM sales_order
WHERE status = 'pending'
  AND entity_id > :last_seen_id
ORDER BY entity_id ASC
LIMIT 500;

8. Hardcodierte IDs: store_id = 1

Hardcodierte IDs wie store_id = 1 oder website_id = 1 funktionieren im Entwicklungssetup, brechen aber in Multi-Store-Umgebungen still. Im Code Review ist auf jeden hardcodierten Wert zu achten, der vom Setup abhängig ist — entity_type_id, attribute_set_id, store_id, website_id und customer_group_id sind die häufigsten Kandidaten.

-- WRONG: Hardcoded store_id breaks in multi-store setups
SELECT entity_id, sku
FROM catalog_product_entity
WHERE store_id = 1;

-- WRONG: Hardcoded attribute_id (varies between installations)
SELECT value FROM catalog_product_entity_decimal
WHERE attribute_id = 77;

-- CORRECT: Resolve IDs dynamically
SELECT cpd.value
FROM catalog_product_entity_decimal cpd
JOIN eav_attribute ea ON ea.attribute_id = cpd.attribute_id
WHERE ea.attribute_code = 'price'
  AND ea.entity_type_id = (
      SELECT entity_type_id FROM eav_entity_type
      WHERE entity_type_code = 'catalog_product'
  );

-- CORRECT: Use store_id from application context, not hardcoded
SELECT entity_id, increment_id
FROM sales_order
WHERE store_id = :store_id   -- injected from Magento store context
  AND status   = 'pending';

9. Raw-Query ohne Parameter-Binding

Rohe SQL-Queries mit String-Konkatenation sind ein SQL-Injection-Risiko. In Magento-Custom-Modulen ist das besonders gefährlich, wenn Sucheingaben, URL-Parameter oder Admin-Formulardaten direkt in Queries eingebaut werden. Im Code Review ist jede Query-Erstellung per String-Konkatenation als kritischer Befund einzustufen.

-- WRONG: Direct string concatenation (SQL injection risk)
-- PHP: $query = "SELECT * FROM sales_order WHERE increment_id = '" . $input . "'";

-- CORRECT: Always use parameter binding via Magento's connection adapter
-- PHP example with Magento's DB adapter:
-- $connection = $resource->getConnection();
-- $select = $connection->select()
--     ->from('sales_order', ['entity_id', 'increment_id', 'grand_total'])
--     ->where('increment_id = ?', $incrementId)
--     ->where('store_id = ?', (int) $storeId);
-- $result = $connection->fetchAll($select);

-- If a raw query is unavoidable, use quoteInto:
-- $query = $connection->quoteInto(
--     'SELECT entity_id FROM sales_order WHERE increment_id = ?',
--     $incrementId
-- );

-- CORRECT SQL (parameterized):
SELECT entity_id, increment_id, grand_total, status
FROM sales_order
WHERE increment_id = :increment_id
  AND store_id     = :store_id;

10. Fehlender Transaktionsscope bei Multi-Step-Operationen

Operationen, die mehrere Tabellen oder mehrere Zeilen in einer logischen Einheit ändern, müssen in einer Transaktion ausgeführt werden. Fehlt der Transaktionsscope, können Teilfehler zu inkonsistenten Datenbankzuständen führen, die sich schwer diagnostizieren lassen. Im Code Review ist jede Schreiboperation, die mehr als eine Tabelle betrifft, auf Transaktionsscoping zu prüfen.

-- WRONG: No transaction — partial failure leaves inconsistent state
INSERT INTO sales_order (increment_id, grand_total, status)
VALUES (:increment_id, :total, 'pending');
-- If the next statement fails, the order exists without any items
INSERT INTO sales_order_item (order_id, sku, qty_ordered, price)
VALUES (LAST_INSERT_ID(), :sku, :qty, :price);

-- CORRECT: Wrap multi-step operations in a transaction
START TRANSACTION;

INSERT INTO sales_order (increment_id, grand_total, status)
VALUES (:increment_id, :total, 'pending');

SET @order_id = LAST_INSERT_ID();

INSERT INTO sales_order_item (order_id, sku, qty_ordered, price)
VALUES (@order_id, :sku, :qty, :price);

-- Only commit if both inserts succeeded
COMMIT;
-- Application code should catch exceptions and call ROLLBACK

11. ORM-Collection ohne Filter lädt gesamte Tabelle

In Magento führt eine Collection ohne addFieldToFilter zu einem SELECT ohne WHERE — und lädt damit die gesamte Tabelle in den Speicher. Das ist das ORM-Äquivalent einer N+1-Query und besonders gefährlich in Observer-Code, der bei jedem Request ausgeführt wird. Im Code Review ist jede Collection-Erstellung auf vorhandene Filter zu prüfen.

-- WRONG ORM pattern (Magento PHP):
-- $collection = $this->orderCollectionFactory->create();
-- foreach ($collection as $order) { ... }
-- Generates: SELECT * FROM sales_order
-- Loads ALL orders into memory

-- CORRECT: Always add filters before iterating
-- $collection = $this->orderCollectionFactory->create();
-- $collection->addFieldToFilter('status', ['in' => ['pending', 'processing']]);
-- $collection->addFieldToFilter('store_id', $storeId);
-- $collection->addFieldToSelect(['entity_id', 'increment_id', 'grand_total']);
-- $collection->setPageSize(500)->setCurPage($page);
-- foreach ($collection as $order) { ... }
-- Generates a filtered, limited query:

SELECT entity_id, increment_id, grand_total
FROM sales_order
WHERE status   IN ('pending', 'processing')
  AND store_id  = :store_id
LIMIT 500;

13. Zusammenfassung

14. FAQ: SQL Code Review