Skip to content

Commit 6a3aef4

Browse files
committed
fix(native): match Python's backtick quoting and opaque argument equality
1 parent 101e9d3 commit 6a3aef4

11 files changed

Lines changed: 134 additions & 13 deletions

File tree

‎crates/sqlbuild-attachments/src/audits/_helpers/rendering.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ pub(crate) fn merged_arguments(
7272
Some(Ok(merged))
7373
}
7474

75-
/// Python's `left != right`, or None for opaque values and numbers differing only in text.
75+
/// Python's `left != right`, or None for nested opaque values and numbers differing in text.
7676
fn python_unequal(left: &ArgumentValue, right: &ArgumentValue) -> Option<bool> {
77-
if matches!(left, ArgumentValue::Opaque) || matches!(right, ArgumentValue::Opaque) {
77+
if contains_opaque(left) || contains_opaque(right) {
7878
return None;
7979
}
8080
if left == right {
@@ -108,3 +108,12 @@ fn python_unequal(left: &ArgumentValue, right: &ArgumentValue) -> Option<bool> {
108108
_ => Some(true),
109109
}
110110
}
111+
112+
/// Whether `value` holds an opaque value at any depth, which only Python can compare.
113+
fn contains_opaque(value: &ArgumentValue) -> bool {
114+
match value {
115+
ArgumentValue::Opaque => true,
116+
ArgumentValue::List(items) => items.iter().any(contains_opaque),
117+
_ => false,
118+
}
119+
}

‎crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ fn given_attached_audits_when_rendering_then_python_rendering_is_returned() {
5656
},
5757
expected_rendering: None,
5858
},
59+
AttachedAuditTestCase {
60+
description: "an override holding a nested opaque value defers to Python's equality",
61+
attachment: AuditAttachment {
62+
implicit_arguments: vec![(
63+
"values".to_owned(),
64+
ArgumentValue::List(vec![ArgumentValue::Opaque]),
65+
)],
66+
..attachment(
67+
vec![(
68+
"values".to_owned(),
69+
ArgumentValue::List(vec![ArgumentValue::Opaque]),
70+
)],
71+
None,
72+
None,
73+
)
74+
},
75+
expected_rendering: None,
76+
},
5977
AttachedAuditTestCase {
6078
description: "a missing argument is Python's error before the policies",
6179
attachment: attachment(Vec::new(), Some("fatal"), None),

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option<Str
5858
};
5959
let mut index = 0;
6060
while index < bytes.len() {
61-
let text_end = match non_code_end(bytes, index, QuotePolicy::COMPILER) {
61+
let text_end = match python_non_code_end(bytes, index) {
6262
Ok(Some(end)) => end,
6363
Ok(None) => index,
6464
Err(Unclosed::BlockComment) => return (UNCLOSED_BLOCK_COMMENT, None),
@@ -97,6 +97,18 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option<Str
9797
}
9898
}
9999

100+
/// Python's comment and quote end at `index`; only `'` and `"` double, so backticks never escape.
101+
fn python_non_code_end(bytes: &[u8], index: usize) -> Result<Option<usize>, Unclosed> {
102+
if bytes[index] != b'`' {
103+
return non_code_end(bytes, index, QuotePolicy::COMPILER);
104+
}
105+
bytes[index + 1..]
106+
.iter()
107+
.position(|byte| *byte == b'`')
108+
.map(|offset| Some(index + 1 + offset + 1))
109+
.ok_or(Unclosed::Quote)
110+
}
111+
100112
/// Python's `_interpolate_sql_segment`: substitute every token inside one quoted literal.
101113
fn substitute_quoted_text(
102114
sql: &str,

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,3 +336,20 @@ pub(crate) fn parse_error_on_ordinary_thread(header: String) -> Option<String> {
336336
pub(crate) fn nesting_error_at(position: usize) -> String {
337337
format!("values nest deeper than 256 levels at position {position}")
338338
}
339+
340+
pub(crate) fn doubled_backticks_close_one_segment_and_open_the_next() -> bool {
341+
let sqls = vec![
342+
"SELECT `@@zz``".to_owned(),
343+
"SELECT `@@region``@@region`, '@@region''s'".to_owned(),
344+
"SELECT `a``b` -- @@region".to_owned(),
345+
];
346+
substitute_batch(&sqls, &[("region".to_owned(), "north".to_owned())])
347+
== vec![
348+
(UNKNOWN_VARIABLE, Some("zz".to_owned())),
349+
(
350+
SUBSTITUTED,
351+
Some("SELECT `north``north`, 'north''s'".to_owned()),
352+
),
353+
(UNCHANGED, None),
354+
]
355+
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
use crate::compiler::tests::helpers::{
2-
dollar_quoted_text_is_quoted_for_substitution, dynamic_or_malformed_sql_requests_fallback,
3-
scalar_variables_preserve_lexical_boundaries, unclosed_dollar_quote_stops_as_unclosed_quote,
2+
dollar_quoted_text_is_quoted_for_substitution,
3+
doubled_backticks_close_one_segment_and_open_the_next,
4+
dynamic_or_malformed_sql_requests_fallback, scalar_variables_preserve_lexical_boundaries,
5+
unclosed_dollar_quote_stops_as_unclosed_quote,
46
};
57
use crate::compiler::tests::test_types::StaticSqlOperationTestCase;
68

79
#[test]
810
fn given_sql_interpolation_cases_when_substituting_then_expected_behavior_holds() {
911
let test_cases = [
12+
StaticSqlOperationTestCase {
13+
description: "doubled backticks close one quoted segment and open the next",
14+
run: doubled_backticks_close_one_segment_and_open_the_next,
15+
expected_success: true,
16+
},
1017
StaticSqlOperationTestCase {
1118
description: "scalar variables preserve lexical boundaries",
1219
run: scalar_variables_preserve_lexical_boundaries,

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,16 @@ def attachment_failure_cases() -> tuple[FailureCase, ...]:
7171
**_staging_audits("floor"),
7272
},
7373
),
74+
failure_case(
75+
name="audit-variable-in-doubled-backticks",
76+
expected_code="P001",
77+
expected_message="unknown project variable '@@floor_amount'",
78+
files={
79+
**config_files('\n[vars]\nregion = "north"\n'),
80+
_FLOOR_AUDIT_PATH: _FLOOR_AUDIT.replace("@minimum", "`@@floor_amount``"),
81+
**_staging_audits("floor"),
82+
},
83+
),
7484
failure_case(
7585
name="sql-function-missing-returns",
7686
expected_code="P001",

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@
6767
expected_macro_calls=(1, 1, 1),
6868
expected_error_types=_ERROR_TYPES,
6969
),
70+
NativeAttachmentErrorTestCase(
71+
description="an unknown project variable inside doubled backticks in audit SQL",
72+
case_name="audit-variable-in-doubled-backticks",
73+
expected_macro_calls=(1, 1, 1),
74+
expected_error_types=_ERROR_TYPES,
75+
),
7076
NativeAttachmentErrorTestCase(
7177
description="a SQL function without returns",
7278
case_name="sql-function-missing-returns",

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,11 @@ def generated_authored_sql(*, rng: random.Random) -> str:
150150
"@@ENV:SQB_ORDERS_REGION",
151151
'@const("sales_cap")',
152152
'@enum("order_status").PLACED',
153+
"`",
154+
"``",
155+
"`@@missing``",
156+
"`@@regions``",
157+
"`@@CTX:run.target``",
153158
)
154159

155160

‎tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ class StaticProjectVarDifferentialTestCase:
7878
seed: int
7979
sql_count: int
8080
effective_vars: dict[str, object]
81+
fragments: tuple[str, ...]
8182
expected_minimum_dollar_quote_substitutions: int
83+
expected_minimum_doubled_backtick_substitutions: int
8284

8385

8486
@dataclass(frozen=True)

‎tests/unit/src/sqlbuild/compiler/compile/_helpers/helpers.py‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -833,9 +833,10 @@ class StaticProjectVarDifferential:
833833
native: dict[str, str]
834834
python: dict[str, str | None]
835835
substituted_dollar_quotes: int
836+
substituted_doubled_backticks: int
836837

837838

838-
_LEXICAL_SQL_FRAGMENTS: tuple[str, ...] = (
839+
LEXICAL_SQL_FRAGMENTS: tuple[str, ...] = (
839840
"$",
840841
"$$",
841842
"$t$",
@@ -860,18 +861,32 @@ class StaticProjectVarDifferential:
860861
"_",
861862
"\u00e9",
862863
)
864+
BACKTICK_SQL_FRAGMENTS: tuple[str, ...] = (
865+
"`",
866+
"``",
867+
"'",
868+
"''",
869+
'"',
870+
"@@revision",
871+
"@@status",
872+
"@@",
873+
" ",
874+
"x",
875+
"--",
876+
"\n",
877+
)
863878
_LEXICAL_SQL_MAX_FRAGMENTS: int = 12
864879
_DIFFERENTIAL_FILE_PATH: Path = Path("models/generated.sql")
865880

866881

867-
def generated_lexical_sqls(*, seed: int, count: int) -> tuple[str, ...]:
882+
def generated_lexical_sqls(*, seed: int, count: int, fragments: tuple[str, ...]) -> tuple[str, ...]:
868883
"""Generate SQL-like strings mixing quotes, dollar quotes, comments and variable tokens."""
869884

870885
rng: random.Random = random.Random(seed)
871886
sqls: list[str] = []
872887
for _ in range(count):
873888
fragment_count: int = rng.randint(1, _LEXICAL_SQL_MAX_FRAGMENTS)
874-
sqls.append("".join(rng.choices(_LEXICAL_SQL_FRAGMENTS, k=fragment_count)))
889+
sqls.append("".join(rng.choices(fragments, k=fragment_count)))
875890
return tuple(sqls)
876891

877892

@@ -896,6 +911,9 @@ def native_static_project_var_differential(
896911
substituted_dollar_quotes=sum(
897912
sql != rendered and "$$" in sql for sql, rendered in native.items()
898913
),
914+
substituted_doubled_backticks=sum(
915+
sql != rendered and "``" in sql for sql, rendered in native.items()
916+
),
899917
)
900918

901919

0 commit comments

Comments
 (0)