-
Notifications
You must be signed in to change notification settings - Fork 158
Implement function materialization #588
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
base: main
Are you sure you want to change the base?
Implement function materialization #588
Conversation
|
Related to #590 |
|
Hi @shachibista! Thanks for the contribution. This is a really interesting functionality and I just started checking the implementation details, so I still don't have a full picture of the work needed. I'm sorry I can't give complete feedback yet, but I wanted to ask a few questions to get your opinion:
|
Ah yes, I missed that.
You are correct, I tried to use the default implementation but did not realise that dbt-adapters needed to be updated too. Should be unnecessary after update.
Unfortunately we cannot use them because ClickHouse functions expect an expression: However the tests defined in the adapters repo expect the function body to be a
What errors are you getting? I did not try to run it from a model but I currently get the following when trying to run from a model. I am not sure what the reason is. |
| {% endmacro %} | ||
|
|
||
| {% macro clickhouse__scalar_function_create_replace_signature_sql(target_relation) %} | ||
| CREATE OR REPLACE FUNCTION {{ target_relation.include(database=false, schema=false) }} {{ on_cluster_clause(this) }} AS ({{ clickhouse__formatted_scalar_function_args_sql() }}) -> |
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.
Inconsistent relation used for cluster clause
Medium Severity
The clickhouse__scalar_function_create_replace_signature_sql macro receives target_relation as a parameter and uses it for the function name, but then uses the global this variable for on_cluster_clause(this). This is inconsistent with the established pattern throughout the codebase where the same relation is used for both the identifier and the cluster clause. If target_relation and this ever differ, the ON CLUSTER clause may be generated incorrectly or omitted unexpectedly.
Summary
Adds support for User Defined Functions.
Checklist
Delete items not relevant to your PR:
Note
Adds first-class support for dbt User Defined Functions and aligns the adapter with dbt-core 1.11.
Functionrelation type indbt/adapters/clickhouse/relation.pyinclude/clickhouse/macros/materializations/functions/scalar.sql(create/replace signature and body)tests/integration/adapter/udf/test_udf.py1.11.0and dependency updates:dbt-core>=1.11.0,dbt-adapters>=1.17.0;setup.pydbt_minor_version="1.11"Written by Cursor Bugbot for commit 2a75b23. This will update automatically on new commits. Configure here.