diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e5030b85e0..3f60e537db5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Added dbt_invocation_id for each BigQuery job to enable performance analysis ([#2808](https://github.com/fishtown-analytics/dbt/issues/2808), [#2809](https://github.com/fishtown-analytics/dbt/pull/2809)) - Save cli and rpc arguments in run_results.json ([#2510](https://github.com/fishtown-analytics/dbt/issues/2510), [#2813](https://github.com/fishtown-analytics/dbt/pull/2813)) - Added support for BigQuery connections using refresh tokens ([#2344](https://github.com/fishtown-analytics/dbt/issues/2344), [#2805](https://github.com/fishtown-analytics/dbt/pull/2805)) +- Remove injected_sql from manifest nodes ([#2762](https://github.com/fishtown-analytics/dbt/issues/2762), [#2834](https://github.com/fishtown-analytics/dbt/pull/2834)) ### Under the hood - Added strategy-specific validation to improve the relevancy of compilation errors for the `timestamp` and `check` snapshot strategies. (([#2787](https://github.com/fishtown-analytics/dbt/issues/2787), [#2791](https://github.com/fishtown-analytics/dbt/pull/2791)) diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 5811227efe4..18e295eb916 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -148,12 +148,15 @@ def initialize(self): make_directory(self.config.target_path) make_directory(self.config.modules_path) + # creates a ModelContext which is converted to + # a dict for jinja rendering of SQL def _create_node_context( self, node: NonSourceCompiledNode, manifest: Manifest, extra_context: Dict[str, Any], ) -> Dict[str, Any]: + context = generate_runtime_model( node, self.config, manifest ) @@ -169,36 +172,6 @@ def add_ephemeral_prefix(self, name: str): relation_cls = adapter.Relation return relation_cls.add_ephemeral_prefix(name) - def _get_compiled_model( - self, - manifest: Manifest, - cte_id: str, - extra_context: Dict[str, Any], - ) -> NonSourceCompiledNode: - - if cte_id not in manifest.nodes: - raise InternalException( - f'During compilation, found a cte reference that could not be ' - f'resolved: {cte_id}' - ) - cte_model = manifest.nodes[cte_id] - if getattr(cte_model, 'compiled', False): - assert isinstance(cte_model, tuple(COMPILED_TYPES.values())) - return cast(NonSourceCompiledNode, cte_model) - elif cte_model.is_ephemeral_model: - # this must be some kind of parsed node that we can compile. - # we know it's not a parsed source definition - assert isinstance(cte_model, tuple(COMPILED_TYPES)) - # update the node so - node = self.compile_node(cte_model, manifest, extra_context) - manifest.sync_update_node(node) - return node - else: - raise InternalException( - f'During compilation, found an uncompiled cte that ' - f'was not an ephemeral model: {cte_id}' - ) - def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str: """ `ctes` is a list of InjectedCTEs like: @@ -260,73 +233,107 @@ def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str: return str(parsed) - def _model_prepend_ctes( - self, - model: NonSourceCompiledNode, - prepended_ctes: List[InjectedCTE] - ) -> NonSourceCompiledNode: - if model.compiled_sql is None: - raise RuntimeException( - 'Cannot prepend ctes to an unparsed node', model - ) - injected_sql = self._inject_ctes_into_sql( - model.compiled_sql, - prepended_ctes, - ) - - model.extra_ctes_injected = True - model.extra_ctes = prepended_ctes - model.injected_sql = injected_sql - model.validate(model.to_dict()) - return model - def _get_dbt_test_name(self) -> str: return 'dbt__CTE__INTERNAL_test' + # This method is called by the 'compile_node' method. Starting + # from the node that it is passed in, it will recursively call + # itself using the 'extra_ctes'. The 'ephemeral' models do + # not produce SQL that is executed directly, instead they + # are rolled up into the models that refer to them by + # inserting CTEs into the SQL. def _recursively_prepend_ctes( self, model: NonSourceCompiledNode, manifest: Manifest, - extra_context: Dict[str, Any], + extra_context: Optional[Dict[str, Any]], ) -> Tuple[NonSourceCompiledNode, List[InjectedCTE]]: + + if model.compiled_sql is None: + raise RuntimeException( + 'Cannot inject ctes into an unparsed node', model + ) if model.extra_ctes_injected: return (model, model.extra_ctes) - if flags.STRICT_MODE: - if not isinstance(model, tuple(COMPILED_TYPES.values())): - raise InternalException( - f'Bad model type: {type(model)}' - ) + # Just to make it plain that nothing is actually injected for this case + if not model.extra_ctes: + model.extra_ctes_injected = True + manifest.update_node(model) + return (model, model.extra_ctes) + # This stores the ctes which will all be recursively + # gathered and then "injected" into the model. prepended_ctes: List[InjectedCTE] = [] dbt_test_name = self._get_dbt_test_name() + # extra_ctes are added to the model by + # RuntimeRefResolver.create_relation, which adds an + # extra_cte for every model relation which is an + # ephemeral model. for cte in model.extra_ctes: if cte.id == dbt_test_name: sql = cte.sql else: - cte_model = self._get_compiled_model( - manifest, - cte.id, - extra_context, - ) - cte_model, new_prepended_ctes = self._recursively_prepend_ctes( - cte_model, manifest, extra_context - ) + if cte.id not in manifest.nodes: + raise InternalException( + f'During compilation, found a cte reference that ' + f'could not be resolved: {cte.id}' + ) + cte_model = manifest.nodes[cte.id] + + if not cte_model.is_ephemeral_model: + raise InternalException(f'{cte.id} is not ephemeral') + + # This model has already been compiled, so it's been + # through here before + if getattr(cte_model, 'compiled', False): + assert isinstance(cte_model, + tuple(COMPILED_TYPES.values())) + cte_model = cast(NonSourceCompiledNode, cte_model) + new_prepended_ctes = cte_model.extra_ctes + + # if the cte_model isn't compiled, i.e. first time here + else: + # This is an ephemeral parsed model that we can compile. + # Compile and update the node + cte_model = self._compile_node( + cte_model, manifest, extra_context) + # recursively call this method + cte_model, new_prepended_ctes = \ + self._recursively_prepend_ctes( + cte_model, manifest, extra_context + ) + # Save compiled SQL file and sync manifest + self._write_node(cte_model) + manifest.sync_update_node(cte_model) + _extend_prepended_ctes(prepended_ctes, new_prepended_ctes) new_cte_name = self.add_ephemeral_prefix(cte_model.name) sql = f' {new_cte_name} as (\n{cte_model.compiled_sql}\n)' + _add_prepended_cte(prepended_ctes, InjectedCTE(id=cte.id, sql=sql)) - model = self._model_prepend_ctes(model, prepended_ctes) + # We don't save injected_sql into compiled sql for ephemeral models + # because it will cause problems with processing of subsequent models. + # Ephemeral models do not produce executable SQL of their own. + if not model.is_ephemeral_model: + injected_sql = self._inject_ctes_into_sql( + model.compiled_sql, + prepended_ctes, + ) + model.compiled_sql = injected_sql + model.extra_ctes_injected = True + model.extra_ctes = prepended_ctes + model.validate(model.to_dict()) manifest.update_node(model) return model, prepended_ctes - def _insert_ctes( + def _add_ctes( self, compiled_node: NonSourceCompiledNode, manifest: Manifest, @@ -352,11 +359,12 @@ def _insert_ctes( compiled_node.extra_ctes.append(cte) compiled_node.compiled_sql = f'\nselect count(*) from {name}' - injected_node, _ = self._recursively_prepend_ctes( - compiled_node, manifest, extra_context - ) - return injected_node + return compiled_node + # creates a compiled_node from the ManifestNode passed in, + # creates a "context" dictionary for jinja rendering, + # and then renders the "compiled_sql" using the node, the + # raw_sql and the context. def _compile_node( self, node: ManifestNode, @@ -374,7 +382,6 @@ def _compile_node( 'compiled_sql': None, 'extra_ctes_injected': False, 'extra_ctes': [], - 'injected_sql': None, }) compiled_node = _compiled_type_for(node).from_dict(data) @@ -390,11 +397,13 @@ def _compile_node( compiled_node.compiled = True - injected_node = self._insert_ctes( + # add ctes for specific test nodes, and also for + # possible future use in adapters + compiled_node = self._add_ctes( compiled_node, manifest, extra_context ) - return injected_node + return compiled_node def write_graph_file(self, linker: Linker, manifest: Manifest): filename = graph_file_name @@ -449,26 +458,26 @@ def compile(self, manifest: Manifest, write=True) -> Graph: return Graph(linker.graph) + # writes the "compiled_sql" into the target/compiled directory def _write_node(self, node: NonSourceCompiledNode) -> ManifestNode: - if not _is_writable(node): + if (not node.extra_ctes_injected or + node.resource_type == NodeType.Snapshot): return node logger.debug(f'Writing injected SQL for node "{node.unique_id}"') - if node.injected_sql is None: - # this should not really happen, but it'd be a shame to crash - # over it - logger.error( - f'Compiled node "{node.unique_id}" had no injected_sql, ' - 'cannot write sql!' - ) - else: + if node.compiled_sql: node.build_path = node.write_node( self.config.target_path, 'compiled', - node.injected_sql + node.compiled_sql ) return node + # This is the main entry point into this code. It's called by + # CompileRunner.compile, GenericRPCRunner.compile, and + # RunTask.get_hook_sql. It calls '_compile_node' to convert + # the node into a compiled node, and then calls the + # recursive method to "prepend" the ctes. def compile_node( self, node: ManifestNode, @@ -478,16 +487,9 @@ def compile_node( ) -> NonSourceCompiledNode: node = self._compile_node(node, manifest, extra_context) - if write and _is_writable(node): + node, _ = self._recursively_prepend_ctes( + node, manifest, extra_context + ) + if write: self._write_node(node) return node - - -def _is_writable(node): - if not node.injected_sql: - return False - - if node.resource_type == NodeType.Snapshot: - return False - - return True diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index 32c4b91db7b..79d93c7247f 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -1217,7 +1217,9 @@ def post_hooks(self) -> List[Dict[str, Any]]: @contextproperty def sql(self) -> Optional[str]: - return getattr(self.model, 'injected_sql', None) + if getattr(self.model, 'extra_ctes_injected', None): + return self.model.compiled_sql + return None @contextproperty def database(self) -> str: diff --git a/core/dbt/contracts/graph/compiled.py b/core/dbt/contracts/graph/compiled.py index fbf45d0c208..d611e4e3614 100644 --- a/core/dbt/contracts/graph/compiled.py +++ b/core/dbt/contracts/graph/compiled.py @@ -42,7 +42,6 @@ class CompiledNode(ParsedNode, CompiledNodeMixin): compiled_sql: Optional[str] = None extra_ctes_injected: bool = False extra_ctes: List[InjectedCTE] = field(default_factory=list) - injected_sql: Optional[str] = None def set_cte(self, cte_id: str, sql: str): """This is the equivalent of what self.extra_ctes[cte_id] = sql would diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 362c8cd4929..1a1134e3a26 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -132,7 +132,7 @@ def data(self): result.update({ 'raw_sql': self.node.raw_sql, # the node isn't always compiled, but if it is, include that! - 'compiled_sql': getattr(self.node, 'injected_sql', None), + 'compiled_sql': getattr(self.node, 'compiled_sql', None), }) return result diff --git a/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql b/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql index 1d60d23658f..e34c6bf22a6 100644 --- a/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql +++ b/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql @@ -214,7 +214,7 @@ {% if not target_relation_exists %} - {% set build_sql = build_snapshot_table(strategy, model['injected_sql']) %} + {% set build_sql = build_snapshot_table(strategy, model['compiled_sql']) %} {% set final_sql = create_table_as(False, target_relation, build_sql) %} {% else %} diff --git a/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql b/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql index 6b5f2a470b5..e3f1966c6af 100644 --- a/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql +++ b/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql @@ -106,7 +106,7 @@ {% macro snapshot_check_all_get_existing_columns(node, target_exists) -%} - {%- set query_columns = get_columns_in_query(node['injected_sql']) -%} + {%- set query_columns = get_columns_in_query(node['compiled_sql']) -%} {%- if not target_exists -%} {# no table yet -> return whatever the query does #} {{ return([false, query_columns]) }} diff --git a/core/dbt/rpc/node_runners.py b/core/dbt/rpc/node_runners.py index 2bbf866d4a5..8598deccef4 100644 --- a/core/dbt/rpc/node_runners.py +++ b/core/dbt/rpc/node_runners.py @@ -65,7 +65,7 @@ class RPCCompileRunner(GenericRPCRunner[RemoteCompileResult]): def execute(self, compiled_node, manifest) -> RemoteCompileResult: return RemoteCompileResult( raw_sql=compiled_node.raw_sql, - compiled_sql=compiled_node.injected_sql, + compiled_sql=compiled_node.compiled_sql, node=compiled_node, timing=[], # this will get added later logs=[], @@ -88,7 +88,7 @@ def from_run_result( class RPCExecuteRunner(GenericRPCRunner[RemoteRunResult]): def execute(self, compiled_node, manifest) -> RemoteRunResult: _, execute_result = self.adapter.execute( - compiled_node.injected_sql, fetch=True + compiled_node.compiled_sql, fetch=True ) table = ResultTable( @@ -98,7 +98,7 @@ def execute(self, compiled_node, manifest) -> RemoteRunResult: return RemoteRunResult( raw_sql=compiled_node.raw_sql, - compiled_sql=compiled_node.injected_sql, + compiled_sql=compiled_node.compiled_sql, node=compiled_node, table=table, timing=[], diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index 449900649a2..086d423477e 100644 --- a/core/dbt/task/run.py +++ b/core/dbt/task/run.py @@ -255,7 +255,7 @@ def raise_on_first_error(self): def get_hook_sql(self, adapter, hook, idx, num_hooks, extra_context): compiler = adapter.get_compiler() compiled = compiler.compile_node(hook, self.manifest, extra_context) - statement = compiled.injected_sql + statement = compiled.compiled_sql hook_index = hook.index or num_hooks hook_obj = get_hook(statement, index=hook_index) return hook_obj.sql or '' diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index 44fec0ac6f4..d6c84a36fe6 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -42,7 +42,7 @@ def print_start_line(self): def execute_data_test(self, test: CompiledDataTestNode): res, table = self.adapter.execute( - test.injected_sql, auto_begin=True, fetch=True + test.compiled_sql, auto_begin=True, fetch=True ) num_rows = len(table.rows) @@ -59,7 +59,7 @@ def execute_data_test(self, test: CompiledDataTestNode): def execute_schema_test(self, test: CompiledSchemaTestNode): res, table = self.adapter.execute( - test.injected_sql, + test.compiled_sql, auto_begin=True, fetch=True, ) diff --git a/plugins/bigquery/dbt/adapters/bigquery/impl.py b/plugins/bigquery/dbt/adapters/bigquery/impl.py index eea5300fe27..364209a79b3 100644 --- a/plugins/bigquery/dbt/adapters/bigquery/impl.py +++ b/plugins/bigquery/dbt/adapters/bigquery/impl.py @@ -385,7 +385,7 @@ def _materialize_as_view(self, model: Dict[str, Any]) -> str: model_database = model.get('database') model_schema = model.get('schema') model_alias = model.get('alias') - model_sql = model.get('injected_sql') + model_sql = model.get('compiled_sql') logger.debug("Model SQL ({}):\n{}".format(model_alias, model_sql)) self.connections.create_view( @@ -505,7 +505,7 @@ def execute_model(self, model, materialization, sql_override=None, decorator=None): if sql_override is None: - sql_override = model.get('injected_sql') + sql_override = model.get('compiled_sql') if flags.STRICT_MODE: connection = self.connections.get_thread_connection() diff --git a/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql b/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql index cbb97ea4ec7..53c7d2f6673 100644 --- a/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql +++ b/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql @@ -11,7 +11,7 @@ {{ log(table_start_time ~ ' | -> Running for day ' ~ date, info=True) }} {% endif %} - {% set fixed_sql = model['injected_sql'] | replace('[DBT__PARTITION_DATE]', date) %} + {% set fixed_sql = model['compiled_sql'] | replace('[DBT__PARTITION_DATE]', date) %} {% set _ = adapter.execute_model(model, 'table', fixed_sql, decorator=date) %} {% endfor %} diff --git a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py index e5b886d142e..46d3deafa8b 100644 --- a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py +++ b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py @@ -919,5 +919,7 @@ def _snapshot_and_assert_revived(self): for result in revived_records: # result is a tuple, the dbt_valid_from is second and dbt_valid_to is latest self.assertIsInstance(result[1], datetime) - self.assertGreaterEqual(result[1].replace(tzinfo=pytz.UTC), self._invalidated_snapshot_datetime) + # there are milliseconds (part of microseconds in datetime objects) in the + # invalidated_snapshot_datetime and not in result datetime so set the microseconds to 0 + self.assertGreaterEqual(result[1].replace(tzinfo=pytz.UTC), self._invalidated_snapshot_datetime.replace(microsecond=0) self.assertIsNone(result[2]) diff --git a/test/integration/015_cli_invocation_tests/test_cli_invocation.py b/test/integration/015_cli_invocation_tests/test_cli_invocation.py index 72c76846b24..d871432cee4 100644 --- a/test/integration/015_cli_invocation_tests/test_cli_invocation.py +++ b/test/integration/015_cli_invocation_tests/test_cli_invocation.py @@ -130,7 +130,7 @@ def test_postgres_toplevel_dbt_run_with_profile_dir_arg(self): # make sure the test runs against `custom_schema` for test_result in res: - self.assertTrue(self.custom_schema, test_result.node.injected_sql) + self.assertTrue(self.custom_schema, test_result.node.compiled_sql) class TestCLIInvocationWithProjectDir(ModelCopyingIntegrationTest): diff --git a/test/integration/020_ephemeral_test/models-n/ephemeral.sql b/test/integration/020_ephemeral_test/models-n/ephemeral.sql new file mode 100644 index 00000000000..44823c9a94f --- /dev/null +++ b/test/integration/020_ephemeral_test/models-n/ephemeral.sql @@ -0,0 +1,6 @@ +{{ + config( + materialized = "ephemeral", + ) +}} +select * from {{ref("ephemeral_level_two")}} diff --git a/test/integration/020_ephemeral_test/models-n/ephemeral_level_two.sql b/test/integration/020_ephemeral_test/models-n/ephemeral_level_two.sql new file mode 100644 index 00000000000..c7d6758137f --- /dev/null +++ b/test/integration/020_ephemeral_test/models-n/ephemeral_level_two.sql @@ -0,0 +1,6 @@ +{{ + config( + materialized = "ephemeral", + ) +}} +select * from {{ ref('source_table') }} diff --git a/test/integration/020_ephemeral_test/models-n/root_view.sql b/test/integration/020_ephemeral_test/models-n/root_view.sql new file mode 100644 index 00000000000..d1bb5689e80 --- /dev/null +++ b/test/integration/020_ephemeral_test/models-n/root_view.sql @@ -0,0 +1 @@ +select * from {{ref("ephemeral")}} diff --git a/test/integration/020_ephemeral_test/models-n/source_table.sql b/test/integration/020_ephemeral_test/models-n/source_table.sql new file mode 100644 index 00000000000..3c9ffd00ab3 --- /dev/null +++ b/test/integration/020_ephemeral_test/models-n/source_table.sql @@ -0,0 +1,12 @@ +{{ config(materialized='table') }} + +with source_data as ( + + select 1 as id + union all + select null as id + +) + +select * +from source_data diff --git a/test/integration/020_ephemeral_test/test_ephemeral.py b/test/integration/020_ephemeral_test/test_ephemeral.py index b916ad4f5d3..6bf5277a48d 100644 --- a/test/integration/020_ephemeral_test/test_ephemeral.py +++ b/test/integration/020_ephemeral_test/test_ephemeral.py @@ -1,7 +1,9 @@ from test.integration.base import DBTIntegrationTest, use_profile +import os +import re -class TestEphemeral(DBTIntegrationTest): +class TestEphemeralMulti(DBTIntegrationTest): @property def schema(self): return "ephemeral_020" @@ -20,6 +22,26 @@ def test__postgres(self): self.assertTablesEqual("seed", "dependent") self.assertTablesEqual("seed", "double_dependent") self.assertTablesEqual("seed", "super_dependent") + self.assertTrue(os.path.exists('./target/run/test/models/double_dependent.sql')) + with open('./target/run/test/models/double_dependent.sql', 'r') as fp: + sql_file = fp.read() + + 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 (' + 'select * from test_ephemeral_.seed' + '), __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'" + 'union all' + "select * from __dbt__CTE__base_copy where gender = 'Female'" + ');') + sql_file = "".join(sql_file.split()) + expected_sql = "".join(expected_sql.split()) + self.assertEqual ( sql_file, expected_sql ) + @use_profile('snowflake') def test__snowflake(self): @@ -32,6 +54,41 @@ def test__snowflake(self): ["SEED", "DEPENDENT", "DOUBLE_DEPENDENT", "SUPER_DEPENDENT"] ) + +class TestEphemeralNested(DBTIntegrationTest): + @property + def schema(self): + return "ephemeral_020" + + @property + def models(self): + return "models-n" + + @use_profile('postgres') + def test__postgres(self): + + results = self.run_dbt() + self.assertEqual(len(results), 2) + + self.assertTrue(os.path.exists('./target/run/test/models-n/root_view.sql')) + + with open('./target/run/test/models-n/root_view.sql', 'r') as fp: + sql_file = fp.read() + + 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 (' + 'select * from "dbt"."test_ephemeral_"."source_table"' + '), __dbt__CTE__ephemeral as (' + 'select * from __dbt__CTE__ephemeral_level_two' + ')select * from __dbt__CTE__ephemeral' + ');') + + sql_file = "".join(sql_file.split()) + expected_sql = "".join(expected_sql.split()) + self.assertEqual ( sql_file, expected_sql ) + class TestEphemeralErrorHandling(DBTIntegrationTest): @property def schema(self): diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py index 994b64bed72..bb414042ce2 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -1114,7 +1114,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(model_sql_path), 'unrendered_config': unrendered_model_config, }, @@ -1188,7 +1187,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(second_model_sql_path), 'unrendered_config': unrendered_second_config }, @@ -1264,7 +1262,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': '', 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': '', 'checksum': self._checksum_file(seed_path), 'unrendered_config': unrendered_seed_config, }, @@ -1301,7 +1298,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': AnyStringWith('count(*)'), 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': AnyStringWith('count(*)'), 'test_metadata': { 'namespace': None, 'name': 'not_null', @@ -1346,7 +1342,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': AnyStringWith('select 0'), 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': AnyStringWith('select 0'), 'test_metadata': { 'namespace': 'test', 'name': 'nothing', @@ -1390,7 +1385,6 @@ def expected_seeded_manifest(self, model_database=None): 'compiled_sql': AnyStringWith('count(*)'), 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': AnyStringWith('count(*)'), 'test_metadata': { 'namespace': None, 'name': 'unique', @@ -1587,7 +1581,6 @@ def expected_postgres_references_manifest(self, model_database=None): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(ephemeral_copy_path), 'unrendered_config': self.unrendered_model_config(materialized='ephemeral'), }, @@ -1645,7 +1638,6 @@ def expected_postgres_references_manifest(self, model_database=None): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [ANY], - 'injected_sql': ANY, 'checksum': self._checksum_file(ephemeral_summary_path), 'unrendered_config': self.unrendered_model_config(materialized='table'), }, @@ -1702,7 +1694,6 @@ def expected_postgres_references_manifest(self, model_database=None): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(view_summary_path), 'unrendered_config': self.unrendered_model_config(materialized='view'), }, @@ -1776,7 +1767,6 @@ def expected_postgres_references_manifest(self, model_database=None): 'compiled_sql': '', 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': '', 'checksum': self._checksum_file(seed_path), 'unrendered_config': self.unrendered_seed_config(), }, @@ -2047,7 +2037,6 @@ def expected_bigquery_complex_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(clustered_sql_path), 'unrendered_config': self.unrendered_model_config( cluster_by=['first_name'], @@ -2129,7 +2118,6 @@ def expected_bigquery_complex_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(multi_clustered_sql_path), 'unrendered_config': self.unrendered_model_config( cluster_by=['first_name', 'email'], @@ -2210,7 +2198,6 @@ def expected_bigquery_complex_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(nested_view_sql_path), 'unrendered_config': self.unrendered_model_config(materialized='view'), }, @@ -2246,7 +2233,6 @@ def expected_bigquery_complex_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(nested_table_sql_path), 'unrendered_config': self.unrendered_model_config(materialized='table'), }, @@ -2323,7 +2309,6 @@ def expected_bigquery_complex_manifest(self): 'compiled_sql': '', 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': '', 'checksum': self._checksum_file(seed_path), 'unrendered_config': self.unrendered_seed_config(), }, @@ -2459,7 +2444,6 @@ def expected_redshift_incremental_view_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(model_sql_path), 'unrendered_config': self.unrendered_model_config(bind=False, materialized='view'), }, @@ -2536,7 +2520,6 @@ def expected_redshift_incremental_view_manifest(self): 'compiled_sql': ANY, 'extra_ctes_injected': True, 'extra_ctes': [], - 'injected_sql': ANY, 'checksum': self._checksum_file(seed_path), 'unrendered_config': self.unrendered_seed_config(), }, @@ -2710,7 +2693,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'model'], - 'injected_sql': compiled_sql, 'meta': {}, 'name': 'model', 'original_file_path': model_sql_path, @@ -2799,7 +2781,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'second_model'], - 'injected_sql': compiled_sql, 'meta': {}, 'name': 'second_model', 'original_file_path': second_model_sql_path, @@ -2883,7 +2864,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'seed'], - 'injected_sql': '', 'meta': {}, 'name': 'seed', 'original_file_path': seed_path, @@ -2930,7 +2910,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'schema_test', 'not_null_model_id'], - 'injected_sql': AnyStringWith('id is null'), 'meta': {}, 'name': 'not_null_model_id', 'original_file_path': model_schema_yml_path, @@ -2985,7 +2964,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'schema_test', 'test_nothing_model_'], - 'injected_sql': AnyStringWith('select 0'), 'meta': {}, 'name': 'test_nothing_model_', 'original_file_path': model_schema_yml_path, @@ -3039,7 +3017,6 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'schema_test', 'unique_model_id'], - 'injected_sql': AnyStringWith('count(*)'), 'meta': {}, 'name': 'unique_model_id', 'original_file_path': model_schema_yml_path, @@ -3129,7 +3106,7 @@ def expected_postgres_references_run_results(self): }, }, 'compiled': True, - 'compiled_sql': ephemeral_compiled_sql, + 'compiled_sql': ephemeral_injected_sql, 'config': self.rendered_model_config(materialized='table'), 'sources': [], 'depends_on': { @@ -3146,7 +3123,6 @@ def expected_postgres_references_run_results(self): ], 'extra_ctes_injected': True, 'fqn': ['test', 'ephemeral_summary'], - 'injected_sql': ephemeral_injected_sql, 'meta': {}, 'name': 'ephemeral_summary', 'original_file_path': ephemeral_summary_path, @@ -3220,7 +3196,6 @@ def expected_postgres_references_run_results(self): 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'view_summary'], - 'injected_sql': view_compiled_sql, 'meta': {}, 'name': 'view_summary', 'original_file_path': view_summary_path, @@ -3308,7 +3283,6 @@ def expected_postgres_references_run_results(self): 'extra_ctes': [], 'extra_ctes_injected': True, 'fqn': ['test', 'seed'], - 'injected_sql': '', 'meta': {}, 'name': 'seed', 'original_file_path': seed_path, diff --git a/test/integration/062_defer_state_test/test_defer_state.py b/test/integration/062_defer_state_test/test_defer_state.py index 892ef863ee9..1d3a4fc7b8d 100644 --- a/test/integration/062_defer_state_test/test_defer_state.py +++ b/test/integration/062_defer_state_test/test_defer_state.py @@ -64,8 +64,8 @@ def run_and_defer(self): # with state it should work though results = self.run_dbt(['run', '-m', 'view_model', '--state', 'state', '--defer', '--target', 'otherschema']) - assert self.other_schema not in results[0].node.injected_sql - assert self.unique_schema() in results[0].node.injected_sql + assert self.other_schema not in results[0].node.compiled_sql + assert self.unique_schema() in results[0].node.compiled_sql with open('target/manifest.json') as fp: data = json.load(fp) diff --git a/test/unit/test_compiler.py b/test/unit/test_compiler.py index a176a5d3e1f..bf6195dfb76 100644 --- a/test/unit/test_compiler.py +++ b/test/unit/test_compiler.py @@ -119,7 +119,6 @@ def test__prepend_ctes__already_has_cte(self): compiled=True, extra_ctes_injected=False, extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql='select * from source_table')], - injected_sql='', compiled_sql=( 'with cte as (select * from something_else) ' 'select * from __dbt__CTE__ephemeral'), @@ -147,7 +146,6 @@ def test__prepend_ctes__already_has_cte(self): compiled_sql='select * from source_table', extra_ctes_injected=False, extra_ctes=[], - injected_sql='', checksum=FileHash.from_contents(''), ), }, @@ -168,7 +166,7 @@ def test__prepend_ctes__already_has_cte(self): self.assertEqual(result, manifest.nodes['model.root.view']) self.assertEqual(result.extra_ctes_injected, True) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, ('with __dbt__CTE__ephemeral as (' 'select * from source_table' '), cte as (select * from something_else) ' @@ -176,7 +174,7 @@ def test__prepend_ctes__already_has_cte(self): self.assertEqual( manifest.nodes['model.root.ephemeral'].extra_ctes_injected, - True) + False) def test__prepend_ctes__no_ctes(self): manifest = Manifest( @@ -204,7 +202,6 @@ def test__prepend_ctes__no_ctes(self): compiled=True, extra_ctes_injected=False, extra_ctes=[], - injected_sql='', compiled_sql=('with cte as (select * from something_else) ' 'select * from source_table'), checksum=FileHash.from_contents(''), @@ -230,7 +227,6 @@ def test__prepend_ctes__no_ctes(self): compiled=True, extra_ctes_injected=False, extra_ctes=[], - injected_sql='', compiled_sql=('select * from source_table'), checksum=FileHash.from_contents(''), ), @@ -254,7 +250,7 @@ def test__prepend_ctes__no_ctes(self): manifest.nodes.get('model.root.view')) self.assertTrue(result.extra_ctes_injected) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, manifest.nodes.get('model.root.view').compiled_sql) compiler = dbt.compilation.Compiler(self.config) @@ -268,7 +264,7 @@ def test__prepend_ctes__no_ctes(self): manifest.nodes.get('model.root.view_no_cte')) self.assertTrue(result.extra_ctes_injected) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, manifest.nodes.get('model.root.view_no_cte').compiled_sql) def test__prepend_ctes(self): @@ -298,7 +294,6 @@ def test__prepend_ctes(self): compiled=True, extra_ctes_injected=False, extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql='select * from source_table')], - injected_sql='', compiled_sql='select * from __dbt__CTE__ephemeral', checksum=FileHash.from_contents(''), ), @@ -323,7 +318,6 @@ def test__prepend_ctes(self): compiled=True, extra_ctes_injected=False, extra_ctes=[], - injected_sql='', compiled_sql='select * from source_table', checksum=FileHash.from_contents(''), ), @@ -347,13 +341,14 @@ def test__prepend_ctes(self): self.assertTrue(result.extra_ctes_injected) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, ('with __dbt__CTE__ephemeral as (' 'select * from source_table' ') ' 'select * from __dbt__CTE__ephemeral')) + print(f"\n---- line 349 ----") - self.assertTrue(manifest.nodes['model.root.ephemeral'].extra_ctes_injected) + self.assertFalse(manifest.nodes['model.root.ephemeral'].extra_ctes_injected) def test__prepend_ctes__cte_not_compiled(self): ephemeral_config = self.model_config.replace(materialized='ephemeral') @@ -397,7 +392,6 @@ def test__prepend_ctes__cte_not_compiled(self): raw_sql='select * from source_table', compiled=True, compiled_sql='select * from source_table', - injected_sql='select * from source_table', extra_ctes_injected=True, extra_ctes=[], checksum=FileHash.from_contents(''), @@ -426,7 +420,6 @@ 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')], - injected_sql='', compiled_sql='select * from __dbt__CTE__ephemeral', checksum=FileHash.from_contents(''), ), @@ -440,7 +433,7 @@ def test__prepend_ctes__cte_not_compiled(self): ) compiler = dbt.compilation.Compiler(self.config) - with patch.object(compiler, 'compile_node') as compile_node: + with patch.object(compiler, '_compile_node') as compile_node: compile_node.return_value = compiled_ephemeral result, _ = compiler._recursively_prepend_ctes( @@ -456,7 +449,7 @@ def test__prepend_ctes__cte_not_compiled(self): self.assertTrue(manifest.nodes['model.root.ephemeral'].compiled) self.assertTrue(result.extra_ctes_injected) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, ('with __dbt__CTE__ephemeral as (' 'select * from source_table' ') ' @@ -491,7 +484,6 @@ def test__prepend_ctes__multiple_levels(self): compiled=True, extra_ctes_injected=False, extra_ctes=[InjectedCTE(id='model.root.ephemeral', sql=None)], - injected_sql=None, compiled_sql='select * from __dbt__CTE__ephemeral', checksum=FileHash.from_contents(''), @@ -554,7 +546,7 @@ def test__prepend_ctes__multiple_levels(self): self.assertEqual(result, manifest.nodes['model.root.view']) self.assertTrue(result.extra_ctes_injected) self.assertEqualIgnoreWhitespace( - result.injected_sql, + result.compiled_sql, ('with __dbt__CTE__ephemeral_level_two as (' 'select * from source_table' '), __dbt__CTE__ephemeral as (' diff --git a/test/unit/test_contracts_graph_compiled.py b/test/unit/test_contracts_graph_compiled.py index 09297e03d48..ad01cdb50c0 100644 --- a/test/unit/test_contracts_graph_compiled.py +++ b/test/unit/test_contracts_graph_compiled.py @@ -74,10 +74,9 @@ def basic_compiled_model(): config=NodeConfig(), meta={}, compiled=True, - compiled_sql='select * from whatever', extra_ctes=[InjectedCTE('whatever', 'select * from other')], extra_ctes_injected=True, - injected_sql='with whatever as (select * from other) select * from whatever', + compiled_sql='with whatever as (select * from other) select * from whatever', checksum=FileHash.from_contents(''), unrendered_config={} ) @@ -183,10 +182,9 @@ def basic_compiled_dict(): 'columns': {}, 'meta': {}, 'compiled': True, - 'compiled_sql': 'select * from whatever', 'extra_ctes': [{'id': 'whatever', 'sql': 'select * from other'}], 'extra_ctes_injected': True, - 'injected_sql': 'with whatever as (select * from other) select * from whatever', + 'compiled_sql': 'with whatever as (select * from other) select * from whatever', 'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'}, 'unrendered_config': {} } @@ -375,10 +373,9 @@ def basic_compiled_schema_test_node(): config=TestConfig(severity='warn'), meta={}, compiled=True, - compiled_sql='select * from whatever', extra_ctes=[InjectedCTE('whatever', 'select * from other')], extra_ctes_injected=True, - injected_sql='with whatever as (select * from other) select * from whatever', + compiled_sql='with whatever as (select * from other) select * from whatever', column_name='id', test_metadata=TestMetadata(namespace=None, name='foo', kwargs={}), checksum=FileHash.from_contents(''), @@ -474,10 +471,9 @@ def basic_compiled_schema_test_dict(): 'columns': {}, 'meta': {}, 'compiled': True, - 'compiled_sql': 'select * from whatever', 'extra_ctes': [{'id': 'whatever', 'sql': 'select * from other'}], 'extra_ctes_injected': True, - 'injected_sql': 'with whatever as (select * from other) select * from whatever', + 'compiled_sql': 'with whatever as (select * from other) select * from whatever', 'column_name': 'id', 'test_metadata': { 'name': 'foo', diff --git a/test/unit/test_manifest.py b/test/unit/test_manifest.py index 87d474a1cda..376573ff1c1 100644 --- a/test/unit/test_manifest.py +++ b/test/unit/test_manifest.py @@ -38,7 +38,6 @@ REQUIRED_COMPILED_NODE_KEYS = frozenset(REQUIRED_PARSED_NODE_KEYS | { 'compiled', 'extra_ctes_injected', 'extra_ctes', 'compiled_sql', - 'injected_sql', }) @@ -482,7 +481,6 @@ def setUp(self): compiled=True, compiled_sql='also does not matter', extra_ctes_injected=True, - injected_sql=None, extra_ctes=[], checksum=FileHash.empty(), ), @@ -508,7 +506,6 @@ def setUp(self): compiled=True, compiled_sql='also does not matter', extra_ctes_injected=True, - injected_sql='and this also does not matter', extra_ctes=[], checksum=FileHash.empty(), ),