Skip to content

Commit 2d8fe1d

Browse files
authored
fix(compile): linear missing-description locations and comment-tolerant expected fixtures (#924)
* perf(compile): index YAML entry lines once for missing-description locations Locating each undescribed seed or source rescanned its whole YAML file with a per-name regex, so error-heavy compiles grew quadratically with entries per file. Index every entry name once per file and look names up from that index, preserving the outermost-entry and pattern semantics exactly. * fix(test): ignore comments around set-operation branches in SQL-test fixtures A comment before or after UNION ALL in an __expected__ fixture stayed attached to the neighbouring branch, so the projection scanners read the comment as an alias and reported a false P001 branch mismatch. Branch and projection splits now drop surrounding comments, alias detection ignores trailing comments, the Python fallback splitter follows the adapter's lexical syntax, and Polyglot's annotated comment wrapper is unwrapped.
1 parent acce554 commit 2d8fe1d

21 files changed

Lines changed: 718 additions & 67 deletions

File tree

‎crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/extraction.rs‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ fn projection_name(expression: &str, file: &str, label: &str) -> Result<String,
512512
if let Some(position) = find_last_top_level_keyword(expression, "AS")? {
513513
let alias_start = skip_ignorable(expression, position + 2)?;
514514
if let Some((alias, end)) = read_identifier(expression, alias_start)
515-
&& expression[end..].trim().is_empty()
515+
&& skip_ignorable(expression, end)? == expression.len()
516516
{
517517
return Ok(alias);
518518
}
@@ -880,8 +880,34 @@ fn scan_code<T>(
880880
Ok(None)
881881
}
882882

883-
fn non_empty_trimmed(value: &str) -> Option<&str> {
884-
Some(value.trim()).filter(|trimmed| !trimmed.is_empty())
883+
/// Return `value` without surrounding whitespace and comments, or `None` when no code remains.
884+
fn non_empty_trimmed(value: &str) -> Result<Option<&str>, String> {
885+
if !value.contains("--") && !value.contains("/*") {
886+
return Ok(Some(value.trim()).filter(|trimmed| !trimmed.is_empty()));
887+
}
888+
let start = skip_ignorable(value, 0)?;
889+
let mut end = start;
890+
let mut index = start;
891+
while index < value.len() {
892+
if let Some(comment) = comment_end(value.as_bytes(), index)
893+
.map_err(|error| scan_error_message(error, "SQL test"))?
894+
{
895+
index = comment;
896+
continue;
897+
}
898+
let next = skip_non_code(value, index)?;
899+
if next != index {
900+
end = next;
901+
index = next;
902+
continue;
903+
}
904+
let width = char_len(value, index);
905+
if !value[index..].starts_with(char::is_whitespace) {
906+
end = index + width;
907+
}
908+
index += width;
909+
}
910+
Ok(Some(&value[start..end]).filter(|trimmed| !trimmed.is_empty()))
885911
}
886912

887913
/// Split on top-level `UNION`, `INTERSECT` and `EXCEPT`, each with an optional `ALL`/`DISTINCT`.
@@ -895,7 +921,7 @@ pub(crate) fn split_set_operations(sql: &str) -> Result<Vec<&str>, String> {
895921
let Some(end) = set_operator_end(sql, index)? else {
896922
return Ok(CodeStep::Advance);
897923
};
898-
values.extend(non_empty_trimmed(&sql[start..index]));
924+
values.extend(non_empty_trimmed(&sql[start..index])?);
899925
let mut resume = skip_ignorable(sql, end)?;
900926
if let Some(quantifier_end) =
901927
consume_keyword(sql, resume, "ALL").or_else(|| consume_keyword(sql, resume, "DISTINCT"))
@@ -905,7 +931,7 @@ pub(crate) fn split_set_operations(sql: &str) -> Result<Vec<&str>, String> {
905931
start = resume;
906932
Ok(CodeStep::Resume(resume))
907933
})?;
908-
values.extend(non_empty_trimmed(&sql[start..]));
934+
values.extend(non_empty_trimmed(&sql[start..])?);
909935
Ok(values)
910936
}
911937

@@ -927,12 +953,12 @@ pub(crate) fn split_top_level(sql: &str, separator: u8) -> Result<Vec<&str>, Str
927953
let mut start = 0;
928954
scan_code::<()>(sql, 0, |index, depth| {
929955
if depth == 0 && byte_at(sql, index) == Some(separator) {
930-
values.extend(non_empty_trimmed(&sql[start..index]));
956+
values.extend(non_empty_trimmed(&sql[start..index])?);
931957
start = index + 1;
932958
}
933959
Ok(CodeStep::Advance)
934960
})?;
935-
values.extend(non_empty_trimmed(&sql[start..]));
961+
values.extend(non_empty_trimmed(&sql[start..])?);
936962
Ok(values)
937963
}
938964

‎crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_extraction.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,26 @@ fn given_expected_cte_sql_when_scanning_top_level_then_set_operations_commas_and
9191
]),
9292
expected_commas: Ok(vec![
9393
"SELECT 'x, UNION (' AS a",
94-
"/* UNION , FROM */ \"b)\"",
94+
"\"b)\"",
9595
"`c,d`",
9696
"f(1, (SELECT 2 FROM t UNION SELECT 3)) -- , FROM\nFROM t",
9797
]),
9898
expected_from: Ok(Some(107)),
9999
},
100+
TopLevelScanTestCase {
101+
description: "comments around branches and projections are not code",
102+
sql: "-- lead\nSELECT 1 AS a, /* b */ 2 AS b -- first\n/* x */ UNION ALL -- second\nSELECT '--' AS a, 3 AS b /* end */",
103+
expected_unions: Ok(vec![
104+
"SELECT 1 AS a, /* b */ 2 AS b",
105+
"SELECT '--' AS a, 3 AS b",
106+
]),
107+
expected_commas: Ok(vec![
108+
"SELECT 1 AS a",
109+
"2 AS b -- first\n/* x */ UNION ALL -- second\nSELECT '--' AS a",
110+
"3 AS b",
111+
]),
112+
expected_from: Ok(None),
113+
},
100114
TopLevelScanTestCase {
101115
description: "keyword boundaries and empty pieces",
102116
sql: ",SELECT reunion, union_id,, x FROMAGE UNION",

‎scripts/cold_compile_performance/_helpers/diagnostic_project.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,34 @@ def write_single_model_diagnostic_project(*, project_dir: Path, diagnostics: int
132132
+ ",\n".join(projections)
133133
+ '\nFROM __ref("orders") AS o\n'
134134
)
135+
136+
137+
def write_undescribed_project(*, project_dir: Path, model_count: int, width: int = 6) -> None:
138+
"""Generate models, seeds, and one large source file whose resources all lack descriptions."""
139+
project_dir.mkdir(parents=True, exist_ok=True)
140+
for folder in ("models", "sources", "seeds"):
141+
(project_dir / folder).mkdir(exist_ok=True)
142+
(project_dir / "sqlbuild_project.toml").write_text(
143+
'name = "undescribed_orders"\nadapter = "duckdb"\n[rules]\nselect = []\n'
144+
)
145+
columns: list[str] = [f"amount_{index:03}" for index in range(width)]
146+
sources: list[str] = ["sources:"]
147+
seeds: list[str] = ["seeds:"]
148+
for index in range(model_count):
149+
sources.extend(
150+
(f" - name: orders_{index:05}", " schema: main", f" table: orders_{index:05}")
151+
)
152+
sources.append(" columns:")
153+
sources.extend(f" - name: {column}\n type: INTEGER" for column in columns)
154+
(project_dir / "models" / f"order_totals_{index:05}.sql").write_text(
155+
"MODEL (materialized view);\n"
156+
f'SELECT {", ".join(columns)} FROM __source("orders_{index:05}")\n'
157+
)
158+
if index % 10 == 0:
159+
seeds.append(f" - name: products_{index:05}\n columns:")
160+
seeds.extend(f" - name: {column}\n type: INTEGER" for column in columns)
161+
(project_dir / "seeds" / f"products_{index:05}.csv").write_text(
162+
",".join(columns) + "\n" + ",".join("1" for _ in columns) + "\n"
163+
)
164+
(project_dir / "sources" / "orders.yml").write_text("\n".join(sources) + "\n")
165+
(project_dir / "seeds" / "products.yml").write_text("\n".join(seeds) + "\n")

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,19 @@
1515
split_set_operation_branches,
1616
)
1717
from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql
18+
from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax
1819

1920
_POLYGLOT_VALUES_EXPRESSION_NAME: str = "Values"
2021
_POLYGLOT_VALUES_SET_ALIAS: str = "_values"
2122
_SCAN_CONTEXT: str = "SQL test"
2223

2324

2425
def extract_expected_branch_column_names_with_sql_analysis(
25-
*, sql: str, file_label: str, label: str = "__expected__<model>"
26+
*,
27+
sql: str,
28+
file_label: str,
29+
syntax: SqlLexicalSyntax,
30+
label: str = "__expected__<model>",
2631
) -> tuple[tuple[str, ...], ...] | None:
2732
"""Return expected SELECT branch names using required Polyglot analysis."""
2833

@@ -32,17 +37,23 @@ def extract_expected_branch_column_names_with_sql_analysis(
3237
parsed_expression: Any = polyglot_module.parse_one(sql, dialect="generic")
3338
except polyglot_module.PolyglotError:
3439
return _extract_unparsed_branch_names(
35-
sql=sql, polyglot_module=polyglot_module, file_label=file_label, label=label
40+
sql=sql,
41+
polyglot_module=polyglot_module,
42+
file_label=file_label,
43+
label=label,
44+
syntax=syntax,
3645
)
3746
return _extract_branch_names(expression=parsed_expression, file_label=file_label, label=label)
3847

3948

4049
def _extract_unparsed_branch_names(
41-
*, sql: str, polyglot_module: Any, file_label: str, label: str
50+
*, sql: str, polyglot_module: Any, file_label: str, label: str, syntax: SqlLexicalSyntax
4251
) -> tuple[tuple[str, ...], ...] | None:
4352
"""Parse each top-level set-operation branch separately when the whole query cannot parse."""
4453

45-
branches: tuple[str, ...] = split_set_operation_branches(sql=sql, context=_SCAN_CONTEXT)
54+
branches: tuple[str, ...] = split_set_operation_branches(
55+
sql=sql, context=_SCAN_CONTEXT, syntax=syntax
56+
)
4657
if len(branches) <= 1:
4758
return None
4859
try:

‎src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -762,12 +762,14 @@ def _extract_expected_branch_column_names(
762762
) -> tuple[tuple[str, ...], ...]:
763763
sql_analysis_column_names: tuple[tuple[str, ...], ...] | None = (
764764
extract_expected_branch_column_names_with_sql_analysis(
765-
sql=sql, file_label=file_label, label=label
765+
sql=sql, file_label=file_label, label=label, syntax=syntax
766766
)
767767
)
768768
if sql_analysis_column_names is not None:
769769
return sql_analysis_column_names
770-
branches: tuple[str, ...] = split_set_operation_branches(sql=sql, context=_CONTEXT)
770+
branches: tuple[str, ...] = split_set_operation_branches(
771+
sql=sql, context=_CONTEXT, syntax=syntax
772+
)
771773
return tuple(
772774
_extract_expected_select_column_names(
773775
branch_sql=branch, file_label=file_label, label=label, syntax=syntax
@@ -837,9 +839,13 @@ def _extract_expected_projection_name(
837839
alias_name: str | None = _extract_as_alias(expression=expression, syntax=syntax)
838840
if alias_name is not None:
839841
return alias_name
840-
stripped_expression: str = expression.strip()
841-
if _is_simple_identifier(stripped_expression):
842-
return stripped_expression
842+
start: int = _skip_ignorable(sql=expression, start=0, syntax=syntax)
843+
if start < len(expression) and is_identifier_start(expression[start]):
844+
name: str
845+
end: int
846+
name, end = _read_identifier(sql=expression, start=start, file_label="projection")
847+
if _skip_ignorable(sql=expression, start=end, syntax=syntax) == len(expression):
848+
return name
843849
raise CompileInputError(
844850
f"SQL test '{file_label}' must alias every non-trivial {label} projection"
845851
)
@@ -862,17 +868,11 @@ def _extract_as_alias(*, expression: str, syntax: SqlLexicalSyntax) -> str | Non
862868
start=alias_index,
863869
file_label="projection",
864870
)
865-
if not expression[alias_end:].strip():
871+
if _skip_ignorable(sql=expression, start=alias_end, syntax=syntax) == len(expression):
866872
last_alias_name = alias_name
867873
return last_alias_name
868874

869875

870-
def _is_simple_identifier(value: str) -> bool:
871-
if not value or not is_identifier_start(value[0]):
872-
return False
873-
return all(is_identifier_character(character) for character in value[1:])
874-
875-
876876
def _contains_select_star(*, sql: str, syntax: SqlLexicalSyntax) -> bool:
877877
index: int
878878
for index, _depth in iter_code_positions(sql=sql, context=_CONTEXT, syntax=syntax):

‎src/sqlbuild/compiler/compile/constants.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
)
9797
POLYGLOT_SELECT_EXPRESSION_NAME: str = "Select"
9898
POLYGLOT_COLUMN_EXPRESSION_NAME: str = "Column"
99-
POLYGLOT_WRAPPER_EXPRESSION_NAMES: frozenset[str] = frozenset({"Subquery", "Paren"})
99+
POLYGLOT_WRAPPER_EXPRESSION_NAMES: frozenset[str] = frozenset({"Annotated", "Subquery", "Paren"})
100100

101101
TABLE_FUNCTION_RETURN_KEYS: frozenset[str] = frozenset({"table"})
102102

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""Index the outermost `- name:` entries of authored YAML declaration files."""
2+
3+
from __future__ import annotations
4+
5+
import re
6+
from collections.abc import Iterator, Mapping
7+
from functools import lru_cache
8+
9+
_ENTRY_PREFIX: re.Pattern[str] = re.compile(r"(?P<indent>\s*)-\s*name:")
10+
_COMMENT_START: str = "#"
11+
_QUOTES: tuple[str, ...] = ("", "'", '"')
12+
13+
14+
@lru_cache(maxsize=16)
15+
def yaml_entry_lines(contents: str) -> Mapping[str, int]:
16+
"""Index every entry name once so per-resource lookups stay linear in file size."""
17+
18+
best: dict[str, tuple[int, int]] = {}
19+
index: int
20+
line: str
21+
for index, line in enumerate(contents.splitlines(), start=1):
22+
prefix: re.Match[str] | None = _ENTRY_PREFIX.match(line)
23+
if prefix is None:
24+
continue
25+
indent: int = len(prefix.group("indent"))
26+
for name in _entry_names(line[prefix.end() :]):
27+
current: tuple[int, int] | None = best.get(name)
28+
if current is None or indent < current[0]:
29+
best[name] = (indent, index)
30+
return {name: line_number for name, (_, line_number) in best.items()}
31+
32+
33+
def _entry_names(value: str) -> Iterator[str]:
34+
"""Yield every name `\\s*(['"]?)<name>\\1\\s*(#.*)?$` accepts for one entry value."""
35+
36+
closings: tuple[int, ...] = _closing_positions(value)
37+
for start in range(_leading_whitespace(text=value, start=0) + 1):
38+
for quote in _QUOTES:
39+
if not value.startswith(quote, start):
40+
continue
41+
body_start: int = start + len(quote)
42+
for closing in closings:
43+
end: int = closing - len(quote)
44+
if end >= body_start and value.startswith(quote, end):
45+
yield value[body_start:end]
46+
47+
48+
def _closing_positions(value: str) -> tuple[int, ...]:
49+
"""Return positions after which only whitespace and an optional comment remain."""
50+
51+
positions: list[int] = []
52+
tail: int = len(value)
53+
while tail >= 0:
54+
position: int = tail
55+
positions.append(position)
56+
while position > 0 and value[position - 1].isspace():
57+
position -= 1
58+
positions.append(position)
59+
tail = value.rfind(_COMMENT_START, 0, tail)
60+
return tuple(sorted(set(positions)))
61+
62+
63+
def _leading_whitespace(*, text: str, start: int) -> int:
64+
position: int = start
65+
while position < len(text) and text[position].isspace():
66+
position += 1
67+
return position

‎src/sqlbuild/compiler/discovery/main/_yaml_entry_line.py‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,10 @@
22

33
from __future__ import annotations
44

5-
import re
5+
from sqlbuild.compiler.discovery._helpers.yml.entry_lines import yaml_entry_lines
66

77

88
def yaml_entry_line(*, contents: str, name: str) -> int | None:
99
"""Return the 1-based line of the outermost `- name: <name>` entry, if present."""
1010

11-
pattern: re.Pattern[str] = re.compile(
12-
rf"^(?P<indent>\s*)-\s*name:\s*(?P<quote>['\"]?){re.escape(name)}(?P=quote)\s*(#.*)?$"
13-
)
14-
best: tuple[int, int] | None = None
15-
index: int
16-
line: str
17-
for index, line in enumerate(contents.splitlines(), start=1):
18-
match: re.Match[str] | None = pattern.match(line)
19-
if match is None:
20-
continue
21-
indent: int = len(match.group("indent"))
22-
if best is None or indent < best[0]:
23-
best = (indent, index)
24-
return None if best is None else best[1]
11+
return yaml_entry_lines(contents).get(name)

‎src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,50 @@ def iter_code_positions_impl(
231231
index += 1
232232

233233

234+
def skip_ignorable_impl(
235+
*, sql: str, start: int, context: str = "SQL", syntax: SqlLexicalSyntax | None = None
236+
) -> int:
237+
"""Return the first offset at or after `start` that is not whitespace or a comment."""
238+
239+
index: int = start
240+
while index < len(sql):
241+
if sql[index].isspace():
242+
index += 1
243+
continue
244+
if sql[index] in SQL_TEXT_START_CHARACTERS:
245+
return index
246+
comment_end: int | None = _non_code_end(
247+
sql=sql, index=index, context=context, syntax=syntax
248+
)
249+
if comment_end is None:
250+
return index
251+
index = comment_end
252+
return index
253+
254+
255+
def strip_ignorable_impl(
256+
*, sql: str, context: str = "SQL", syntax: SqlLexicalSyntax | None = None
257+
) -> str:
258+
"""Return the SQL without leading or trailing whitespace and comments."""
259+
260+
code_start: int = skip_ignorable_impl(sql=sql, start=0, context=context, syntax=syntax)
261+
code_end: int = code_start
262+
index: int = code_start
263+
while index < len(sql):
264+
non_code_end: int | None = _non_code_end(
265+
sql=sql, index=index, context=context, syntax=syntax
266+
)
267+
if non_code_end is None:
268+
index += 1
269+
if not sql[index - 1].isspace():
270+
code_end = index
271+
continue
272+
if sql[index] in SQL_TEXT_START_CHARACTERS:
273+
code_end = non_code_end
274+
index = non_code_end
275+
return sql[code_start:code_end]
276+
277+
234278
def _non_code_end(
235279
*, sql: str, index: int, context: str, syntax: SqlLexicalSyntax | None
236280
) -> int | None:

0 commit comments

Comments
 (0)