Skip to content
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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

themisvaltinos
Copy link
Contributor

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:

MODEL (
  name db.customers 
);

SELECT 
  item_id
FROM 
  db.seed_model;

ON_VIRTUAL_UPDATE_BEGIN; 

GRANT SELECT ON VIEW @this_model TO ROLE role_name;

INJA_STATEMENT_BEGIN;     
GRANT REFERENCES, SELECT ON FUTURE VIEWS IN DATABASE {{ db }} TO ROLE owner_name;  
JINJA_END;  

ON_VIRTUAL_UPDATE_END;

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 named dev, the db.customers as well as this_model would resolve to db__dev.customers rather than the physical table.

For python models these statements are defined in the @model decorator:

@model( 
  "db.customers",  
  on_virtual_update=["GRANT SELECT ON VIEW @this_model TO ROLE role_name;"]
)

sqlmesh/core/dialect.py Outdated Show resolved Hide resolved
sqlmesh/core/dialect.py Outdated Show resolved Hide resolved
sqlmesh/core/model/definition.py Outdated Show resolved Hide resolved
sqlmesh/core/plan/evaluator.py Outdated Show resolved Hide resolved
sqlmesh/core/snapshot/evaluator.py Outdated Show resolved Hide resolved
tests/core/test_model.py Outdated Show resolved Hide resolved
tests/core/test_model.py Outdated Show resolved Hide resolved
tests/core/test_model.py Show resolved Hide resolved
@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from f57f88e to f012e96 Compare December 18, 2024 18:46
sqlmesh/core/dialect.py Outdated Show resolved Hide resolved
sqlmesh/core/dialect.py Outdated Show resolved Hide resolved
sqlmesh/core/dialect.py Outdated Show resolved Hide resolved
sqlmesh/core/model/definition.py Outdated Show resolved Hide resolved
sqlmesh/core/snapshot/evaluator.py Outdated Show resolved Hide resolved
@izeigerman
Copy link
Member

Can we please make sure this feature is documented before we merge.

@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from 10006db to 14d1f49 Compare December 23, 2024 14:13
@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from 14d1f49 to 46bb623 Compare January 2, 2025 19:52
@themisvaltinos themisvaltinos requested a review from a team January 3, 2025 08:22
@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from 46bb623 to d225eeb Compare January 7, 2025 08:14
@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from d225eeb to a2b8955 Compare January 7, 2025 21:47
@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from 7d1e30d to 6343569 Compare January 8, 2025 16:41
view_name,
exp.select("*").from_(table_name, dialect=self.adapter.dialect),
table_description=model.description if is_prod else None,
column_descriptions=model.column_descriptions if is_prod else None,
view_properties=model.virtual_properties,
)
if on_virtual_update := model.on_virtual_update:
adapter.execute(
model._render_statements(on_virtual_update, engine_adapter=adapter, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not call private methods here. What you had previously (render_on_virtual_update) was great.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I don't think kwargs contains anything meaningful here. This is mostly for future expansions, since this interface is public. No need to pass in into renderer I don't think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right changing it to use a public method instead similar to pre/post statements

Regarding the kwargs I am passing the table mapping of qualified view names, which is then propagated down to render and used here:

table_mapping=table_mapping,

so it is needed to resolve model names to their corresponding view names of the current environment, so I'm not sure without passing them how I could do this

@themisvaltinos themisvaltinos force-pushed the themis/vlayer_post_statements branch from 3e4492c to a4d4de6 Compare January 9, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants