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

merge main changes for rc2 #407

Merged
merged 13 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changes/unreleased/Features-20230112-162620.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
kind: Features
body: add back optional old uses of temp tables for incremental models in snowflake
for perf increase in some cases
time: 2023-01-12T16:26:20.813848-06:00
custom:
Author: McKnight-42 dbeatty10
Issue: "323"
PR: "389"
7 changes: 7 additions & 0 deletions .changes/unreleased/Under the Hood-20230119-103909.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Under the Hood
body: 'Enable a `tmp_relation_type` config for applicable incremental models'
time: 2023-01-19T10:39:09.774872-06:00
custom:
Author: McKnight-42 dbeatty10
Issue: "402"
PR: "403"
59 changes: 59 additions & 0 deletions .github/workflows/main-branch-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# **what?**
# The purpose of this workflow is to trigger CI to run for main on a regular
# cadence. If the CI workflow fails for a branch, it will post to
# dev-core-alerts to raise awareness. The 'aurelien-baudet/workflow-dispatch'

# **why?**
# Ensures main is always shippable and not broken. Also, can catch any
# dependencies shifting beneath us that might introduce breaking changes
# (could also impact Cloud).

# **when?**
# Mainly on a schedule of 9:00, 13:00, 18:00 UTC everyday.
# Manual trigger can also test on demand

name: Main branch scheduled testing

on:
schedule:
- cron: '0 9,13,18 * * *' # 9:00, 13:00, 18:00 UTC

workflow_dispatch: # for manual triggering

# no special access is needed
permissions: read-all

jobs:
kick-off-ci:
name: Kick-off CI
runs-on: ubuntu-latest

strategy:
# must run CI 1 branch at a time b/c the workflow-dispatch Action polls for
# latest run for results and it gets confused when we kick off multiple runs
# at once. There is a race condition so we will just run in sequential order.
max-parallel: 1
fail-fast: false
matrix:
branch: [main]
workflow_name: [main.yml, integration.yml]

steps:
- name: Call CI workflow for ${{ matrix.branch }} branch
id: trigger-step
uses: aurelien-baudet/workflow-dispatch@v2.1.1
with:
workflow: ${{ matrix.workflow_name }}
ref: ${{ matrix.branch }}
token: ${{ secrets.FISHTOWN_BOT_PAT }}

- name: Post failure to Slack
uses: ravsamhq/notify-slack-action@v1
if: ${{ always() && !contains(steps.trigger-step.outputs.workflow-conclusion,'success') }}
with:
status: ${{ job.status }}
notification_title: 'dbt-snowflake scheduled run of ${{ matrix.workflow_name }} on "${{ matrix.branch }}" branch not successful'
message_format: ':x: ${{ matrix.workflow_name }} CI on branch "${{ matrix.branch }}" ${{ steps.trigger-step.outputs.workflow-conclusion }}'
footer: 'Linked failed CI run ${{ steps.trigger-step.outputs.workflow-url }}'
env:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_DEV_CORE_ALERTS }}
2 changes: 1 addition & 1 deletion .github/workflows/release-branch-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
max-parallel: 1
fail-fast: false
matrix:
branch: [1.0.latest, 1.1.latest, 1.2.latest, 1.3.latest, main]
branch: [1.0.latest, 1.1.latest, 1.2.latest, 1.3.latest, 1.4.latest]
workflow_name: [main.yml, integration.yml]

steps:
Expand Down
1 change: 1 addition & 0 deletions dbt/adapters/snowflake/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class SnowflakeConfig(AdapterConfig):
copy_grants: Optional[bool] = None
snowflake_warehouse: Optional[str] = None
query_tag: Optional[str] = None
tmp_relation_type: Optional[str] = None
merge_update_columns: Optional[str] = None


Expand Down
59 changes: 49 additions & 10 deletions dbt/include/snowflake/macros/materializations/incremental.sql
Original file line number Diff line number Diff line change
@@ -1,19 +1,58 @@
{% macro dbt_snowflake_get_tmp_relation_type(strategy, unique_key, language) %}

{%- set tmp_relation_type = config.get('tmp_relation_type') -%}
/* {#
High-level principles:
If we are running multiple statements (DELETE + INSERT),
we must first save the model query results as a temporary table
in order to guarantee consistent inputs to both statements.

and we want to guarantee identical inputs to both statements,
then we must first save the model query results as a temporary table
(which presumably comes with a performance cost).
If we are running a single statement (MERGE or INSERT alone),
we can save the model query definition as a view instead,
for faster overall incremental processing.
we _may_ save the model query definition as a view instead,
for (presumably) faster overall incremental processing.

Low-level specifics:
If an invalid option is specified, then we will raise an
excpetion with corresponding message.

Languages other than SQL (like Python) will use a temporary table.
With the default strategy of merge, the user may choose between a temporary
table and view (defaulting to view).

The append strategy can use a view because it will run a single INSERT statement.

When unique_key is none, the delete+insert strategy can use a view beacuse a
single INSERT statement is run with no DELETES as part of the statement.
Otherwise, play it safe by using a temporary table.
#} */

{% if language == 'sql' and (strategy in ('default', 'append', 'merge') or (unique_key is none)) %}
{{ return('view') }}
{% else %} {#-- play it safe -- #}
{{ return('table') }}
{% if language == "python" and tmp_relation_type is not none %}
{% do exceptions.raise_compiler_error(
"Python models currently only support 'table' for tmp_relation_type but "
~ tmp_relation_type ~ " was specified."
) %}
{% endif %}

{% if strategy == "delete+insert" and tmp_relation_type is not none and tmp_relation_type != "table" and unique_key is not none %}
{% do exceptions.raise_compiler_error(
"In order to maintain consistent results when `unique_key` is not none,
the `delete+insert` strategy only supports `table` for `tmp_relation_type` but "
~ tmp_relation_type ~ " was specified."
)
%}
{% endif %}

{% if language != "sql" %}
{{ return("table") }}
{% elif tmp_relation_type == "table" %}
{{ return("table") }}
{% elif tmp_relation_type == "view" %}
{{ return("view") }}
{% elif strategy in ("default", "merge", "append") %}
{{ return("view") }}
{% elif strategy == "delete+insert" and unique_key is none %}
{{ return("view") }}
{% else %}
{{ return("table") }}
{% endif %}
{% endmacro %}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from dbt.tests.adapter.incremental.test_incremental_on_schema_change import BaseIncrementalOnSchemaChange

class TestIncrementalOnSchemaChange(BaseIncrementalOnSchemaChange):
pass

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading