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

Make generated CTE test names lowercase to match style guide #3028

Merged
merged 4 commits into from
Jan 25, 2021
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Fixes

- Fix exit code from dbt debug not returning a failure when one of the tests fail ([#3017](/~https://github.com/fishtown-analytics/dbt/issues/3017))
- Auto-generated CTEs in tests and ephemeral models have lowercase names to comply with dbt coding conventions ([#3027](/~https://github.com/fishtown-analytics/dbt/issues/3027), [#3028](/~https://github.com/fishtown-analytics/dbt/issues/3028))

### Features
- Add optional configs for `require_partition_filter` and `partition_expiration_days` in BigQuery ([#1843](/~https://github.com/fishtown-analytics/dbt/issues/1843), [#2928](/~https://github.com/fishtown-analytics/dbt/pull/2928))
Expand All @@ -12,6 +13,7 @@ Contributors:
- [@yu-iskw](/~https://github.com/yu-iskw) ([#2928](/~https://github.com/fishtown-analytics/dbt/pull/2928))
- [@sdebruyn](/~https://github.com/sdebruyn) / [@lynxcare](/~https://github.com/lynxcare) ([#3018](/~https://github.com/fishtown-analytics/dbt/pull/3018))
- [@rvacaru](/~https://github.com/rvacaru) ([#2974](/~https://github.com/fishtown-analytics/dbt/pull/2974))
- [@NiallRees](/~https://github.com/NiallRees) ([#3028](/~https://github.com/fishtown-analytics/dbt/pull/3028))

## dbt 0.19.0 (Release TBD)

Expand Down
2 changes: 1 addition & 1 deletion core/dbt/adapters/base/relation.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def create_from_source(

@staticmethod
def add_ephemeral_prefix(name: str):
return f'__dbt__CTE__{name}'
return f'__dbt__cte__{name}'

@classmethod
def create_ephemeral_from_node(
Expand Down
10 changes: 5 additions & 5 deletions core/dbt/compilation.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str:
[
InjectedCTE(
id="cte_id_1",
sql="__dbt__CTE__ephemeral as (select * from table)",
sql="__dbt__cte__ephemeral as (select * from table)",
),
InjectedCTE(
id="cte_id_2",
sql="__dbt__CTE__events as (select id, type from events)",
sql="__dbt__cte__events as (select id, type from events)",
),
]

Expand All @@ -206,8 +206,8 @@ def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str:

This will spit out:

"with __dbt__CTE__ephemeral as (select * from table),
__dbt__CTE__events as (select id, type from events),
"with __dbt__cte__ephemeral as (select * from table),
__dbt__cte__events as (select id, type from events),
with internal_cte as (select * from sessions)
select * from internal_cte"

Expand Down Expand Up @@ -246,7 +246,7 @@ def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str:
return str(parsed)

def _get_dbt_test_name(self) -> str:
return 'dbt__CTE__INTERNAL_test'
return 'dbt__cte__internal_test'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably not be hard-coded within the compiler, and should instead do something like adapter.add_ephemeral_prefix('internal_test'), but that's a problem for another day. We're planning to clean up some inconsistencies around data tests for the v0.20.0 release, anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be nice to see 🙂


# This method is called by the 'compile_node' method. Starting
# from the node that it is passed in, it will recursively call
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def filter_null_values(input: Dict[K_T, Optional[V_T]]) -> Dict[K_T, V_T]:


def add_ephemeral_model_prefix(s: str) -> str:
return '__dbt__CTE__{}'.format(s)
return '__dbt__cte__{}'.format(s)
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, this method is no longer used, since #2712 swapped it out in favor of adapter.add_ephemeral_prefix.



def timestring() -> str:
Expand Down
18 changes: 9 additions & 9 deletions test/integration/020_ephemeral_test/test_ephemeral.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ def test__postgres(self):

sql_file = re.sub(r'\d+', '', sql_file)
expected_sql = ('create view "dbt"."test_ephemeral_"."double_dependent__dbt_tmp" as ('
'with __dbt__CTE__base as ('
'with __dbt__cte__base as ('
'select * from test_ephemeral_.seed'
'), __dbt__CTE__base_copy as ('
'select * from __dbt__CTE__base'
'), __dbt__cte__base_copy as ('
'select * from __dbt__cte__base'
')-- base_copy just pulls from base. Make sure the listed'
'-- graph of CTEs all share the same dbt_cte__base cte'
"select * from __dbt__CTE__base where gender = 'Male'"
"select * from __dbt__cte__base where gender = 'Male'"
'union all'
"select * from __dbt__CTE__base_copy where gender = 'Female'"
"select * from __dbt__cte__base_copy where gender = 'Female'"
');')
sql_file = "".join(sql_file.split())
expected_sql = "".join(expected_sql.split())
Expand Down Expand Up @@ -79,11 +79,11 @@ def test__postgres(self):
sql_file = re.sub(r'\d+', '', sql_file)
expected_sql = (
'create view "dbt"."test_ephemeral_"."root_view__dbt_tmp" as ('
'with __dbt__CTE__ephemeral_level_two as ('
'with __dbt__cte__ephemeral_level_two as ('
'select * from "dbt"."test_ephemeral_"."source_table"'
'), __dbt__CTE__ephemeral as ('
'select * from __dbt__CTE__ephemeral_level_two'
')select * from __dbt__CTE__ephemeral'
'), __dbt__cte__ephemeral as ('
'select * from __dbt__cte__ephemeral_level_two'
')select * from __dbt__cte__ephemeral'
');')

sql_file = "".join(sql_file.split())
Expand Down
4 changes: 2 additions & 2 deletions test/integration/100_rpc_test/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ def query_url(url, query):
return requests.post(url, headers=headers, data=json.dumps(query))


_select_from_ephemeral = '''with __dbt__CTE__ephemeral_model as (
_select_from_ephemeral = '''with __dbt__cte__ephemeral_model as (


select 1 as id
)select * from __dbt__CTE__ephemeral_model'''
)select * from __dbt__cte__ephemeral_model'''


def addr_in_use(err, *args):
Expand Down
30 changes: 15 additions & 15 deletions test/unit/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def setUp(self):

def mock_generate_runtime_model_context(model, config, manifest):
def ref(name):
result = f'__dbt__CTE__{name}'
result = f'__dbt__cte__{name}'
unique_id = f'model.root.{name}'
model.extra_ctes.append(InjectedCTE(id=unique_id, sql=None))
return result
Expand Down Expand Up @@ -121,7 +121,7 @@ def test__prepend_ctes__already_has_cte(self):
extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql='select * from source_table')],
compiled_sql=(
'with cte as (select * from something_else) '
'select * from __dbt__CTE__ephemeral'),
'select * from __dbt__cte__ephemeral'),
checksum=FileHash.from_contents(''),
),
'model.root.ephemeral': CompiledModelNode(
Expand Down Expand Up @@ -168,10 +168,10 @@ def test__prepend_ctes__already_has_cte(self):
self.assertEqual(result.extra_ctes_injected, True)
self.assertEqualIgnoreWhitespace(
result.compiled_sql,
('with __dbt__CTE__ephemeral as ('
('with __dbt__cte__ephemeral as ('
'select * from source_table'
'), cte as (select * from something_else) '
'select * from __dbt__CTE__ephemeral'))
'select * from __dbt__cte__ephemeral'))

self.assertEqual(
manifest.nodes['model.root.ephemeral'].extra_ctes_injected,
Expand Down Expand Up @@ -296,7 +296,7 @@ def test__prepend_ctes(self):
compiled=True,
extra_ctes_injected=False,
extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql='select * from source_table')],
compiled_sql='select * from __dbt__CTE__ephemeral',
compiled_sql='select * from __dbt__cte__ephemeral',
checksum=FileHash.from_contents(''),
),
'model.root.ephemeral': CompiledModelNode(
Expand Down Expand Up @@ -345,10 +345,10 @@ def test__prepend_ctes(self):
self.assertTrue(result.extra_ctes_injected)
self.assertEqualIgnoreWhitespace(
result.compiled_sql,
('with __dbt__CTE__ephemeral as ('
('with __dbt__cte__ephemeral as ('
'select * from source_table'
') '
'select * from __dbt__CTE__ephemeral'))
'select * from __dbt__cte__ephemeral'))
print(f"\n---- line 349 ----")

self.assertFalse(manifest.nodes['model.root.ephemeral'].extra_ctes_injected)
Expand Down Expand Up @@ -423,7 +423,7 @@ def test__prepend_ctes__cte_not_compiled(self):
compiled=True,
extra_ctes_injected=False,
extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql='select * from source_table')],
compiled_sql='select * from __dbt__CTE__ephemeral',
compiled_sql='select * from __dbt__cte__ephemeral',
checksum=FileHash.from_contents(''),
),
'model.root.ephemeral': parsed_ephemeral,
Expand Down Expand Up @@ -454,10 +454,10 @@ def test__prepend_ctes__cte_not_compiled(self):
self.assertTrue(result.extra_ctes_injected)
self.assertEqualIgnoreWhitespace(
result.compiled_sql,
('with __dbt__CTE__ephemeral as ('
('with __dbt__cte__ephemeral as ('
'select * from source_table'
') '
'select * from __dbt__CTE__ephemeral'))
'select * from __dbt__cte__ephemeral'))

self.assertTrue(manifest.nodes['model.root.ephemeral'].extra_ctes_injected)

Expand Down Expand Up @@ -488,7 +488,7 @@ def test__prepend_ctes__multiple_levels(self):
compiled=True,
extra_ctes_injected=False,
extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql=None)],
compiled_sql='select * from __dbt__CTE__ephemeral',
compiled_sql='select * from __dbt__cte__ephemeral',
checksum=FileHash.from_contents(''),

),
Expand Down Expand Up @@ -552,12 +552,12 @@ def test__prepend_ctes__multiple_levels(self):
self.assertTrue(result.extra_ctes_injected)
self.assertEqualIgnoreWhitespace(
result.compiled_sql,
('with __dbt__CTE__ephemeral_level_two as ('
('with __dbt__cte__ephemeral_level_two as ('
'select * from source_table'
'), __dbt__CTE__ephemeral as ('
'select * from __dbt__CTE__ephemeral_level_two'
'), __dbt__cte__ephemeral as ('
'select * from __dbt__cte__ephemeral_level_two'
') '
'select * from __dbt__CTE__ephemeral'))
'select * from __dbt__cte__ephemeral'))

self.assertTrue(manifest.nodes['model.root.ephemeral'].compiled)
self.assertTrue(manifest.nodes['model.root.ephemeral_level_two'].compiled)
Expand Down