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-575] [Bug] 1.0.0 Cannot execute an empty query #5183

Closed
1 task done
Fouad-Kichauat opened this issue Apr 28, 2022 · 7 comments
Closed
1 task done

[CT-575] [Bug] 1.0.0 Cannot execute an empty query #5183

Fouad-Kichauat opened this issue Apr 28, 2022 · 7 comments
Labels
bug Something isn't working postgres Team:Adapters Issues designated for the adapter area of the code

Comments

@Fouad-Kichauat
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

dbt is errorring with cannot execute and empty query for a model that does have columns description in a schema.yml file, and having persist_docs: {columns: true} configured.

Database Error in model my_third_dbt_model (models/example/my_third_dbt_model.sql)

Expected Behavior

No response

Steps To Reproduce

  1. dbt init
  2. Create a third simple model with column description: e.g.: models/example/my_third_dbt_model.sql
select *
from {{ ref('my_second_dbt_model') }}
  1. Add the documentation of newly created model to schema.sql: i.e: models/example/schema.sql
  - name: my_third_dbt_model
    description: "An additional starter dbt model"
    columns:
      - name: Id
        description: 'The primary key for this table'
        tests:
          - unique
          - not_null
  1. Enable column-level docs persistence; i.e.: dbt_project.yml
models:
  ...
  +persist_docs:
    relation: true
    columns: true
  1. Run dbt run and observe the error

Relevant log output

No response

Environment

- OS: MacOS
- Python: Python 3.9.9
- dbt: 1.0.0

What database are you using dbt with?

postgres

Additional Context

No response

@Fouad-Kichauat Fouad-Kichauat added bug Something isn't working triage labels Apr 28, 2022
@github-actions github-actions bot changed the title [Bug] 1.0.0 Cannot execute an empty query [CT-575] [Bug] 1.0.0 Cannot execute an empty query Apr 28, 2022
@jtcohen6 jtcohen6 added the Team:Adapters Issues designated for the adapter area of the code label Apr 28, 2022
@muscovitebob
Copy link

Seems related to #2439 but in reverse.

@jtcohen6
Copy link
Contributor

@Fouad-Kichauat Thanks for opening, and for the clear reproduction case!

This is due to the mismatched casing for your column name: Id in your .yml file, and yet id in your model (flowing through from my_first_dbt_model):

If you change your Id to id, the issue goes away.

Better error?

One straightforward change we could make: Check to ensure that

{% do run_query(alter_column_comment(relation, model.columns)) %}

That would still result in the column comment not being persisted, but it would at least avoid the "can't execute an empty query" error. Instead, it would fail silently. Is that actually better...?

Case-insensitive comparison?

This just came up over on dbt-bigquery as well: dbt-labs/dbt-bigquery#169

For databases that retain case-sensitive names, I think it's reasonable for dbt to expect users to match case as well. For instance, on Postgres, this is possible:

jerco=# create table dbt_jcohen.my_wonky_tbl as (select 1 as "id", 2 as "ID", 3 as "Id");
SELECT 1
jerco=# select * from dbt_jcohen.my_wonky_tbl;
 id | ID | Id
----+----+----
  1 |  2 |  3
(1 row)

We could add case-insensitive comparison logic here, but then how could we handle the case above?

Update docs

My inclination is to say, our best bet is to update the docs on persist_docs, to really drive home that column names must match across yaml + sql, with regard to their casing as well: https://docs.getdbt.com/reference/resource-configs/persist_docs

@muscovitebob
Copy link

Is it possible to warn when the yml contains columns which are not created by the model sql file I wonder?

@Fouad-Kichauat
Copy link
Author

Fouad-Kichauat commented Apr 28, 2022

@jtcohen6 Thank for the fast reply!

Using the prior example and matching the exact case of the source column solves the issue for the demo code.
But in our current project it does not. To better replicate the issue we are facing, following changes need to be made to the above mentioned demo:

models/example/schema.sql

models:
  - name: my_third_dbt_model
    description: "An additional starter dbt model"
    columns:
      - name: Id
        description: 'The primary key for this table'
        tests:
          - unique
          - not_null
      - name: SecondColumn

models/example/my_third_dbt_model.sql

select id as Id,
'test' as SecondColumn
from {{ ref('my_second_dbt_model') }}

EDIT:
Further examination shows that if you use Id as your column name, this issue pops-up.
Using id mitigates that. So should we always use id as the default id field in postgres?

@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 28, 2022

It all depends on whether you're quoting the column name or not. In Postgres, column names are lowercased by default, but they retain case-sensitivity if they're quoted.

If you change your model to quote the column names, this should now work:

select id as "Id",
'test' as "SecondColumn"
from {{ ref('my_second_dbt_model') }}
models:
  - name: my_third_dbt_model
    description: "An additional starter dbt model"
    columns:
      - name: Id
        quote: true  # note this is necessary too
        description: 'The primary key for this table'
        tests:
          - unique
          - not_null
      - name: SecondColumn
        quote: true
        description: 'Another column'

To be clear, I don't recommend this. It's all much simpler if you lowercase (and snake case) column names across the board:

models:
  - name: my_third_dbt_model
    description: "An additional starter dbt model"
    columns:
      - name: id
        description: 'The primary key for this table'
        tests:
          - unique
          - not_null
      - name: second_column
select id,
'test' as second_column
from {{ ref('my_second_dbt_model') }}

@Fouad-Kichauat
Copy link
Author

Thank you for the many clarifications!! We can close the issue now.

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

No branches or pull requests

3 participants