diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/extraction.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/extraction.rs index 7e7688c379..dd207b475b 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/extraction.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/extraction.rs @@ -512,7 +512,7 @@ fn projection_name(expression: &str, file: &str, label: &str) -> Result( Ok(None) } -fn non_empty_trimmed(value: &str) -> Option<&str> { - Some(value.trim()).filter(|trimmed| !trimmed.is_empty()) +/// Return `value` without surrounding whitespace and comments, or `None` when no code remains. +fn non_empty_trimmed(value: &str) -> Result, String> { + if !value.contains("--") && !value.contains("/*") { + return Ok(Some(value.trim()).filter(|trimmed| !trimmed.is_empty())); + } + let start = skip_ignorable(value, 0)?; + let mut end = start; + let mut index = start; + while index < value.len() { + if let Some(comment) = comment_end(value.as_bytes(), index) + .map_err(|error| scan_error_message(error, "SQL test"))? + { + index = comment; + continue; + } + let next = skip_non_code(value, index)?; + if next != index { + end = next; + index = next; + continue; + } + let width = char_len(value, index); + if !value[index..].starts_with(char::is_whitespace) { + end = index + width; + } + index += width; + } + Ok(Some(&value[start..end]).filter(|trimmed| !trimmed.is_empty())) } /// 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, String> { let Some(end) = set_operator_end(sql, index)? else { return Ok(CodeStep::Advance); }; - values.extend(non_empty_trimmed(&sql[start..index])); + values.extend(non_empty_trimmed(&sql[start..index])?); let mut resume = skip_ignorable(sql, end)?; if let Some(quantifier_end) = 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, String> { start = resume; Ok(CodeStep::Resume(resume)) })?; - values.extend(non_empty_trimmed(&sql[start..])); + values.extend(non_empty_trimmed(&sql[start..])?); Ok(values) } @@ -927,12 +953,12 @@ pub(crate) fn split_top_level(sql: &str, separator: u8) -> Result, Str let mut start = 0; scan_code::<()>(sql, 0, |index, depth| { if depth == 0 && byte_at(sql, index) == Some(separator) { - values.extend(non_empty_trimmed(&sql[start..index])); + values.extend(non_empty_trimmed(&sql[start..index])?); start = index + 1; } Ok(CodeStep::Advance) })?; - values.extend(non_empty_trimmed(&sql[start..])); + values.extend(non_empty_trimmed(&sql[start..])?); Ok(values) } diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_extraction.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_extraction.rs index e5b777fb29..c96e10fa00 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_extraction.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_extraction.rs @@ -91,12 +91,26 @@ fn given_expected_cte_sql_when_scanning_top_level_then_set_operations_commas_and ]), expected_commas: Ok(vec![ "SELECT 'x, UNION (' AS a", - "/* UNION , FROM */ \"b)\"", + "\"b)\"", "`c,d`", "f(1, (SELECT 2 FROM t UNION SELECT 3)) -- , FROM\nFROM t", ]), expected_from: Ok(Some(107)), }, + TopLevelScanTestCase { + description: "comments around branches and projections are not code", + sql: "-- lead\nSELECT 1 AS a, /* b */ 2 AS b -- first\n/* x */ UNION ALL -- second\nSELECT '--' AS a, 3 AS b /* end */", + expected_unions: Ok(vec![ + "SELECT 1 AS a, /* b */ 2 AS b", + "SELECT '--' AS a, 3 AS b", + ]), + expected_commas: Ok(vec![ + "SELECT 1 AS a", + "2 AS b -- first\n/* x */ UNION ALL -- second\nSELECT '--' AS a", + "3 AS b", + ]), + expected_from: Ok(None), + }, TopLevelScanTestCase { description: "keyword boundaries and empty pieces", sql: ",SELECT reunion, union_id,, x FROMAGE UNION", diff --git a/scripts/cold_compile_performance/_helpers/diagnostic_project.py b/scripts/cold_compile_performance/_helpers/diagnostic_project.py index 71a8d54044..b84cd99849 100644 --- a/scripts/cold_compile_performance/_helpers/diagnostic_project.py +++ b/scripts/cold_compile_performance/_helpers/diagnostic_project.py @@ -132,3 +132,34 @@ def write_single_model_diagnostic_project(*, project_dir: Path, diagnostics: int + ",\n".join(projections) + '\nFROM __ref("orders") AS o\n' ) + + +def write_undescribed_project(*, project_dir: Path, model_count: int, width: int = 6) -> None: + """Generate models, seeds, and one large source file whose resources all lack descriptions.""" + project_dir.mkdir(parents=True, exist_ok=True) + for folder in ("models", "sources", "seeds"): + (project_dir / folder).mkdir(exist_ok=True) + (project_dir / "sqlbuild_project.toml").write_text( + 'name = "undescribed_orders"\nadapter = "duckdb"\n[rules]\nselect = []\n' + ) + columns: list[str] = [f"amount_{index:03}" for index in range(width)] + sources: list[str] = ["sources:"] + seeds: list[str] = ["seeds:"] + for index in range(model_count): + sources.extend( + (f" - name: orders_{index:05}", " schema: main", f" table: orders_{index:05}") + ) + sources.append(" columns:") + sources.extend(f" - name: {column}\n type: INTEGER" for column in columns) + (project_dir / "models" / f"order_totals_{index:05}.sql").write_text( + "MODEL (materialized view);\n" + f'SELECT {", ".join(columns)} FROM __source("orders_{index:05}")\n' + ) + if index % 10 == 0: + seeds.append(f" - name: products_{index:05}\n columns:") + seeds.extend(f" - name: {column}\n type: INTEGER" for column in columns) + (project_dir / "seeds" / f"products_{index:05}.csv").write_text( + ",".join(columns) + "\n" + ",".join("1" for _ in columns) + "\n" + ) + (project_dir / "sources" / "orders.yml").write_text("\n".join(sources) + "\n") + (project_dir / "seeds" / "products.yml").write_text("\n".join(seeds) + "\n") diff --git a/src/sqlbuild/compiler/compile/_helpers/analysis/tests.py b/src/sqlbuild/compiler/compile/_helpers/analysis/tests.py index c56f1ce44b..9a5350e941 100644 --- a/src/sqlbuild/compiler/compile/_helpers/analysis/tests.py +++ b/src/sqlbuild/compiler/compile/_helpers/analysis/tests.py @@ -15,6 +15,7 @@ split_set_operation_branches, ) from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax _POLYGLOT_VALUES_EXPRESSION_NAME: str = "Values" _POLYGLOT_VALUES_SET_ALIAS: str = "_values" @@ -22,7 +23,11 @@ def extract_expected_branch_column_names_with_sql_analysis( - *, sql: str, file_label: str, label: str = "__expected__" + *, + sql: str, + file_label: str, + syntax: SqlLexicalSyntax, + label: str = "__expected__", ) -> tuple[tuple[str, ...], ...] | None: """Return expected SELECT branch names using required Polyglot analysis.""" @@ -32,17 +37,23 @@ def extract_expected_branch_column_names_with_sql_analysis( parsed_expression: Any = polyglot_module.parse_one(sql, dialect="generic") except polyglot_module.PolyglotError: return _extract_unparsed_branch_names( - sql=sql, polyglot_module=polyglot_module, file_label=file_label, label=label + sql=sql, + polyglot_module=polyglot_module, + file_label=file_label, + label=label, + syntax=syntax, ) return _extract_branch_names(expression=parsed_expression, file_label=file_label, label=label) def _extract_unparsed_branch_names( - *, sql: str, polyglot_module: Any, file_label: str, label: str + *, sql: str, polyglot_module: Any, file_label: str, label: str, syntax: SqlLexicalSyntax ) -> tuple[tuple[str, ...], ...] | None: """Parse each top-level set-operation branch separately when the whole query cannot parse.""" - branches: tuple[str, ...] = split_set_operation_branches(sql=sql, context=_SCAN_CONTEXT) + branches: tuple[str, ...] = split_set_operation_branches( + sql=sql, context=_SCAN_CONTEXT, syntax=syntax + ) if len(branches) <= 1: return None try: diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py index 045bc918f2..bd3e1c9e91 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py @@ -762,12 +762,14 @@ def _extract_expected_branch_column_names( ) -> tuple[tuple[str, ...], ...]: sql_analysis_column_names: tuple[tuple[str, ...], ...] | None = ( extract_expected_branch_column_names_with_sql_analysis( - sql=sql, file_label=file_label, label=label + sql=sql, file_label=file_label, label=label, syntax=syntax ) ) if sql_analysis_column_names is not None: return sql_analysis_column_names - branches: tuple[str, ...] = split_set_operation_branches(sql=sql, context=_CONTEXT) + branches: tuple[str, ...] = split_set_operation_branches( + sql=sql, context=_CONTEXT, syntax=syntax + ) return tuple( _extract_expected_select_column_names( branch_sql=branch, file_label=file_label, label=label, syntax=syntax @@ -837,9 +839,13 @@ def _extract_expected_projection_name( alias_name: str | None = _extract_as_alias(expression=expression, syntax=syntax) if alias_name is not None: return alias_name - stripped_expression: str = expression.strip() - if _is_simple_identifier(stripped_expression): - return stripped_expression + start: int = _skip_ignorable(sql=expression, start=0, syntax=syntax) + if start < len(expression) and is_identifier_start(expression[start]): + name: str + end: int + name, end = _read_identifier(sql=expression, start=start, file_label="projection") + if _skip_ignorable(sql=expression, start=end, syntax=syntax) == len(expression): + return name raise CompileInputError( f"SQL test '{file_label}' must alias every non-trivial {label} projection" ) @@ -862,17 +868,11 @@ def _extract_as_alias(*, expression: str, syntax: SqlLexicalSyntax) -> str | Non start=alias_index, file_label="projection", ) - if not expression[alias_end:].strip(): + if _skip_ignorable(sql=expression, start=alias_end, syntax=syntax) == len(expression): last_alias_name = alias_name return last_alias_name -def _is_simple_identifier(value: str) -> bool: - if not value or not is_identifier_start(value[0]): - return False - return all(is_identifier_character(character) for character in value[1:]) - - def _contains_select_star(*, sql: str, syntax: SqlLexicalSyntax) -> bool: index: int for index, _depth in iter_code_positions(sql=sql, context=_CONTEXT, syntax=syntax): diff --git a/src/sqlbuild/compiler/compile/constants.py b/src/sqlbuild/compiler/compile/constants.py index 063b1f129f..793916ddc0 100644 --- a/src/sqlbuild/compiler/compile/constants.py +++ b/src/sqlbuild/compiler/compile/constants.py @@ -96,7 +96,7 @@ ) POLYGLOT_SELECT_EXPRESSION_NAME: str = "Select" POLYGLOT_COLUMN_EXPRESSION_NAME: str = "Column" -POLYGLOT_WRAPPER_EXPRESSION_NAMES: frozenset[str] = frozenset({"Subquery", "Paren"}) +POLYGLOT_WRAPPER_EXPRESSION_NAMES: frozenset[str] = frozenset({"Annotated", "Subquery", "Paren"}) TABLE_FUNCTION_RETURN_KEYS: frozenset[str] = frozenset({"table"}) diff --git a/src/sqlbuild/compiler/discovery/_helpers/yml/entry_lines.py b/src/sqlbuild/compiler/discovery/_helpers/yml/entry_lines.py new file mode 100644 index 0000000000..f92a845f91 --- /dev/null +++ b/src/sqlbuild/compiler/discovery/_helpers/yml/entry_lines.py @@ -0,0 +1,67 @@ +"""Index the outermost `- name:` entries of authored YAML declaration files.""" + +from __future__ import annotations + +import re +from collections.abc import Iterator, Mapping +from functools import lru_cache + +_ENTRY_PREFIX: re.Pattern[str] = re.compile(r"(?P\s*)-\s*name:") +_COMMENT_START: str = "#" +_QUOTES: tuple[str, ...] = ("", "'", '"') + + +@lru_cache(maxsize=16) +def yaml_entry_lines(contents: str) -> Mapping[str, int]: + """Index every entry name once so per-resource lookups stay linear in file size.""" + + best: dict[str, tuple[int, int]] = {} + index: int + line: str + for index, line in enumerate(contents.splitlines(), start=1): + prefix: re.Match[str] | None = _ENTRY_PREFIX.match(line) + if prefix is None: + continue + indent: int = len(prefix.group("indent")) + for name in _entry_names(line[prefix.end() :]): + current: tuple[int, int] | None = best.get(name) + if current is None or indent < current[0]: + best[name] = (indent, index) + return {name: line_number for name, (_, line_number) in best.items()} + + +def _entry_names(value: str) -> Iterator[str]: + """Yield every name `\\s*(['"]?)\\1\\s*(#.*)?$` accepts for one entry value.""" + + closings: tuple[int, ...] = _closing_positions(value) + for start in range(_leading_whitespace(text=value, start=0) + 1): + for quote in _QUOTES: + if not value.startswith(quote, start): + continue + body_start: int = start + len(quote) + for closing in closings: + end: int = closing - len(quote) + if end >= body_start and value.startswith(quote, end): + yield value[body_start:end] + + +def _closing_positions(value: str) -> tuple[int, ...]: + """Return positions after which only whitespace and an optional comment remain.""" + + positions: list[int] = [] + tail: int = len(value) + while tail >= 0: + position: int = tail + positions.append(position) + while position > 0 and value[position - 1].isspace(): + position -= 1 + positions.append(position) + tail = value.rfind(_COMMENT_START, 0, tail) + return tuple(sorted(set(positions))) + + +def _leading_whitespace(*, text: str, start: int) -> int: + position: int = start + while position < len(text) and text[position].isspace(): + position += 1 + return position diff --git a/src/sqlbuild/compiler/discovery/main/_yaml_entry_line.py b/src/sqlbuild/compiler/discovery/main/_yaml_entry_line.py index 375abe4f80..f3fe5ab0d0 100644 --- a/src/sqlbuild/compiler/discovery/main/_yaml_entry_line.py +++ b/src/sqlbuild/compiler/discovery/main/_yaml_entry_line.py @@ -2,23 +2,10 @@ from __future__ import annotations -import re +from sqlbuild.compiler.discovery._helpers.yml.entry_lines import yaml_entry_lines def yaml_entry_line(*, contents: str, name: str) -> int | None: """Return the 1-based line of the outermost `- name: ` entry, if present.""" - pattern: re.Pattern[str] = re.compile( - rf"^(?P\s*)-\s*name:\s*(?P['\"]?){re.escape(name)}(?P=quote)\s*(#.*)?$" - ) - best: tuple[int, int] | None = None - index: int - line: str - for index, line in enumerate(contents.splitlines(), start=1): - match: re.Match[str] | None = pattern.match(line) - if match is None: - continue - indent: int = len(match.group("indent")) - if best is None or indent < best[0]: - best = (indent, index) - return None if best is None else best[1] + return yaml_entry_lines(contents).get(name) diff --git a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py index ee018e4914..e88be3e72d 100644 --- a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py +++ b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py @@ -231,6 +231,50 @@ def iter_code_positions_impl( index += 1 +def skip_ignorable_impl( + *, sql: str, start: int, context: str = "SQL", syntax: SqlLexicalSyntax | None = None +) -> int: + """Return the first offset at or after `start` that is not whitespace or a comment.""" + + index: int = start + while index < len(sql): + if sql[index].isspace(): + index += 1 + continue + if sql[index] in SQL_TEXT_START_CHARACTERS: + return index + comment_end: int | None = _non_code_end( + sql=sql, index=index, context=context, syntax=syntax + ) + if comment_end is None: + return index + index = comment_end + return index + + +def strip_ignorable_impl( + *, sql: str, context: str = "SQL", syntax: SqlLexicalSyntax | None = None +) -> str: + """Return the SQL without leading or trailing whitespace and comments.""" + + code_start: int = skip_ignorable_impl(sql=sql, start=0, context=context, syntax=syntax) + code_end: int = code_start + index: int = code_start + while index < len(sql): + non_code_end: int | None = _non_code_end( + sql=sql, index=index, context=context, syntax=syntax + ) + if non_code_end is None: + index += 1 + if not sql[index - 1].isspace(): + code_end = index + continue + if sql[index] in SQL_TEXT_START_CHARACTERS: + code_end = non_code_end + index = non_code_end + return sql[code_start:code_end] + + def _non_code_end( *, sql: str, index: int, context: str, syntax: SqlLexicalSyntax | None ) -> int | None: diff --git a/src/sqlbuild/compiler/sql_analysis/_helpers/set_operations.py b/src/sqlbuild/compiler/sql_analysis/_helpers/set_operations.py index f42cb54430..9cb2201df3 100644 --- a/src/sqlbuild/compiler/sql_analysis/_helpers/set_operations.py +++ b/src/sqlbuild/compiler/sql_analysis/_helpers/set_operations.py @@ -5,9 +5,10 @@ from sqlbuild.compiler.sql_analysis._helpers.scanning import ( is_identifier_character_impl, iter_code_positions_impl, - skip_block_comment_impl, - skip_line_comment_impl, + skip_ignorable_impl, + strip_ignorable_impl, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax _SET_OPERATOR_KEYWORDS: tuple[str, ...] = ("UNION", "INTERSECT") _EXCEPT_KEYWORD: str = "EXCEPT" @@ -16,8 +17,10 @@ _STAR_CHARACTER: str = "*" -def split_set_operation_branches_impl(*, sql: str, context: str) -> tuple[str, ...]: - """Split SQL on top-level `UNION`, `INTERSECT` and `EXCEPT` with optional quantifiers.""" +def split_set_operation_branches_impl( + *, sql: str, context: str, syntax: SqlLexicalSyntax | None = None +) -> tuple[str, ...]: + """Split SQL on top-level set operators; branches exclude surrounding comments.""" branches: list[str] = [] branch_start: int = 0 @@ -26,7 +29,7 @@ def split_set_operation_branches_impl(*, sql: str, context: str) -> tuple[str, . previous_depth: int = 0 index: int depth: int - for index, depth in iter_code_positions_impl(sql=sql, context=context): + for index, depth in iter_code_positions_impl(sql=sql, context=context, syntax=syntax): if depth != previous_depth: previous_code = None previous_depth = depth @@ -39,17 +42,23 @@ def split_set_operation_branches_impl(*, sql: str, context: str) -> tuple[str, . previous_code = sql[index] if operator_end is None: continue - branch_sql: str = sql[branch_start:index].strip() + branch_sql: str = strip_ignorable_impl( + sql=sql[branch_start:index], context=context, syntax=syntax + ) if branch_sql: branches.append(branch_sql) - resume = _skip_ignorable(sql=sql, start=operator_end, context=context) + resume = skip_ignorable_impl(sql=sql, start=operator_end, context=context, syntax=syntax) quantifier_end: int | None = _keyword_end( sql=sql, start=resume, keyword=_ALL_KEYWORD ) or _keyword_end(sql=sql, start=resume, keyword=_DISTINCT_KEYWORD) if quantifier_end is not None: - resume = _skip_ignorable(sql=sql, start=quantifier_end, context=context) + resume = skip_ignorable_impl( + sql=sql, start=quantifier_end, context=context, syntax=syntax + ) branch_start = resume - final_branch_sql: str = sql[branch_start:].strip() + final_branch_sql: str = strip_ignorable_impl( + sql=sql[branch_start:], context=context, syntax=syntax + ) if final_branch_sql: branches.append(final_branch_sql) return tuple(branches) @@ -77,19 +86,3 @@ def _keyword_end(*, sql: str, start: int, keyword: str) -> int | None: if start > 0 and is_identifier_character_impl(sql[start - 1]): return None return keyword_end - - -def _skip_ignorable(*, sql: str, start: int, context: str) -> int: - index: int = start - while index < len(sql): - if sql[index].isspace(): - index += 1 - continue - if sql.startswith("--", index): - index = skip_line_comment_impl(sql=sql, start=index) - continue - if sql.startswith("/*", index): - index = skip_block_comment_impl(sql=sql, start=index, context=context) - continue - return index - return index diff --git a/src/sqlbuild/compiler/sql_analysis/main/_split_set_operation_branches.py b/src/sqlbuild/compiler/sql_analysis/main/_split_set_operation_branches.py index b0d1ad4714..7fc70d7421 100644 --- a/src/sqlbuild/compiler/sql_analysis/main/_split_set_operation_branches.py +++ b/src/sqlbuild/compiler/sql_analysis/main/_split_set_operation_branches.py @@ -3,9 +3,12 @@ from sqlbuild.compiler.sql_analysis._helpers.set_operations import ( split_set_operation_branches_impl, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax -def split_set_operation_branches(*, sql: str, context: str = "SQL") -> tuple[str, ...]: +def split_set_operation_branches( + *, sql: str, context: str = "SQL", syntax: SqlLexicalSyntax | None = None +) -> tuple[str, ...]: """Split SQL on top-level `UNION`, `INTERSECT` and `EXCEPT`, ignoring quotes and comments.""" - return split_set_operation_branches_impl(sql=sql, context=context) + return split_set_operation_branches_impl(sql=sql, context=context, syntax=syntax) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 3208cd0112..e0ece67ea9 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -517,6 +517,14 @@ class RequiredDescriptionAggregateCase: expected_kinds: frozenset[str] +@dataclass(frozen=True) +class RequiredDescriptionExactOutputCase: + description: str + files: tuple[tuple[str, str], ...] + expected_diagnostics: tuple[tuple[str, str, str, int, int], ...] + expected_help: tuple[str, ...] + + @dataclass(frozen=True) class RequiredDescriptionPlanCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py index 509a435296..92d5ab0f7c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py @@ -18,6 +18,7 @@ write_diagnostic_project, write_function_name_project, write_single_model_diagnostic_project, + write_undescribed_project, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import DiagnosticPerformanceCase @@ -102,6 +103,40 @@ def test_given_one_model_with_many_diagnostics_when_compiling_then_scales_linear assert timings[1] <= timings[0] * 4.5 + 1.0 +@pytest.mark.parametrize( + "test_case", + [ + DiagnosticPerformanceCase( + "many_undescribed_resources", + diagnostic_count=500, + expected_max_wall_seconds=15.0, + ) + ], + ids=lambda case: case.description, +) +def test_given_many_undescribed_resources_when_compiling_then_scales_linearly( + test_case: DiagnosticPerformanceCase, tmp_path: Path +) -> None: + timings: list[float] = [] + for multiplier in (1, 4): + project: Path = tmp_path / f"undescribed_{multiplier}" + count: int = test_case.diagnostic_count * multiplier + write_undescribed_project(project_dir=project, model_count=count) + elapsed, payload = measure_diagnostic_compile( + project_dir=project, timeout=test_case.expected_timeout_seconds + ) + kinds: Counter[tuple[str, str]] = Counter( + (item["code"], item["resource_type"]) for item in payload["diagnostics"] + ) + assert kinds == Counter( + {("P010", "model"): count, ("P010", "source"): count, ("P010", "seed"): count // 10} + ) + timings.append(elapsed) + _LOGGER.info("undescribed resources 1x/4x wall=%s", timings) + assert max(timings) < test_case.expected_max_wall_seconds + assert timings[1] <= timings[0] * 5.0 + 1.0 + + @pytest.mark.parametrize( "test_case", [ diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_required_descriptions.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_required_descriptions.py index 8c5edfcebc..211d9c61e4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_required_descriptions.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_required_descriptions.py @@ -17,6 +17,7 @@ PathDefaultDescriptionCase, RequiredDescriptionAggregateCase, RequiredDescriptionCase, + RequiredDescriptionExactOutputCase, RequiredDescriptionPlanCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( @@ -43,6 +44,27 @@ ) +_SHADOWED_SOURCES_YAML: str = ( + "sources:\n" + " - name: raw_orders\n" + " schema: main\n" + " table: orders\n" + " columns:\n" + " - name: raw_refunds\n" + " type: INTEGER\n" + ' - name: "raw_refunds" # refunds feed\n' + " schema: main\n" + " table: refunds\n" + " - name: 'raw_customers'\n" + " schema: main\n" + " table: customers\n" +) +_TWO_SEEDS_YAML: str = ( + "seeds:\n" + " - name: product_types\n" + _SEED_COLUMNS + " - name: product_labels\n" + _SEED_COLUMNS +) + + _PROVIDER_DESCRIBED: str = ( "from sqlbuild.providers import Provider\n\n\n" "class OrdersApi(Provider):\n" @@ -534,6 +556,89 @@ def test_given_every_resource_kind_undescribed_when_compiling_then_one_run_repor assert kinds == test_case.expected_kinds +@pytest.mark.parametrize( + "test_case", + [ + RequiredDescriptionExactOutputCase( + description="seed and source entries located at their outermost declarations", + files=( + ("models/order_summary.sql", "MODEL ();\nSELECT 1 AS order_count\n"), + ("seeds/product_types.csv", _SEED_CSV), + ("seeds/product_labels.csv", _SEED_CSV), + ("seeds/products.yml", _TWO_SEEDS_YAML), + ("sources/raw.yml", _SHADOWED_SOURCES_YAML), + ), + expected_diagnostics=( + ( + "P010", + "model 'order_summary' has no description", + "models/order_summary.sql", + 1, + 1, + ), + ("P010", "seed 'product_types' has no description", "seeds/products.yml", 2, 1), + ("P010", "seed 'product_labels' has no description", "seeds/products.yml", 8, 1), + ("P010", "source 'raw_orders' has no description", "sources/raw.yml", 2, 1), + ("P010", "source 'raw_refunds' has no description", "sources/raw.yml", 8, 1), + ("P010", "source 'raw_customers' has no description", "sources/raw.yml", 11, 1), + ), + expected_help=( + "to describe model 'order_summary', add this to the MODEL header in " + "models/order_summary.sql:\n" + " MODEL (\n" + ' description "What one row of order_summary represents",\n' + " ...\n" + " );", + ( + "to describe seed 'product_types', add `description` to its entry in seeds/products.yml:\n" + " - name: product_types\n" + " description: What one row of product_types represents" + ), + ( + "to describe seed 'product_labels', add `description` to its entry in seeds/products.yml:\n" + " - name: product_labels\n" + " description: What one row of product_labels represents" + ), + ( + "to describe source 'raw_orders', add `description` to its entry in sources/raw.yml:\n" + " - name: raw_orders\n" + " description: What one row of raw_orders represents" + ), + ( + "to describe source 'raw_refunds', add `description` to its entry in sources/raw.yml:\n" + " - name: raw_refunds\n" + " description: What one row of raw_refunds represents" + ), + ( + "to describe source 'raw_customers', add `description` to its entry in sources/raw.yml:\n" + " - name: raw_customers\n" + " description: What one row of raw_customers represents" + ), + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_undescribed_yaml_entries_when_compiling_then_reports_exact_ordered_diagnostics( + test_case: RequiredDescriptionExactOutputCase, tmp_path: Path +) -> None: + result: subprocess.CompletedProcess[str] = compile_inline_files( + tmp_path=tmp_path, files=test_case.files + ) + + payload: dict[str, Any] = json.loads(result.stdout) + diagnostics: list[dict[str, Any]] = payload["diagnostics"] + assert result.returncode == 1, result.stdout + result.stderr + assert ( + tuple( + (item["code"], item["message"], item["path"], item["line"], item["column"]) + for item in diagnostics + ) + == test_case.expected_diagnostics + ) + assert tuple(item["help"] for item in diagnostics) == test_case.expected_help + + @pytest.mark.parametrize( "test_case", [ diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py index ab022a8c92..b7a821740c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py @@ -212,3 +212,13 @@ class AuthoredSqlParityTestCase: description: str compiled_test_name: str expected_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class CommentedFixtureE2ETestCase: + """SQL tests whose fixture CTEs carry comments around set-operation branches.""" + + description: str + test_files: dict[str, str] + expected_exit_code: int + expected_output_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_commented_fixtures.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_commented_fixtures.py new file mode 100644 index 0000000000..5b50be7a74 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_commented_fixtures.py @@ -0,0 +1,189 @@ +"""E2E coverage for comments around set-operation branches in SQL-test fixtures.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ( + CommentedFixtureE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( + execute_duckdb, + prepare_inline_project, + run_sqb, +) + +_PROJECT_FILES: dict[str, str] = { + "sqlbuild_project.toml": ( + 'name = "commented_fixtures"\nadapter = "duckdb"\n\n' + '[connection]\ndatabase = "commented_fixtures.duckdb"\n\n' + '[defaults]\nmaterialized = "table"\n' + ), + "macros/status.py": ( + 'def normalize_status(value: str) -> str:\n return f"LOWER(TRIM({value}))"\n' + ), + "sources/raw_orders.yml": ( + "sources:\n" + " - name: raw_orders\n description: Test source raw_orders.\n" + " schema: main\n" + " table: raw_orders\n" + " columns:\n" + " - name: order_id\n type: INTEGER\n" + " - name: status\n type: VARCHAR\n" + ), + "models/orders.sql": ( + "MODEL (description 'Test model orders.');\n\n" + 'SELECT order_id, @normalize_status("status") AS status FROM __source("raw_orders")\n' + ), + "models/order_counts.sql": ( + "MODEL (description 'Test model order_counts.');\n\n" + 'SELECT status, COUNT(*) AS order_count FROM __ref("orders") GROUP BY status\n' + ), +} +_LINE_COMMENTED_SOURCE: str = ( + "__source__raw_orders AS (\n" + " -- first order\n" + " SELECT 1 AS order_id, ' PAID ' AS status -- trailing note\n" + " -- before the union\n" + " UNION ALL\n" + " -- second order\n" + " SELECT 2 AS order_id, 'void' AS status\n" + " -- after the last branch\n" + "),\n" +) +_LINE_COMMENTED_EXPECTED: str = ( + "__expected__orders AS (\n" + " SELECT 1 AS order_id, 'paid' AS status -- paid order\n" + " -- the voided order follows\n" + " UNION ALL\n" + " SELECT 2 AS order_id, 'void' AS status -- '--' inside a comment\n" + ")\nSELECT 1\n" +) +_BLOCK_COMMENTED_SOURCE: str = ( + "__source__raw_orders AS (\n" + " /* first order */ SELECT 1 AS order_id, ' PAID ' AS status /* note */\n" + " UNION ALL /* second order */\n" + " SELECT 2 AS order_id, 'void' AS status\n" + "),\n" +) +_BLOCK_COMMENTED_EXPECTED: str = ( + "__expected__orders AS (\n" + " SELECT 1 AS order_id, 'paid' AS status\n" + " /* nested /* UNION ALL SELECT 9 AS other_id */ still a comment */\n" + " UNION ALL\n" + " /* voided */ SELECT 2 AS order_id, 'void' AS status /* last */\n" + ")\nSELECT 1\n" +) +_MISMATCH_FRAGMENT: str = ( + "must use the same __expected__orders projection names and order in every " + "set-operation branch; branch 2 does not match branch 1" +) + + +@pytest.mark.parametrize( + "test_case", + [ + CommentedFixtureE2ETestCase( + description="line and block comments around fixture branches pass", + test_files={ + "tests/unit/test_orders_line_comments.sql": ( + 'TEST (name "orders_line_comments");\n\nWITH\n' + + _LINE_COMMENTED_SOURCE + + _LINE_COMMENTED_EXPECTED + ), + "tests/unit/test_orders_block_comments.sql": ( + 'TEST (name "orders_block_comments");\n\nWITH\n' + + _BLOCK_COMMENTED_SOURCE + + _BLOCK_COMMENTED_EXPECTED + ), + "tests/unit/test_order_counts_commented_ref.sql": ( + 'TEST (name "order_counts_commented_ref");\n\nWITH\n' + "__ref__orders AS (\n" + " SELECT 1 AS order_id, 'paid' AS status -- first\n" + " /* between */ UNION ALL -- second\n" + " SELECT 2 AS order_id, 'paid' AS status\n" + "),\n" + "__expected__order_counts AS (\n" + " SELECT 'paid' AS status, CAST(2 AS BIGINT) AS order_count -- only row\n" + ")\nSELECT 1\n" + ), + "tests/unit/test_normalize_status_commented.sql": ( + 'TEST (mode macro, name "normalizes_status_commented");\n\nWITH\n' + "input_values AS (SELECT ' PAID ' AS raw_status),\n" + "__macro_actual__ AS (\n" + ' SELECT @normalize_status("raw_status") AS status FROM input_values\n' + "),\n" + "__macro_expected__ AS (\n" + " SELECT 'paid' AS status -- normalized\n" + " -- duplicates collapse\n" + " INTERSECT DISTINCT SELECT 'paid' AS status\n" + ")\nSELECT 1\n" + ), + }, + expected_exit_code=0, + expected_output_fragments=("PASS=4", "FAIL=0"), + ), + CommentedFixtureE2ETestCase( + description="line-commented branch mismatch still fails", + test_files={ + "tests/unit/test_orders_mismatch.sql": ( + 'TEST (name "orders_mismatch");\n\nWITH\n' + + _LINE_COMMENTED_SOURCE + + "__expected__orders AS (\n" + " SELECT 1 AS order_id, 'paid' AS status -- paid order\n" + " UNION ALL\n" + " SELECT 2 AS order_id, 'void' AS order_status -- renamed\n" + ")\nSELECT 1\n" + ), + }, + expected_exit_code=1, + expected_output_fragments=("P001", _MISMATCH_FRAGMENT), + ), + CommentedFixtureE2ETestCase( + description="block-commented branch mismatch still fails", + test_files={ + "tests/unit/test_orders_mismatch.sql": ( + 'TEST (name "orders_mismatch");\n\nWITH\n' + + _BLOCK_COMMENTED_SOURCE + + "__expected__orders AS (\n" + " SELECT 1 AS order_id, 'paid' AS status /* paid */\n" + " UNION ALL /* voided */\n" + " SELECT 'void' AS status, 2 AS order_id\n" + ")\nSELECT 1\n" + ), + }, + expected_exit_code=1, + expected_output_fragments=("P001", _MISMATCH_FRAGMENT), + ), + ], + ids=lambda case: case.description, +) +def test_given_commented_fixture_branches_when_testing_then_comments_are_not_projections( + test_case: CommentedFixtureE2ETestCase, + tmp_path: Path, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="commented_fixtures", + repo_files={**_PROJECT_FILES, **test_case.test_files}, + ) + execute_duckdb( + db_path=project_dir / "commented_fixtures.duckdb", + sql="CREATE TABLE raw_orders AS SELECT 1 AS order_id, 'paid' AS status", + ) + + tested: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "test"), project_dir=project_dir + ) + + output: str = tested.stdout + tested.stderr + assert tested.returncode == test_case.expected_exit_code, output + for fragment in test_case.expected_output_fragments: + assert fragment in output, output + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py index 28b16aacf7..cae6af4a7f 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py @@ -1129,3 +1129,11 @@ class RequiredDescriptionLocationCase: description: str build: Callable[[], RequiredDescriptionInputs] expected_location: tuple[str, int, int] + + +@dataclass(frozen=True) +class DialectSetOperationSplitTestCase: + description: str + sql: str + syntax: SqlLexicalSyntax | None + expected_branches: tuple[str, ...] diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_expected_projection_scanning.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_expected_projection_scanning.py index 6daaf4791a..430e299ee2 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_expected_projection_scanning.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_expected_projection_scanning.py @@ -10,6 +10,7 @@ ) from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from tests.unit.src.sqlbuild.compiler.compile._helpers._test_types import ( + DialectSetOperationSplitTestCase, ExpectedProjectionScanTestCase, ) @@ -37,6 +38,19 @@ expected_alias=None, expected_contains_select_star=False, ), + ExpectedProjectionScanTestCase( + description="comments around branches and projections are not code", + sql="-- lead\nSELECT 1 AS a /* one */\n-- before\nUNION ALL /* after */ SELECT '--' AS a -- end", + expected_branches=("SELECT 1 AS a", "SELECT '--' AS a"), + expected_select_list_end=len( + "-- lead\nSELECT 1 AS a /* one */\n-- before\nUNION ALL /* after */ SELECT '--' AS a -- end" + ), + expected_commas=( + "-- lead\nSELECT 1 AS a /* one */\n-- before\nUNION ALL /* after */ SELECT '--' AS a -- end", + ), + expected_alias="a", + expected_contains_select_star=False, + ), ExpectedProjectionScanTestCase( description="nested parentheses hide commas, FROM and AS", sql="SELECT f(a, (SELECT 1 AS x FROM y)) AS total", @@ -129,3 +143,49 @@ def test_given_expected_cte_sql_when_scanning_top_level_then_results_are_charact assert sql_test_core._contains_select_star(sql=test_case.sql, syntax=_GENERIC_SQL_SYNTAX) is ( test_case.expected_contains_select_star ) + + +@pytest.mark.parametrize( + "test_case", + ( + DialectSetOperationSplitTestCase( + description="generic rules end a block comment at the first close", + sql="SELECT 1 AS a /* x /* UNION ALL */ SELECT 9 AS b */ UNION ALL SELECT 2 AS a", + syntax=None, + expected_branches=( + "SELECT 1 AS a /* x /* UNION ALL */ SELECT 9 AS b */", + "SELECT 2 AS a", + ), + ), + DialectSetOperationSplitTestCase( + description="nested block comments hide a set operation", + sql="SELECT 1 AS a /* x /* UNION ALL */ SELECT 9 AS b */ UNION ALL SELECT 2 AS a", + syntax=SqlLexicalSyntax(nested_block_comments=True), + expected_branches=("SELECT 1 AS a", "SELECT 2 AS a"), + ), + DialectSetOperationSplitTestCase( + description="dialect line comment prefixes hide a set operation", + sql="SELECT 1 AS a # UNION ALL SELECT 9 AS b\nUNION ALL SELECT 2 AS a # end", + syntax=SqlLexicalSyntax(line_comment_prefixes=frozenset({"--", "#"})), + expected_branches=("SELECT 1 AS a", "SELECT 2 AS a"), + ), + DialectSetOperationSplitTestCase( + description="backslash escapes keep a quote open", + sql="SELECT 'it\\' UNION ALL SELECT 9' AS a UNION ALL SELECT 'x' AS a", + syntax=SqlLexicalSyntax(backslash_escape_quotes=frozenset({"'"})), + expected_branches=("SELECT 'it\\' UNION ALL SELECT 9' AS a", "SELECT 'x' AS a"), + ), + ), + ids=lambda case: case.description, +) +def test_given_dialect_lexical_syntax_when_splitting_set_operations_then_follows_dialect_rules( + test_case: DialectSetOperationSplitTestCase, +) -> None: + assert ( + split_set_operation_branches(sql=test_case.sql, context="SQL test", syntax=test_case.syntax) + == test_case.expected_branches + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_tests.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_tests.py index 236430d065..c29f7d81c8 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_tests.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_tests.py @@ -7,6 +7,7 @@ from sqlbuild.compiler.compile._helpers.analysis.tests import ( # noqa: E402 extract_expected_branch_column_names_with_sql_analysis, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax # noqa: E402 from tests.unit.src.sqlbuild.compiler.compile._helpers._test_types import ( # noqa: E402 ExtractSqlAnalysisExpectedBranchesErrorTestCase, ExtractSqlAnalysisExpectedBranchesTestCase, @@ -89,6 +90,7 @@ def test_given_sql_analysis_available_when_extracting_expected_branches_then_it_ extract_expected_branch_column_names_with_sql_analysis( sql=test_case.sql, file_label="tests/unit/orders.sql", + syntax=SqlLexicalSyntax(), ) ) @@ -128,4 +130,5 @@ def test_given_invalid_sql_analysis_expected_branches_when_extracting_then_it_ra extract_expected_branch_column_names_with_sql_analysis( sql=test_case.sql, file_label="tests/unit/orders.sql", + syntax=SqlLexicalSyntax(), ) diff --git a/tests/unit/src/sqlbuild/compiler/discovery/main/_test_types.py b/tests/unit/src/sqlbuild/compiler/discovery/main/_test_types.py index cd0f8f07c7..00fbc1470f 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/main/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/main/_test_types.py @@ -8,3 +8,11 @@ class SerializeHookEntriesTestCase: sql_fields: tuple[str, ...] python_hook_fields: dict[str, dict[str, object]] expected_hooks: list[dict[str, object]] + + +@dataclass(frozen=True) +class YamlEntryLineTestCase: + description: str + contents: str + name: str + expected_line: int | None diff --git a/tests/unit/src/sqlbuild/compiler/discovery/main/test_yaml_entry_line.py b/tests/unit/src/sqlbuild/compiler/discovery/main/test_yaml_entry_line.py new file mode 100644 index 0000000000..278ad2b932 --- /dev/null +++ b/tests/unit/src/sqlbuild/compiler/discovery/main/test_yaml_entry_line.py @@ -0,0 +1,49 @@ +"""Unit tests for locating named entries in authored YAML declaration files.""" + +from __future__ import annotations + +import pytest + +from sqlbuild.compiler.discovery.main._yaml_entry_line import yaml_entry_line +from tests.unit.src.sqlbuild.compiler.discovery.main._test_types import YamlEntryLineTestCase + +_SOURCES: str = ( + "sources:\n" + " - name: raw_orders\n" + " columns:\n" + " - name: raw_refunds\n" + ' - name: "raw_refunds" # refunds feed\n' + " - name: 'raw_customers'\n" + " - name:raw_products # products\n" + " - name: raw_orders\n" +) + + +@pytest.mark.parametrize( + "test_case", + ( + YamlEntryLineTestCase("plain entry", _SOURCES, "raw_orders", 2), + YamlEntryLineTestCase( + "outermost wins over earlier nested column", _SOURCES, "raw_refunds", 5 + ), + YamlEntryLineTestCase("single quoted entry", _SOURCES, "raw_customers", 6), + YamlEntryLineTestCase("compact entry with trailing comment", _SOURCES, "raw_products", 7), + YamlEntryLineTestCase("comment text is not a name", _SOURCES, "products", None), + YamlEntryLineTestCase( + "unquoted line also matches its comment", _SOURCES, "raw_products # products", 7 + ), + YamlEntryLineTestCase("missing entry", _SOURCES, "raw_shipments", None), + YamlEntryLineTestCase("mismatched quotes", "- name: \"raw_orders'\n", "raw_orders", None), + ), + ids=lambda case: case.description, +) +def test_given_yaml_declarations_when_locating_entry_then_returns_outermost_line( + test_case: YamlEntryLineTestCase, +) -> None: + assert yaml_entry_line(contents=test_case.contents, name=test_case.name) == ( + test_case.expected_line + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"])