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 fix #180

Closed
Closed

Conversation

gfunc
Copy link
Contributor

@gfunc gfunc commented Aug 17, 2023

Summary

bug fix for distributed_table materialization

@CLAassistant
Copy link

CLAassistant commented Aug 17, 2023

CLA assistant check
All committers have signed the CLA.

@@ -46,7 +46,7 @@
{% elif existing_relation.can_exchange %}
-- We can do an atomic exchange, so no need for an intermediate
{% call statement('main') -%}
{% do run_query(create_empty_table_from_relation(backup_relation, view_relation)) or '' %}
{{ create_empty_table_from_relation(backup_relation, view_relation) }}
{%- endcall %}
{% do exchange_tables_atomic(backup_relation, existing_relation) %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, we should modify exchange_tables_atomic for distributed tables. In my case(#179), the SYSTEM SYNC REPLICA on the distributed table bugged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the error message with the actual version.

  ...
�[0m12:10:06.320142 [debug] [Thread-1  ]: dbt_clickhouse adapter: Error running SQL: /* {"app": "dbt", "dbt_version": "1.4.6", "profile_name": "stage", "target_name": "clickhouse_stage", "node_id": "model.dbt_prototype_version.event"} */

    SYSTEM SYNC REPLICA 
  
    
    ON CLUSTER "default" default_db_test_dbt.event
  
�[0m12:10:06.321265 [debug] [Thread-1  ]: Timing info for model.dbt_prototype_version.event (execute): 2023-08-18 12:10:05.375159 => 2023-08-18 12:10:06.321126
�[0m12:10:06.364239 [debug] [Thread-1  ]: Database Error in model event (models/event.sql)
  Code: 36.
  DB::Exception: There was an error on [clickhouse-server..........]: Code: 36. DB::Exception: Table default_db_test_dbt.event (zzzzzzzz.............zzzzzzzz) is not replicated. (BAD_ARGUMENTS) (version 23.3.1.2823 (official build)). Stack trace:
  

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. IMO this is not only a bug for distributed_table materialization but for every other materialization using exchange_tables_atomic macro with cluster setting.

I will commit the change with more fixes.

@zli06160
Copy link
Contributor

I forked the project to make a better versioning on my side.

Can you please tell me how do you think about the following modifications?
https://github.com/zli06160/dbt-clickhouse/pull/1/files

It worked for me (multiple runs: well distributed, correct row counts).

@genzgd
Copy link
Contributor

genzgd commented Aug 22, 2023

I forked the project to make a better versioning on my side.

Can you please tell me how do you think about the following modifications? https://github.com/zli06160/dbt-clickhouse/pull/1/files

It worked for me (multiple runs: well distributed, correct row counts).

I think this fix makes sense -- @GUNC do you want to add it, or @zli06160 do you want to open a new PR?

@gfunc
Copy link
Contributor Author

gfunc commented Aug 22, 2023

I think @zli06160 you should open a PR for this fix. I will close this PR. I am currently working on cluster tests and distributed materialization tests.

@zli06160
Copy link
Contributor

@gfunc @genzgd I opened the #184.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants