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-2420] [Bug] Attribute quote on a column not used on constraint generation #7370

Closed
2 tasks done
Tracked by #7372
lustefaniak opened this issue Apr 17, 2023 · 2 comments · Fixed by #7537
Closed
2 tasks done
Tracked by #7372

[CT-2420] [Bug] Attribute quote on a column not used on constraint generation #7370

lustefaniak opened this issue Apr 17, 2023 · 2 comments · Fixed by #7537
Assignees
Labels
bug Something isn't working model_contracts Team:Adapters Issues designated for the adapter area of the code
Milestone

Comments

@lustefaniak
Copy link

lustefaniak commented Apr 17, 2023

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

It is possible to define quote: true on a column, so e.g. from will work. But when adding data_type constraint it won't be correctly quoted creating invalid SQL (at least for Bigquery):

    select * from (
        select
    cast(null as int) as from, cast(null as int) as id, cast(null as int) as currency, cast(null as int) as currency_fullname, cast(null as datetime) as timestamp
    ) as __dbt_sbq
    where false
    limit 0

Expected Behavior

    select * from (
        select
    cast(null as int) as `from`, cast(null as int) as id, cast(null as int) as currency, cast(null as int) as currency_fullname, cast(null as datetime) as timestamp
    ) as __dbt_sbq
    where false
    limit 0

Steps To Reproduce

models:
  - name: txs_large
    config:
      materialized: table
      contract:
        enforced: true
    constraints:
      - type: primary_key
        columns: [id]
    columns:
      - name: from
        quote: true
        data_type: int
        tests:
          - not_null

Relevant log output

❯ dbt build
14:34:04  Running with dbt=1.5.0-rc1
14:34:04  [WARNING]: Configuration paths exist in your dbt_project.yml file which do not apply to any resources.
There are 1 unused configuration paths:
- models.mini-dbt.example
14:34:04  Found 6 models, 7 tests, 1 snapshot, 1 analysis, 353 macros, 0 operations, 1 seed file, 1 source, 1 exposure, 2 metrics, 0 groups
14:34:04
14:34:06  Concurrency: 8 threads (target='dev')
14:34:06
14:34:06  1 of 15 START seed file mini_dbt.currencies .................................... [RUN]
14:34:10  1 of 15 OK loaded seed file mini_dbt.currencies ................................ [INSERT 8 in 4.30s]
14:34:10  2 of 15 START test not_null_currencies_currency ................................ [RUN]
14:34:10  3 of 15 START test unique_currencies_currency .................................. [RUN]
14:34:11  3 of 15 PASS unique_currencies_currency ........................................ [PASS in 0.98s]
14:34:12  2 of 15 PASS not_null_currencies_currency ...................................... [PASS in 1.77s]
14:34:12  4 of 15 START sql table model mini_dbt.txs_large ............................... [RUN]
14:34:13  4 of 15 ERROR creating sql table model mini_dbt.txs_large ...................... [ERROR in 0.92s]
14:34:13  5 of 15 SKIP test not_null_txs_large__from_ .................................... [SKIP]
14:34:13  6 of 15 SKIP test not_null_txs_large_currency .................................. [SKIP]
14:34:13  7 of 15 SKIP test not_null_txs_large_currency_fullname ......................... [SKIP]
14:34:13  8 of 15 SKIP test not_null_txs_large_id ........................................ [SKIP]
14:34:13  9 of 15 SKIP test not_null_txs_large_timestamp ................................. [SKIP]
14:34:13  10 of 15 SKIP relation mini_dbt.txs_large_czk .................................. [SKIP]
14:34:13  11 of 15 SKIP relation mini_dbt.txs_large_dkk .................................. [SKIP]
14:34:13  12 of 15 SKIP relation mini_dbt.txs_large_gbp .................................. [SKIP]
14:34:13  13 of 15 SKIP relation mini_dbt.txs_large_huf .................................. [SKIP]
14:34:13  14 of 15 SKIP relation mini_dbt.txs_large_pln .................................. [SKIP]
14:34:13  15 of 15 SKIP relation snapshots.txs_large_snapshot ............................ [SKIP]
14:34:13
14:34:13  Finished running 1 seed, 7 tests, 1 table model, 5 view models, 1 snapshot in 0 hours 0 minutes and 8.24 seconds (8.24s).
14:34:13
14:34:13  Completed with 1 error and 0 warnings:
14:34:13
14:34:13  Database Error in model txs_large (models/base/txs_large.sql)
14:34:13    Syntax error: Unexpected keyword FROM at [4:26]
14:34:13
14:34:13  Done. PASS=3 WARN=0 ERROR=1 SKIP=11 TOTAL=15

Environment

- OS: Macos
- Python: 3.11.3
- dbt: 1.5.0-rc1

Which database adapter are you using with dbt?

bigquery

Additional Context

No response

@lustefaniak lustefaniak added bug Something isn't working triage labels Apr 17, 2023
@github-actions github-actions bot changed the title [Bug] Attribute quote on a column not used on constraint generation [CT-2420] [Bug] Attribute quote on a column not used on constraint generation Apr 17, 2023
@jtcohen6
Copy link
Contributor

@lustefaniak Good catch! Thanks for opening the bug report.

I think this is the line in the get_empty_schema_sql macro that needs updating, for the "pre-flight" check (column names + data types), which is where you're seeing the error currently:

cast(null as {{ col['data_type'] }}) as {{ col['name'] }}{{ ", " if not loop.last }}

Although we'd also want to take a look at the code that renders out the eventual DDL, including column name + data type + constraints (if any), which is the (Python) adapter method render_raw_columns_constraints:

rendered_column_constraint = [f"{v['name']} {v['data_type']}"]

IMO we do NOT need or want to support a contracted model with columns that are identical, save for a difference of quoting/casing (e.g. "from" and "FROM"). So our equality checks can remain case-insensitive and unquoted.

Exit criteria:

  • It's possible to define a contracted model with a column name that's a reserved word (e.g. from) that requires quote: true

@jtcohen6 jtcohen6 added Team:Language Team:Adapters Issues designated for the adapter area of the code model_contracts and removed triage labels Apr 17, 2023
@jtcohen6 jtcohen6 added this to the v1.5.x milestone Apr 18, 2023
@emmyoop
Copy link
Member

emmyoop commented May 3, 2023

Create a test just for postgres. Manually ensure the other adapters continue to work but no tests needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working model_contracts Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants