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

[CT-2157][Regression] Bug between adapter.get_relation & dbt_utils._is_relation #7024

Closed
2 tasks done
patkearns10 opened this issue Feb 21, 2023 · 12 comments
Closed
2 tasks done
Assignees
Labels
bug Something isn't working Team:Adapters Issues designated for the adapter area of the code

Comments

@patkearns10
Copy link

Is this a regression in a recent version of dbt-core?

  • I believe this is a regression in dbt-core functionality
  • I have searched the existing issues, and I could not find an existing issue for this regression

Current Behavior

code snippets commonly used in the audit-helper workflow, adapter.get_relation now cause errors:

{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema="dbt_pkearns",
      identifier="customers"
) -%}


{{ dbt_utils.get_filtered_columns_in_relation(from=old_etl_relation) }}

Screen Shot 2023-02-22 at 10 06 07 AM

  • get_relation creates a relation from a fully qualified table name
  • pass relation to the dbt_utils.get_filtered_columns_in_relation macro, where dbt_utils._is_relation says this is not a relation.
  • returns error message from adapter._is_relation (code)

Expected/Previous Behavior

image

In a previous version, this returned no errors.

dbt 1.1
dbt_utils 0.8.4

Steps To Reproduce

In latest version (or anything from dbt 1.2 & dbt_utils version: 0.9.0 onwards):
<replace schema and identifier with a table that does exist>

{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema="dbt_pkearns",
      identifier="customers"
) -%}


{{ dbt_utils.get_filtered_columns_in_relation(from=old_etl_relation) }}

Swap to version dbt 1.1 & dbt_utils 0.84 and it will run successfully.

Relevant log output

Compilation Error in rpc request (from remote system) Macro get_filtered_columns_in_relation expected a Relation but received the value: None > in macro _is_relation (macros/jinja_helpers/_is_relation.sql) > called by macro default__get_filtered_columns_in_relation (macros/sql/get_filtered_columns_in_relation.sql) > called by macro get_filtered_columns_in_relation (macros/sql/get_filtered_columns_in_relation.sql) > called by rpc request (from remote system)

Environment

- OS:
- Python:
- dbt (working version):1.1
- dbt (regression version):1.2

Which database adapter are you using with dbt?

snowflake

Additional Context

Slack thread:
https://dbt-labs.slack.com/archives/C010X3HKX2L/p1676945381425329

So, it seems like this code:

{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema="dbt_pkearns",
      identifier="customers"
) -%}

to “get a relation” is returning a thing that _is_relation does not consider to be a relation.

so we could fix adapter.get_relation to return something that _is_relation agrees with
or we could fix _is_relation to be more lenient, based on whatever get_relation now returns

Used some logic from this old issue to diagnose the _is_relation failure as well:
Stealing some logic from this old PR: #2938
image

@patkearns10 patkearns10 added bug Something isn't working triage regression labels Feb 21, 2023
@github-actions github-actions bot changed the title [Regression] <title> [CT-2157] [Regression] <title> Feb 21, 2023
@patkearns10 patkearns10 changed the title [CT-2157] [Regression] <title> [Regression] Bug between adapter.get_relation & dbt_utils._is_relation Feb 21, 2023
@jtcohen6 jtcohen6 added the Team:Adapters Issues designated for the adapter area of the code label Feb 22, 2023
@Fleid Fleid changed the title [Regression] Bug between adapter.get_relation & dbt_utils._is_relation [CT-2157][Regression] Bug between adapter.get_relation & dbt_utils._is_relation Feb 22, 2023
@Fleid
Copy link
Contributor

Fleid commented Feb 22, 2023

I can observe all of that, but have got no clue what to do with it.
Looping in someone more clever to decide what to do... @nathaniel-may

@Fleid Fleid added Refinement Maintainer input needed and removed triage labels Feb 22, 2023
@mikealfare mikealfare self-assigned this Mar 6, 2023
@mikealfare
Copy link
Contributor

@patkearns10, thanks for doing all of that investigative work! I don't know that the issue is in _is_relation(); it might be in get_relation(). My knee jerk reaction is that this smells like a case sensitivity change. get_relation() might not return a relation because it might (need to confirm) be case sensitive. And from your last screen shot, the name of the relation is in all caps. I started digging in and will continue, but if you have your example handy, could you try changing the initial call to all caps and see if it works?

@mikealfare
Copy link
Contributor

@patkearns10 One more question, what's your quoting config set to?

@patkearns10
Copy link
Author

I do not have any quoting set in my project. And when I switch to all caps, still fails:
Screen Shot 2023-03-08 at 11 21 24 AM
Screen Shot 2023-03-08 at 11 30 19 AM

...

This is actually weird though...
_is_relation runs two tests
https://github.com/dbt-labs/dbt-utils/blob/main/macros/jinja_helpers/_is_relation.sql

{% macro _is_relation(obj, macro) %}
    {%- if not (obj is mapping and obj.get('metadata', {}).get('type', '').endswith('Relation')) -%}
        {%- do exceptions.raise_compiler_error("Macro " ~ macro ~ " expected a Relation but received the value: " ~ obj) -%}
    {%- endif -%}
{% endmacro %}

if I do those individually, it passes,

Screen Shot 2023-03-08 at 11 33 05 AM
Screen Shot 2023-03-08 at 11 32 08 AM
Screen Shot 2023-03-08 at 11 31 31 AM
...
but within the macro it fails?
Screen Shot 2023-03-08 at 11 34 36 AM

@mikealfare
Copy link
Contributor

@patkearns10 I added some tests to troubleshoot. They're on this PR: dbt-labs/dbt-snowflake#504. But I can't get the tests to fail. I believe this macro summarizes your issue: https://github.com/dbt-labs/dbt-snowflake/blob/3dd1d4ca6a8e28d68dd3883a529056046182c393/tests/functional/adapter/get_relation_tests/macros.py#L26. Does that look right?

The test is basically executing the macro and seeing if it makes it through. The macro returns a valid SnowflakeRelation object, i.e. it passed through the if block where you're seeing the error.

@patkearns10
Copy link
Author

The logic looks right to me, so, I would have expected that to fail based on what we were seeing last week.

@mikealfare
Copy link
Contributor

@patkearns10 I added some additional tests. It looks like I can get this to fail if I create a state where the macro can run before the model is created (if nothing is tied together, it all gets created in parallel with no dependencies). Additionally, I've gotten both un-quoted and quoted versions of the relation name based on those scenarios. Could you do me one more favor and run your scenario after wrapping the double-quoted schema and identifier in single quotes? In other words:

{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema='"DBT_PKEARNS"',
      identifier='"CUSTOMERS"'
) -%}


{{ dbt_utils.get_filtered_columns_in_relation(from=old_etl_relation) }}

@patkearns10
Copy link
Author

I am seeing the same error there:
Screen Shot 2023-03-14 at 2 36 42 PM

@fix-a-thing
Copy link

I'm still facing this issue, did it get resolved? I see the above PR has passed tested but lacks a review and that was in March?

@fabryandrea
Copy link

I'm still facing this issue, did it get resolved? I see the above PR has passed tested but lacks a review and that was in March?

Me too. Are there any news on this? Thank you.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 8, 2023

So the immediate way to fix this is by wrapping the call to get_filtered_columns_in_relation, so that it doesn't get executed at parse time:

{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema="dbt_jcohen",
      identifier="customers"
) -%}

{% if execute %} {# or if old_etl_relation #}
    {{ dbt_utils.get_filtered_columns_in_relation(from=old_etl_relation) }}
{% endif %}

The reason is that, at parse time, get_relation returns None, because we haven't yet made a connection to the database. So we need to wrap get_filtered_columns_in_relation in either if execute or if old_etl_relation.

For my colleagues: Internal reference doc on the distinctions between parsing <> compilation" <> runtime (which I'm long overdue for turning into a public README in this repo).


I am pretty sure this is not, in fact, a regression. I get the same error if I use dbt-core v1.1 + dbt-utils v0.8.4:

-- models/my_models.sql
{% set old_etl_relation=adapter.get_relation(
      database=target.database,
      schema="dbt_jcohen",
      identifier="customers"
) -%}

{{ dbt_utils.get_filtered_columns_in_relation(from=old_etl_relation) }}
$ dbt deps
09:53:16  Running with dbt=1.1.4
09:53:17  Installing dbt-labs/dbt_utils
09:53:17    Installed from version 0.8.4
09:53:17    Updated version available: 1.1.1
09:53:17
09:53:17  Updates available for packages: ['dbt-labs/dbt_utils']
Update your versions in packages.yml, then run dbt deps

$ dbt compile -s my_model
09:53:21  Running with dbt=1.1.4
09:53:21  Unable to do partial parsing because a project config has changed
09:53:22  Encountered an error:
Compilation Error in model my_model (models/my_model.sql)
  Macro get_filtered_columns_in_relation expected a Relation but received the value: None

  > in macro _is_relation (macros/cross_db_utils/_is_relation.sql)
  > called by macro default__get_filtered_columns_in_relation (macros/sql/get_filtered_columns_in_relation.sql)
  > called by macro get_filtered_columns_in_relation (macros/sql/get_filtered_columns_in_relation.sql)
  > called by model my_model (models/my_model.sql)

I think there are two reasonable ways to fix this within dbt-utils:

I'm going to open a new issue over there, and close this one.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 8, 2023

Closing in favor of: dbt-labs/dbt-utils#833

@jtcohen6 jtcohen6 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
6 participants