Skip to content

Commit c9b0985

Browse files
authored
fix(compiler): treat dollar-quoted strings as quoted text in variable substitution (#1020)
* fix(compiler): treat dollar-quoted strings as quoted text in variable substitution The native @@var fast path on the model path now scans with the shared compiler scanner, so dollar-quoted text is quoted exactly as in the pure-Python scanner and an unclosed dollar quote falls back to Python's interpolation error. Adds a native-versus-Python differential test, Rust cases, an E2E compile test on each engine, and harness cases. * fix(compiler): route dollar-quoted authored SQL through the native attachments path The native substitution scanner now reads dollar quotes like Python, so the preview ATTACHMENTS stage no longer hands SQL containing $ to the Python scanner. Adds dollar-heavy authored SQL parity and dollar-quoted attachment project SQL.
1 parent 620bca0 commit c9b0985

15 files changed

Lines changed: 406 additions & 48 deletions

File tree

‎crates/sqlbuild-sqltext/src/compiler/_helpers/sql_interpolation/substitution.rs‎

Lines changed: 34 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
//! Conservative native fast path for compile-wide scalar SQL variables.
22
3+
use crate::sql_scan::main::non_code_end::non_code_end;
4+
use crate::sql_scan::models::QuotePolicy;
35
use std::collections::HashMap;
6+
use std::ops::Range;
47

58
pub(crate) const UNCHANGED: u8 = 0;
69
pub(crate) const SUBSTITUTED: u8 = 1;
@@ -35,48 +38,23 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option<Str
3538
copy_start: 0,
3639
};
3740
let mut index = 0;
38-
let mut quote: Option<u8> = None;
3941
while index < bytes.len() {
40-
if let Some(quote_byte) = quote {
41-
if bytes[index] == quote_byte {
42-
if matches!(quote_byte, b'\'' | b'"')
43-
&& bytes.get(index + 1).copied() == Some(quote_byte)
44-
{
45-
index += 2;
46-
continue;
47-
}
48-
quote = None;
49-
index += 1;
50-
continue;
51-
}
52-
if bytes[index..].starts_with(b"@@") {
53-
let Some((end, next_state)) = substitute_token(sql, index, variables, state) else {
42+
let text_end = match non_code_end(bytes, index, QuotePolicy::COMPILER) {
43+
Ok(Some(end)) => end,
44+
Ok(None) => index,
45+
Err(_) => return (FALLBACK, None),
46+
};
47+
if text_end > index {
48+
let comment = bytes[index..].starts_with(b"--") || bytes[index..].starts_with(b"/*");
49+
if !comment {
50+
let Some(next_state) =
51+
substitute_quoted_text(sql, index..text_end, variables, state)
52+
else {
5453
return (FALLBACK, None);
5554
};
5655
state = next_state;
57-
index = end;
58-
continue;
5956
}
60-
index += 1;
61-
continue;
62-
}
63-
64-
if bytes[index..].starts_with(b"--") {
65-
index = sql[index + 2..]
66-
.find('\n')
67-
.map_or(bytes.len(), |offset| index + 3 + offset);
68-
continue;
69-
}
70-
if bytes[index..].starts_with(b"/*") {
71-
let Some(offset) = sql[index + 2..].find("*/") else {
72-
return (FALLBACK, None);
73-
};
74-
index += offset + 4;
75-
continue;
76-
}
77-
if matches!(bytes[index], b'\'' | b'"' | b'`') {
78-
quote = Some(bytes[index]);
79-
index += 1;
57+
index = text_end;
8058
continue;
8159
}
8260
if bytes[index..].starts_with(b"@@") {
@@ -89,9 +67,6 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option<Str
8967
}
9068
index += 1;
9169
}
92-
if quote.is_some() {
93-
return (FALLBACK, None);
94-
}
9570
match state.output {
9671
Some(mut rendered) => {
9772
rendered.push_str(&sql[state.copy_start..]);
@@ -101,6 +76,25 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option<Str
10176
}
10277
}
10378

79+
/// Python's `_interpolate_sql_segment`: substitute every token inside one quoted literal.
80+
fn substitute_quoted_text(
81+
sql: &str,
82+
text: Range<usize>,
83+
variables: &HashMap<&str, &str>,
84+
mut state: SubstitutionState,
85+
) -> Option<SubstitutionState> {
86+
let mut index = text.start;
87+
while let Some(offset) = sql[index..text.end].find("@@") {
88+
let (token_end, next_state) = substitute_token(sql, index + offset, variables, state)?;
89+
if token_end > text.end {
90+
return None;
91+
}
92+
state = next_state;
93+
index = token_end;
94+
}
95+
Some(state)
96+
}
97+
10498
fn substitute_token(
10599
sql: &str,
106100
start: usize,

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,40 @@ pub(crate) fn dynamic_or_malformed_sql_requests_fallback() -> bool {
4545
== vec![(FALLBACK, None); sqls.len()]
4646
}
4747

48+
pub(crate) fn dollar_quoted_text_is_quoted_for_substitution() -> bool {
49+
let sqls = vec![
50+
"SELECT $$--@@region$$ AS x".to_owned(),
51+
"SELECT $$ /* $$ AS a, '@@region' AS b -- */".to_owned(),
52+
"SELECT $tag$ it's $$ -- @@region $tag$, @@region".to_owned(),
53+
"SELECT price$1$ -- @@region\n, $1 /* @@region */".to_owned(),
54+
"SELECT a$$b$$ -- @@region".to_owned(),
55+
];
56+
substitute_batch(&sqls, &[("region".to_owned(), "north".to_owned())])
57+
== vec![
58+
(SUBSTITUTED, Some("SELECT $$--north$$ AS x".to_owned())),
59+
(
60+
SUBSTITUTED,
61+
Some("SELECT $$ /* $$ AS a, 'north' AS b -- */".to_owned()),
62+
),
63+
(
64+
SUBSTITUTED,
65+
Some("SELECT $tag$ it's $$ -- north $tag$, north".to_owned()),
66+
),
67+
(UNCHANGED, None),
68+
(UNCHANGED, None),
69+
]
70+
}
71+
72+
pub(crate) fn unclosed_dollar_quote_requests_fallback() -> bool {
73+
let sqls = vec![
74+
"SELECT $tag$ @@region".to_owned(),
75+
"SELECT @@region, $$ open".to_owned(),
76+
"-- @@region\nSELECT $t$ closes with another tag $u$".to_owned(),
77+
];
78+
substitute_batch(&sqls, &[("region".to_owned(), "north".to_owned())])
79+
== vec![(FALLBACK, None); sqls.len()]
80+
}
81+
4882
pub(crate) fn simple_references_preserve_authored_order() -> bool {
4983
extract(
5084
"SELECT * FROM __source(\"orders\") UNION ALL SELECT * FROM __dbt_ref(\"shop\" , \"customers\")",

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::compiler::tests::helpers::{
2-
dynamic_or_malformed_sql_requests_fallback, scalar_variables_preserve_lexical_boundaries,
2+
dollar_quoted_text_is_quoted_for_substitution, dynamic_or_malformed_sql_requests_fallback,
3+
scalar_variables_preserve_lexical_boundaries, unclosed_dollar_quote_requests_fallback,
34
};
45
use crate::compiler::tests::test_types::StaticSqlOperationTestCase;
56

@@ -16,6 +17,16 @@ fn given_sql_interpolation_cases_when_substituting_then_expected_behavior_holds(
1617
run: dynamic_or_malformed_sql_requests_fallback,
1718
expected_success: true,
1819
},
20+
StaticSqlOperationTestCase {
21+
description: "dollar-quoted text is quoted text, not comments or code",
22+
run: dollar_quoted_text_is_quoted_for_substitution,
23+
expected_success: true,
24+
},
25+
StaticSqlOperationTestCase {
26+
description: "unclosed dollar quotes request Python fallback",
27+
run: unclosed_dollar_quote_requests_fallback,
28+
expected_success: true,
29+
},
1930
];
2031

2132
for test_case in test_cases {

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,15 @@ def _interpolation_cases() -> tuple[FailureCase, ...]:
145145
columns="amount, status, '@@ENV:SQB_DIFFERENTIAL_UNSET' AS channel"
146146
),
147147
),
148+
failure_case(
149+
name="unclosed-dollar-quote-with-project-variable",
150+
expected_code="P001",
151+
expected_message="SQL interpolation contains an unclosed quoted string",
152+
files={
153+
**config_files('\n[vars]\nregion = "north"\n'),
154+
**_staging_columns(columns="amount, status, $tag$ @@region AS region"),
155+
},
156+
),
148157
failure_case(
149158
name="context-variable-in-model-sql",
150159
expected_code="P001",

‎scripts/compiler_differential/classes/render_features.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ def _interpolation(self) -> None:
208208
body=(
209209
"SELECT id,\n"
210210
" '@@site_label' AS site,\n"
211+
" $$--@@site_label /* $$ AS site_note,\n"
211212
f" '@@ENV:{GENERATOR_CHANNEL_ENV_VAR}' AS channel,\n"
212213
" amount * @@discount_rate AS discounted\n"
213214
f'FROM __ref("{base.name}")\n'

‎src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
from sqlbuild.compiler.compile.constants import (
2222
COMPILE_INPUT_READS,
2323
SQL_CONTEXT_NAME_EXTRA_TOKENS,
24-
SQL_DOLLAR_QUOTE_TOKEN,
2524
SQL_IDENTIFIER_EXTRA_TOKEN,
2625
SQL_INTERPOLATION_TOKEN,
2726
SQL_QUOTE_TOKENS,
@@ -81,7 +80,7 @@ def expand_authored_sql_result( # noqa: PLR0913
8180
native: bool = native_stage_enabled(NativeStage.ATTACHMENTS)
8281
prepared_sql: str | None = (
8382
prepare_static_project_vars_batch(sqls=(sql,), effective_vars=effective_vars)[0]
84-
if native and SQL_DOLLAR_QUOTE_TOKEN not in sql
83+
if native
8584
else None
8685
)
8786
interpolated_sql: str = (

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@
9090
SQL_STATEMENT_TERMINATOR_TOKEN: str = ";"
9191
SQL_SINGLE_QUOTE_TOKEN: str = "'"
9292
SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`", "$"})
93-
SQL_DOLLAR_QUOTE_TOKEN: str = "$"
9493
SQL_QUALIFIER_SEPARATOR_TOKEN: str = "."
9594
SQL_REFERENCE_NAME_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"'})
9695
SQL_IDENTIFIER_EXTRA_TOKEN: str = "_"

‎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
@@ -993,3 +993,11 @@ class DeepCallNestingCompileCase:
993993
depth: int
994994
engines: tuple[str, ...]
995995
expected_fragments: tuple[str, ...]
996+
997+
998+
@dataclass(frozen=True)
999+
class DollarQuotedProjectVarCompileCase:
1000+
description: str
1001+
engine: str
1002+
model_sql: str
1003+
expected_fragment: str
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
"""E2E coverage for project variables around dollar-quoted text on every compiler engine."""
2+
3+
from __future__ import annotations
4+
5+
import subprocess
6+
from pathlib import Path
7+
8+
import pytest
9+
10+
from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import (
11+
DollarQuotedProjectVarCompileCase,
12+
)
13+
from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb
14+
15+
_ENGINES: tuple[str, ...] = ("python", "native", "native-preview")
16+
_PROJECT_TOML: str = 'name = "orders"\nadapter = "duckdb"\n\n[vars]\nregion = "north"\n'
17+
_MODEL_HEADER: str = "MODEL (description 'Order region labels.', materialized view);\n\n"
18+
_COMPILED_MODEL: Path = Path("target/compiled/models/order_labels.sql")
19+
20+
21+
@pytest.mark.parametrize(
22+
"test_case",
23+
[
24+
DollarQuotedProjectVarCompileCase(
25+
description=f"comment_markers_inside_dollar_quotes_are_text_{engine}",
26+
engine=engine,
27+
model_sql=(
28+
"SELECT $$--@@region$$ AS label, $tag$ it's /* $tag$ AS note, "
29+
"$$ /* $$ AS a, '@@region' AS b -- */\n"
30+
),
31+
expected_fragment=(
32+
"SELECT $$--north$$ AS label, $tag$ it's /* $tag$ AS note, "
33+
"$$ /* $$ AS a, 'north' AS b -- */"
34+
),
35+
)
36+
for engine in _ENGINES
37+
],
38+
ids=lambda case: case.description,
39+
)
40+
def test_given_dollar_quoted_model_sql_when_compiling_then_vars_substitute_inside_quotes(
41+
test_case: DollarQuotedProjectVarCompileCase, tmp_path: Path
42+
) -> None:
43+
project_dir: Path = prepare_inline_project(
44+
tmp_path=tmp_path,
45+
project_name="orders",
46+
repo_files={
47+
"sqlbuild_project.toml": _PROJECT_TOML,
48+
"models/order_labels.sql": _MODEL_HEADER + test_case.model_sql,
49+
},
50+
)
51+
52+
result: subprocess.CompletedProcess[str] = run_sqb(
53+
command=("--no-color", "--compiler-engine", test_case.engine, "compile"),
54+
project_dir=project_dir,
55+
)
56+
57+
assert result.returncode == 0, result.stdout + result.stderr
58+
compiled_sql: str = (project_dir / _COMPILED_MODEL).read_text(encoding="utf-8")
59+
assert test_case.expected_fragment in compiled_sql
60+
61+
62+
@pytest.mark.parametrize(
63+
"test_case",
64+
[
65+
DollarQuotedProjectVarCompileCase(
66+
description=f"unclosed_dollar_quote_reports_interpolation_error_{engine}",
67+
engine=engine,
68+
model_sql="SELECT $tag$ @@region AS label\n",
69+
expected_fragment="error[P001]: SQL interpolation contains an unclosed quoted string",
70+
)
71+
for engine in _ENGINES
72+
],
73+
ids=lambda case: case.description,
74+
)
75+
def test_given_unclosed_dollar_quote_with_var_when_compiling_then_interpolation_reports_it(
76+
test_case: DollarQuotedProjectVarCompileCase, tmp_path: Path
77+
) -> None:
78+
project_dir: Path = prepare_inline_project(
79+
tmp_path=tmp_path,
80+
project_name="orders",
81+
repo_files={
82+
"sqlbuild_project.toml": _PROJECT_TOML,
83+
"models/order_labels.sql": _MODEL_HEADER + test_case.model_sql,
84+
},
85+
)
86+
87+
result: subprocess.CompletedProcess[str] = run_sqb(
88+
command=("--no-color", "--compiler-engine", test_case.engine, "compile"),
89+
project_dir=project_dir,
90+
)
91+
92+
output: str = result.stdout + result.stderr
93+
assert result.returncode == 1, output
94+
assert test_case.expected_fragment in output
95+
96+
97+
if __name__ == "__main__":
98+
pytest.main([__file__, "-vv"])

‎tests/integration/src/sqlbuild/compiler/attachments/_test_types.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
22

3+
import random
4+
from collections.abc import Callable
35
from dataclasses import dataclass
46

57

@@ -10,6 +12,7 @@ class AuthoredSqlParityTestCase:
1012
description: str
1113
seed: int
1214
count: int
15+
generate: Callable[[random.Random], str]
1316
expected_minimum_expanded: int
1417
expected_minimum_python_errors: int
1518

0 commit comments

Comments
 (0)