dbt-datamocktool icon indicating copy to clipboard operation
dbt-datamocktool copied to clipboard

Failure to replace refs introduced by commit 5a4c311

Open Chizbro opened this issue 1 year ago • 3 comments

Describe the bug

__render_sql_and_replace_references will fail to replace refs with rendered input mappings when the ref in the model is not formatted with precisely one leading and one following space

Steps to reproduce

  • Create a model with a ref formatted without that spacing. e.g. {{ref('some_model')}}
  • Create a unit test for that model specifying an input mapping for ref('some_model')
  • Run the test

Expected results

The ref should be replaced successfully and the test should run

Actual results

__render_sql_and_replace_references fails to replace the ref due to the code below (lines 148 to 151) matching on the specific format {{ ref('') }} and not dealing with varying or missing whitespace between the ref call and the brackets

    {#- Replace the keys first, before the sql code is rendered -#}
    {% for k, v in ns.rendered_mappings.items() %}
        {% set ns.test_sql = ns.test_sql|replace("{{ "~render(k)~" }}", v) %}
    {% endfor %}

The standard dependency failure error is thrown from render dbt was unable to infer all dependencies for the model This typically happens when ref() is placed within a conditional block ... To fix this, add the following hint to the top of the model -- depends_on: {{ ref('some_model) }}

System information

The contents of your packages.yml file: The relevant information is that the issue was introduced in the above referenced commit, first present in ~v0.3.2~ v0.3.5. The issue does not occur in v0.3.1

Which database are you using dbt with?

  • [ ] postgres
  • [ ] redshift
  • [ ] bigquery
  • [ ] snowflake
  • [ x ] other (specify: databricks, but this isn't relevant for this issue )

The output of dbt --version:

We're still on 1.4.6 waiting for some library updates but this also isn't relevant to this issue

Are you interested in contributing the fix?

Sure!

Edits:

  • 2023-07-31 - Introduced in v0.3.5, not v0.3.2

Chizbro avatar Jul 28 '23 06:07 Chizbro

You can add the depends_on for that datamock test:

ex:

      input_mapping:
        ref('some_model'): ref('A_seed')
      expected_output: ref('model_expected')
      depends_on: ["-- depends_on: {{ ref('some_model') }}"]

NGL91 avatar Jul 31 '23 02:07 NGL91

@NGL91 that causes data mock tool to use the built some_model, and not the mock data defined in the input mapping, so causes the test to fail. The issue is with with the replace call for rendered mappings, as it will only recognise {{ ref('some_model') }} and not (e.g.) {{ ref('some_model')}}

One solution might be to use regex replace (~untested~ tested, but untidy, code to follow)

{% set re = modules.re %}
{% for k, v in ns.rendered_mappings.items() %}
    {% set ns.test_sql = re.sub( '\{\{[\s]*' ~ k.replace("(", "\(").replace(")", "\)") ~ '[\s]*\}\}', v, ns.test_sql ) %}
{% endfor %}

Edit: Replaced untested code with tested code

Chizbro avatar Jul 31 '23 03:07 Chizbro

To clarify on "that causes data mock tool to use the built some_model"

Because the ref hasn't been replaced with the seed (due to the whitespace issues mentioned) the compiled test uses the existing ref and not the seed data, resulting in this behaviour. I'm not trying to suggest that dbt_datamocktool is looking at the depends_on and doing anything specific in this case besides adding the comments to the compiled test

Chizbro avatar Jul 31 '23 05:07 Chizbro