diff --git a/backend/src/services/maintenance/_dashboard_scanner.py b/backend/src/services/maintenance/_dashboard_scanner.py index a5a248a91..1347e3de3 100644 --- a/backend/src/services/maintenance/_dashboard_scanner.py +++ b/backend/src/services/maintenance/_dashboard_scanner.py @@ -90,7 +90,7 @@ async def find_affected_dashboards( try: virtual_query = { "columns": ["id", "table_name", "schema", "sql"], - "filters": [{"col": "sql", "opr": "is_not_null", "value": None}], + "filters": [{"col": "sql", "opr": "is_not_null", "value": ""}], } _, virtual_datasets = await superset_client.get_datasets(query=virtual_query) except Exception as e: diff --git a/backend/src/services/sql_table_extractor.py b/backend/src/services/sql_table_extractor.py index 6e0f717aa..8257f5677 100644 --- a/backend/src/services/sql_table_extractor.py +++ b/backend/src/services/sql_table_extractor.py @@ -146,28 +146,37 @@ def extract_tables_from_sql_span(sql_text: str) -> set[str]: if not raw_matches: return tables - # Use sqlparse to identify string literal positions - parsed = sqlparse.parse(sql_text) + # Use sqlparse to identify string literal positions. Some production virtual + # datasets contain SQL so large that sqlparse refuses to group it + # (MAX_GROUPING_TOKENS = 10000 tokens → SQLParseError). In that case fall back + # to regex-only extraction (accepting potential string-literal false positives) + # rather than failing the whole maintenance scan. string_literal_ranges: list[tuple[int, int]] = [] + try: + parsed = sqlparse.parse(sql_text) - def walk_tokens(tokens: Iterable[Token], base_offset: int = 0) -> None: - offset = base_offset - for token in tokens: - if isinstance(token, TokenList): - walk_tokens(token.flatten(), offset) - else: - ttype = token.ttype - val = token.value - if is_string_literal(token): - string_literal_ranges.append( - (offset, offset + len(val)) - ) - offset += len(val) + def walk_tokens(tokens: Iterable[Token], base_offset: int = 0) -> None: + offset = base_offset + for token in tokens: + if isinstance(token, TokenList): + walk_tokens(token.flatten(), offset) + else: + ttype = token.ttype + val = token.value + if is_string_literal(token): + string_literal_ranges.append( + (offset, offset + len(val)) + ) + offset += len(val) - for stmt in parsed: - if stmt is None: - continue - walk_tokens(stmt.flatten(), base_offset=0) + for stmt in parsed: + if stmt is None: + continue + walk_tokens(stmt.flatten(), base_offset=0) + except Exception: + # sqlparse failed (e.g. token-limit) — treat no text as a string literal so + # every regex match is kept. Best-effort matching over hard failure. + string_literal_ranges = [] def is_in_string(pos: int) -> bool: for s_start, s_end in string_literal_ranges: diff --git a/backend/tests/services/maintenance/test_dashboard_scanner.py b/backend/tests/services/maintenance/test_dashboard_scanner.py index c8f65f6b4..4026f2f64 100644 --- a/backend/tests/services/maintenance/test_dashboard_scanner.py +++ b/backend/tests/services/maintenance/test_dashboard_scanner.py @@ -105,7 +105,7 @@ class TestFindAffectedDashboards: query = virtual_call.kwargs.get("query") or virtual_call.args[0] assert "sql" in query["columns"] assert query["filters"] == [ - {"col": "sql", "opr": "is_not_null", "value": None} + {"col": "sql", "opr": "is_not_null", "value": ""} ] @pytest.mark.asyncio @@ -231,6 +231,43 @@ class TestFindAffectedDashboards: result = await find_affected_dashboards([table], mock_superset) assert 80 in result + @pytest.mark.asyncio + async def test_huge_virtual_dataset_sql_does_not_fail_discovery(self, mock_superset): + """A virtual dataset whose SQL exceeds sqlparse's token limit must not abort the scan. + + The extractor falls back to regex matching for oversized SQL; other datasets + are still scanned and matched. + """ + from src.services.maintenance._dashboard_scanner import find_affected_dashboards + huge_sql = ( + "SELECT " + " + ".join(f"c{i}" for i in range(15000)) + + " FROM dm_view.some_other_table" + ) + matching = [ + {"id": 9, "schema": "", "table_name": "Orders", "sql": "SELECT * FROM raw.orders"} + ] + oversized = [ + {"id": 8, "schema": "", "table_name": "Huge", "sql": huge_sql} + ] + + async def side_effect(**kwargs): + kind = self._classify_query(kwargs.get("query")) + if kind == "virtual_primary": + # physical-ish result plus one oversized virtual dataset returned by the + # client scan; both must be deduped and the oversized one must not raise. + return (2, matching + oversized) + return (0, []) + + mock_superset.get_datasets.side_effect = side_effect + mock_superset.get_dataset_detail.return_value = { + "linked_dashboards": [{"id": 90, "title": "Dashboard 90"}] + } + + # The oversized dataset references dm_view.some_other_table; matching raw.orders + # must still surface its dashboard without the whole scan raising. + result = await find_affected_dashboards(["raw.orders"], mock_superset) + assert 90 in result + @pytest.mark.asyncio async def test_virtual_and_physical_matches_combined(self, mock_superset): """Physical table matches and virtual SQL matches are both returned.""" diff --git a/backend/tests/test_sql_table_extractor.py b/backend/tests/test_sql_table_extractor.py index 8c5d74285..88980a4fc 100644 --- a/backend/tests/test_sql_table_extractor.py +++ b/backend/tests/test_sql_table_extractor.py @@ -258,6 +258,22 @@ class TestSqlSpanEdgeCases: result = extract_tables_from_sql_span(sql) assert "raw.sales" in result + def test_huge_sql_falls_back_to_regex_instead_of_raising(self): + """SQL exceeding sqlparse's token limit (MAX_GROUPING_TOKENS=10000) must not raise. + + Production virtual datasets can be enormous; before this fix a SQLParseError + ("Maximum number of tokens exceeded (10000)") aborted the whole maintenance scan. + It now falls back to regex-only extraction and still finds the target table. + """ + from src.services.sql_table_extractor import extract_tables_from_sql + sql = ( + "SELECT " + + " + ".join(f"c{i}" for i in range(15000)) + + " FROM dm_view.counterparty_td" + ) + result = extract_tables_from_sql(sql) + assert "dm_view.counterparty_td" in result + class TestExtractTablesFromJinjaEdge: """Edge coverage for extract_tables_from_jinja.""" diff --git a/docs/adr/ADR-0020-maintenance-virtual-dataset-discovery.md b/docs/adr/ADR-0020-maintenance-virtual-dataset-discovery.md index b4af848b4..14e32efbe 100644 --- a/docs/adr/ADR-0020-maintenance-virtual-dataset-discovery.md +++ b/docs/adr/ADR-0020-maintenance-virtual-dataset-discovery.md @@ -52,7 +52,7 @@ ``` Tier 1 (предпочтительно, маленький результат): GET /dataset/?q={"columns":["id","table_name","schema","sql"], - "filters":[{"col":"sql","opr":"is_not_null","value":None}]} + "filters":[{"col":"sql","opr":"is_not_null","value":""}]} Tier 2 (fallback, клиентский скан), если Tier 1 отклонён/упал: GET /dataset/?q={"columns":["id","table_name","schema","sql"]} @@ -85,6 +85,14 @@ Tier 2 (fallback, клиентский скан), если Tier 1 отклонё обнаружение виртуальности строится на `sql`, а не на флаге. Клиентская проверка `is_virtual` в цикле сопоставления уже опирается на непустой `sql`. +### Примечание по `value` в фильтре + +`value: None` в rison-фильтре невалидно (схема требует number/string/boolean/array), +Superset отклоняет его HTTP 400. Для `is_not_null` значение игнорируется оператором, +поэтому используется `value: ""` — валидная строка, которую `col.isnot(None)` не +использует. Это позволяет Tier 1 работать (маленький серверный фильтр), а не каждый +раз откатываться к полному клиентскому скану. + --- ## Decision 2: AsyncAPIClient.request поднимает не-2xx ответы @@ -103,6 +111,35 @@ Tier 2 (fallback, клиентский скан), если Tier 1 отклонё --- +## Decision 3: sqlparse token-limit fallback при извлечении таблиц из SQL + +### Проблема + +После починки обнаружения виртуальных датасетов runtime-сбой сместился в извлечение +таблиц. `sql_table_extractor.extract_tables_from_sql_span()` вызывает +`sqlparse.parse(sql_text)`; для SQL, у которого больше `MAX_GROUPING_TOKENS = 10000` +токенов, sqlparse поднимает `SQLParseError: "Maximum number of tokens exceeded (10000)"`. +В `find_affected_dashboards` вызов `extract_tables_from_sql(ds_sql)` в цикле по всем +виртуальным датасетам **не был обёрнут в try/except**, поэтому один гигантский +виртуальный датасет срывал всё обнаружение (`preview-dashboards` → 502, +`start_maintenance` → discovery failed). Это и есть «капа слишком маленькая» из логов. + +### Решение + +`extract_tables_from_sql_span()` оборачивает `sqlparse.parse` + обход токенов в +`try/except`. При ошибке (в т.ч. token-limit) список строковых литералов остаётся +пустым → `is_in_string()` возвращает False → сохраняются **все** regex-совпадения +`schema.table`. Это best-effort сопоставление (допускает возможные false positives из +строковых литералов) вместо жёсткого отказа всего скана. + +### Отклонённая альтернатива + +Поднимать `MAX_GROUPING_TOKENS` в `sqlparse` (монакий-патч или правка константы) — +хрупко и влияет на производительность парсинга для всех датасетов. Обход только +конкретного сбоя безопаснее. + +--- + ## Последствия 1. **Виртуальные датасеты теперь обнаруживаются.** Физические + виртуальные совпадения @@ -119,11 +156,17 @@ Tier 2 (fallback, клиентский скан), если Tier 1 отклонё pagination cap (best-effort: при капе виртуальное совпадение пропускается, физическое сохраняется). -4. **Проверяемость.** Корневая причина подтверждена по исходникам Superset +4. **Гигантские виртуальные датасеты не валят discovery.** `extract_tables_from_sql_span` + при token-limit sqlparse (10000) откатывается к regex-only, а не бросает исключение. + Один большой виртуальный датасет больше не срывает `preview-dashboards`/`start_maintenance`. + Покрыто тестами: `tests/test_sql_table_extractor.py::test_huge_sql_falls_back_to_regex_instead_of_raising` + и `tests/services/maintenance/test_dashboard_scanner.py::test_huge_virtual_dataset_sql_does_not_fail_discovery`. + +5. **Проверяемость.** Корневая причина подтверждена по исходникам Superset (`superset/datasets/api.py: search_columns` и `list_columns`; `is_sqllab_view` — - модельная колонка в `connectors/sqla/models.py`). Механизм покрыт тестами в - `backend/tests/services/maintenance/test_dashboard_scanner.py` (включая end-to-end с - реальным `sql_table_extractor` и реальным SQL из продакшн-сценария) и - `backend/tests/test_core/test_async_network.py`. + модельная колонка в `connectors/sqla/models.py`) и по `sqlparse` (`MAX_GROUPING_TOKENS`). + Механизм покрыт тестами в `backend/tests/services/maintenance/test_dashboard_scanner.py` + (включая end-to-end с реальным `sql_table_extractor` и реальным SQL из продакшн-сценария), + `backend/tests/test_sql_table_extractor.py` и `backend/tests/test_core/test_async_network.py`. # [/DEF:Doc.Adr.ADR0020:ADR] diff --git a/docs/adr/README.md b/docs/adr/README.md index 901138866..82ea429a4 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -37,7 +37,7 @@ ADR фиксирует принятое и проверяемое архитек | [0017](ADR-0017-agent-centric-logging.md) | `ACCEPTED` | Agent-centric traces achieved by editing call sites (meaningful intents, removal of per-request noise) rather than only adding complexity to the logger. | | [0018](ADR-0018-rejected-dataset-review.md) | `REJECTED` | Dataset Review is removed from the active product; `specs/027-dataset-llm-orchestration/` is retained only as archived history. | | [0019](ADR-0019-dashboard-import-mechanism.md) | `ACCEPTED` | UUID-трансформация БД (не strip_databases), cross-filter patching через IdMappingService, password injection flow (await_input→wait_for_input→retry), разделение dry-run/execute, парсинг имён YAML-файлов БД. | -| [0020](ADR-0020-maintenance-virtual-dataset-discovery.md) | `ACCEPTED` | Обнаружение виртуальных (SQL) датасетов в maintenance discovery через серверный фильтр `sql is_not_null` (fallback — клиентский скан по непустому `sql`); `AsyncAPIClient.request` поднимает не-2xx ответы (`raise_for_status`), восстанавливая fallback фильтрованный→полный скан. | +| [0020](ADR-0020-maintenance-virtual-dataset-discovery.md) | `ACCEPTED` | Обнаружение виртуальных (SQL) датасетов в maintenance discovery через серверный фильтр `sql is_not_null` (fallback — клиентский скан по непустому `sql`); `AsyncAPIClient.request` поднимает не-2xx ответы (`raise_for_status`); sqlparse token-limit (10000) fallback к regex-only для гигантских виртуальных датасетов. | ## Правила сопровождения