Merge remote-tracking branch 'origin/main' into copilot/fix-mlv-schema-mismatch-216

Author: Copilot

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## v1.12.5
+
+### Bug Fixes
+
+- Fix `UnexpectedJinjaBlockDeprecation` (D023) emitted by `create_table_as.sql`: a stray `{% do %}` tag inside a C-style `/* … */` comment was being parsed as a top-level Jinja block. Switched the wrapper to a Jinja comment `{# … #}`. Added a unit-test regression that scans every macro file for `unexpected_block` warnings. ([#46](https://github.com/microsoft/dbt-fabricspark/issues/46))
+- Pin `dbt-core<2.0` in `tools/scripts/run-local-e2e.sh` so the local end-to-end harness keeps resolving a `fabricspark`-aware dbt-core (avoids the unrelated `2.0.0a1` alpha).
+- Fix latent typo in `alter_column_set_constraints` dispatcher macro (`create_table_as.sql`): the body was missing its `{{ … }}` wrap, so the macro emitted its own source as text instead of dispatching. Added unit guards: (a) the dispatcher now renders via `adapter.dispatch` correctly, and (b) `FabricSparkRelation` cannot silently re-introduce the legacy `Cannot set database in spark!` runtime check that v1.9.3 removed to enable schema-enabled lakehouses and cross-lakehouse writes. ([#46](https://github.com/microsoft/dbt-fabricspark/issues/46))
+
 ## v1.12.4
 
 ### Bug Fixes

contrib/bootstrap-dev-env.ps1

@@ -43,10 +43,61 @@ foreach ($distro in $distros) {
     wsl --unregister $distro
 }
 
+$RECOMMENDED_CORES = 16
+$RECOMMENDED_FREE_GB = 128
+
 $memGB=[math]::Floor((Get-CimInstance Win32_ComputerSystem).TotalPhysicalMemory/1GB)
 $cpu=[Environment]::ProcessorCount
 $swap=[math]::Floor($memGB/4)
 
+if ($cpu -lt $RECOMMENDED_CORES) {
+    Write-Host "WARNING: This machine has $cpu cores, which is below the recommended $RECOMMENDED_CORES cores." -ForegroundColor DarkYellow
+    $requiredResponse = "I am OK with having a subpar development experience"
+    do {
+        $response = Read-Host "Please type '$requiredResponse' **EXACTLY** as is to continue"
+    } while ($response -ne $requiredResponse)
+} else {
+    Write-Host "(detected $cpu cores, ${memGB}GB RAM)" -ForegroundColor Green
+}
+
+$driveOptions = @(Get-CimInstance Win32_LogicalDisk -Filter "DriveType=3" |
+    Sort-Object -Property FreeSpace -Descending |
+    ForEach-Object {
+        [PSCustomObject]@{
+            Path   = "$($_.DeviceID)\VHD"
+            FreeGB = [math]::Floor($_.FreeSpace / 1GB)
+        }
+    })
+
+if ($driveOptions.Count -eq 0) {
+    Write-Error "No local fixed drives found to host the WSL VHD."
+    exit 1
+}
+
+Write-Host ""
+Write-Host "Choose a drive to store the WSL VHD (the repo recommends at least ${RECOMMENDED_FREE_GB} GB free):" -ForegroundColor Cyan
+for ($i = 0; $i -lt $driveOptions.Count; $i++) {
+    $opt = $driveOptions[$i]
+    Write-Host ("  [{0}] {1}  ({2} GB free)" -f ($i + 1), $opt.Path, $opt.FreeGB)
+}
+
+do {
+    $pick = Read-Host "Pick a drive number [1-$($driveOptions.Count)]"
+} while (-not ($pick -as [int]) -or [int]$pick -lt 1 -or [int]$pick -gt $driveOptions.Count)
+
+$wslLocation = $driveOptions[[int]$pick - 1]
+
+if ($wslLocation.FreeGB -lt $RECOMMENDED_FREE_GB) {
+    Write-Host "WARNING: $($wslLocation.Path) has only $($wslLocation.FreeGB) GB free, which is below the recommended ${RECOMMENDED_FREE_GB} GB." -ForegroundColor DarkYellow
+    $requiredResponse = "I am OK with having my WSL crash when my hard disk fills up"
+    do {
+        $response = Read-Host "Please type '$requiredResponse' **EXACTLY** as is to continue"
+    } while ($response -ne $requiredResponse)
+} else {
+    Write-Host "Using $($wslLocation.Path) for the WSL VHD." -ForegroundColor Green
+}
+
+New-Item -ItemType Directory -Path $wslLocation.Path -Force | Out-Null
 @"
 [wsl2]
 memory=${memGB}GB
@@ -60,5 +111,28 @@ wsl --shutdown
 
 winget install -e --id Microsoft.GitCredentialManagerCore
 
-Write-Host "Installing Ubuntu"
-wsl --install -d Ubuntu-24.04
+Write-Host "Updating WSL (the --location flag requires WSL 2.4.4+)"
+wsl --update
+
+$minWslVersion = [version]'2.4.4'
+$prevEncoding = [Console]::OutputEncoding
+try {
+    [Console]::OutputEncoding = [System.Text.Encoding]::Unicode
+    $wslVersionRaw = (wsl --version) 2>$null
+} finally {
+    [Console]::OutputEncoding = $prevEncoding
+}
+$versionMatch = [regex]::Match(($wslVersionRaw -join "`n"), '(\d+)\.(\d+)\.(\d+)')
+if (-not $versionMatch.Success) {
+    Write-Error "Could not determine WSL version from 'wsl --version'. Run 'wsl --update' manually and re-run this script."
+    exit 1
+}
+$wslVersion = [version]("$($versionMatch.Groups[1].Value).$($versionMatch.Groups[2].Value).$($versionMatch.Groups[3].Value)")
+if ($wslVersion -lt $minWslVersion) {
+    Write-Error "Detected WSL $wslVersion, but $minWslVersion is required for 'wsl --install --location'. Run 'wsl --update' manually and re-run this script."
+    exit 1
+}
+Write-Host "WSL version $wslVersion detected." -ForegroundColor Green
+
+Write-Host "Installing Ubuntu to $($wslLocation.Path)"
+wsl --install -d Ubuntu-24.04 --location $wslLocation.Path

src/dbt/include/fabricspark/macros/materializations/models/table/create_table_as.sql

@@ -165,10 +165,10 @@
   {{ return(adapter.dispatch('persist_constraints', 'dbt')(relation, model)) }}
 {% endmacro %}
 
-/* TODO: alter table {{ relation }} change column {{ quoted_name }} set not null;
+{# TODO: alter table {{ relation }} change column {{ quoted_name }} set not null;
      is not supported in Fabric Runtime 1.2/Spark 3.4.1
     {% do alter_column_set_constraints(relation, model.columns) %}
-*/
+#}
 {% macro fabricspark__persist_constraints(relation, model) %}
   {%- set contract_config = config.get('contract') -%}
   {% if contract_config.enforced and config.get('file_format', 'delta') == 'delta' %}
@@ -194,7 +194,7 @@
 
 
 {% macro alter_column_set_constraints(relation, column_dict) %}
-  return(adapter.dispatch('alter_column_set_constraints', 'dbt')(relation, column_dict))
+  {{ return(adapter.dispatch('alter_column_set_constraints', 'dbt')(relation, column_dict)) }}
 {% endmacro %}
 
 {% macro fabricspark__alter_column_set_constraints(relation, column_dict) %}

tests/unit/test_alter_column_set_constraints_dispatcher.py

@@ -0,0 +1,69 @@
+from __future__ import annotations
+
+from pathlib import Path
+from unittest import mock
+
+import pytest
+from jinja2 import Environment, FileSystemLoader
+
+MACROS_DIR = (
+    Path(__file__).resolve().parents[2]
+    / "src"
+    / "dbt"
+    / "include"
+    / "fabricspark"
+    / "macros"
+    / "materializations"
+    / "models"
+    / "table"
+)
+
+
+@pytest.fixture
+def env() -> Environment:
+    return Environment(
+        loader=FileSystemLoader(str(MACROS_DIR)),
+        extensions=["jinja2.ext.do"],
+    )
+
+
+def _dispatch_to_recorder(recorded: list[tuple]):
+    def _dispatch(macro_name, macro_namespace=None, packages=None):
+        def _impl(*args, **kwargs):
+            recorded.append((macro_name, args, kwargs))
+            return ""
+
+        return _impl
+
+    return _dispatch
+
+
+def test_alter_column_set_constraints_dispatches(env: Environment) -> None:
+    template = env.get_template(
+        "create_table_as.sql",
+        globals={
+            "validation": mock.Mock(),
+            "model": mock.Mock(),
+            "exceptions": mock.Mock(),
+            "config": mock.Mock(),
+            "return": lambda r: r,
+        },
+    )
+    recorded: list[tuple] = []
+    adapter = mock.Mock()
+    adapter.dispatch = _dispatch_to_recorder(recorded)
+
+    template.globals["adapter"] = adapter
+
+    rendered = template.module.alter_column_set_constraints("my_relation", {"col": {}})
+
+    assert "return(adapter.dispatch" not in str(rendered), (
+        "alter_column_set_constraints macro is emitting its own source as text — "
+        "the dispatcher is missing its `{{ … }}` wrap. Got:\n" + str(rendered)
+    )
+    assert recorded == [
+        ("alter_column_set_constraints", ("my_relation", {"col": {}}), {}),
+    ], (
+        "Expected the dispatcher to invoke adapter.dispatch exactly once "
+        "with the original args. Got: " + repr(recorded)
+    )

tests/unit/test_macros_no_unexpected_blocks.py

@@ -0,0 +1,37 @@
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from dbt_common.clients._jinja_blocks import (
+    BlockIterator,
+    ExtractWarning,
+    TagIterator,
+)
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+MACROS_DIR = REPO_ROOT / "src" / "dbt" / "include" / "fabricspark" / "macros"
+ALLOWED_BLOCKS = {"macro", "materialization", "test", "data_test"}
+
+
+def _macro_files() -> list[Path]:
+    return sorted(MACROS_DIR.rglob("*.sql"))
+
+
+@pytest.mark.parametrize(
+    "macro_path",
+    _macro_files(),
+    ids=lambda p: str(p.relative_to(REPO_ROOT)),
+)
+def test_macro_file_has_no_unexpected_block_warnings(macro_path: Path) -> None:
+    warnings: list[ExtractWarning] = []
+    BlockIterator(
+        TagIterator(macro_path.read_text()),
+        warning_callback=warnings.append,
+    ).lex_for_blocks(allowed_blocks=ALLOWED_BLOCKS, collect_raw_data=False)
+
+    formatted = "\n".join(f"  {w.warning_type}: {w.msg}" for w in warnings)
+    assert not warnings, (
+        f"{macro_path.relative_to(REPO_ROOT)} emitted unexpected-block warnings "
+        f"(would trigger UnexpectedJinjaBlockDeprecation in dbt-core):\n{formatted}"
+    )

tests/unit/test_relation_no_database_check.py

@@ -0,0 +1,47 @@
+from __future__ import annotations
+
+import inspect
+
+from dbt.adapters.contracts.relation import RelationType
+from dbt.adapters.fabricspark.relation import FabricSparkRelation
+
+_FORBIDDEN_LITERAL = "Cannot set database in spark!"
+
+
+def test_relation_with_distinct_database_and_schema_does_not_raise() -> None:
+    rel = FabricSparkRelation.create(
+        database="lakehouse_a",
+        schema="dbo",
+        identifier="my_table",
+        type=RelationType.Table,
+    )
+
+    assert rel.database == "lakehouse_a"
+    assert rel.schema == "dbo"
+    assert rel.identifier == "my_table"
+    assert rel.matches(database="lakehouse_a", schema="dbo", identifier="my_table")
+
+
+def test_relation_with_distinct_database_and_schema_schemas_enabled_does_not_raise() -> None:
+    FabricSparkRelation._schemas_enabled = True
+    try:
+        rel = FabricSparkRelation.create(
+            database="lakehouse_a",
+            schema="dbo",
+            identifier="my_table",
+            type=RelationType.Table,
+        )
+        assert str(rel) == "`lakehouse_a`.`dbo`.my_table"
+    finally:
+        FabricSparkRelation._schemas_enabled = False
+
+
+def test_relation_source_does_not_contain_legacy_database_check_message() -> None:
+    src = inspect.getsource(FabricSparkRelation)
+    assert _FORBIDDEN_LITERAL not in src, (
+        f"FabricSparkRelation source contains the legacy "
+        f"{_FORBIDDEN_LITERAL!r} message. This guard was removed in v1.9.3 "
+        "(commit 2dba5bc) to support schema-enabled lakehouses and "
+        "cross-lakehouse / cross-workspace writes — it must not be "
+        "re-introduced."
+    )