-
Notifications
You must be signed in to change notification settings - Fork 164
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?
Changes from all commits
0ff4536
5963bea
6632fe1
077e3fa
a84c68c
271df27
37472f6
f71adf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| version = '1.10.0' | ||
| version = "1.11.0" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| {% macro clickhouse__formatted_scalar_function_args_sql() %} | ||
| {% set args = [] %} | ||
| {% for arg in model.arguments -%} | ||
| {%- do args.append(arg.name) -%} | ||
| {%- endfor %} | ||
| {{ args | join(', ') }} | ||
| {% 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 commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent relation used for cluster clauseMedium Severity The |
||
| {% endmacro %} | ||
|
|
||
| {% macro clickhouse__scalar_function_body_sql() %} | ||
| {{ model.compiled_code }} | ||
| {% endmacro %} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| dbt-core>=1.10.0,<1.11 | ||
| dbt-core>=1.11.0 | ||
| dbt-tests-adapter>=1.10,<2.0 | ||
| pytest>=7.2.0 | ||
| pytest-dotenv==0.5.2 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| """ | ||
| test UDF creation support for dbt-clickhouse | ||
| """ | ||
|
|
||
| import pytest | ||
|
|
||
| import dbt.tests.adapter.functions.files as files | ||
| from dbt.tests.adapter.functions.test_udfs import UDFsBasic | ||
|
|
||
| # we'll import helper used by the core test project to write directories | ||
|
|
||
| MY_UDF_SQL = """ | ||
| price * 2 | ||
| """.strip() | ||
|
|
||
|
|
||
| class TestUDFBasics(UDFsBasic): | ||
| @pytest.fixture(scope="class") | ||
| def functions(self): | ||
| return { | ||
| "price_for_xlarge.sql": MY_UDF_SQL, | ||
| "price_for_xlarge.yml": files.MY_UDF_YML, | ||
| } |
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.
Functions never get ON CLUSTER clause in clusters
Medium Severity
The
elifbranch forNODE_TYPE_FUNCTIONincreate_fromsetsschema = Nonebut falls through without readingclusteranddatabase_enginefromquoting.credentials, since that logic is in theelseblock. This leavesclusteras""andcan_on_clusterasNone, soon_cluster_clause(this)inscalar.sqlwill never emit theON CLUSTERclause — even when a cluster is configured. Thecluster/database_enginecredential reads need to happen for function types too.Additional Locations (1)
dbt/include/clickhouse/macros/materializations/functions/scalar.sql#L9-L10