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

use adapter to execute sql #4987

Closed
wants to merge 16 commits into from
Closed

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Apr 1, 2022

part of trying to resolve bigquery testing issues in new pytest format

Description

trying to fix bigquery connection issue by changing function call to more vauge run_sql_with_adapter

allows Bigquery and hopefully all other adapter repos to use the same function to connect for these testing purposes ignoring restrictive things between adapters like cursor

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have added information about my change to be included in the CHANGELOG.

@cla-bot cla-bot bot added the cla:yes label Apr 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2022

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@kwigley kwigley force-pushed the fix-tests-for-bigquery-incremental branch from bc605cf to eae14e1 Compare April 1, 2022 20:18
@kwigley
Copy link
Contributor

kwigley commented Apr 1, 2022

We are still running into issues when using this branch with dbt-bigquery. See the query below

with diff_count as (
    SELECT
        1 as id,
        COUNT(*) as num_missing FROM (
HERE ---->  (SELECT  FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.one_str__overwrite EXCEPT DISTINCT
             SELECT  FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.str_unique_key)
             UNION ALL
            (SELECT  FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.str_unique_key EXCEPT DISTINCT
             SELECT  FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.one_str__overwrite)
        ) as a
), table_a as (
    SELECT COUNT(*) as num_rows FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.one_str__overwrite
), table_b as (
    SELECT COUNT(*) as num_rows FROM dbt-dev-168022.test16488446899771240366_test_incremental_unique_id.str_unique_key
), row_count_diff as (
    select
        1 as id,
        table_a.num_rows - table_b.num_rows as difference
    from table_a, table_b
)
select
    row_count_diff.difference as row_count_difference,
    diff_count.num_missing as num_mismatched
from row_count_diff
join diff_count using (id)

Notice there are no columns for the select statement. We are somehow getting an empty list of columns for the relation when generating sql to compare tables here

sql = self._assert_tables_equal_sql(relation_a, relation_b)

@McKnight-42 McKnight-42 requested review from gshank and ChenyuLInx April 1, 2022 20:52
@McKnight-42
Copy link
Contributor Author

McKnight-42 commented Apr 1, 2022

@gshank @ChenyuLInx would love your thoughts on this, me and @kwigley were trying to get bigquery working with the conversion for unique_id tests.

#4778 this seems possibly related

@gshank
Copy link
Contributor

gshank commented Apr 1, 2022

I'm removing dbt.tests.tables in my current branch in pr #4986. That tables.py code should not be used in any tests going in the adapter zone. The 'check_relations_equal' code in dbt.tests.util is much more likely to work.

@gshank
Copy link
Contributor

gshank commented Apr 1, 2022

Sorry I didn't notice that TableComparison was being used in your other pull request.

@McKnight-42 McKnight-42 self-assigned this Apr 4, 2022
@McKnight-42
Copy link
Contributor Author

McKnight-42 commented Apr 4, 2022

@gshank, @ChenyuLInx I've made the swap to check_relations_equal as you suggested in the dbt-core and tests are yet again passing on postgres, redshift, and snowflake but still getting similar error to that of the one this message points out. where we are not adding a column after the select when testing against bigquery.

@gshank
Copy link
Contributor

gshank commented Apr 4, 2022

It looks like you get an empty list of columns if BigQuery doesn't find the table. You might want to look at how the relations are defined in 'check_relations_equal'. It could be that we need to tweak how 'relation_from_name' works?

@ChenyuLInx
Copy link
Contributor

@McKnight-42 and I looked at it the other day and noticed that the issue was caused by some models didn't finish correctly. We should probably make it a habit to check run results.

@McKnight-42
Copy link
Contributor Author

seem to have found the issue with casting this out so closing for now.

@McKnight-42 McKnight-42 closed this Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants