Skip to content

Commit ba15cf7

Browse files
committed
fix(native): project nested header values without Python recursion
Python projected native header values recursively, spending about four frames per call level, so call syntax nested 231 to 256 levels deep raised RecursionError in every engine. The projection now walks values with an explicit stack, and a plain call counts as the two levels it builds (a hook's arguments count as one more), so the 256-level limit measures the values discovery actually produces. Lint suggests the same flatten help for a header nested too deeply.
1 parent 5ac1887 commit ba15cf7

12 files changed

Lines changed: 409 additions & 42 deletions

File tree

‎crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ impl HeaderParser {
201201
self.advance();
202202
if self.peek().0 == SYMBOL_TOKEN && self.peek().1 == OPEN_PAREN {
203203
self.advance();
204-
return self.nested(token.2, |parser| parser.parse_call(token.1, options));
204+
return self.nested(token.2, |parser| parser.parse_call(token, options));
205205
}
206206
return Ok(parse_word(token.1));
207207
}
@@ -230,17 +230,17 @@ impl HeaderParser {
230230
Err(format!("expected value at position {}", token.2))
231231
}
232232

233-
/// The value after `name(`, which has been consumed.
233+
/// The value after a consumed `name(`; a plain call nests a map in a map, two levels.
234234
fn parse_call(
235235
&mut self,
236-
name: String,
236+
(_, name, position): HeaderToken,
237237
options: ParseValueOptions,
238238
) -> Result<AuthoredValue, String> {
239239
if matches!(name.as_str(), "__ref" | "__seed" | "__source") {
240240
return self.parse_relation(&name);
241241
}
242242
if matches!(name.as_str(), "inline_sql" | "sql" | "python") {
243-
return self.parse_hook(&name);
243+
return self.parse_hook(&name, position);
244244
}
245245
if name == CONSTANT_CALL {
246246
return Ok(AuthoredValue::TypedConstant(self.parse_map(
@@ -250,23 +250,26 @@ impl HeaderParser {
250250
},
251251
)?));
252252
}
253-
Ok(AuthoredValue::Map(vec![(
254-
name,
255-
AuthoredValue::Map(self.parse_map(ParseMapOptions {
253+
let arguments: Vec<(String, AuthoredValue)> = self.nested(position, |parser| {
254+
parser.parse_map(ParseMapOptions {
256255
end: Some(")"),
257256
threshold_policy: options.threshold_policy,
258257
allow_outside: options.allow_outside,
259258
..ParseMapOptions::default()
260-
})?),
259+
})
260+
})?;
261+
Ok(AuthoredValue::Map(vec![(
262+
name,
263+
AuthoredValue::Map(arguments),
261264
)]))
262265
}
263266

264267
/// Parse one container opened at `position`, refusing to nest deeper than the limit.
265-
fn nested(
268+
fn nested<T>(
266269
&mut self,
267270
position: usize,
268-
parse: impl FnOnce(&mut Self) -> Result<AuthoredValue, String>,
269-
) -> Result<AuthoredValue, String> {
271+
parse: impl FnOnce(&mut Self) -> Result<T, String>,
272+
) -> Result<T, String> {
270273
if self.depth >= MAX_HEADER_NESTING_DEPTH {
271274
return Err(nesting_error(position));
272275
}
@@ -334,7 +337,7 @@ impl HeaderParser {
334337
if !matches!(token.1.as_str(), "inline_sql" | "sql" | "python") {
335338
return Err(error());
336339
}
337-
self.nested(token.2, |parser| parser.parse_hook(&token.1))
340+
self.nested(token.2, |parser| parser.parse_hook(&token.1, token.2))
338341
}
339342

340343
fn parse_relation(&mut self, name: &str) -> Result<AuthoredValue, String> {
@@ -352,7 +355,8 @@ impl HeaderParser {
352355
)))
353356
}
354357

355-
fn parse_hook(&mut self, name: &str) -> Result<AuthoredValue, String> {
358+
/// The hook after `name(` at `position`; its keyword arguments are one more level.
359+
fn parse_hook(&mut self, name: &str, position: usize) -> Result<AuthoredValue, String> {
356360
if name == INLINE_SQL_HOOK {
357361
let token = self.peek().clone();
358362
if token.0 != STRING_TOKEN {
@@ -373,7 +377,7 @@ impl HeaderParser {
373377
if token.1.chars().all(is_python_whitespace) {
374378
return Err(format!("{name}(...) requires a non-empty hook name"));
375379
}
376-
let kwargs = self.parse_hook_kwargs()?;
380+
let kwargs = self.nested(position, Self::parse_hook_kwargs)?;
377381
if name == SQL_HOOK {
378382
Ok(AuthoredValue::NamedSqlHook(token.1, kwargs))
379383
} else {

‎crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,32 @@ fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_c
8484
depth: 100_000,
8585
expected_error_position: Some(1_285),
8686
},
87+
HeaderNestingTestCase {
88+
description: "calls count their map and arguments, so 128 reach the limit",
89+
prefix: "meta ",
90+
open: "call(key ",
91+
close: ")",
92+
suffix: "",
93+
depth: 128,
94+
expected_error_position: None,
95+
},
96+
HeaderNestingTestCase {
97+
description: "calls one past the limit fail at the outermost extra call",
98+
prefix: "meta ",
99+
open: "call(key ",
100+
close: ")",
101+
suffix: "",
102+
depth: 129,
103+
expected_error_position: Some(1_157),
104+
},
87105
HeaderNestingTestCase {
88106
description: "calls 20k deep",
89107
prefix: "meta ",
90108
open: "call(key ",
91109
close: ")",
92110
suffix: "",
93111
depth: 20_000,
94-
expected_error_position: Some(2_309),
112+
expected_error_position: Some(1_157),
95113
},
96114
HeaderNestingTestCase {
97115
description: "typed constants 1k deep",
@@ -103,13 +121,13 @@ fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_c
103121
expected_error_position: Some(3_845),
104122
},
105123
HeaderNestingTestCase {
106-
description: "hook arguments 100k deep count the hook list and each hook",
124+
description: "hook arguments 100k deep count the hook list, each hook and its arguments",
107125
prefix: "post_hooks [",
108126
open: "sql(\"refresh\", nested: ",
109127
close: ")",
110128
suffix: "]",
111129
depth: 100_000,
112-
expected_error_position: Some(5_877),
130+
expected_error_position: Some(2_933),
113131
},
114132
];
115133

‎scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
_MACRO_MODULE: str = "def cents(value):\n return f'{value} * 100'\n"
3535
_DEEP_HEADER_DEPTH: int = 20_000
3636
_DEEP_HEADER_VALUE: str = "[" * _DEEP_HEADER_DEPTH + '"placed"' + "]" * _DEEP_HEADER_DEPTH
37+
_DEEP_HEADER_CALLS: str = "limit(level " * _DEEP_HEADER_DEPTH + "1" + ")" * _DEEP_HEADER_DEPTH
3738
_DEEP_HEADER_MESSAGE: str = "contains invalid SQLBuild header syntax: values nest deeper than 256"
3839
_SEED_DECLARATION: str = (
3940
"seeds:\n - name: order_channels\n description: Order channels.\n"
@@ -165,7 +166,7 @@ def _sql_file_cases() -> tuple[FailureCase, ...]:
165166
expected_code="D013",
166167
files={
167168
"models/staging/_sqlbuild/_constants/statuses.sql": (
168-
f"CONSTANT (name kept_statuses, value {_DEEP_HEADER_VALUE});\n"
169+
f"CONSTANT (name kept_statuses, value {_DEEP_HEADER_CALLS});\n"
169170
)
170171
},
171172
expected_message=_DEEP_HEADER_MESSAGE,

‎src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py‎

Lines changed: 89 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
import re
66
from bisect import bisect_right
77
from collections import OrderedDict
8-
from dataclasses import dataclass
8+
from collections.abc import Callable
9+
from dataclasses import dataclass, field
910
from pathlib import Path
1011
from typing import cast
1112

@@ -52,6 +53,7 @@
5253
_MODEL_HEADER_INTEGER_PATTERN: re.Pattern[str] = re.compile(r"^[+-]?\d+$")
5354
_MODEL_HEADER_FLOAT_PATTERN: re.Pattern[str] = re.compile(r"^[+-]?(?:\d+\.\d*|\d*\.\d+)$")
5455
_MODEL_HEADER_TOKEN_CACHE_SIZE: int = 4096
56+
_PROJECTION_PENDING: object = object()
5557
_MODEL_HEADER_NESTING_ERROR_PATTERN: re.Pattern[str] = re.compile(
5658
r"values nest deeper than (?P<limit>\d+) levels at position (?P<position>\d+)"
5759
)
@@ -79,6 +81,15 @@
7981
_SELECT_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"['\"`()sSfFuU]")
8082

8183

84+
@dataclass
85+
class _ProjectionFrame:
86+
"""One container being projected: its native children, projected so far, and its builder."""
87+
88+
children: tuple[object, ...]
89+
build: Callable[[list[object]], object]
90+
projected: list[object] = field(default_factory=list)
91+
92+
8293
@dataclass(frozen=True)
8394
class _ModelHeaderToken:
8495
kind: str
@@ -575,17 +586,22 @@ def parse_header_values(
575586
raise
576587
except ModelHeaderSyntaxError as error:
577588
nesting: re.Match[str] | None = _MODEL_HEADER_NESTING_ERROR_PATTERN.fullmatch(str(error))
589+
nesting_help: str | None = (
590+
None
591+
if nesting is None
592+
else f"flatten the value so it nests at most {nesting.group('limit')} levels deep"
593+
)
578594
if nesting is None or header_line is None:
579595
raise error_class(
580596
f"{statement_name}(...) in '{file_path}' contains invalid SQLBuild header syntax: "
581-
f"{error}"
597+
f"{error}",
598+
help=nesting_help,
582599
) from error
583-
limit: str = nesting.group("limit")
584600
line: int = header_line + header.count("\n", 0, int(nesting.group("position")))
585601
raise error_class(
586602
f"{statement_name}(...) in '{file_path}:{line}' contains invalid SQLBuild header "
587-
f"syntax: values nest deeper than {limit} levels",
588-
help=f"flatten the value so it nests at most {limit} levels deep",
603+
f"syntax: values nest deeper than {nesting.group('limit')} levels",
604+
help=nesting_help,
589605
) from error
590606

591607

@@ -653,47 +669,98 @@ def project_native_header_values(values: dict[str, object]) -> dict[str, object]
653669

654670

655671
def _project_native_header_map(values: dict[str, object]) -> dict[str, object]:
656-
return {key: _project_native_header_value(value) for key, value in values.items()}
672+
return cast(dict[str, object], _project_native_header_value(values))
657673

658674

659675
def _project_native_header_value(value: object) -> object:
676+
"""Project one native value depth-first with an explicit stack, never Python recursion."""
677+
678+
frames: list[_ProjectionFrame] = []
679+
node: object = value
680+
while True:
681+
expansion: _ProjectionFrame | None = _projection_frame(node)
682+
if expansion is None:
683+
projected: object = _project_native_leaf(node)
684+
else:
685+
frames.append(expansion)
686+
projected = _PROJECTION_PENDING
687+
while True:
688+
if projected is not _PROJECTION_PENDING:
689+
if not frames:
690+
return projected
691+
frames[-1].projected.append(projected)
692+
frame: _ProjectionFrame = frames[-1]
693+
if len(frame.projected) < len(frame.children):
694+
node = frame.children[len(frame.projected)]
695+
break
696+
frames.pop()
697+
projected = frame.build(frame.projected)
698+
699+
700+
def _projection_frame(value: object) -> _ProjectionFrame | None:
660701
if isinstance(value, dict):
661-
return _project_native_header_map(cast(dict[str, object], value))
702+
return _map_frame(values=cast(dict[str, object], value), build=dict)
662703
if isinstance(value, list):
663-
return [_project_native_header_value(item) for item in value]
704+
return _ProjectionFrame(children=tuple(value), build=list)
664705
if (
665706
not isinstance(value, tuple)
666707
or len(value) != _NATIVE_MARKER_LENGTH
667708
or not isinstance(value[0], str)
668709
):
669-
return value
710+
return None
670711
kind: str = value[0]
671712
payload: object = value[1]
672-
if kind == _MODEL_HEADER_WORD_TOKEN and isinstance(payload, str):
673-
return _parse_word_value(payload)
674713
if kind == _NATIVE_SET_MARKER and isinstance(payload, list):
675-
return AuthoredSqlSet(tuple(_project_native_header_value(item) for item in payload))
714+
return _ProjectionFrame(
715+
children=tuple(payload), build=lambda items: AuthoredSqlSet(tuple(items))
716+
)
676717
if kind == _NATIVE_TUPLE_MARKER and isinstance(payload, list):
677-
return tuple(_project_native_header_value(item) for item in payload)
718+
return _ProjectionFrame(children=tuple(payload), build=tuple)
678719
if kind == _NATIVE_CONSTANT_MARKER and isinstance(payload, dict):
679-
return AuthoredSqlValueCall(
680-
arguments=tuple(_project_native_header_map(cast(dict[str, object], payload)).items())
720+
return _map_frame(
721+
values=cast(dict[str, object], payload),
722+
build=lambda items: AuthoredSqlValueCall(arguments=tuple(items)),
681723
)
682-
if kind == _NATIVE_INLINE_SQL_MARKER and isinstance(payload, str):
683-
return SqlHookEntry(statement=payload)
684724
if (
685725
kind in {_NATIVE_SQL_MARKER, _NATIVE_PYTHON_MARKER}
686726
and isinstance(payload, tuple)
687727
and len(payload) == _NATIVE_MARKER_LENGTH
688728
):
689729
name, kwargs = payload
690730
if isinstance(name, str) and isinstance(kwargs, dict):
691-
projected_kwargs: dict[str, object] = _project_native_header_map(
692-
cast(dict[str, object], kwargs)
731+
entry_type: type[NamedSqlHookEntry] | type[PythonHookEntry] = (
732+
NamedSqlHookEntry if kind == _NATIVE_SQL_MARKER else PythonHookEntry
693733
)
694-
if kind == _NATIVE_SQL_MARKER:
695-
return NamedSqlHookEntry(name=name, kwargs=projected_kwargs)
696-
return PythonHookEntry(name=name, kwargs=projected_kwargs)
734+
return _map_frame(
735+
values=cast(dict[str, object], kwargs),
736+
build=lambda items: entry_type(name=name, kwargs=dict(items)),
737+
)
738+
return None
739+
740+
741+
def _map_frame(
742+
*, values: dict[str, object], build: Callable[[list[tuple[str, object]]], object]
743+
) -> _ProjectionFrame:
744+
keys: tuple[str, ...] = tuple(values)
745+
return _ProjectionFrame(
746+
children=tuple(values.values()),
747+
build=lambda projected: build(list(zip(keys, projected, strict=True))),
748+
)
749+
750+
751+
def _project_native_leaf(value: object) -> object:
752+
if (
753+
not isinstance(value, tuple)
754+
or len(value) != _NATIVE_MARKER_LENGTH
755+
or not isinstance(value[0], str)
756+
):
757+
return value
758+
kind: str = value[0]
759+
payload: object = value[1]
760+
if kind == _MODEL_HEADER_WORD_TOKEN and isinstance(payload, str):
761+
return _parse_word_value(payload)
762+
if kind == _NATIVE_INLINE_SQL_MARKER and isinstance(payload, str):
763+
return SqlHookEntry(statement=payload)
697764
raise ModelHeaderSyntaxError(f"Native MODEL header parser returned invalid '{kind}' marker")
698765

699766

‎src/sqlbuild/lint/_helpers/native.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
source_line_starts,
1717
source_position,
1818
)
19-
from sqlbuild.compiler.discovery.exceptions import ModelSqlParseError
19+
from sqlbuild.compiler.discovery.exceptions import DiscoveryError, ModelSqlParseError
2020
from sqlbuild.lint.constants import (
2121
CARRIAGE_RETURN_LINE_FEED,
2222
CLOSING_PAREN_CHARACTER,
@@ -245,7 +245,7 @@ def _lint_header_values(
245245
message=f"{header.kind}() header could not be parsed: {error}",
246246
severity=VIOLATION_SEVERITY_FAULT,
247247
engine=LINT_ENGINE_SQLBUILD,
248-
remediation=f"Correct the {header.kind}() header syntax.",
248+
remediation=_header_parse_remediation(kind=header.kind, error=error),
249249
),
250250
)
251251

@@ -304,6 +304,11 @@ def _lint_header_values(
304304
return tuple(violations)
305305

306306

307+
def _header_parse_remediation(*, kind: str, error: Exception) -> str:
308+
help_text: str | None = error.help if isinstance(error, DiscoveryError) else None
309+
return f"Correct the {kind}() header syntax." if help_text is None else help_text
310+
311+
307312
def _parse_header_values(*, kind: str, header_text: str) -> dict[str, object]:
308313
inner: str = _inner_header_text(header_text=header_text)
309314
return parse_header_values(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,3 +985,11 @@ class DeepHeaderNestingCompileCase:
985985
suffix: str
986986
depth: int
987987
expected_fragments: tuple[str, ...]
988+
989+
990+
@dataclass(frozen=True)
991+
class DeepCallNestingCompileCase:
992+
description: str
993+
depth: int
994+
engines: tuple[str, ...]
995+
expected_fragments: tuple[str, ...]

0 commit comments

Comments
 (0)