diff --git a/README.md b/README.md index 3407276bcb..c36bc52140 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ The playground runs on local DuckDB, with no warehouse credentials. update every reference, and the next build [migrates](https://sqlbuild.com/docs/concepts/models/migrations/) the existing table instead of rebuilding it. The old name keeps working through a compatibility view. - `sqb rename column:.` renames an incremental model's column in place. + `sqb rename .` renames an incremental model's column in place. - **Replay on change.** When a model's SQL changes, choose how far back to reprocess, from only the new data to the last 14 days to a full rebuild, with [`replay_on_change`](https://sqlbuild.com/docs/concepts/incremental/#replay-on-change). diff --git a/src/sqlbuild/.agents/skills/sqlbuild/SKILL.md b/src/sqlbuild/.agents/skills/sqlbuild/SKILL.md index fa13f57633..a35c616793 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/SKILL.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/SKILL.md @@ -82,7 +82,7 @@ the trigger applies. | Know what a model can see: which macros, enums and constants are visible, used, or unavailable | `sqb scope model:` | | Fix "unknown macro/enum/constant", or decide where a declaration should live | `sqb scope model: --explain macro:` | | Preview whether moving a file breaks declaration visibility | `sqb scope model: --as-path ` | -| Rename a model, move a model file, or rename a column | `sqb rename model: `, `sqb mv model: `, `sqb rename column:. ` | +| Rename a model, move a model file, or rename a column | `sqb rename `, `sqb mv `, `sqb rename . ` (`model:`/`column:` prefixes optional) | | Find what a model depends on, or what breaks if it changes | `sqb lineage --direction both` | | Trace where a column comes from, or who consumes it before renaming or dropping it | `sqb lineage . --direction downstream` | | See which intermediate models a SQL test will really execute | `sqb test --select --inspect` | diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md index 132044ab26..e5081706ac 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md @@ -24,19 +24,21 @@ Nothing is written until the edited project compiles. The command applies its ed ## Usage ```bash -sqb rename model: [flags] -sqb mv model: [flags] -sqb rename column:. [--cascade] [flags] +sqb rename [flags] +sqb mv [flags] +sqb rename . [--cascade] [flags] ``` ```bash -sqb rename model:stg_orders stg_order_lines -sqb mv model:fact_orders models/reporting/ # keeps the name -sqb mv model:fact_orders models/reporting/order_facts.sql # moves and renames -sqb rename column:stg_orders.amount revenue -sqb rename column:stg_orders.amount revenue --cascade +sqb rename stg_orders stg_order_lines +sqb mv fact_orders models/reporting/ # keeps the name +sqb mv fact_orders models/reporting/order_facts.sql # moves and renames +sqb rename stg_orders.amount revenue +sqb rename stg_orders.amount revenue --cascade ``` +Targets are written the way [`sqb lineage`](lineage.md) reads them: a bare name is a model, and `.` is one of its columns. The `model:` and `column:` prefixes are optional, so `sqb rename model:stg_orders stg_order_lines` and `sqb rename column:stg_orders.amount revenue` still work. A target that names a source, seed, or function is refused, because those are not renamed by these commands. + A model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` takes a new file path, which also sets the model name, or a folder ending in `/`, which keeps it. ## What changes diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/column-migrations.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/column-migrations.md index eff7b76918..dc077b8caa 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/column-migrations.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/column-migrations.md @@ -21,7 +21,7 @@ Column migrations apply to incremental models, including microbatch models, and ## Declaring a rename -[`sqb rename column:.`](../../cli/rename.md) renames the column in the model and every query that reads it, and adds the `migrate_from` declaration below for you. +[`sqb rename .`](../../cli/rename.md) renames the column in the model and every query that reads it, and adds the `migrate_from` declaration below for you. Add `migrate_from` to the renamed column in the model header, naming the old column: diff --git a/src/sqlbuild/cli/commands/_helpers/entry/parser.py b/src/sqlbuild/cli/commands/_helpers/entry/parser.py index 4ea29c6640..26c367b3d0 100644 --- a/src/sqlbuild/cli/commands/_helpers/entry/parser.py +++ b/src/sqlbuild/cli/commands/_helpers/entry/parser.py @@ -153,7 +153,9 @@ def _add_refactor_parsers( CliCommand.RENAME, help="rename a model or column and update every reference" ) rename_parser.add_argument( - "refactor_target", metavar="TARGET", help="model: or column:." + "refactor_target", + metavar="TARGET", + help=" or .; the model: and column: prefixes are optional", ) rename_parser.add_argument("refactor_new_name", metavar="NEW_NAME") rename_parser.add_argument( @@ -166,7 +168,9 @@ def _add_refactor_parsers( move_parser: argparse.ArgumentParser = subparsers.add_parser( CliCommand.MV, help="move a model file, optionally renaming it, and update references" ) - move_parser.add_argument("refactor_target", metavar="TARGET", help="model:") + move_parser.add_argument( + "refactor_target", metavar="TARGET", help="; the model: prefix is optional" + ) move_parser.add_argument( "refactor_destination", metavar="DESTINATION", help="new .sql path or folder/" ) diff --git a/src/sqlbuild/cli/commands/_helpers/lineage/selection.py b/src/sqlbuild/cli/commands/_helpers/lineage/selection.py index 3a75fbb85b..9d00fcf380 100644 --- a/src/sqlbuild/cli/commands/_helpers/lineage/selection.py +++ b/src/sqlbuild/cli/commands/_helpers/lineage/selection.py @@ -7,6 +7,7 @@ from sqlbuild.cli.commands.constants import ( BOTH_DIRECTIONS, + COLUMN_TARGET_KIND, COLUMN_TARGET_SEPARATOR, DOWNSTREAM_DIRECTION, PATH_BETWEEN_MARKER, @@ -25,6 +26,7 @@ LineageGraph, LineageNode, LineageSelectionAnchors, + LineageTarget, ParsedLineagePathSelector, ParsedLineageSelector, RelationLineageIndex, @@ -64,6 +66,7 @@ class _ColumnLineageCandidateSelection: str(CompiledResourceType.SEED), str(CompiledResourceType.UDF), str(CompiledResourceType.TABLE_FN), + COLUMN_TARGET_KIND, } ) @@ -85,14 +88,39 @@ def parse_depth(raw_depth: str) -> int | None: def normalize_lineage_target(*, graph: ProjectGraph | RelationLineageIndex, target: str) -> str: """Strip a printed kind prefix such as `model:` after checking it matches the resource.""" - kind, separator, remainder = target.partition(_KIND_SEPARATOR) - if not separator or kind not in _TARGET_KINDS: + resolved: LineageTarget = resolve_lineage_target(all_keys=graph.all_keys, target=target) + if resolved.kind is None: return target - name: str = remainder.split(COLUMN_TARGET_SEPARATOR, maxsplit=1)[0] - key: CompiledObjectKey | None = graph.all_keys.get(name) - if key is None or str(key.resource_type) != kind: + if not resolved.kind_matches: raise _unknown_target_error(target) - return remainder + return resolved.name + + +def resolve_lineage_target(*, all_keys: dict[str, CompiledObjectKey], target: str) -> LineageTarget: + """Resolve `[kind:]` or `[kind:].` against project resource names.""" + + kind, separator, remainder = target.partition(_KIND_SEPARATOR) + written_kind: str | None = kind if separator and kind in _TARGET_KINDS else None + name: str = remainder if written_kind is not None else target + resource_name, column_separator, column_name = name.partition(COLUMN_TARGET_SEPARATOR) + key: CompiledObjectKey | None = all_keys.get(resource_name) + column: str | None = column_name if column_separator else None + kind_matches: bool + if written_kind is None: + kind_matches = True + elif written_kind == COLUMN_TARGET_KIND: + kind_matches = ( + key is not None and key.resource_type == CompiledResourceType.MODEL and bool(column) + ) + else: + kind_matches = key is not None and str(key.resource_type) == written_kind + return LineageTarget( + kind=written_kind, + name=name, + key=key, + column_name=column, + kind_matches=kind_matches, + ) def select_target_lineage( diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/command.py b/src/sqlbuild/cli/commands/_helpers/refactor/command.py index 34e0a57b8d..8286b058bf 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/command.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/command.py @@ -14,15 +14,10 @@ render_refactor_json, render_refactor_text, ) +from sqlbuild.cli.commands._helpers.refactor.target import refactor_request_for_target from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.cli.commands.models import RefactorCommandRequest, RefactorCompile -from sqlbuild.cli.commands.types import CliCommand from sqlbuild.compiler.compile.models import CompilerDiagnostic -from sqlbuild.compiler.refactoring.constants import ( - COLUMN_KIND_PREFIX, - COLUMN_TARGET_SEPARATOR, - MODEL_KIND_PREFIX, -) from sqlbuild.compiler.refactoring.exceptions import RefactorInputError from sqlbuild.compiler.refactoring.main.commit_refactor_plan import commit_refactor_plan from sqlbuild.compiler.refactoring.main.plan_column_rename import plan_column_rename @@ -69,7 +64,9 @@ def _run( _write_errors(errors=before.errors) return 1 status.complete(message=f"Compiled project. ({time.monotonic() - started:.2f}s)") - refactor_request: RefactorRequest = _refactor_request(request=request) + refactor_request: RefactorRequest = refactor_request_for_target( + request=request, all_keys=before.project.graph.all_keys + ) status.start("Planning edits...") plan: RefactorPlan = ( plan_column_rename(project=before.project, request=refactor_request) @@ -171,50 +168,6 @@ def _files(plan: RefactorPlan) -> str: return format_count_noun(count=len(plan.changes), singular="file") -def _refactor_request(*, request: RefactorCommandRequest) -> RefactorRequest: - target: str = request.target.strip() - if request.command == CliCommand.MV: - if not target.startswith(MODEL_KIND_PREFIX): - raise CliUserError( - f"sqb mv moves models; got '{target}'", - code="C954", - help="write the target as model:", - ) - return RefactorRequest( - operation=RefactorOperation.MOVE_MODEL, - model_name=target.removeprefix(MODEL_KIND_PREFIX), - new_name="", - destination=request.destination or "", - ) - new_name: str = (request.new_name or "").strip() - if target.startswith(MODEL_KIND_PREFIX): - if request.cascade: - raise CliUserError("--cascade applies to column renames only", code="C954") - return RefactorRequest( - operation=RefactorOperation.RENAME_MODEL, - model_name=target.removeprefix(MODEL_KIND_PREFIX), - new_name=new_name, - ) - if target.startswith(COLUMN_KIND_PREFIX) and COLUMN_TARGET_SEPARATOR in target: - model_name: str - column_name: str - model_name, column_name = target.removeprefix(COLUMN_KIND_PREFIX).rsplit( - COLUMN_TARGET_SEPARATOR, 1 - ) - return RefactorRequest( - operation=RefactorOperation.RENAME_COLUMN, - model_name=model_name, - column_name=column_name, - new_name=new_name, - cascade=request.cascade, - ) - raise CliUserError( - f"cannot rename '{target}'", - code="C954", - help="write the target as model: or column:.", - ) - - def _relative( *, errors: tuple[CompilerDiagnostic, ...], staging_dir: Path ) -> tuple[CompilerDiagnostic, ...]: diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/target.py b/src/sqlbuild/cli/commands/_helpers/refactor/target.py new file mode 100644 index 0000000000..19a78f0325 --- /dev/null +++ b/src/sqlbuild/cli/commands/_helpers/refactor/target.py @@ -0,0 +1,83 @@ +"""Read the target of `sqb rename` and `sqb mv` the way `sqb lineage` reads targets.""" + +from __future__ import annotations + +from sqlbuild.cli.commands._helpers.lineage.selection import resolve_lineage_target +from sqlbuild.cli.commands.constants import COLUMN_TARGET_KIND, COLUMN_TARGET_SEPARATOR +from sqlbuild.cli.commands.exceptions import CliUserError +from sqlbuild.cli.commands.models import LineageTarget, RefactorCommandRequest +from sqlbuild.cli.commands.types import CliCommand +from sqlbuild.compiler.compile.models import CompiledObjectKey +from sqlbuild.compiler.compile.types import CompiledResourceType +from sqlbuild.compiler.refactoring.models import RefactorRequest +from sqlbuild.compiler.refactoring.types import RefactorOperation + +_RENAME_USAGE: str = ( + "write the target as or .; the model: and column: prefixes are optional" +) +_MOVE_USAGE: str = "write the target as ; the model: prefix is optional" +_RENAME_SCOPE: str = "sqb rename renames models and model columns" +_MOVE_SCOPE: str = "sqb mv moves models" + + +def refactor_request_for_target( + *, request: RefactorCommandRequest, all_keys: dict[str, CompiledObjectKey] +) -> RefactorRequest: + """Read a `sqb rename` or `sqb mv` target the way `sqb lineage` reads its targets.""" + + raw: str = request.target.strip() + target: LineageTarget = resolve_lineage_target(all_keys=all_keys, target=raw) + is_move: bool = request.command == CliCommand.MV + _check_target(raw=raw, target=target, is_move=is_move) + model_name: str = target.name.partition(COLUMN_TARGET_SEPARATOR)[0] + if is_move: + return RefactorRequest( + operation=RefactorOperation.MOVE_MODEL, + model_name=model_name, + new_name="", + destination=request.destination or "", + ) + new_name: str = (request.new_name or "").strip() + if target.column_name is None: + if request.cascade: + raise CliUserError("--cascade applies to column renames only", code="C954") + return RefactorRequest( + operation=RefactorOperation.RENAME_MODEL, + model_name=model_name, + new_name=new_name, + ) + return RefactorRequest( + operation=RefactorOperation.RENAME_COLUMN, + model_name=model_name, + column_name=target.column_name, + new_name=new_name, + cascade=request.cascade, + ) + + +def _check_target(*, raw: str, target: LineageTarget, is_move: bool) -> None: + verb: str = "move" if is_move else "rename" + usage: str = _MOVE_USAGE if is_move else _RENAME_USAGE + key: CompiledObjectKey | None = target.key + if not target.name or (target.column_name is not None and not target.column_name): + raise CliUserError(f"cannot {verb} '{raw}'", code="C954", help=usage) + if key is not None and key.resource_type != CompiledResourceType.MODEL: + raise CliUserError( + f"cannot {verb} '{raw}': {key.name} is a {key.resource_type}; " + f"{_MOVE_SCOPE if is_move else _RENAME_SCOPE}", + code="C954", + help=usage, + ) + if key is not None and not target.kind_matches: + found: str = ( + f"column: needs ., and {key.name} names a model" + if target.kind == COLUMN_TARGET_KIND + else f"{key.name} is a {key.resource_type}, not a {target.kind}" + ) + raise CliUserError(f"cannot {verb} '{raw}': {found}", code="C954", help=usage) + if is_move and target.column_name is not None: + raise CliUserError( + f"sqb mv moves models; '{raw}' names a column", + code="C954", + help=f"rename a column with sqb rename {target.name} ", + ) diff --git a/src/sqlbuild/cli/commands/constants.py b/src/sqlbuild/cli/commands/constants.py index 1a83eab25f..3935075e65 100644 --- a/src/sqlbuild/cli/commands/constants.py +++ b/src/sqlbuild/cli/commands/constants.py @@ -42,6 +42,7 @@ PATH_SEPARATOR: str = "/" SELECTOR_EXPANSION_MARKER: str = "+" COLUMN_TARGET_SEPARATOR: str = "." +COLUMN_TARGET_KIND: str = "column" SUPPORTED_TYPED_SELECTOR_KINDS: frozenset[str] = frozenset( { SelectorKind.SEED, diff --git a/src/sqlbuild/cli/commands/models.py b/src/sqlbuild/cli/commands/models.py index c42b6dbd76..11feb979d9 100644 --- a/src/sqlbuild/cli/commands/models.py +++ b/src/sqlbuild/cli/commands/models.py @@ -764,6 +764,17 @@ class LineageSelectionAnchors: retained: frozenset[CompiledObjectKey] = field(default_factory=frozenset) +@dataclass(frozen=True) +class LineageTarget: + """One positional target, `[kind:]` or `[kind:].`, resolved by name.""" + + kind: str | None + name: str + key: CompiledObjectKey | None + column_name: str | None + kind_matches: bool + + @dataclass(frozen=True) class ParsedLineageSelector: """One parsed non-path lineage selector.""" diff --git a/src/sqlbuild/compiler/planner/constants.py b/src/sqlbuild/compiler/planner/constants.py index b6bcd0e737..5af959ebbd 100644 --- a/src/sqlbuild/compiler/planner/constants.py +++ b/src/sqlbuild/compiler/planner/constants.py @@ -88,7 +88,7 @@ SOURCE_INPUT_FUNCTION: str = "__source" REF_INPUT_FUNCTION: str = "__ref" MANUAL_RENAME_HINT: str = ( - "matched by unchanged definition; sqb rename model: rewrites references and " + "matched by unchanged definition; sqb rename rewrites references and " "declares migrate_from so the move does not depend on discovery" ) HIDDEN_ORIGIN_REMEDY: str = ( diff --git a/src/sqlbuild/compiler/refactoring/_helpers/renames/model_planning.py b/src/sqlbuild/compiler/refactoring/_helpers/renames/model_planning.py index b566d7490d..f498a4ccdf 100644 --- a/src/sqlbuild/compiler/refactoring/_helpers/renames/model_planning.py +++ b/src/sqlbuild/compiler/refactoring/_helpers/renames/model_planning.py @@ -59,12 +59,12 @@ def find_model(*, project: CompiledProject, name: str) -> CompiledModel: - """Return the compiled model a kind-qualified target names.""" + """Return the compiled model a target names.""" model: CompiledModel | None = next((item for item in project.models if item.name == name), None) if model is None: raise RefactorInputError( - f"{MODEL_KIND_PREFIX}{name} is not a model in this project", + f"no model named '{name}' in this project", code="C951", help="list models with sqb dag --json or sqb scope --browse models", ) diff --git a/src/sqlbuild/compiler/refactoring/constants.py b/src/sqlbuild/compiler/refactoring/constants.py index 7cee4805a0..1ef0eabe47 100644 --- a/src/sqlbuild/compiler/refactoring/constants.py +++ b/src/sqlbuild/compiler/refactoring/constants.py @@ -12,7 +12,6 @@ ) MODEL_KIND_PREFIX: str = "model:" -COLUMN_KIND_PREFIX: str = "column:" SQL_SUFFIX: str = ".sql" PATH_SEPARATOR: str = "/" IDENTIFIER_PATTERN: re.Pattern[str] = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") diff --git a/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py b/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py index eca9d7224f..8b9c4aad86 100644 --- a/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py @@ -13,3 +13,15 @@ class RefactorStatusMessagesTestCase: new_name: str | None destination: str | None expected_lines: tuple[str, ...] + + +@dataclass(frozen=True) +class BareTargetRefactorTestCase: + description: str + command: str + target: str + new_name: str | None + destination: str | None + expected_path: str + expected_fragment: str + expected_missing_paths: tuple[str, ...] diff --git a/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/test_command.py b/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/test_command.py index d754295ac9..db5666b4cf 100644 --- a/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/test_command.py +++ b/tests/integration/src/sqlbuild/cli/commands/_helpers/refactor/test_command.py @@ -10,6 +10,7 @@ from sqlbuild.cli.commands.models import RefactorCommandRequest from sqlbuild.cli.commands.types import CliCommand from tests.integration.src.sqlbuild.cli.commands._helpers.refactor._test_types import ( + BareTargetRefactorTestCase, RefactorStatusMessagesTestCase, ) @@ -113,5 +114,62 @@ def test_given_refactor_when_running_then_progress_messages_agree_in_number( assert expected in lines, lines +@pytest.mark.parametrize( + "test_case", + [ + BareTargetRefactorTestCase( + description="bare model.column renames the column downstream", + command=CliCommand.RENAME, + target="stg_orders.amount", + new_name="revenue", + destination=None, + expected_path="models/marts/fact_orders.sql", + expected_fragment="revenue AS amount", + expected_missing_paths=(), + ), + BareTargetRefactorTestCase( + description="bare model name moves the model file", + command=CliCommand.MV, + target="fact_orders", + new_name=None, + destination="models/finance/", + expected_path="models/finance/fact_orders.sql", + expected_fragment='FROM __ref("stg_orders")', + expected_missing_paths=("models/marts/fact_orders.sql",), + ), + ], + ids=lambda case: case.description, +) +def test_given_bare_target_when_running_then_applies_refactor( + test_case: BareTargetRefactorTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + relative_path: str + content: str + for relative_path, content in _PROJECT_FILES.items(): + path: Path = tmp_path / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content) + + exit_code: int = run_refactor_command( + request=RefactorCommandRequest( + command=test_case.command, + target=test_case.target, + new_name=test_case.new_name, + destination=test_case.destination, + project_dir=tmp_path, + no_color=True, + ) + ) + + output: str = capsys.readouterr().out + assert exit_code == 0, output + assert test_case.expected_fragment in (tmp_path / test_case.expected_path).read_text() + missing_path: str + for missing_path in test_case.expected_missing_paths: + assert not (tmp_path / missing_path).exists() + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/cli/commands/main/model_migrations/test_identity_handover.py b/tests/integration/src/sqlbuild/cli/commands/main/model_migrations/test_identity_handover.py index 6b1af280d5..7802a14a9d 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/model_migrations/test_identity_handover.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/model_migrations/test_identity_handover.py @@ -150,7 +150,7 @@ def test_given_renamed_chain_with_full_replay_when_building_then_migrated_histor "├── customer_orders_enriched migrate main.orders_enriched -> " "main.customer_orders_enriched\n ├── transfer recreate (view)\n" " ├── discovery automatic\n" - " ├── hint matched by unchanged definition; sqb rename model: " + " ├── hint matched by unchanged definition; sqb rename " "rewrites references and declares migrate_from so the move does not depend on discovery\n" " └── old name main.orders_enriched\n" " └── left for janitor old_name_view false" diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/_test_types.py index 6d016fad87..8683957ff4 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/_test_types.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/_test_types.py @@ -98,3 +98,17 @@ class LineagePathSelectorErrorTestCase: select: tuple[str, ...] expected_error_code: str expected_error_fragment: str + + +@dataclass(frozen=True) +class NormalizeLineageTargetTestCase: + description: str + target: str + expected_target: str + + +@dataclass(frozen=True) +class NormalizeLineageTargetErrorTestCase: + description: str + target: str + expected_code: str diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/test_selection.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/test_selection.py index f23331dd46..5b776f78b1 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/test_selection.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/lineage/test_selection.py @@ -5,6 +5,7 @@ import pytest from sqlbuild.cli.commands._helpers.lineage.selection import ( + normalize_lineage_target, select_column_target_lineage, select_selector_lineage, select_target_lineage, @@ -25,6 +26,8 @@ LineagePathSelectorTestCase, LineageSelectionTestCase, LineageSelectorDepthErrorTestCase, + NormalizeLineageTargetErrorTestCase, + NormalizeLineageTargetTestCase, ) from tests.unit.src.sqlbuild.cli.commands._helpers.lineage.helpers import ( build_lineage_test_graph, @@ -334,3 +337,81 @@ def test_given_unreachable_path_selector_when_selecting_lineage_then_raises_user assert error.value.code == test_case.expected_error_code assert test_case.expected_error_fragment in str(error.value) + + +@pytest.mark.parametrize( + "test_case", + [ + NormalizeLineageTargetTestCase( + description="bare model is unchanged", + target="stg_orders", + expected_target="stg_orders", + ), + NormalizeLineageTargetTestCase( + description="model prefix is stripped", + target="model:stg_orders", + expected_target="stg_orders", + ), + NormalizeLineageTargetTestCase( + description="model prefix on a column keeps the column", + target="model:stg_orders.amount", + expected_target="stg_orders.amount", + ), + NormalizeLineageTargetTestCase( + description="column prefix is stripped", + target="column:stg_orders.amount", + expected_target="stg_orders.amount", + ), + NormalizeLineageTargetTestCase( + description="source prefix is stripped", + target="source:raw_orders", + expected_target="raw_orders", + ), + ], + ids=lambda case: case.description, +) +def test_given_target_when_normalizing_then_strips_matching_kind_prefix( + test_case: NormalizeLineageTargetTestCase, +) -> None: + graph: ProjectGraph = build_lineage_test_graph() + + assert ( + normalize_lineage_target(graph=graph, target=test_case.target) == test_case.expected_target + ) + + +@pytest.mark.parametrize( + "test_case", + [ + NormalizeLineageTargetErrorTestCase( + description="prefix naming another kind is unknown", + target="source:stg_orders", + expected_code="C305", + ), + NormalizeLineageTargetErrorTestCase( + description="column prefix without a column is unknown", + target="column:stg_orders", + expected_code="C305", + ), + NormalizeLineageTargetErrorTestCase( + description="column prefix on a source column is unknown", + target="column:raw_orders.id", + expected_code="C305", + ), + NormalizeLineageTargetErrorTestCase( + description="prefixed missing model is unknown", + target="model:missing", + expected_code="C305", + ), + ], + ids=lambda case: case.description, +) +def test_given_mismatched_kind_prefix_when_normalizing_then_raises_unknown_target( + test_case: NormalizeLineageTargetErrorTestCase, +) -> None: + graph: ProjectGraph = build_lineage_test_graph() + + with pytest.raises(CliUserError) as raised: + _ = normalize_lineage_target(graph=graph, target=test_case.target) + + assert raised.value.code == test_case.expected_code diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py index b0a7df1bd0..3be56f3915 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py @@ -4,7 +4,7 @@ from dataclasses import dataclass -from sqlbuild.compiler.refactoring.types import RefactorStatus +from sqlbuild.compiler.refactoring.types import RefactorOperation, RefactorStatus @dataclass(frozen=True) @@ -13,3 +13,24 @@ class RefactorStatusLineTestCase: status: RefactorStatus changed_files: int expected_line: str + + +@dataclass(frozen=True) +class RefactorTargetTestCase: + description: str + command: str + target: str + cascade: bool + expected_operation: RefactorOperation + expected_model_name: str + expected_column_name: str | None + + +@dataclass(frozen=True) +class RefactorTargetErrorTestCase: + description: str + command: str + target: str + cascade: bool + expected_message: str + expected_help: str | None diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_target.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_target.py new file mode 100644 index 0000000000..d1be6aef66 --- /dev/null +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_target.py @@ -0,0 +1,232 @@ +"""Reading `sqb rename` and `sqb mv` targets, with and without kind prefixes.""" + +from __future__ import annotations + +import pytest + +from sqlbuild.cli.commands._helpers.refactor.target import refactor_request_for_target +from sqlbuild.cli.commands.exceptions import CliUserError +from sqlbuild.cli.commands.models import RefactorCommandRequest +from sqlbuild.cli.commands.types import CliCommand +from sqlbuild.compiler.compile.models import CompiledObjectKey +from sqlbuild.compiler.compile.types import CompiledResourceType +from sqlbuild.compiler.refactoring.models import RefactorRequest +from sqlbuild.compiler.refactoring.types import RefactorOperation +from tests.unit.src.sqlbuild.cli.commands._helpers.refactor._test_types import ( + RefactorTargetErrorTestCase, + RefactorTargetTestCase, +) + +_ALL_KEYS: dict[str, CompiledObjectKey] = { + "stg_orders": CompiledObjectKey(CompiledResourceType.MODEL, "stg_orders"), + "raw_orders": CompiledObjectKey(CompiledResourceType.SOURCE, "raw_orders"), + "order_types": CompiledObjectKey(CompiledResourceType.SEED, "order_types"), + "is_completed": CompiledObjectKey(CompiledResourceType.UDF, "is_completed"), +} +_RENAME_HELP: str = ( + "write the target as or .; the model: and column: prefixes are optional" +) + + +@pytest.mark.parametrize( + "test_case", + [ + RefactorTargetTestCase( + description="bare model renames the model", + command=CliCommand.RENAME, + target="stg_orders", + cascade=False, + expected_operation=RefactorOperation.RENAME_MODEL, + expected_model_name="stg_orders", + expected_column_name=None, + ), + RefactorTargetTestCase( + description="bare model column renames the column", + command=CliCommand.RENAME, + target="stg_orders.amount", + cascade=True, + expected_operation=RefactorOperation.RENAME_COLUMN, + expected_model_name="stg_orders", + expected_column_name="amount", + ), + RefactorTargetTestCase( + description="model prefix renames the model", + command=CliCommand.RENAME, + target="model:stg_orders", + cascade=False, + expected_operation=RefactorOperation.RENAME_MODEL, + expected_model_name="stg_orders", + expected_column_name=None, + ), + RefactorTargetTestCase( + description="column prefix renames the column", + command=CliCommand.RENAME, + target="column:stg_orders.amount", + cascade=False, + expected_operation=RefactorOperation.RENAME_COLUMN, + expected_model_name="stg_orders", + expected_column_name="amount", + ), + RefactorTargetTestCase( + description="bare model moves the model", + command=CliCommand.MV, + target="stg_orders", + cascade=False, + expected_operation=RefactorOperation.MOVE_MODEL, + expected_model_name="stg_orders", + expected_column_name=None, + ), + RefactorTargetTestCase( + description="model prefix moves the model", + command=CliCommand.MV, + target="model:stg_orders", + cascade=False, + expected_operation=RefactorOperation.MOVE_MODEL, + expected_model_name="stg_orders", + expected_column_name=None, + ), + RefactorTargetTestCase( + description="unknown bare name is left for the planner to report", + command=CliCommand.RENAME, + target="missing_model", + cascade=False, + expected_operation=RefactorOperation.RENAME_MODEL, + expected_model_name="missing_model", + expected_column_name=None, + ), + ], + ids=lambda case: case.description, +) +def test_given_target_when_reading_then_returns_matching_request( + test_case: RefactorTargetTestCase, +) -> None: + result: RefactorRequest = refactor_request_for_target( + request=RefactorCommandRequest( + command=test_case.command, + target=test_case.target, + new_name="renamed", + destination="models/marts/", + cascade=test_case.cascade, + ), + all_keys=_ALL_KEYS, + ) + + assert result.operation == test_case.expected_operation + assert result.model_name == test_case.expected_model_name + assert result.column_name == test_case.expected_column_name + + +@pytest.mark.parametrize( + "test_case", + [ + RefactorTargetErrorTestCase( + description="bare source is refused and named", + command=CliCommand.RENAME, + target="raw_orders", + cascade=False, + expected_message=( + "cannot rename 'raw_orders': raw_orders is a source; " + "sqb rename renames models and model columns" + ), + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="source column is refused and named", + command=CliCommand.RENAME, + target="raw_orders.id", + cascade=False, + expected_message=( + "cannot rename 'raw_orders.id': raw_orders is a source; " + "sqb rename renames models and model columns" + ), + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="seed is refused even with its own prefix", + command=CliCommand.RENAME, + target="seed:order_types", + cascade=False, + expected_message=( + "cannot rename 'seed:order_types': order_types is a seed; " + "sqb rename renames models and model columns" + ), + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="function cannot be moved", + command=CliCommand.MV, + target="is_completed", + cascade=False, + expected_message=( + "cannot move 'is_completed': is_completed is a udf; sqb mv moves models" + ), + expected_help="write the target as ; the model: prefix is optional", + ), + RefactorTargetErrorTestCase( + description="prefix that contradicts the resource names what it is", + command=CliCommand.RENAME, + target="source:stg_orders", + cascade=False, + expected_message="cannot rename 'source:stg_orders': stg_orders is a model, not a source", + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="column prefix without a column is refused", + command=CliCommand.RENAME, + target="column:stg_orders", + cascade=False, + expected_message=( + "cannot rename 'column:stg_orders': column: needs ., " + "and stg_orders names a model" + ), + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="empty column name is refused", + command=CliCommand.RENAME, + target="stg_orders.", + cascade=False, + expected_message="cannot rename 'stg_orders.'", + expected_help=_RENAME_HELP, + ), + RefactorTargetErrorTestCase( + description="moving a column points at sqb rename", + command=CliCommand.MV, + target="stg_orders.amount", + cascade=False, + expected_message="sqb mv moves models; 'stg_orders.amount' names a column", + expected_help="rename a column with sqb rename stg_orders.amount ", + ), + RefactorTargetErrorTestCase( + description="cascade on a bare model is refused", + command=CliCommand.RENAME, + target="stg_orders", + cascade=True, + expected_message="--cascade applies to column renames only", + expected_help=None, + ), + ], + ids=lambda case: case.description, +) +def test_given_unsupported_target_when_reading_then_raises_c954( + test_case: RefactorTargetErrorTestCase, +) -> None: + with pytest.raises(CliUserError) as raised: + _ = refactor_request_for_target( + request=RefactorCommandRequest( + command=test_case.command, + target=test_case.target, + new_name="renamed", + destination="models/marts/", + cascade=test_case.cascade, + ), + all_keys=_ALL_KEYS, + ) + + assert raised.value.code == "C954" + assert raised.value.message == test_case.expected_message + assert raised.value.help == test_case.expected_help + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/planner/main/pre_build/test_pre_build_work_display.py b/tests/unit/src/sqlbuild/compiler/planner/main/pre_build/test_pre_build_work_display.py index cfec0adb75..f63ccc5e6b 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/main/pre_build/test_pre_build_work_display.py +++ b/tests/unit/src/sqlbuild/compiler/planner/main/pre_build/test_pre_build_work_display.py @@ -139,7 +139,7 @@ def test_given_migration_decision_when_formatting_then_applies_colour_roles( " ├── compatibility compatible\n" " ├── transfer physical copy, promote by transactional rename\n" " ├── discovery automatic\n" - " └── hint matched by unchanged definition; sqb rename model: " + " └── hint matched by unchanged definition; sqb rename " "rewrites references and declares migrate_from so the move does not depend on discovery" ), ), diff --git a/website/examples/README.md b/website/examples/README.md index 0a53b6bd35..f5ad8b2a44 100644 --- a/website/examples/README.md +++ b/website/examples/README.md @@ -52,7 +52,7 @@ cd waffle-shop mv models/marts/daily_order_rollup.sql models/marts/daily_activity_rollup.sql sed -i 's/daily_order_rollup/daily_activity_rollup/g' models/marts/hourly_activity_with_daily_context.sql sqb build -sqb rename model:daily_activity_rollup daily_order_rollup +sqb rename daily_activity_rollup daily_order_rollup sqb plan ``` @@ -65,7 +65,7 @@ cd hero-shop mkdir -p models/marts mv models/finance/daily_revenue.sql models/marts/revenue.sql sqb build -sqb mv model:revenue models/finance/daily_revenue.sql +sqb mv revenue models/finance/daily_revenue.sql sqb plan ``` diff --git a/website/src/content/docs/docs/cli/rename.mdx b/website/src/content/docs/docs/cli/rename.mdx index 9230251a2a..6b41ffb884 100644 --- a/website/src/content/docs/docs/cli/rename.mdx +++ b/website/src/content/docs/docs/cli/rename.mdx @@ -10,19 +10,21 @@ Nothing is written until the edited project compiles. The command applies its ed ## Usage ```bash -sqb rename model: [flags] -sqb mv model: [flags] -sqb rename column:. [--cascade] [flags] +sqb rename [flags] +sqb mv [flags] +sqb rename . [--cascade] [flags] ``` ```bash -sqb rename model:stg_orders stg_order_lines -sqb mv model:fact_orders models/reporting/ # keeps the name -sqb mv model:fact_orders models/reporting/order_facts.sql # moves and renames -sqb rename column:stg_orders.amount revenue -sqb rename column:stg_orders.amount revenue --cascade +sqb rename stg_orders stg_order_lines +sqb mv fact_orders models/reporting/ # keeps the name +sqb mv fact_orders models/reporting/order_facts.sql # moves and renames +sqb rename stg_orders.amount revenue +sqb rename stg_orders.amount revenue --cascade ``` +Targets are written the way [`sqb lineage`](/docs/cli/lineage) reads them: a bare name is a model, and `.` is one of its columns. The `model:` and `column:` prefixes are optional, so `sqb rename model:stg_orders stg_order_lines` and `sqb rename column:stg_orders.amount revenue` still work. A target that names a source, seed, or function is refused, because those are not renamed by these commands. + A model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` takes a new file path, which also sets the model name, or a folder ending in `/`, which keeps it. ## What changes diff --git a/website/src/content/docs/docs/concepts/models/column-migrations.mdx b/website/src/content/docs/docs/concepts/models/column-migrations.mdx index 05408005d0..df5dd93edf 100644 --- a/website/src/content/docs/docs/concepts/models/column-migrations.mdx +++ b/website/src/content/docs/docs/concepts/models/column-migrations.mdx @@ -9,7 +9,7 @@ Column migrations apply to incremental models, including microbatch models, and ## Declaring a rename -[`sqb rename column:.`](/docs/cli/rename) renames the column in the model and every query that reads it, and adds the `migrate_from` declaration below for you. +[`sqb rename .`](/docs/cli/rename) renames the column in the model and every query that reads it, and adds the `migrate_from` declaration below for you. Add `migrate_from` to the renamed column in the model header, naming the old column: