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

[Bug] Same temp table is used for different unit tests #222

Open
2 tasks done
afillatre opened this issue May 22, 2024 · 4 comments
Open
2 tasks done

[Bug] Same temp table is used for different unit tests #222

afillatre opened this issue May 22, 2024 · 4 comments
Labels
bug Something isn't working unit tests Issues related to built-in dbt unit testing functionality

Comments

@afillatre
Copy link

Is this a new bug in dbt-bigquery?

  • I believe this is a new bug in dbt-bigquery
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When some unit tests in different models have the same name (like test_compute_one_line), some tests can fail depending on racing conditions (threads > 1).
The temp table name is <test_name>__dbt_tmp, so 1 test can override the content of another one.

Expected Behavior

Unit tests should run in parallel, without impacting other one, regardless their naming.
ATM I'm forced to run my tests with --threads 1 to prevent the issue

Steps To Reproduce

  1. Create 2 models
  2. Create a unit test within each model, both with the same name
  3. Run the test with --threads > 1

Relevant log output

No response

Environment

- OS: OS X 14.4.1
- Python: 3.11
- dbt-core: 1.8.0
- dbt-bigquery: 1.8.1

Additional Context

No response

@afillatre afillatre added bug Something isn't working triage labels May 22, 2024
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 23, 2024

Thanks @afillatre, I'm going to transfer this to dbt-adapters because I believe it's relevant to adapters beyond just BigQuery.

Here's where we generate the temp name:

{%- set target_relation = this.incorporate(type='table') -%}
{%- set temp_relation = make_temp_relation(target_relation)-%}

I think the options are:

  1. Create a temporary table name including both the unit test name and the model it's testing, to avoid this collision
  2. Document that this is a known limitation of unit tests, and each unit test should have a globally unique name

@jtcohen6 jtcohen6 removed the triage label May 23, 2024
@jtcohen6 jtcohen6 transferred this issue from dbt-labs/dbt-bigquery May 23, 2024
@afillatre
Copy link
Author

Thanks @jtcohen6. I had that first solution in mind as well. However I do not know if there's a limitation regarding the table's name length in any supported database.

@jtcohen6
Copy link
Contributor

@afillatre On Postgres the max identifier length is 63 (which is very short!), so the postgres__make_relation_with_suffix macro (called by make_temp_relation) will hash names longer than 63.

If this is just a matter of passing both the model name + unit test name into make_temp_relation, a naïve version of that change might look like:

  {%- set model_test_identifier = model['unique_id'].split('.')[2:] | join('__') -%} -- returns model_name__unit_test_name
  {%- set target_relation = this.incorporate(type='table', path={"identifier": model_test_identifier}) -%}
  {%- set temp_relation = make_temp_relation(target_relation)-%}

@afillatre
Copy link
Author

Wonderful, thanks for the tips.

I made it work like that (overriding a macro):

{% macro bigquery__make_temp_relation(base_relation, suffix) %}
    {% set model_test_identifier = model['unique_id'].split('.')[2:] | join('__') %} -- returns model_name__unit_test_name
    {% set target_relation = this.incorporate(type='table', path={"identifier": model_test_identifier}) %}
    {{ return(target_relation) }}
{% endmacro %}

Should I create a PR in the bigQuery connector ?

@jtcohen6 jtcohen6 added the unit tests Issues related to built-in dbt unit testing functionality label May 28, 2024
@dbeatty10 dbeatty10 reopened this Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unit tests Issues related to built-in dbt unit testing functionality
Projects
None yet
Development

No branches or pull requests

3 participants