-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat!: Add support for virtual statements to be executed post update #3524
base: main
Are you sure you want to change the base?
Conversation
@@ -6266,3 +6266,174 @@ def assert_metadata_only(): | |||
model = load_sql_based_model(expressions, signal_definitions=signal.get_registry()) | |||
model.signals.clear() | |||
assert_metadata_only() | |||
|
|||
|
|||
def test_model_on_virtual_update(make_snapshot: t.Callable): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add an end-to-end test for this?
f57f88e
to
f012e96
Compare
sqlmesh/core/snapshot/evaluator.py
Outdated
snapshot_deps = {snapshots[p_sid].name: snapshots[p_sid] for p_sid in snapshot.parents} | ||
snapshot_deps[snapshot.name] = snapshot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we wanna make a helper for this, there are a few places where we do the same thing in the codebase:
sqlmesh/sqlmesh/core/snapshot/evaluator.py
Lines 722 to 725 in 2adf5a1
parent_snapshots_by_name = { snapshots[p_sid].name: snapshots[p_sid] for p_sid in snapshot.parents } parent_snapshots_by_name[snapshot.name] = snapshot sqlmesh/sqlmesh/core/snapshot/evaluator.py
Lines 821 to 824 in 2adf5a1
parent_snapshots_by_name = { snapshots[p_sid].name: snapshots[p_sid] for p_sid in snapshot.parents } parent_snapshots_by_name[snapshot.name] = snapshot sqlmesh/sqlmesh/core/scheduler.py
Lines 164 to 167 in 2adf5a1
snapshots = { self.snapshots[p_sid].name: self.snapshots[p_sid] for p_sid in snapshot.parents } snapshots[snapshot.name] = snapshot
cc @izeigerman
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, good catch lol. Yeah, we can add one in the snapshot/definition.py
Can we please make sure this feature is documented before we merge. |
10006db
to
14d1f49
Compare
This update introduces statements that are executed after a virtual update. These can be used for example to grant privileges on views of the virtual layer.
These expressions should be defined within a
ON_VIRTUAL_UPDATE_BEGIN; ...; ON_VIRTUAL_UPDATE_END;
block. For example:Jinja expressions are also supported within this block by properly nesting the Jinja block as shown in the example above.
In these statements table resolution occurs at the virtual level, meaning that qualified view names (including
@this_model
) are used instead of the physical table names. For example, when running the plan in an environment nameddev
, thedb.customers
as well asthis_model
would resolve todb__dev.customers
rather than the physical table.For python models these statements are defined in the
@model
decorator: