|
|
@@ -0,0 +1,59 @@
|
|
|
+# Database Bug Fix Design
|
|
|
+
|
|
|
+Date: 2026-03-15
|
|
|
+Scope: `database/db/mysql.py`, `database/dao/mysql_dao.py`
|
|
|
+Approach: Surgical bug fixes only (no architectural changes)
|
|
|
+
|
|
|
+## Problems
|
|
|
+
|
|
|
+### mysql.py
|
|
|
+
|
|
|
+1. `connect_database`: try/except only wraps connection string construction, not `create_engine`. Real connection failures are uncaught.
|
|
|
+2. `load_data_with_page`: Uses `query.replace("SELECT *", "SELECT COUNT(*)")` to build count query. Breaks if query doesn't start with `SELECT *`.
|
|
|
+3. `fetch_all` / `fetch_one`: Catch `SQLAlchemyError`, print, and return `None` silently. Callers cannot detect failures.
|
|
|
+
|
|
|
+### mysql_dao.py
|
|
|
+
|
|
|
+4. `get_cust_by_ids`, `get_shop_by_ids`, `get_product_by_ids`, `get_order_by_product_ids`: Build `IN (...)` clause via string concatenation — SQL injection risk.
|
|
|
+5. `get_product_by_id`: Queries a single record using `load_data_with_page` (pagination overhead for one row).
|
|
|
+6. `get_cust_list`, `get_product_from_order`: Call `fetch_all(text(query))` directly while all other methods use `load_data_with_page` — inconsistent interface.
|
|
|
+
|
|
|
+## Fixes
|
|
|
+
|
|
|
+### mysql.py
|
|
|
+
|
|
|
+**Fix 1 — `connect_database`**
|
|
|
+Move `create_engine(...)` inside the try block so connection failures are caught and re-raised as `ConnectionAbortedError`.
|
|
|
+
|
|
|
+**Fix 2 — `load_data_with_page`**
|
|
|
+Replace string substitution with subquery wrapping:
|
|
|
+```sql
|
|
|
+SELECT COUNT(*) FROM (<original_query>) AS _count_subq
|
|
|
+```
|
|
|
+Works regardless of the original SELECT clause.
|
|
|
+
|
|
|
+**Fix 3 — `fetch_all` / `fetch_one`**
|
|
|
+Add `raise` in the except block after printing the error. No interface change.
|
|
|
+
|
|
|
+### mysql_dao.py
|
|
|
+
|
|
|
+**Fix 4 — SQL injection in IN clauses**
|
|
|
+Replace string concatenation with SQLAlchemy parameterized binding:
|
|
|
+```python
|
|
|
+from sqlalchemy import bindparam, text
|
|
|
+query = text("... AND col IN :ids").bindparams(bindparam("ids", expanding=True))
|
|
|
+params = {"city_uuid": city_uuid, "ids": list(id_list)}
|
|
|
+```
|
|
|
+
|
|
|
+**Fix 5 — `get_product_by_id`**
|
|
|
+Replace `load_data_with_page` with `fetch_one`, return a single-row DataFrame.
|
|
|
+
|
|
|
+**Fix 6 — `get_cust_list` / `get_product_from_order`**
|
|
|
+Replace direct `fetch_all(text(query))` calls with `load_data_with_page(query, params)`.
|
|
|
+
|
|
|
+## Out of Scope
|
|
|
+
|
|
|
+- No architectural changes (no read/write separation, no ORM models)
|
|
|
+- No changes to Redis layer
|
|
|
+- No changes to DAO method signatures
|
|
|
+- No error handling changes beyond re-raising in fetch methods
|
Sherlock