Repository: TobikoData/sqlmesh
Pull request #2347 (2024-03-26 20:52:45)

fix: ensure start if using end
Prior to this PR a user would get validation error if defining an end without start that wasn't clear on the issue. This validates prior to the actual value validation if an end is defined without a start and gives a more clear error.

I think requiring start with end seems reasonable. Open to other thoughts though. 


Patch:
diff --git a/setup.py b/setup.py
index 0a53ef73..fa7e5e11 100644
--- a/setup.py
+++ b/setup.py
@@ -45,7 +45,7 @@ setup(
         "requests",
         "rich[jupyter]",
         "ruamel.yaml",
-        "sqlglot[rs]~=23.0.3",
+        "sqlglot[rs]~=23.2.0",
     ],
     extras_require={
         "bigquery": [
diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py
index c808215c..0e8e57d9 100644
--- a/sqlmesh/core/context.py
+++ b/sqlmesh/core/context.py
@@ -1587,9 +1587,9 @@ class GenericContext(BaseContext, t.Generic[C]):
         for path in self.configs:
             rmtree(path / c.CACHE)
 
-    def _run_tests(self) -> t.Tuple[unittest.result.TestResult, str]:
+    def _run_tests(self, verbose: bool = False) -> t.Tuple[unittest.result.TestResult, str]:
         test_output_io = StringIO()
-        result = self.test(stream=test_output_io)
+        result = self.test(stream=test_output_io, verbose=verbose)
         return result, test_output_io.getvalue()
 
     def _run_plan_tests(
diff --git a/sqlmesh/core/engine_adapter/base.py b/sqlmesh/core/engine_adapter/base.py
index d72ec2e8..3eae8b5d 100644
--- a/sqlmesh/core/engine_adapter/base.py
+++ b/sqlmesh/core/engine_adapter/base.py
@@ -1273,7 +1273,11 @@ class EngineAdapter:
             source_table, columns_to_types, target_table=target_table, batch_size=0
         )
         columns_to_types = columns_to_types or self.columns(target_table)
-        if valid_from_name not in columns_to_types or valid_to_name not in columns_to_types:
+        if (
+            valid_from_name not in columns_to_types
+            or valid_to_name not in columns_to_types
+            or not columns_to_types_all_known(columns_to_types)
+        ):
             columns_to_types = self.columns(target_table)
         if not columns_to_types:
             raise SQLMeshError(f"Could not get columns_to_types. Does {target_table} exist?")
diff --git a/sqlmesh/core/model/kind.py b/sqlmesh/core/model/kind.py
index 630ec9d4..163c3a1b 100644
--- a/sqlmesh/core/model/kind.py
+++ b/sqlmesh/core/model/kind.py
@@ -370,12 +370,13 @@ class _SCDType2Kind(_ModelKind):
     valid_from_name: SQLGlotString = "valid_from"
     valid_to_name: SQLGlotString = "valid_to"
     invalidate_hard_deletes: SQLGlotBool = False
-    time_data_type: exp.DataType = exp.DataType.build("TIMESTAMP")
+    time_data_type: exp.DataType = Field(exp.DataType.build("TIMESTAMP"), validate_default=True)
 
     forward_only: SQLGlotBool = True
     disable_restatement: SQLGlotBool = True
 
-    @field_validator("time_data_type", mode="before")
+    # always=True can be removed once Pydantic 1 is deprecated
+    @field_validator("time_data_type", mode="before", always=True)
     @classmethod
     def _time_data_type_validator(
         cls, v: t.Union[str, exp.Expression], values: t.Any
@@ -383,7 +384,10 @@ class _SCDType2Kind(_ModelKind):
         values = values if isinstance(values, dict) else values.data
         if isinstance(v, exp.Expression) and not isinstance(v, exp.DataType):
             v = v.name
-        return exp.DataType.build(v, dialect=values.get("dialect"))
+        dialect = values.get("dialect")
+        data_type = exp.DataType.build(v, dialect=dialect)
+        data_type.meta["dialect"] = dialect
+        return data_type
 
     @property
     def managed_columns(self) -> t.Dict[str, exp.DataType]:
diff --git a/sqlmesh/core/node.py b/sqlmesh/core/node.py
index 080b5619..c71c7bc9 100644
--- a/sqlmesh/core/node.py
+++ b/sqlmesh/core/node.py
@@ -258,7 +258,11 @@ class _Node(PydanticModel):
                 raise ConfigError(
                     f"Interval unit of '{interval_unit}' is larger than cron period of '{cron}'"
                 )
-        validate_date_range(values.get("start"), values.get("end"))
+        start = values.get("start")
+        end = values.get("end")
+        if end is not None and start is None:
+            raise ConfigError("Must define a start date if an end date is defined.")
+        validate_date_range(start, end)
         return values
 
     @property
diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py
index 1e37e21c..ddf8cc65 100644
--- a/sqlmesh/integrations/github/cicd/controller.py
+++ b/sqlmesh/integrations/github/cicd/controller.py
@@ -456,7 +456,7 @@ class GithubController:
         """
         Run tests for the PR
         """
-        return self._context._run_tests()
+        return self._context._run_tests(verbose=True)
 
     def _get_or_create_comment(self, header: str = BOT_HEADER_MSG) -> IssueComment:
         comment = seq_get(


Test patch:
diff --git a/tests/core/test_model.py b/tests/core/test_model.py
index 74ce12a3..67ebe5cd 100644
--- a/tests/core/test_model.py
+++ b/tests/core/test_model.py
@@ -444,6 +444,40 @@ def test_json_serde():
     assert deserialized_model == model
 
 
+def test_scd_type_2_by_col_serde():
+    expressions = parse(
+        """
+        MODEL(
+            name test_model,
+            KIND SCD_TYPE_2_BY_COLUMN (
+                unique_key a,
+                columns *,
+            ),
+            cron '@daily',
+            owner 'reakman',
+            grain a,
+            dialect bigquery
+        );
+
+        SELECT a, ds FROM `tbl`
+    """,
+        default_dialect="bigquery",
+    )
+
+    model = load_sql_based_model(expressions)
+
+    model_json = model.json()
+    model_json_parsed = json.loads(model.json())
+    assert model_json_parsed["kind"]["dialect"] == "bigquery"
+    assert model_json_parsed["kind"]["unique_key"] == ["`a`"]
+    assert model_json_parsed["kind"]["columns"] == "*"
+    # Bigquery converts TIMESTAMP -> DATETIME
+    assert model_json_parsed["kind"]["time_data_type"] == "DATETIME"
+
+    deserialized_model = SqlModel.parse_raw(model_json)
+    assert deserialized_model.dict() == model.dict()
+
+
 def test_column_descriptions(sushi_context, assert_exp_eq):
     assert sushi_context.models[
         '"memory"."sushi"."customer_revenue_by_day"'
@@ -3323,3 +3357,22 @@ def test_end_date():
             """
             )
         )
+
+
+def test_end_no_start():
+    expressions = d.parse(
+        f"""
+        MODEL (
+            name db.table,
+            kind INCREMENTAL_BY_TIME_RANGE (
+                time_column ts,
+            ),
+            end '2023-06-01'
+        );
+
+        SELECT 1::int AS a, 2::int AS b, now::timestamp as ts
+        """
+    )
+    with pytest.raises(ConfigError, match="Must define a start date if an end date is defined"):
+        load_sql_based_model(expressions)
+    load_sql_based_model(expressions, defaults={"start": "2023-01-01"})
diff --git a/tests/integrations/github/cicd/test_github_commands.py b/tests/integrations/github/cicd/test_github_commands.py
index 2129ea4c..cdbf7df5 100644
--- a/tests/integrations/github/cicd/test_github_commands.py
+++ b/tests/integrations/github/cicd/test_github_commands.py
@@ -68,7 +68,9 @@ def test_run_all_success_with_approvers_approved(
         github_client,
         bot_config=GithubCICDBotConfig(invalidate_environment_after_deploy=False),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -193,7 +195,9 @@ def test_run_all_success_with_approvers_approved_merge_delete(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -320,7 +324,9 @@ def test_run_all_missing_approval(
         github_client,
         bot_config=GithubCICDBotConfig(invalidate_environment_after_deploy=False),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -440,7 +446,7 @@ def test_run_all_test_failed(
     test_result = TestResult()
     test_result.addFailure(TestCase(), (None, None, None))
     controller._context._run_tests = mocker.MagicMock(
-        side_effect=lambda: (test_result, "some error")
+        side_effect=lambda **kwargs: (test_result, "some error")
     )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
@@ -561,7 +567,9 @@ def test_pr_update_failure(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -681,7 +689,9 @@ def test_prod_update_failure(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -815,7 +825,9 @@ def test_comment_command_invalid(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -882,7 +894,9 @@ def test_comment_command_deploy_prod(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE, enable_deploy_command=True),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = [
         User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
     ]
@@ -1002,7 +1016,9 @@ def test_comment_command_deploy_prod_not_enabled(
         github_client,
         bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE),
     )
-    controller._context._run_tests = mocker.MagicMock(side_effect=lambda: (TestResult(), ""))
+    controller._context._run_tests = mocker.MagicMock(
+        side_effect=lambda **kwargs: (TestResult(), "")
+    )
     controller._context.users = []
     controller._context.invalidate_environment = mocker.MagicMock()