check-script-has-no-table-name is failing when using lateral flatten

[MartinGuindon]

Describe the bug
See updated description of the bug.

The check-script-has-no-table-name pre-commit hook is confusing CTEs with tables, and fails with code like this:

with source as (

    select * from {{ source('stripe', 'payments') }}

),

renamed as (

    select
        id as payment_id,
        order_id,
        payment_method,

        --`amount` is currently stored in cents, so we convert it to dollars
        amount / 100 as amount

    from source

)

select * from renamed

It reports that "source" and "renamed" are tables even though they are not, even though it looks the same from a code perspective.

I think this hook should perhaps fail only at the presence of schema.table or database.schema.table references, unless we can make this hook smarter by being aware of the CTEs defined in the model.

Version:
v0.1.1

[tomsej]

Hi Martin, thanks for bug reporting! I appreciate you are trying pre-commit-dbt. Unfortunately I am not able to reconstruct your bug. What I did:

1. I created a new folder
2. created a new file test.sql and copy-pasted your code.
3. created following .pre-commit-config.yaml file:

repos:
- repo: https://github.com/offbi/pre-commit-dbt
  rev: v0.1.1
  hooks:
  - id: check-script-has-no-table-name

4. Ran git init and git add .
5. Ran pre-commit clean to start fresh.
6. Ran pre-commit run and it succeeded.

Can you try to run pre-commit clean (or pre-commit autoupdate) and then pre-commit run? In version 0.1.1 i fixed a small bug in check-script-has-no-table-name so if you firstly installed version 0.1.0 it did not have to be updated.

tldr; I do not think it is needed to use the presence of schema.table or database.schema.table.
What is the script currently doing is it splits sql to tokens with any whitespace and then:

• it tries to find all ctes - by finding token where the next sequence of tokens is ["as", "("]. This token is then considered as cte name.
• it tries to find all tables - by finding the token where the previous token is "from" or "join". If the next token is "(" (subselect) it is ignored. Also {{ source }} and {{ ref }} macros are ignored.
  Then the difference between tables and ctes are tables where macro for a table is not specified. This should be more robust than looking for presence of dot in table name.

[MartinGuindon]

Glad to hear that the parsing is actually CTE-aware. My apologies, I made assumptions and oversimplified the example model in the bug report. Upon closer inspection, I found the root cause. The error is actually caused by the usage of a lateral flatten function (Snowflake), I had not noticed the comma in the error message:

transform/dbt/models/staging/skuvault/stg_skuvault__purchase_orders.sql: does not use source() or ref() macros for tables:
 - source,

The code of that model looks like this:

with source as (
    select * from {{ source('aws_lambda', 'purchase_orders') }}
),

flattened as (
    select * from source, lateral flatten(line_items)
),

...

When I remove the lateral flatten, the script passes successfully.

[tomsej]

Ahhh it seems it parse table name as source, (with a comma) I was not aware of this option. Think the fix should be easy. I am going to release a new version by the end of the month, so will fix this bug too.

[tomsej]

Fixed in #11. If you do not wait for a new version of pre-commit-dbt you can run pre-commit autoupdate --bleeding-edge to update to HEAD.

[nizar2yas]

hi @tomsej ,
I update to head using pre-commit autoupdate --bleeding-edge but I'm still getting the same exception :

does not use source() or ref() macros for tables:
- {{ref