Fix: Propagation of boolean model fields in Python models (#4130)

Author: izeigerman

sqlmesh/core/config/connection.py

@@ -152,7 +152,8 @@ def to_sql(self, alias: str) -> str:
         # TODO: Add support for Postgres schema. Currently adding it blocks access to the information_schema
         if self.type == "motherduck":
             # MotherDuck does not support aliasing
-            if (md_db := self.path.replace("md:", "")) != alias.replace('"', ""):
+            md_db = self.path.replace("md:", "")
+            if md_db != alias.replace('"', ""):
                 raise ConfigError(
                     f"MotherDuck does not support assigning an alias different from the database name {md_db}."
                 )
@@ -195,7 +196,8 @@ def _validate_database_catalogs(cls, data: t.Any) -> t.Any:
         if not isinstance(data, dict):
             return data
 
-        if db_path := data.get("database") and data.get("catalogs"):
+        db_path = data.get("database")
+        if db_path and data.get("catalogs"):
             raise ConfigError(
                 "Cannot specify both `database` and `catalogs`. Define all your catalogs in `catalogs` and have the first entry be the default catalog"
             )
@@ -302,7 +304,8 @@ def create_engine_adapter(self, register_comments_override: bool = False) -> Eng
         data_files.discard(":memory:")
         for data_file in data_files:
             key = data_file if isinstance(data_file, str) else data_file.path
-            if adapter := BaseDuckDBConnectionConfig._data_file_to_adapter.get(key):
+            adapter = BaseDuckDBConnectionConfig._data_file_to_adapter.get(key)
+            if adapter is not None:
                 logger.info(
                     f"Using existing DuckDB adapter due to overlapping data file: {self._mask_motherduck_token(key)}"
                 )

sqlmesh/core/engine_adapter/base.py

@@ -1046,17 +1046,25 @@ def create_view(
         if materialized_properties:
             partitioned_by = materialized_properties.pop("partitioned_by", None)
             clustered_by = materialized_properties.pop("clustered_by", None)
-            if partitioned_by and (
-                partitioned_by_prop := self._build_partitioned_by_exp(
-                    partitioned_by, **materialized_properties
+            if (
+                partitioned_by
+                and (
+                    partitioned_by_prop := self._build_partitioned_by_exp(
+                        partitioned_by, **materialized_properties
+                    )
                 )
+                is not None
             ):
                 materialized_properties["catalog_name"] = exp.to_table(view_name).catalog
                 properties.append("expressions", partitioned_by_prop)
-            if clustered_by and (
-                clustered_by_prop := self._build_clustered_by_exp(
-                    clustered_by, **materialized_properties
+            if (
+                clustered_by
+                and (
+                    clustered_by_prop := self._build_clustered_by_exp(
+                        clustered_by, **materialized_properties
+                    )
                 )
+                is not None
             ):
                 properties.append("expressions", clustered_by_prop)
 

sqlmesh/core/engine_adapter/bigquery.py

@@ -830,7 +830,8 @@ def _build_struct_with_descriptions(
             return exp.DataType(this=col_type.this, expressions=column_expressions, nested=True)
 
         # Recursively build column definitions for BigQuery's RECORDs (struct) and REPEATED RECORDs (array of struct)
-        if isinstance(col_type, exp.DataType) and (expressions := col_type.expressions):
+        if isinstance(col_type, exp.DataType) and col_type.expressions:
+            expressions = col_type.expressions
             if col_type.is_type(exp.DataType.Type.STRUCT):
                 col_type = _build_struct_with_descriptions(col_type, nested_names + [col_name])
             elif col_type.is_type(exp.DataType.Type.ARRAY) and expressions[0].is_type(

sqlmesh/core/engine_adapter/clickhouse.py

@@ -785,8 +785,9 @@ def _build_table_properties_exp(
                 )
             )
 
-        if partitioned_by and (
-            partitioned_by_prop := self._build_partitioned_by_exp(partitioned_by)
+        if (
+            partitioned_by
+            and (partitioned_by_prop := self._build_partitioned_by_exp(partitioned_by)) is not None
         ):
             properties.append(partitioned_by_prop)
 

sqlmesh/core/engine_adapter/snowflake.py

@@ -221,7 +221,10 @@ def _build_table_properties_exp(
                 )
             )
 
-        if clustered_by and (clustered_by_prop := self._build_clustered_by_exp(clustered_by)):
+        if (
+            clustered_by
+            and (clustered_by_prop := self._build_clustered_by_exp(clustered_by)) is not None
+        ):
             properties.append(clustered_by_prop)
 
         if table_properties:

sqlmesh/core/engine_adapter/trino.py

@@ -353,8 +353,8 @@ def _schema_location(self, schema_name: SchemaName) -> t.Optional[str]:
             match_key = schema.db
 
             # only consider the catalog if it is present
-            if catalog := schema.catalog:
-                match_key = f"{catalog}.{match_key}"
+            if schema.catalog:
+                match_key = f"{schema.catalog}.{match_key}"
 
             for k, v in mapping.items():
                 if re.match(k, match_key):

sqlmesh/core/environment.py

@@ -233,7 +233,7 @@ def execute_environment_statements(
     end: t.Optional[TimeLike] = None,
     execution_time: t.Optional[TimeLike] = None,
 ) -> None:
-    if rendered_expressions := [
+    rendered_expressions = [
         expr
         for statements in environment_statements
         for expr in render_statements(
@@ -250,7 +250,8 @@ def execute_environment_statements(
             runtime_stage=runtime_stage,
             engine_adapter=adapter,
         )
-    ]:
+    ]
+    if rendered_expressions:
         with adapter.transaction():
             for expr in rendered_expressions:
                 adapter.execute(expr)

sqlmesh/core/model/definition.py

@@ -2406,7 +2406,8 @@ def _create_model(
     # since rendering shifted from load time to run time.
     # Note: we check for Tuple since that's what we expect from _resolve_properties
     for property_name in PROPERTIES:
-        if isinstance(property_values := kwargs.get(property_name), exp.Tuple):
+        property_values = kwargs.get(property_name)
+        if isinstance(property_values, exp.Tuple):
             statements.extend(property_values.expressions)
 
     jinja_macro_references, used_variables = extract_macro_references_and_variables(
@@ -2682,30 +2683,41 @@ def render_field_value(value: t.Any) -> t.Any:
 
     for field_name, field_info in ModelMeta.all_field_infos().items():
         field = field_info.alias or field_name
-        if field not in RUNTIME_RENDERED_MODEL_FIELDS and (field_value := fields.get(field)):
-            if isinstance(field_value, dict):
-                rendered_dict = {}
-                for key, value in field_value.items():
-                    if key in RUNTIME_RENDERED_MODEL_FIELDS:
-                        rendered_dict[key] = value
-                    elif rendered := render_field_value(value):
-                        rendered_dict[key] = rendered
-                if rendered_dict:
-                    fields[field] = rendered_dict
-                else:
-                    fields.pop(field)
-            elif isinstance(field_value, list):
-                if rendered_list := [
-                    rendered for value in field_value if (rendered := render_field_value(value))
-                ]:
-                    fields[field] = rendered_list
-                else:
-                    fields.pop(field)
+
+        if field in RUNTIME_RENDERED_MODEL_FIELDS:
+            continue
+
+        field_value = fields.get(field)
+        if field_value is None:
+            continue
+
+        if isinstance(field_value, dict):
+            rendered_dict = {}
+            for key, value in field_value.items():
+                if key in RUNTIME_RENDERED_MODEL_FIELDS:
+                    rendered_dict[key] = value
+                elif (rendered := render_field_value(value)) is not None:
+                    rendered_dict[key] = rendered
+            if rendered_dict:
+                fields[field] = rendered_dict
             else:
-                if rendered_field := render_field_value(field_value):
-                    fields[field] = rendered_field
-                else:
-                    fields.pop(field)
+                fields.pop(field)
+        elif isinstance(field_value, list):
+            rendered_list = [
+                rendered
+                for value in field_value
+                if (rendered := render_field_value(value)) is not None
+            ]
+            if rendered_list:
+                fields[field] = rendered_list
+            else:
+                fields.pop(field)
+        else:
+            rendered_field = render_field_value(field_value)
+            if rendered_field is not None:
+                fields[field] = rendered_field
+            else:
+                fields.pop(field)
 
     return fields
 
@@ -2733,12 +2745,13 @@ def render_model_defaults(
 
     # Validate defaults that have macros are rendered to boolean
     for boolean in {"optimize_query", "allow_partials", "enabled"}:
-        if var := rendered_defaults.get(boolean):
-            if not isinstance(var, (exp.Boolean, bool)):
-                raise ConfigError(f"Expected boolean for '{var}', got '{type(var)}' instead")
+        var = rendered_defaults.get(boolean)
+        if var is not None and not isinstance(var, (exp.Boolean, bool)):
+            raise ConfigError(f"Expected boolean for '{var}', got '{type(var)}' instead")
 
     # Validate the 'interval_unit' if present is an Interval Unit
-    if (var := rendered_defaults.get("interval_unit")) and isinstance(var, str):
+    var = rendered_defaults.get("interval_unit")
+    if isinstance(var, str):
         try:
             rendered_defaults["interval_unit"] = IntervalUnit(var)
         except ValueError as e:
@@ -2751,10 +2764,10 @@ def parse_defaults_properties(
     defaults: t.Dict[str, t.Any], dialect: DialectType
 ) -> t.Dict[str, t.Any]:
     for prop in PROPERTIES:
-        if default_properties := defaults.get(prop):
-            for key, value in default_properties.items():
-                if isinstance(key, str) and d.SQLMESH_MACRO_PREFIX in str(value):
-                    defaults[prop][key] = exp.maybe_parse(value, dialect=dialect)
+        default_properties = defaults.get(prop)
+        for key, value in (default_properties or {}).items():
+            if isinstance(key, str) and d.SQLMESH_MACRO_PREFIX in str(value):
+                defaults[prop][key] = exp.maybe_parse(value, dialect=dialect)
 
     return defaults
 

sqlmesh/core/model/kind.py

@@ -996,9 +996,10 @@ def create_model_kind(v: t.Any, dialect: str, defaults: t.Dict[str, t.Any]) -> M
             # we dont want to throw an error here because we still want Models with a CustomKind to be able
             # to be serialized / deserialized in contexts where the custom materialization class may not be available,
             # such as in HTTP request handlers
-            if custom_materialization := get_custom_materialization_type(
+            custom_materialization = get_custom_materialization_type(
                 validate_string(props.get("materialization")), raise_errors=False
-            ):
+            )
+            if custom_materialization is not None:
                 actual_kind_type, _ = custom_materialization
                 return actual_kind_type(**props)
 

sqlmesh/core/plan/evaluator.py

@@ -500,15 +500,16 @@ def _restatement_intervals_across_all_environments(
         # for any affected full_history_restatement_only snapshots, we need to widen the intervals being restated to
         # include the whole time range for that snapshot. This requires a call to state to load the full snapshot record,
         # so we only do it if necessary
-        if full_history_restatement_snapshot_ids := [
+        full_history_restatement_snapshot_ids = [
             # FIXME: full_history_restatement_only is just one indicator that the snapshot can only be fully refreshed, the other one is Model.depends_on_self
             # however, to figure out depends_on_self, we have to render all the model queries which, alongside having to fetch full snapshots from state,
             # is problematic in secure environments that are deliberately isolated from arbitrary user code (since rendering a query may require user macros to be present)
             # So for now, these are not considered
             s_id
             for s_id, s in snapshots_to_restate.items()
             if s[0].full_history_restatement_only
-        ]:
+        ]
+        if full_history_restatement_snapshot_ids:
             # only load full snapshot records that we havent already loaded
             additional_snapshots = self.state_sync.get_snapshots(
                 [