Fix medium/low severity issues: script splitter, EXPLAIN, keyring, imports
execute_script() semicolon splitter (all drivers)
- Add BaseDriver._split_statements() that tracks single/double-quoted
strings and -- / /* */ comments so semicolons inside procedure bodies
are not treated as statement boundaries.
- Replace the naive sql.split(';') in all four drivers with this helper.
- MSSQL execute_script() also pre-splits on GO (case-insensitive, own
line) so scripts pasted from SSMS work correctly.
MSSQL EXPLAIN
- SET SHOWPLAN_TEXT ON/execute/SET SHOWPLAN_TEXT OFF must be separate
execute() calls; pyodbc rejects multiple statements in one call.
Hold self._lock for the entire sequence to keep session state atomic.
Keyring (connections.py)
- Log warnings (with traceback) on load/save failures instead of
silently returning "".
- save_password() now calls delete_password() when password is empty,
so clearing a saved password actually removes the old keyring entry
rather than leaving a stale one behind.
SQLite get_tables() O(N) lock acquisitions
- Run all COUNT(*) queries inside the same `with self._cur() as c:`
block, reducing N+1 lock acquisitions to 1.
Unused import: remove psycopg2.extras from postgres_driver.py.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,9 @@ from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
from app.models.connection_model import ConnectionProfile
|
||||
from app.utils.logger import get_logger
|
||||
|
||||
_log = get_logger(__name__)
|
||||
|
||||
_CONN_FILE = Path.home() / ".dbclient" / "connections.json"
|
||||
|
||||
@@ -23,8 +26,15 @@ _SERVICE = "DBClient"
|
||||
# ── Keyring helpers ──────────────────────────────────────────────────────────
|
||||
|
||||
def save_password(profile_id: str, password: str) -> None:
|
||||
if _KEYRING_OK and password:
|
||||
if not _KEYRING_OK:
|
||||
return
|
||||
if password:
|
||||
try:
|
||||
keyring.set_password(_SERVICE, profile_id, password)
|
||||
except Exception:
|
||||
_log.warning("keyring: failed to save password for %s", profile_id, exc_info=True)
|
||||
else:
|
||||
delete_password(profile_id)
|
||||
|
||||
|
||||
def load_password(profile_id: str) -> str:
|
||||
@@ -32,6 +42,7 @@ def load_password(profile_id: str) -> str:
|
||||
try:
|
||||
return keyring.get_password(_SERVICE, profile_id) or ""
|
||||
except Exception:
|
||||
_log.warning("keyring: failed to load password for %s", profile_id, exc_info=True)
|
||||
return ""
|
||||
return ""
|
||||
|
||||
@@ -41,7 +52,7 @@ def delete_password(profile_id: str) -> None:
|
||||
try:
|
||||
keyring.delete_password(_SERVICE, profile_id)
|
||||
except Exception:
|
||||
pass
|
||||
pass # Already absent is fine
|
||||
|
||||
|
||||
# ── Profile CRUD ─────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -197,6 +197,52 @@ class BaseDriver(ABC):
|
||||
"""Rename a column via ALTER TABLE."""
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
def _split_statements(sql: str) -> list:
|
||||
"""Split SQL text on unquoted semicolons, preserving quoted strings and comments."""
|
||||
stmts, buf = [], []
|
||||
i, n = 0, len(sql)
|
||||
while i < n:
|
||||
ch = sql[i]
|
||||
if ch == '-' and i + 1 < n and sql[i + 1] == '-':
|
||||
end = sql.find('\n', i)
|
||||
end = end + 1 if end != -1 else n
|
||||
buf.append(sql[i:end])
|
||||
i = end
|
||||
elif ch == '/' and i + 1 < n and sql[i + 1] == '*':
|
||||
end = sql.find('*/', i + 2)
|
||||
end = end + 2 if end != -1 else n
|
||||
buf.append(sql[i:end])
|
||||
i = end
|
||||
elif ch in ("'", '"'):
|
||||
q, j = ch, i + 1
|
||||
while j < n:
|
||||
if sql[j] == '\\':
|
||||
j += 2
|
||||
elif sql[j] == q:
|
||||
j += 1
|
||||
if j < n and sql[j] == q:
|
||||
j += 1
|
||||
continue
|
||||
break
|
||||
else:
|
||||
j += 1
|
||||
buf.append(sql[i:j])
|
||||
i = j
|
||||
elif ch == ';':
|
||||
stmt = ''.join(buf).strip()
|
||||
if stmt:
|
||||
stmts.append(stmt)
|
||||
buf = []
|
||||
i += 1
|
||||
else:
|
||||
buf.append(ch)
|
||||
i += 1
|
||||
stmt = ''.join(buf).strip()
|
||||
if stmt:
|
||||
stmts.append(stmt)
|
||||
return stmts
|
||||
|
||||
@property
|
||||
def is_connected(self) -> bool:
|
||||
return self._connection is not None
|
||||
|
||||
@@ -233,9 +233,11 @@ class MSSQLDriver(BaseDriver):
|
||||
return [], [], c.rowcount
|
||||
|
||||
def execute_script(self, sql: str) -> list:
|
||||
import re
|
||||
results = []
|
||||
stmts = [s.strip() for s in sql.split(';') if s.strip()]
|
||||
for stmt in stmts:
|
||||
batches = re.split(r'^\s*GO\s*$', sql, flags=re.IGNORECASE | re.MULTILINE)
|
||||
for batch in batches:
|
||||
for stmt in self._split_statements(batch):
|
||||
try:
|
||||
with self._cur() as c:
|
||||
c.execute(stmt)
|
||||
@@ -305,9 +307,20 @@ class MSSQLDriver(BaseDriver):
|
||||
# ── Server tools ──────────────────────────────────────────────────────────
|
||||
|
||||
def explain_query(self, sql: str) -> tuple:
|
||||
with self._cur() as c:
|
||||
c.execute(f"SET SHOWPLAN_TEXT ON; {sql}; SET SHOWPLAN_TEXT OFF")
|
||||
return ["Plan"], c.fetchall()
|
||||
with self._lock:
|
||||
cur = self._connection.cursor()
|
||||
try:
|
||||
cur.execute("SET SHOWPLAN_TEXT ON")
|
||||
cur.execute(sql)
|
||||
cols = [d[0] for d in cur.description] if cur.description else ["Plan"]
|
||||
rows = cur.fetchall()
|
||||
cur.execute("SET SHOWPLAN_TEXT OFF")
|
||||
finally:
|
||||
try:
|
||||
cur.close()
|
||||
except Exception:
|
||||
pass
|
||||
return cols, rows
|
||||
|
||||
def get_process_list(self) -> tuple:
|
||||
with self._cur() as c:
|
||||
|
||||
@@ -254,9 +254,8 @@ class MySQLDriver(BaseDriver):
|
||||
|
||||
def execute_script(self, sql: str) -> list:
|
||||
results = []
|
||||
stmts = [s.strip() for s in sql.split(';') if s.strip()]
|
||||
with self._cur() as c:
|
||||
for stmt in stmts:
|
||||
for stmt in self._split_statements(sql):
|
||||
try:
|
||||
c.execute(stmt)
|
||||
if c.description:
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
"""PostgreSQL driver implementation using psycopg2."""
|
||||
import psycopg2
|
||||
import psycopg2.extras
|
||||
from contextlib import contextmanager
|
||||
from typing import Optional
|
||||
from app.drivers.base import (
|
||||
@@ -230,9 +229,8 @@ class PostgreSQLDriver(BaseDriver):
|
||||
|
||||
def execute_script(self, sql: str) -> list:
|
||||
results = []
|
||||
stmts = [s.strip() for s in sql.split(';') if s.strip()]
|
||||
with self._cur() as c:
|
||||
for stmt in stmts:
|
||||
for stmt in self._split_statements(sql):
|
||||
try:
|
||||
c.execute(stmt)
|
||||
if c.description:
|
||||
|
||||
@@ -72,9 +72,8 @@ class SQLiteDriver(BaseDriver):
|
||||
tables = []
|
||||
for name in names:
|
||||
try:
|
||||
with self._cur() as rc:
|
||||
rc.execute(f'SELECT COUNT(*) FROM "{name}"')
|
||||
row_count = rc.fetchone()[0]
|
||||
c.execute(f'SELECT COUNT(*) FROM "{name}"')
|
||||
row_count = c.fetchone()[0]
|
||||
except Exception:
|
||||
row_count = 0
|
||||
tables.append(TableInfo(name=name, schema="main", row_count=row_count))
|
||||
@@ -164,8 +163,7 @@ class SQLiteDriver(BaseDriver):
|
||||
|
||||
def execute_script(self, sql: str) -> list:
|
||||
results = []
|
||||
stmts = [s.strip() for s in sql.split(';') if s.strip()]
|
||||
for stmt in stmts:
|
||||
for stmt in self._split_statements(sql):
|
||||
try:
|
||||
with self._cur() as c:
|
||||
c.execute(stmt)
|
||||
|
||||
Reference in New Issue
Block a user