Rifqi Hafizuddin Claude Opus 4.8 commited on
Commit Β·
3669290
1
Parent(s): 446c575
/fix infer FKs for FK-less dedorch catalog + log repr(e) in db executor
Browse filesThe Go-owned dedorch catalog ships no foreign_keys, so the IR validator rejected every cross-table join the planner proposed (revenue-by-product, etc.). Infer the obvious <base>_id -> <table>.id edges at catalog read: conservative (single unambiguous target, matching data_type, schema sources only) and self-disabling once Go introspection emits real FKs. Stopgap, not a keeper.
Also log repr(e) instead of str(e) in the executor + prewarm paths: a Fernet InvalidToken (empty str()) from a credential-key mismatch had surfaced as "error": "", hiding the real failure.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- src/catalog/fk_inference.py +97 -0
- src/catalog/store.py +5 -1
- src/query/executor/db.py +6 -2
src/catalog/fk_inference.py
ADDED
|
@@ -0,0 +1,97 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Heuristic foreign-key inference for catalogs that ship no declared FKs.
|
| 2 |
+
|
| 3 |
+
The dedorch catalog (written by Go's introspection) currently carries **no**
|
| 4 |
+
`foreign_keys`, so the FK-backed-joins-only IR validator rejects every join the
|
| 5 |
+
planner proposes β cross-table questions ("revenue by product") can't run even
|
| 6 |
+
though the planner picks the right columns. Until Go captures real FK
|
| 7 |
+
constraints, we infer the obvious relational edges from naming conventions so the
|
| 8 |
+
planner and the validator agree on the same catalog.
|
| 9 |
+
|
| 10 |
+
Conservative by design (a wrong edge would silently corrupt joined results):
|
| 11 |
+
- `schema` (database) sources only β joins are DB-only anyway
|
| 12 |
+
- a foreign key is only inferred from a column named ``<base>_id``
|
| 13 |
+
- the target must be the SINGLE other table whose name matches ``<base>``
|
| 14 |
+
(singular/plural) and exposes an ``id`` column of the SAME data_type
|
| 15 |
+
- ambiguous matches (0 or >1 candidate tables) are skipped, never guessed
|
| 16 |
+
- sources that already declare ANY foreign key are left untouched (trust Go)
|
| 17 |
+
"""
|
| 18 |
+
|
| 19 |
+
from __future__ import annotations
|
| 20 |
+
|
| 21 |
+
import re
|
| 22 |
+
|
| 23 |
+
from src.catalog.models import ForeignKey, Source
|
| 24 |
+
from src.middlewares.logging import get_logger
|
| 25 |
+
|
| 26 |
+
from .models import Catalog
|
| 27 |
+
|
| 28 |
+
logger = get_logger("fk_inference")
|
| 29 |
+
|
| 30 |
+
# `<base>_id` β the conventional foreign-key column name (base must be non-empty).
|
| 31 |
+
_ID_COL = re.compile(r"^(?P<base>.+)_id$", re.IGNORECASE)
|
| 32 |
+
|
| 33 |
+
|
| 34 |
+
def _table_matches_base(table_name: str, base: str) -> bool:
|
| 35 |
+
"""Whether `table_name` is the table `<base>` refers to (singular/plural)."""
|
| 36 |
+
n = table_name.lower()
|
| 37 |
+
b = base.lower()
|
| 38 |
+
# `orders`β`order`, `products`β`product`, `sales_agents`β`agent` (suffix),
|
| 39 |
+
# plus the singular form and the `-es` plural.
|
| 40 |
+
return n == b or n == b + "es" or n.endswith(b + "s")
|
| 41 |
+
|
| 42 |
+
|
| 43 |
+
def _infer_source(source: Source) -> int:
|
| 44 |
+
"""Add inferred FK edges to one source's tables in place; return the count."""
|
| 45 |
+
added = 0
|
| 46 |
+
for table in source.tables:
|
| 47 |
+
for col in table.columns:
|
| 48 |
+
m = _ID_COL.match(col.name)
|
| 49 |
+
if not m:
|
| 50 |
+
continue
|
| 51 |
+
base = m.group("base")
|
| 52 |
+
candidates: list[tuple[str, str]] = [] # (target_table_id, target_column_id)
|
| 53 |
+
for tgt in source.tables:
|
| 54 |
+
if tgt.table_id == table.table_id:
|
| 55 |
+
continue
|
| 56 |
+
if not _table_matches_base(tgt.name, base):
|
| 57 |
+
continue
|
| 58 |
+
id_col = next(
|
| 59 |
+
(
|
| 60 |
+
c
|
| 61 |
+
for c in tgt.columns
|
| 62 |
+
if c.name.lower() == "id" and c.data_type == col.data_type
|
| 63 |
+
),
|
| 64 |
+
None,
|
| 65 |
+
)
|
| 66 |
+
if id_col is not None:
|
| 67 |
+
candidates.append((tgt.table_id, id_col.column_id))
|
| 68 |
+
# Only act on an unambiguous single match β never guess between many.
|
| 69 |
+
if len(candidates) != 1:
|
| 70 |
+
continue
|
| 71 |
+
target_table_id, target_column_id = candidates[0]
|
| 72 |
+
table.foreign_keys.append(
|
| 73 |
+
ForeignKey(
|
| 74 |
+
column_id=col.column_id,
|
| 75 |
+
target_table_id=target_table_id,
|
| 76 |
+
target_column_id=target_column_id,
|
| 77 |
+
)
|
| 78 |
+
)
|
| 79 |
+
added += 1
|
| 80 |
+
return added
|
| 81 |
+
|
| 82 |
+
|
| 83 |
+
def infer_foreign_keys(catalog: Catalog) -> Catalog:
|
| 84 |
+
"""Infer FK edges in place for schema sources that declare none. Returns `catalog`.
|
| 85 |
+
|
| 86 |
+
Sources that already carry any declared FK are left as-is (Go's real FKs win).
|
| 87 |
+
"""
|
| 88 |
+
total = 0
|
| 89 |
+
for source in catalog.sources:
|
| 90 |
+
if source.source_type != "schema":
|
| 91 |
+
continue
|
| 92 |
+
if any(t.foreign_keys for t in source.tables):
|
| 93 |
+
continue # real FKs present β trust them, infer nothing
|
| 94 |
+
total += _infer_source(source)
|
| 95 |
+
if total:
|
| 96 |
+
logger.info("inferred foreign keys", user_id=catalog.user_id, count=total)
|
| 97 |
+
return catalog
|
src/catalog/store.py
CHANGED
|
@@ -13,6 +13,7 @@ from src.db.postgres.connection import AsyncSessionLocal
|
|
| 13 |
from src.db.postgres.models import Catalog as CatalogRow
|
| 14 |
from src.middlewares.logging import get_logger
|
| 15 |
|
|
|
|
| 16 |
from .models import Catalog
|
| 17 |
|
| 18 |
logger = get_logger("catalog_store")
|
|
@@ -37,7 +38,10 @@ class CatalogStore:
|
|
| 37 |
row = result.scalar_one_or_none()
|
| 38 |
if row is None:
|
| 39 |
return None
|
| 40 |
-
|
|
|
|
|
|
|
|
|
|
| 41 |
|
| 42 |
async def upsert(self, catalog: Catalog) -> None:
|
| 43 |
# Legacy: Go's catalog.Service owns catalog writes now. Kept working (and
|
|
|
|
| 13 |
from src.db.postgres.models import Catalog as CatalogRow
|
| 14 |
from src.middlewares.logging import get_logger
|
| 15 |
|
| 16 |
+
from .fk_inference import infer_foreign_keys
|
| 17 |
from .models import Catalog
|
| 18 |
|
| 19 |
logger = get_logger("catalog_store")
|
|
|
|
| 38 |
row = result.scalar_one_or_none()
|
| 39 |
if row is None:
|
| 40 |
return None
|
| 41 |
+
# dedorch catalogs ship no foreign_keys (Go introspection drops them),
|
| 42 |
+
# but the IR validator only allows FK-backed joins. Infer the obvious
|
| 43 |
+
# edges so the planner and validator agree. No-op once Go emits real FKs.
|
| 44 |
+
return infer_foreign_keys(Catalog.model_validate(row))
|
| 45 |
|
| 46 |
async def upsert(self, catalog: Catalog) -> None:
|
| 47 |
# Legacy: Go's catalog.Service owns catalog writes now. Kept working (and
|
src/query/executor/db.py
CHANGED
|
@@ -121,7 +121,9 @@ class DbExecutor(BaseExecutor):
|
|
| 121 |
logger.error(
|
| 122 |
"db executor failed",
|
| 123 |
source_id=ir.source_id,
|
| 124 |
-
|
|
|
|
|
|
|
| 125 |
elapsed_ms=elapsed_ms,
|
| 126 |
)
|
| 127 |
return QueryResult(
|
|
@@ -235,7 +237,9 @@ class DbExecutor(BaseExecutor):
|
|
| 235 |
creds = decrypt_credentials_dict(client.credentials)
|
| 236 |
await asyncio.to_thread(cls._warm_sync, client_id, client.db_type, creds)
|
| 237 |
except Exception as exc: # noqa: BLE001 β best-effort warming
|
| 238 |
-
|
|
|
|
|
|
|
| 239 |
|
| 240 |
@staticmethod
|
| 241 |
def _warm_sync(client_id: str, db_type: str, creds: dict) -> None:
|
|
|
|
| 121 |
logger.error(
|
| 122 |
"db executor failed",
|
| 123 |
source_id=ir.source_id,
|
| 124 |
+
# repr, not str: some exceptions (e.g. Fernet InvalidToken) have an
|
| 125 |
+
# empty str(), which hides the real failure as error="".
|
| 126 |
+
error=repr(e),
|
| 127 |
elapsed_ms=elapsed_ms,
|
| 128 |
)
|
| 129 |
return QueryResult(
|
|
|
|
| 237 |
creds = decrypt_credentials_dict(client.credentials)
|
| 238 |
await asyncio.to_thread(cls._warm_sync, client_id, client.db_type, creds)
|
| 239 |
except Exception as exc: # noqa: BLE001 β best-effort warming
|
| 240 |
+
# repr, not str: empty-str exceptions (e.g. Fernet InvalidToken)
|
| 241 |
+
# would otherwise log as error="".
|
| 242 |
+
logger.info("prewarm skipped", source_id=source.source_id, error=repr(exc))
|
| 243 |
|
| 244 |
@staticmethod
|
| 245 |
def _warm_sync(client_id: str, db_type: str, creds: dict) -> None:
|