Skip to content

Commit 5adf6a4

Browse files
authored
fix: keep stub-named CTEs from capturing references and the layered benchmark compiling (#880)
* test: keep the layered production benchmark project compiling cleanly The layered generator placed its contract schema in project-wide schemas/ although only the first staging model uses it (S024), and its seed-joined models referenced id unqualified (B003). Place the schema beside its only consumer, qualify input columns, and refresh the stale hook count in the disabled layered performance guard. Add a small non-performance E2E compile of the layered project so the generator cannot silently break again. Refs: CHI-414 * fix: keep authored stub-named CTEs from capturing relation references Compact analysis without a binding schema replaced relation references with __sqlbuild_project_input_N stubs without checking the authored SQL, so a CTE with that name (any case) captured the reference and analysis inferred the CTE's shape instead of the input's. Share the binding-mode sharing guard and fall back to real relation names when the stub prefix appears in the SQL or a relation name. Refs: CHI-412
1 parent d895760 commit 5adf6a4

8 files changed

Lines changed: 358 additions & 21 deletions

File tree

‎src/sqlbuild/compiler/compile/_helpers/analysis/compact.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
binding_query_fields,
3232
lineage_reference_map,
3333
prepare_binding_queries,
34+
relation_stubs_collide,
3435
remembered_shared_results,
3536
reused_shared_results,
3637
shared_result_is_exact,
@@ -723,22 +724,24 @@ def _prepare_compact_analysis_batch(
723724
shared: SharedBindingQuery | None = (
724725
binding_query.shared if binding_query is not None else None
725726
)
726-
qualified_reference_names: frozenset[str] = (
727-
_qualified_reference_names(
727+
stubbed_reference_names: frozenset[str] = (
728+
frozenset()
729+
if binding_schema is not None
730+
or relation_stubs_collide(cleaned_sql=cleaned_sql, names=lineage_references)
731+
else frozenset(lineage_references)
732+
- _qualified_reference_names(
728733
query_sql=cleaned_sql,
729734
reference_names=lineage_references.keys(),
730735
)
731-
if binding_schema is None
732-
else frozenset()
733736
)
734737
canonical_stubs: dict[str, str] = (
735738
shared.stubs
736739
if shared is not None
737740
else {
738741
name: (
739-
name
740-
if binding_schema is not None or name in qualified_reference_names
741-
else f"{COMPACT_RELATION_STUB_PREFIX}{index}"
742+
f"{COMPACT_RELATION_STUB_PREFIX}{index}"
743+
if name in stubbed_reference_names
744+
else name
742745
)
743746
for index, name in enumerate(lineage_references)
744747
}

‎src/sqlbuild/compiler/compile/_helpers/sharing/binding.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import re
66
from collections import Counter
7-
from collections.abc import Mapping, Sequence
7+
from collections.abc import Iterable, Mapping, Sequence
88
from dataclasses import replace
99
from typing import Any, cast
1010

@@ -190,6 +190,15 @@ def shared_result_is_exact(*, validation: object, template: object) -> bool:
190190
return isinstance(validation, dict) and cast(dict[str, object], validation).get("errors") == []
191191

192192

193+
def relation_stubs_collide(*, cleaned_sql: str, names: Iterable[str]) -> bool:
194+
"""Return whether authored SQL or relation names already use the relation stub prefix."""
195+
196+
return (
197+
any(name.startswith(COMPACT_RELATION_STUB_PREFIX) for name in names)
198+
or COMPACT_RELATION_STUB_PREFIX.casefold() in cleaned_sql.casefold()
199+
)
200+
201+
193202
def _shared_reference_names(
194203
*,
195204
cleaned_sql: str,
@@ -199,9 +208,7 @@ def _shared_reference_names(
199208
names: list[str] = list(lineage_reference_map(references))
200209
if not names or len({name.casefold() for name in names}) != len(names):
201210
return None
202-
if any(name.startswith(COMPACT_RELATION_STUB_PREFIX) for name in names):
203-
return None
204-
if COMPACT_RELATION_STUB_PREFIX.casefold() in cleaned_sql.casefold():
211+
if relation_stubs_collide(cleaned_sql=cleaned_sql, names=names):
205212
return None
206213
if SQL_QUOTED_IDENTIFIER_DELIMITER in cleaned_sql:
207214
return None

‎tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ class SourceSemanticBindingCase:
1212
expected_code: str = "B002"
1313

1414

15+
@dataclass(frozen=True)
16+
class RelationStubNameCase:
17+
description: str
18+
stub_cte_name: str
19+
expected_downstream_type: str
20+
expected_ghost_column_diagnostics: frozenset[tuple[str, str]]
21+
22+
1523
@dataclass(frozen=True)
1624
class SemanticCorpusCase:
1725
description: str
@@ -106,6 +114,19 @@ class LayeredProductionCompilePerformanceGuardTestCase:
106114
expected_config_edit_max_seconds: float
107115

108116

117+
@dataclass(frozen=True)
118+
class LayeredProductionCompileTestCase:
119+
description: str
120+
model_count: int
121+
source_count: int
122+
seed_count: int
123+
function_count: int
124+
macro_count: int
125+
test_count: int
126+
audit_count: int
127+
expected_diagnostic_codes: tuple[str, ...]
128+
129+
109130
@dataclass(frozen=True)
110131
class SemanticCompilePerformanceGuardTestCase:
111132
description: str

‎tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py‎

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
FreshProcessCompileCachePerformanceGuardTestCase,
3131
SemanticCorpusCase,
3232
)
33+
from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project
3334

3435
_DBT_SHAPED_SQL_SIZE_PROFILE: tuple[tuple[float, int], ...] = (
3536
(0.50, 1_800),
@@ -1474,8 +1475,12 @@ def _scope_single_folder_macros(*, project_dir: Path) -> None:
14741475

14751476

14761477
def _layered_write_schemas(*, project_dir: Path) -> None:
1477-
schemas_dir: Path = project_dir / "schemas"
1478-
schemas_dir.mkdir()
1478+
"""Write the contract schema beside its only consumer, the first staging model."""
1479+
1480+
schemas_dir: Path = (
1481+
project_dir / "models" / _layered_model_folder(index=0) / "_sqlbuild" / "_schemas"
1482+
)
1483+
schemas_dir.mkdir(parents=True)
14791484
(schemas_dir / "benchmark_row.sql").write_text(
14801485
"""SCHEMA (
14811486
name benchmark_row,
@@ -1619,16 +1624,17 @@ def _layered_dependent_model_sql(
16191624
previous_name: str = f"model_{index - 1:05d}"
16201625
macro_index: int = (index // _MACRO_INTERVAL) % macro_count
16211626
id_expression: str = {
1622-
True: f'@macro_{macro_index:05d}("id")',
1623-
False: f"id + {index % 7}",
1627+
True: f'@macro_{macro_index:05d}("previous.id")',
1628+
False: f"previous.id + {index % 7}",
16241629
}[index % _MACRO_INTERVAL == 0]
16251630
generated_amount_expression: str = (
1626-
f"amount + {index % 11} + {_layered_generated_mapping_expression()} "
1631+
f"previous.amount + {index % 11} + "
1632+
f"{_layered_generated_mapping_expression(column='previous.id')} "
16271633
"+ CAST(@@benchmark_revision AS INTEGER)"
16281634
)
16291635
function_index: int = index % function_count
16301636
amount_expression: str = {
1631-
True: f'__udf("fn_{function_index:05d}")(amount)',
1637+
True: f'__udf("fn_{function_index:05d}")(previous.amount)',
16321638
False: generated_amount_expression,
16331639
}[index % _FUNCTION_INTERVAL == 0]
16341640
seed_index: int = index % seed_count
@@ -1639,7 +1645,7 @@ def _layered_dependent_model_sql(
16391645
query: str = f'''SELECT
16401646
{id_expression} AS id,
16411647
{amount_expression} AS amount,
1642-
CASE WHEN id % 2 = 0 THEN 'even' ELSE 'odd' END AS status
1648+
CASE WHEN previous.id % 2 = 0 THEN 'even' ELSE 'odd' END AS status
16431649
FROM __ref("{previous_name}") AS previous{join_sql}
16441650
'''
16451651
direct_sql: str = f"{_layered_model_header(index=index, audit_count=audit_count)}\n\n{query}"
@@ -2061,8 +2067,10 @@ def _layered_model_sql_size_target(*, index: int, model_count: int) -> int:
20612067
return round(lower_size + position * (upper_size - lower_size))
20622068

20632069

2064-
def _layered_generated_mapping_expression() -> str:
2065-
clauses: str = "".join(f"WHEN id = {value:05d} THEN {value % 13:02d} " for value in range(45))
2070+
def _layered_generated_mapping_expression(*, column: str = "id") -> str:
2071+
clauses: str = "".join(
2072+
f"WHEN {column} = {value:05d} THEN {value % 13:02d} " for value in range(45)
2073+
)
20662074
return f"CASE {clauses} ELSE 0 END"
20672075

20682076

@@ -2276,3 +2284,44 @@ def plan_reasons_and_migrations(
22762284

22772285
def _query_changed(model: dict[str, Any]) -> bool:
22782286
return model["reason"] == "query_changed"
2287+
2288+
2289+
def write_relation_stub_project(
2290+
*, tmp_path: Path, stub_cte_name: str, selected_column: str
2291+
) -> Path:
2292+
"""Write a project whose middle model authors a CTE named like a relation stub."""
2293+
2294+
return prepare_inline_project(
2295+
tmp_path=tmp_path,
2296+
project_name="relation_stub_names",
2297+
repo_files={
2298+
"sqlbuild_project.toml": (
2299+
'name = "relation_stub_names"\n'
2300+
'adapter = "duckdb"\n'
2301+
'default_target = "dev"\n\n'
2302+
"[connection]\n"
2303+
'database = "orders.duckdb"\n\n'
2304+
"[targets.dev]\n"
2305+
'schema = "main"\n'
2306+
),
2307+
"models/orders.sql": (
2308+
"MODEL (materialized table);\n\nSELECT CAST(7 AS INTEGER) AS order_id\n"
2309+
),
2310+
"models/order_status.sql": (
2311+
"MODEL (materialized table);\n\n"
2312+
f"WITH {stub_cte_name} AS (\n"
2313+
" SELECT 'pending' AS order_id, 'pending' AS ghost_status\n"
2314+
")\n"
2315+
f'SELECT {selected_column} FROM __ref("orders")\n'
2316+
),
2317+
"models/next_order.sql": (
2318+
"MODEL (\n"
2319+
" materialized table,\n"
2320+
" contract enforced,\n"
2321+
" columns (order_id (type INTEGER), next_order_id (type INTEGER)),\n"
2322+
");\n\n"
2323+
"SELECT order_id, order_id + 1 AS next_order_id\n"
2324+
'FROM __ref("order_status")\n'
2325+
),
2326+
},
2327+
)

‎tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_performance.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ def test_given_test_heavy_project_when_compiling_then_finishes_within_budget(
237237
macro_count=12,
238238
test_count=130,
239239
expected_audit_count=700,
240-
expected_hook_count=2,
240+
expected_hook_count=0,
241241
expected_min_model_sql_bytes=5_500_000,
242242
expected_max_model_sql_bytes=7_000_000,
243243
expected_min_compiled_test_bytes=3_000_000,
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
"""E2E coverage that the layered production benchmark project compiles cleanly."""
2+
3+
from __future__ import annotations
4+
5+
import json
6+
import subprocess
7+
from pathlib import Path
8+
from typing import Any
9+
10+
import pytest
11+
12+
from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import (
13+
LayeredProductionCompileTestCase,
14+
)
15+
from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import (
16+
write_layered_production_compile_project,
17+
)
18+
from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import run_sqb
19+
20+
21+
@pytest.mark.parametrize(
22+
"test_case",
23+
[
24+
LayeredProductionCompileTestCase(
25+
description="small layered project with seed joins, macros, functions and contract",
26+
model_count=350,
27+
source_count=83,
28+
seed_count=16,
29+
function_count=8,
30+
macro_count=4,
31+
test_count=47,
32+
audit_count=251,
33+
expected_diagnostic_codes=(),
34+
),
35+
],
36+
ids=lambda case: case.description,
37+
)
38+
def test_given_layered_production_project_when_compiling_then_reports_no_diagnostics(
39+
test_case: LayeredProductionCompileTestCase,
40+
tmp_path: Path,
41+
) -> None:
42+
project_dir: Path = tmp_path / "layered_production"
43+
write_layered_production_compile_project(
44+
project_dir=project_dir,
45+
model_count=test_case.model_count,
46+
source_count=test_case.source_count,
47+
seed_count=test_case.seed_count,
48+
function_count=test_case.function_count,
49+
macro_count=test_case.macro_count,
50+
test_count=test_case.test_count,
51+
audit_count=test_case.audit_count,
52+
)
53+
model_sql: str = "".join(
54+
path.read_text(encoding="utf-8") for path in (project_dir / "models").rglob("*.sql")
55+
)
56+
for fragment in ("__seed(", "__udf(", "@macro_", "model_schema benchmark_row"):
57+
assert fragment in model_sql
58+
59+
result: subprocess.CompletedProcess[str] = run_sqb(
60+
project_dir=project_dir,
61+
command=("compile", "--no-cache", "--json"),
62+
)
63+
64+
assert result.returncode == 0, result.stdout + result.stderr
65+
payload: dict[str, Any] = json.loads(result.stdout)
66+
diagnostic_codes: tuple[str, ...] = tuple(
67+
str(diagnostic["code"]) for diagnostic in payload["diagnostics"]
68+
)
69+
assert diagnostic_codes == test_case.expected_diagnostic_codes, payload["diagnostics"]
70+
assert payload["summary"]["errors"] == 0
71+
assert payload["summary"]["warnings"] == 0
72+
assert payload["summary"]["models"] == test_case.model_count
73+
assert payload["summary"]["tests"] == test_case.test_count
74+
75+
76+
if __name__ == "__main__":
77+
pytest.main([__file__, "-vv"])

0 commit comments

Comments
 (0)