From 40d6db1e6bbfd20aeaf0edb6b17d1a2205bacb1f Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 13 Sep 2023 14:02:09 +0100 Subject: [PATCH 01/10] first pass: add limit in show sql --- .../global_project/macros/adapters/show.sql | 21 +++++++++++++++++++ core/dbt/task/show.py | 16 +++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 core/dbt/include/global_project/macros/adapters/show.sql diff --git a/core/dbt/include/global_project/macros/adapters/show.sql b/core/dbt/include/global_project/macros/adapters/show.sql new file mode 100644 index 00000000000..2ff97dd68fe --- /dev/null +++ b/core/dbt/include/global_project/macros/adapters/show.sql @@ -0,0 +1,21 @@ +{% macro show(limit) -%} + {%- set sql_header = config.get('sql_header', none) -%} + {{ sql_header if sql_header is not none }} + {%- if limit is not none -%} + {{ get_limit_subquery_sql(compiled_code, limit) }} + {%- else -%} + {{ compiled_code }} + {%- endif -%} +{% endmacro %} + +{% macro get_limit_subquery_sql(sql, limit) %} + {{ adapter.dispatch('get_limit_subquery_sql', 'dbt')(sql, limit) }} +{% endmacro %} + +{% macro default__get_limit_subquery_sql(sql, limit) %} + select * + from ( + {{ sql }} + ) as model_limit_subq + limit {{ limit }} +{% endmacro %} diff --git a/core/dbt/task/show.py b/core/dbt/task/show.py index f9af847e874..eb52a392757 100644 --- a/core/dbt/task/show.py +++ b/core/dbt/task/show.py @@ -2,6 +2,7 @@ import threading import time +from dbt.context.providers import generate_runtime_model_context from dbt.contracts.graph.nodes import SeedNode from dbt.contracts.results import RunResult, RunStatus from dbt.events.base_types import EventLevel @@ -23,14 +24,15 @@ def execute(self, compiled_node, manifest): # Allow passing in -1 (or any negative number) to get all rows limit = None if self.config.args.limit < 0 else self.config.args.limit - if "sql_header" in compiled_node.unrendered_config: - compiled_node.compiled_code = ( - compiled_node.unrendered_config["sql_header"] + compiled_node.compiled_code - ) - - adapter_response, execute_result = self.adapter.execute( - compiled_node.compiled_code, fetch=True, limit=limit + model_context = generate_runtime_model_context(compiled_node, self.config, manifest) + show_limit_sql = self.adapter.execute_macro( + macro_name="show", + manifest=manifest, + context_override=model_context, + kwargs={"limit": limit}, ) + adapter_response, execute_result = self.adapter.execute(show_limit_sql, fetch=True) + end_time = time.time() return RunResult( From d77bfe2eea66791e8e667607a03792317372e157 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 13 Sep 2023 15:15:39 +0100 Subject: [PATCH 02/10] test sql_header w jinja --- tests/functional/show/fixtures.py | 2 +- tests/functional/show/test_show.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/functional/show/fixtures.py b/tests/functional/show/fixtures.py index 89b698abea8..1fc9b9fd797 100644 --- a/tests/functional/show/fixtures.py +++ b/tests/functional/show/fixtures.py @@ -37,7 +37,7 @@ models__sql_header = """ {% call set_sql_header(config) %} -set session time zone 'Asia/Kolkata'; +set session time zone '{{ var("timezone", "Europe/Paris") }}'; {%- endcall %} select current_setting('timezone') as timezone """ diff --git a/tests/functional/show/test_show.py b/tests/functional/show/test_show.py index 42bddb2ddee..1dc59c27cb9 100644 --- a/tests/functional/show/test_show.py +++ b/tests/functional/show/test_show.py @@ -167,9 +167,10 @@ def test_seed(self, project): class TestShowSqlHeader(ShowBase): def test_sql_header(self, project): - run_dbt(["build"]) - (_, log_output) = run_dbt_and_capture(["show", "--select", "sql_header"]) - assert "Asia/Kolkata" in log_output + run_dbt(["build", "--vars", "timezone: Asia/Kolkata"]) + (_, log_output) = run_dbt_and_capture( + ["show", "--select", "sql_header", "--vars", "timezone: Asia/Kolkata"] + ) class TestShowModelVersions: From 0cafc074de824360e84454dd1e3e2bc0eb02acd3 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 13 Sep 2023 15:34:21 +0100 Subject: [PATCH 03/10] test limit included in show sql --- core/dbt/task/show.py | 4 ++-- tests/functional/show/test_show.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/core/dbt/task/show.py b/core/dbt/task/show.py index eb52a392757..94677bd85db 100644 --- a/core/dbt/task/show.py +++ b/core/dbt/task/show.py @@ -25,13 +25,13 @@ def execute(self, compiled_node, manifest): limit = None if self.config.args.limit < 0 else self.config.args.limit model_context = generate_runtime_model_context(compiled_node, self.config, manifest) - show_limit_sql = self.adapter.execute_macro( + compiled_node.compiled_code = self.adapter.execute_macro( macro_name="show", manifest=manifest, context_override=model_context, kwargs={"limit": limit}, ) - adapter_response, execute_result = self.adapter.execute(show_limit_sql, fetch=True) + adapter_response, execute_result = self.adapter.execute(compiled_node.compiled_code, fetch=True) end_time = time.time() diff --git a/tests/functional/show/test_show.py b/tests/functional/show/test_show.py index 1dc59c27cb9..7a0c4a3f99a 100644 --- a/tests/functional/show/test_show.py +++ b/tests/functional/show/test_show.py @@ -157,6 +157,9 @@ def test_limit(self, project, args, expected): dbt_args = ["show", "--inline", models__second_ephemeral_model, *args] results = run_dbt(dbt_args) assert len(results.results[0].agate_table) == expected + # ensure limit was injected in compiled_code when limit specified in command args + if results.args.get('limit') > 0: + assert 'limit' in results.results[0].node.compiled_code class TestShowSeed(ShowBase): From 29445b4d4100a3c9026db7caf01ea2d5fcfe7c5e Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 13 Sep 2023 15:39:31 +0100 Subject: [PATCH 04/10] changelog entry --- .changes/unreleased/Fixes-20230913-153924.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230913-153924.yaml diff --git a/.changes/unreleased/Fixes-20230913-153924.yaml b/.changes/unreleased/Fixes-20230913-153924.yaml new file mode 100644 index 00000000000..c61452174de --- /dev/null +++ b/.changes/unreleased/Fixes-20230913-153924.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: 'update dbt show to include limit in DWH query ' +time: 2023-09-13T15:39:24.591805+01:00 +custom: + Author: michelleark + Issue: 8496, 8417 From a8d01ee67a2fa4431d6bcf5fdd5d5f1178e3f249 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 13 Sep 2023 15:45:20 +0100 Subject: [PATCH 05/10] linting --- tests/functional/show/test_show.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/show/test_show.py b/tests/functional/show/test_show.py index 7a0c4a3f99a..85532d81cf8 100644 --- a/tests/functional/show/test_show.py +++ b/tests/functional/show/test_show.py @@ -158,8 +158,8 @@ def test_limit(self, project, args, expected): results = run_dbt(dbt_args) assert len(results.results[0].agate_table) == expected # ensure limit was injected in compiled_code when limit specified in command args - if results.args.get('limit') > 0: - assert 'limit' in results.results[0].node.compiled_code + if results.args.get("limit") > 0: + assert "limit" in results.results[0].node.compiled_code class TestShowSeed(ShowBase): From a2ace01504652b7f38f2d4bc59c0d6ec0268eba5 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 18 Sep 2023 13:16:17 +0100 Subject: [PATCH 06/10] add adapter-zone tests for dbt show --- core/dbt/task/show.py | 4 +- .../tests/adapter/dbt_show/test_dbt_show.py | 44 +++++++++++++++++++ tests/functional/show/test_show.py | 27 ------------ 3 files changed, 47 insertions(+), 28 deletions(-) create mode 100644 tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py diff --git a/core/dbt/task/show.py b/core/dbt/task/show.py index 94677bd85db..439246340b8 100644 --- a/core/dbt/task/show.py +++ b/core/dbt/task/show.py @@ -31,7 +31,9 @@ def execute(self, compiled_node, manifest): context_override=model_context, kwargs={"limit": limit}, ) - adapter_response, execute_result = self.adapter.execute(compiled_node.compiled_code, fetch=True) + adapter_response, execute_result = self.adapter.execute( + compiled_node.compiled_code, fetch=True + ) end_time = time.time() diff --git a/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py new file mode 100644 index 00000000000..14d96b90320 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py @@ -0,0 +1,44 @@ +import pytest +from dbt.tests.util import run_dbt_and_capture, run_dbt + +from tests.functional.show.test_show import ShowBase +from tests.functional.show.fixtures import ( + models__second_ephemeral_model +) + + +# -- Below we define base classes for tests you import the one based on if your adapter uses dbt clone or not -- +class BaseShowLimit(ShowBase): + @pytest.mark.parametrize( + "args,expected", + [ + ([], 5), # default limit + (["--limit", 3], 3), # fetch 3 rows + (["--limit", -1], 7), # fetch all rows + ], + ) + def test_limit(self, project, args, expected): + run_dbt(["build"]) + dbt_args = ["show", "--inline", models__second_ephemeral_model, *args] + results = run_dbt(dbt_args) + assert len(results.results[0].agate_table) == expected + # ensure limit was injected in compiled_code when limit specified in command args + limit = results.args.get("limit") + if limit > 0: + assert f"limit {limit}" in results.results[0].node.compiled_code + + +class BaseShowSqlHeader(ShowBase): + def test_sql_header(self, project): + run_dbt(["build", "--vars", "timezone: Asia/Kolkata"]) + (_, log_output) = run_dbt_and_capture( + ["show", "--select", "sql_header", "--vars", "timezone: Asia/Kolkata"] + ) + + +class TestPostgresShowSqlHeader(BaseShowSqlHeader): + pass + + +class TestPostgresShowLimit(BaseShowLimit): + pass diff --git a/tests/functional/show/test_show.py b/tests/functional/show/test_show.py index 85532d81cf8..8be3befa056 100644 --- a/tests/functional/show/test_show.py +++ b/tests/functional/show/test_show.py @@ -143,39 +143,12 @@ def test_second_ephemeral_model(self, project): assert "col_hundo" in log_output -class TestShowLimit(ShowBase): - @pytest.mark.parametrize( - "args,expected", - [ - ([], 5), # default limit - (["--limit", 3], 3), # fetch 3 rows - (["--limit", -1], 7), # fetch all rows - ], - ) - def test_limit(self, project, args, expected): - run_dbt(["build"]) - dbt_args = ["show", "--inline", models__second_ephemeral_model, *args] - results = run_dbt(dbt_args) - assert len(results.results[0].agate_table) == expected - # ensure limit was injected in compiled_code when limit specified in command args - if results.args.get("limit") > 0: - assert "limit" in results.results[0].node.compiled_code - - class TestShowSeed(ShowBase): def test_seed(self, project): (_, log_output) = run_dbt_and_capture(["show", "--select", "sample_seed"]) assert "Previewing node 'sample_seed'" in log_output -class TestShowSqlHeader(ShowBase): - def test_sql_header(self, project): - run_dbt(["build", "--vars", "timezone: Asia/Kolkata"]) - (_, log_output) = run_dbt_and_capture( - ["show", "--select", "sql_header", "--vars", "timezone: Asia/Kolkata"] - ) - - class TestShowModelVersions: @pytest.fixture(scope="class") def models(self): From 1de567791b39e3c8969c22a8f874c7ceb6e08ecc Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 18 Sep 2023 14:59:23 +0100 Subject: [PATCH 07/10] linting --- .../dbt/tests/adapter/dbt_show/test_dbt_show.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py index 14d96b90320..00551550eec 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py +++ b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py @@ -2,12 +2,10 @@ from dbt.tests.util import run_dbt_and_capture, run_dbt from tests.functional.show.test_show import ShowBase -from tests.functional.show.fixtures import ( - models__second_ephemeral_model -) +from tests.functional.show.fixtures import models__second_ephemeral_model -# -- Below we define base classes for tests you import the one based on if your adapter uses dbt clone or not -- +# -- Below we define base classes for tests you import based on if your adapter supports dbt show or not -- class BaseShowLimit(ShowBase): @pytest.mark.parametrize( "args,expected", @@ -37,8 +35,8 @@ def test_sql_header(self, project): class TestPostgresShowSqlHeader(BaseShowSqlHeader): - pass + pass class TestPostgresShowLimit(BaseShowLimit): - pass + pass From 1ebfd74c6890eb1f9dd7d8a346584d3ad206aec4 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 18 Sep 2023 18:43:49 +0100 Subject: [PATCH 08/10] reorganize dbt_show adapter test fixtures --- .../dbt/tests/adapter/dbt_show/fixtures.py | 34 ++++++++++++++++++ .../tests/adapter/dbt_show/test_dbt_show.py | 36 ++++++++++++++----- tests/functional/show/test_show.py | 2 -- 3 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 tests/adapter/dbt/tests/adapter/dbt_show/fixtures.py diff --git a/tests/adapter/dbt/tests/adapter/dbt_show/fixtures.py b/tests/adapter/dbt/tests/adapter/dbt_show/fixtures.py new file mode 100644 index 00000000000..6eda5a695f3 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/dbt_show/fixtures.py @@ -0,0 +1,34 @@ +models__sql_header = """ +{% call set_sql_header(config) %} +set session time zone '{{ var("timezone", "Europe/Paris") }}'; +{%- endcall %} +select current_setting('timezone') as timezone +""" + +models__ephemeral_model = """ +{{ config(materialized = 'ephemeral') }} +select + coalesce(sample_num, 0) + 10 as col_deci +from {{ ref('sample_model') }} +""" + +models__second_ephemeral_model = """ +{{ config(materialized = 'ephemeral') }} +select + col_deci + 100 as col_hundo +from {{ ref('ephemeral_model') }} +""" + +models__sample_model = """ +select * from {{ ref('sample_seed') }} +""" + +seeds__sample_seed = """sample_num,sample_bool +1,true +2,false +3,true +4,false +5,true +6,false +7,true +""" diff --git a/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py index 00551550eec..a93bb9dd2ab 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py +++ b/tests/adapter/dbt/tests/adapter/dbt_show/test_dbt_show.py @@ -1,12 +1,28 @@ import pytest -from dbt.tests.util import run_dbt_and_capture, run_dbt +from dbt.tests.util import run_dbt -from tests.functional.show.test_show import ShowBase -from tests.functional.show.fixtures import models__second_ephemeral_model +from dbt.tests.adapter.dbt_show.fixtures import ( + models__sql_header, + models__ephemeral_model, + models__second_ephemeral_model, + models__sample_model, + seeds__sample_seed, +) # -- Below we define base classes for tests you import based on if your adapter supports dbt show or not -- -class BaseShowLimit(ShowBase): +class BaseShowLimit: + @pytest.fixture(scope="class") + def models(self): + return { + "sample_model.sql": models__sample_model, + "ephemeral_model.sql": models__ephemeral_model, + } + + @pytest.fixture(scope="class") + def seeds(self): + return {"sample_seed.csv": seeds__sample_seed} + @pytest.mark.parametrize( "args,expected", [ @@ -26,12 +42,16 @@ def test_limit(self, project, args, expected): assert f"limit {limit}" in results.results[0].node.compiled_code -class BaseShowSqlHeader(ShowBase): +class BaseShowSqlHeader: + @pytest.fixture(scope="class") + def models(self): + return { + "sql_header.sql": models__sql_header, + } + def test_sql_header(self, project): run_dbt(["build", "--vars", "timezone: Asia/Kolkata"]) - (_, log_output) = run_dbt_and_capture( - ["show", "--select", "sql_header", "--vars", "timezone: Asia/Kolkata"] - ) + run_dbt(["show", "--select", "sql_header", "--vars", "timezone: Asia/Kolkata"]) class TestPostgresShowSqlHeader(BaseShowSqlHeader): diff --git a/tests/functional/show/test_show.py b/tests/functional/show/test_show.py index 8be3befa056..85f27261c11 100644 --- a/tests/functional/show/test_show.py +++ b/tests/functional/show/test_show.py @@ -11,7 +11,6 @@ models__second_model, models__ephemeral_model, schema_yml, - models__sql_header, private_model_yml, ) @@ -25,7 +24,6 @@ def models(self): "sample_number_model_with_nulls.sql": models__sample_number_model_with_nulls, "second_model.sql": models__second_model, "ephemeral_model.sql": models__ephemeral_model, - "sql_header.sql": models__sql_header, } @pytest.fixture(scope="class") From 7c118f77fd59fe4af937235815e0f4f68200f94e Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 25 Sep 2023 13:30:08 +0100 Subject: [PATCH 09/10] rename show macro, pass kwargs explicitly --- core/dbt/include/global_project/macros/adapters/show.sql | 4 ++-- core/dbt/task/show.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/core/dbt/include/global_project/macros/adapters/show.sql b/core/dbt/include/global_project/macros/adapters/show.sql index 2ff97dd68fe..d71aa3a7572 100644 --- a/core/dbt/include/global_project/macros/adapters/show.sql +++ b/core/dbt/include/global_project/macros/adapters/show.sql @@ -1,5 +1,5 @@ -{% macro show(limit) -%} - {%- set sql_header = config.get('sql_header', none) -%} +{% macro get_show_sql(compiled_code, sql_header, limit) -%} + {%- set sql_header = sql_header -%} {{ sql_header if sql_header is not none }} {%- if limit is not none -%} {{ get_limit_subquery_sql(compiled_code, limit) }} diff --git a/core/dbt/task/show.py b/core/dbt/task/show.py index 439246340b8..eba7dc663c6 100644 --- a/core/dbt/task/show.py +++ b/core/dbt/task/show.py @@ -26,10 +26,14 @@ def execute(self, compiled_node, manifest): model_context = generate_runtime_model_context(compiled_node, self.config, manifest) compiled_node.compiled_code = self.adapter.execute_macro( - macro_name="show", + macro_name="get_show_sql", manifest=manifest, context_override=model_context, - kwargs={"limit": limit}, + kwargs={ + "compiled_code": model_context["compiled_code"], + "sql_header": model_context["config"].get("sql_header"), + "limit": limit, + }, ) adapter_response, execute_result = self.adapter.execute( compiled_node.compiled_code, fetch=True From 81f68e65f7b9eb0c39b9bf11be69633838e49557 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 25 Sep 2023 15:40:06 +0100 Subject: [PATCH 10/10] formatting get_show_sql macro --- core/dbt/include/global_project/macros/adapters/show.sql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/dbt/include/global_project/macros/adapters/show.sql b/core/dbt/include/global_project/macros/adapters/show.sql index d71aa3a7572..2e23a919d29 100644 --- a/core/dbt/include/global_project/macros/adapters/show.sql +++ b/core/dbt/include/global_project/macros/adapters/show.sql @@ -1,6 +1,9 @@ {% macro get_show_sql(compiled_code, sql_header, limit) -%} {%- set sql_header = sql_header -%} {{ sql_header if sql_header is not none }} + {%- if sql_header -%} + {{ sql_header }} + {%- endif -%} {%- if limit is not none -%} {{ get_limit_subquery_sql(compiled_code, limit) }} {%- else -%}