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] Custom test name causes dbt test to fail when persisting failures with --store-failures #9752

Closed
2 tasks done
mkielar opened this issue Mar 12, 2024 · 3 comments
Closed
2 tasks done
Labels
bug Something isn't working stale Issues that have gone stale

Comments

@mkielar
Copy link

mkielar commented Mar 12, 2024

Is this a new bug in dbt-core?

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

Current Behavior

This relates to #9740. I have added names to my tests so that I could generate a human-readable report for my end-users. The tests are named like this:

columns:
  - name: node_id
      tests:
        - dbt_expectations.expect_column_values_to_be_unique:
          name: 'Expect "node_id" values to be unique.'

However, when I run dbt test --store-failures, the command wails with:

Database Error in test Expect "node_id" values to be unique.. (models/staging/[MASKED]/models.yml)
  Invalid table ID "Expect "node_id" values to be unique.".

Expected Behavior

I'd expect dbt to either use the "original" name of the test, or internally produce a name that is acceptable by my SQL Engine (in my case BigQuery) and actually store the results.

Steps To Reproduce

  1. Define a test with overridden name, and put whitespace and quotes in the name.
  2. Run dbt test --store-failures

Relevant log output

Database Error in test Expect "node_id" values to be unique.. (models/staging/[MASKED]/models.yml)
  Invalid table ID "Expect "node_id" values to be unique.".

Environment

- OS: fedoraremix, v.38, via Windows Subsystem for Linux, v.1.
- Python: 3.11.7
- dbt: 1.7.9

Which database adapter are you using with dbt?

bigquery

Additional Context

No response

@mkielar mkielar added bug Something isn't working triage labels Mar 12, 2024
@dbeatty10 dbeatty10 self-assigned this Mar 12, 2024
@dbeatty10 dbeatty10 changed the title [Bug] Custom test name causes "dbt test" to fail when persisting failures with "--store-failures" [Bug] Custom test name causes dbt test to fail when persisting failures with --store-failures Mar 12, 2024
@dbeatty10
Copy link
Contributor

Sorry you bumped up against this @mkielar -- thank you for opening this issue.

Short story

I believe the relevant section of code is here.

This isn't likely to be a high priority for us to address in the near-term, but you can work around it in the meantime by doing either of these in your YAML:

  1. Removing double quotes from your custom data test name, or
  2. Adding the relevant escape character for double quotes for SQL identifiers within your custom test name

Reprex

models/my_model.sql

select 1 as id, null as status

models/_models.yml

models:
  - name: my_model
    columns:
      - name: status
        tests:
          - not_null:
              name: 'Expect "node_id" values to be unique.'
              config:
                store_failures_as: view
dbt build -s my_model

More detail

Since it is affecting store_failures for you, I'd expect it to affect store_failures_as also.

This looks to have the same underlying behavior as described in dbt-labs/dbt-adapters#105 wherein the characters within the database identifier are not being escaped.

Acceptance criteria

High-level

Ideally, here's how it would behave depending if identifier quoting is true or false:

  • if true, then adapter-specific escaping is applied to the identifier value during the quoting process. This would allow any valid identifier to be used when creating tables, views, etc.
  • if false, then adapter-specific escaping is not applied to the identifier value and neither is any quoting.
    • Less ideal: a database error may result depending on the value of the identifier (i.e., any spaces will yield an error).
    • Better: if dbt can determine which values won't work and it preemptively raises a compilation error along with a helpful description so the user knows how to fix the issue.

Low-level

  • There exist adapter-specific test(s) that test out all the characters that are valid within an unquoted identifier.
    • Do the same for database and schemas.
  • There exist adapter-specific test(s) that test out all the characters that are valid within a quoted identifier
    • Do the same for database and schemas.
  • Implement adapter-specific escaping here or somewhere else.

Note

The character(s) used for quoted identifiers may vary by adapter. Likewise, the characters that are valid within an identifier may vary as well. Other rules like maximum length, etc. may also vary.

Copy link
Contributor

github-actions bot commented Sep 9, 2024

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Sep 9, 2024
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 16, 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 stale Issues that have gone stale
Projects
None yet
Development

No branches or pull requests

2 participants