Feat: Adds support for macros in model_defaults and conditional properties by themisvaltinos · Pull Request #3933 · SQLMesh/sqlmesh

themisvaltinos · 2025-03-04T12:32:48Z

This update adds:

Macro support in model_defaults attributes.
The ability to conditionally enable these, as well as for physical_properties, session_properties, virtual_properties.
For session_properties, virtual_properties rendering is now moved to runtime to align with physical_properties.
Introduces the model_kind_name macro variable, which holds the name of the current model kind.

This macro is intended for example to disable from physical_properties the creatable_type for your project's VIEW-type models and set it to TRANSIENT for the rest of the model kinds, by having in model_defaults:

model_defaults:
  dialect: snowflake
  start: 2022-01-01
  physical_properties:
    creatable_type: "@IF(@model_kind_name != 'VIEW', 'TRANSIENT', NULL)"

sqlmesh/core/model/definition.py

tests/core/test_model.py

themisvaltinos · 2025-03-06T12:15:42Z

Thanks @izeigerman addressed comments and generalised logic for all model defaults and rendering of the three kind of properties during runtime for consistency, if you want to have another look

izeigerman · 2025-03-07T06:48:05Z

sqlmesh/core/model/definition.py

+    def render_physical_properties(self, **render_kwargs: t.Any) -> t.Dict[str, t.Any]:
+        return self.render_properties(properties=self.physical_properties, **render_kwargs)
+
+    def render_virtual_properties(self, **render_kwargs: t.Any) -> t.Dict[str, t.Any]:


@erindru didn't you argue that virtual properties should be rendered at load time?

I think we were discussing that for virtual properties there wasn't a use case for it yet like a specific use case for physical_properties so they don't have to be rendered at runtime. Unless you think @erindru that indeed virtual properties should be rendered at load time?

I dont have a strong opinion. I needed physical_properties to be rendered at creation/evaluation time because I needed access to @this_model for a specific use case, but I didn't touch virtual_properties at the time because there was no clear reason to.

Right I don't have a strong opinion either, this allows the use of this_model or model_kind_name in virtual properties as well, but at the same time I don't know how this could be helpful or what property it will make sense to unset. So happy to move it back to load time

izeigerman · 2025-03-07T06:49:51Z

sqlmesh/core/config/model.py

-    allow_partials: t.Optional[bool] = None
-    interval_unit: t.Optional[IntervalUnit] = None
-    enabled: t.Optional[bool] = None
+    optimize_query: t.Optional[t.Any] = None


Why did we erase the types entirely? Shouldn't they be t.Union[str, bool] etc?

When do we validate that the value passed here is correct?

Yes you're right will need to adjust the type and add validation somewhere. I suppose after the rendering since it can't occur before resolving the macros

Revised it and added validation so that after rendering the values to raise if the values are not of the correct type

sqlmesh/core/model/definition.py

themisvaltinos · 2025-03-07T20:10:33Z

Addressed comments @izeigerman regarding virtual_properties and session_properties let me know if you'd prefer to move them back to loading time

izeigerman · 2025-03-11T19:37:32Z

sqlmesh/core/model/definition.py


-    def render_physical_properties(self, **render_kwargs: t.Any) -> t.Dict[str, exp.Expression]:
-        def _render(expression: exp.Expression) -> exp.Expression:
+    def render_properties(


Shouldn't this be private?

izeigerman · 2025-03-11T19:40:20Z

sqlmesh/core/model/definition.py

-                raise SQLMeshError(
-                    f"Failed to render model attribute `{fields['name']}` at `{path}`\n"
-                    f"'{expression.sql(dialect=dialect)}' must return an expression"
+            # Warn instead of raising for cases where an attribute is conditionally assigned


I don't think we support this behavior for all meta fields, are we not? Eg. if the macro returns NULL for something like allow_partials we do want to fail, don't we? Should we then distinguish between macro not returning anything and macro returning NULL? The former could be a bug, the latter seems intentional.

Looks like this validation happens later in render_model_defaults. Please correct me if I'm wrong

Right the validation happens in render_model_defaults and it will be a bug if the rendering didn't return anything at all here, so revised to error out in that case and not for the intentional set to null by the user

izeigerman · 2025-03-11T19:44:04Z

sqlmesh/core/snapshot/evaluator.py

            table_description=model.description if is_prod else None,
            column_descriptions=model.column_descriptions if is_prod else None,
-            view_properties=model.virtual_properties,
+            view_properties=model.render_virtual_properties(**kwargs),


Let's not use kwargs. Let's make an explicit dict render_kwargs instead.

Sure revised this

izeigerman

Please address the remaining comments. LGTM otherwise

…l_kind_name macro

…on time

…rties (#3933)

themisvaltinos requested a review from a team March 4, 2025 12:42

izeigerman reviewed Mar 4, 2025

View reviewed changes

sqlmesh/core/model/definition.py Outdated Show resolved Hide resolved

izeigerman reviewed Mar 4, 2025

View reviewed changes

sqlmesh/core/model/definition.py Show resolved Hide resolved

izeigerman reviewed Mar 4, 2025

View reviewed changes

tests/core/test_model.py Show resolved Hide resolved

themisvaltinos force-pushed the themis/props_parametrisation branch 2 times, most recently from 84049c1 to b31eeaf Compare March 6, 2025 07:38

themisvaltinos changed the title ~~Feat: Add support for conditional physical_properties, add model_kind_name macro~~ Feat: Adds support for macros in model_defaults and conditional properties Mar 6, 2025

themisvaltinos requested a review from izeigerman March 6, 2025 12:15

izeigerman reviewed Mar 7, 2025

View reviewed changes

sqlmesh/core/model/definition.py Show resolved Hide resolved

themisvaltinos force-pushed the themis/props_parametrisation branch from 4ab8efc to 9402ec6 Compare March 7, 2025 17:27

themisvaltinos force-pushed the themis/props_parametrisation branch from 338c20a to db7c331 Compare March 8, 2025 17:08

themisvaltinos requested a review from izeigerman March 10, 2025 21:52

izeigerman reviewed Mar 11, 2025

View reviewed changes

izeigerman approved these changes Mar 11, 2025

View reviewed changes

themisvaltinos added 10 commits March 11, 2025 21:50

Feat: Add support for conditional physical_properties, introduce mode…

12c5d24

…l_kind_name macro

Remove unnecessary assignment; revise test

1a41614

Allow macros in model defaults; move properties rendering to evaluati…

b6aa6fc

…on time

Add python test; handle variables in pythons attributes

4c000e2

Provide more info on the comment

aaa4c42

Correct variable name

a06f30f

Revise obsolete handling

2d0db32

Refactor, resolve rebase

91bc55e

Add validations and types for model defaults

77bdf1e

Address comments

6e15835

themisvaltinos force-pushed the themis/props_parametrisation branch from db7c331 to 6e15835 Compare March 12, 2025 04:51

themisvaltinos merged commit db507bc into main Mar 12, 2025
22 checks passed

themisvaltinos deleted the themis/props_parametrisation branch March 12, 2025 05:37

afzaljasani pushed a commit that referenced this pull request Apr 8, 2025

Feat: Adds support for macros in model_defaults and conditional prope…

a563504

…rties (#3933)

Conversation

themisvaltinos commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

themisvaltinos commented Mar 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

themisvaltinos Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

themisvaltinos commented Mar 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

izeigerman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

themisvaltinos commented Mar 4, 2025 •

edited

Loading

themisvaltinos Mar 7, 2025 •

edited

Loading