dbt-bigquery: [ADAP-769] [Regression] bq jobs labeling doesn't work on 1.6.0

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

Hello!

I have a macro that reads a list of environment variables and builds a dictionary that later is being caught by query-comment.job-label feature.

macros/bq_labels.sql:

{% macro bq_labels() %}{
    "system": "{{ env_var('LABEL_SYSTEM', 'airflow') }}",
    "owner": "{{ env_var('LABEL_OWNER', 'unknown') }}",
    "envtype": "{{ env_var('LABEL_ENV', 'dev') }}",
    "dag_id": "{{ env_var('LABEL_DAG_ID', 'unknown') }}"
}{% endmacro %}

dbt_project.yml:

...
macro-paths:
  - macros
query-comment:
  comment: '{{ bq_labels() }}'
  job-label: true
  append: true

It works perfectly when using dbt-bigquery==1.5.3. When I run the bq show command for my job, I can see the following output in the “Labels” column:

owner:monideep
system:airflow
envtype:local
dbt_invocation_id:58a6c0f6-3001-4c2c-8a73-84847f95662d
dag_id:demo-dag_test_dbt_dag_mde

But when using 1.6.0, it looks like it can’t “parse” the macro. The output is the following:

dbt_invocation_id:fbf2d221-dd52-4baf-851a-9de0933689e5
query_comment:___bq_labels_____

Expected Behavior

The labels are parsed correctly, like in 1.5.3

Steps To Reproduce

Create a macro as described
Update dbt_project.yml as described
Run dbt run command

Relevant log output

No response

Environment

- OS: macos 13.4.1, ubuntu 20.04
- Python: 3.8
- dbt-core: 1.6.0
- dbt-bigquery: 1.6.0

Additional Context

No response

About this issue

Original URL
State: closed
Created a year ago
Reactions: 7
Comments: 21 (16 by maintainers)

Most upvoted comments

Hi, Based on the previous investigations on this issue and by exploring the code of dbt-core and dbt-bigquery, I might have found a fix with #955 (I basically reverted the 2 lines previously identified and added checks to make sure the value is not null).

@mikealfare Given that I copy-pasted your test highlighting the regression with a minor modification (the labels should be a valid JSON so I removed the trailing comma), I included you in the Changie entry.

Let me know if you think this solution isn’t valid.

kodaho on Oct 5, 2023

@mikealfare @dbeatty10 I have worked on this in a local environment and have been able to get it working again. I believe that these two lines are the culprit and reverted them back to before the change mentioned to get the job-labels working again.

Firstly, this change talks about adding a limit to the execute signature to align with dbt-core; which in itself is absolutely fine. But I don’t think changing these two lines were necessary to achieve that and an oversight given the below reasoning.

The implementation of query_header.comment.query_comment is different to profile.query_comment in that the query_header version implements MacroQueryStringSetter which seems to be performing some security checks around SQL injection within a query comment after obtaining it from the profile.query_comment rather than just pulling directly. It seems odd to have removed these checks?

I feel I PR to change the two lines in question back to their original state will fix things. What do people think?

RGreen90 on Aug 25, 2023