ClickHouse / ClickHouse/dbt-clickhouse

Add missing Snapshot features, fix current issues and increase test coverage

Open
#662 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
362
Forks
176
Avg merge
2d 10h
Merged PRs (30d)
8

Description

Base issue to summarize and discuss all the pending work needed to improve the current Snapshot status:

**About changes needed to make Snapsots work:**
To make Snapshots work we needed to reimplement some of the SQL macros related to this materialisation. The reasons to do this are the following ones:
- `clickhouse__snapshot_merge_sql`: ClickHouse has **no `MERGE`/`UPDATE`**. The target is rebuilt via `insert` + `EXCHANGE TABLES` (or drop+rename).
- `build_snapshot_staging_table`: (non-dispatched) Uses a **non-temporary** staging table (`create_table_as(False, …)`) because the rebuild reads it across several separate statements; ClickHouse temp tables don't persist that way.
- `clickhouse__snapshot_hash_arguments`: `halfMD5(... cast as varchar ...)` instead of `md5` needed as `md5` is not a CH function. MD5 is but it doesn't return needed hex text. As we need a 32-char string, halfMD5 do the trick.
- `clickhouse__post_snapshot`: drop the staging relation.
- `clickhouse__snapshot_staging_table`:
- `snapshot_time` CTE (check strategy): pins `now()` so it is consistent across the whole query

We need to keep this documented in the code so we have all the context on these decissions.

**Missing features:**
Since we added the current Snapshot implementation, the code in dbt-core has evolved but we have not kept up with all the new added features. Some of them are:
- [ ] Multi-column `unique_key` https://github.com/ClickHouse/dbt-clickhouse/issues/544
- [ ] Different `dbt_valid_to`/`dbt_scd_id` literals
- [ ] `snapshot_meta_column_names` https://github.com/ClickHouse/dbt-clickhouse/issues/571
- [ ] `dbt_valid_to_current` https://github.com/ClickHouse/dbt-clickhouse/issues/481 https://github.com/ClickHouse/dbt-clickhouse/pull/630
- [ ] `hard_deletes: new_record` https://github.com/ClickHouse/dbt-clickhouse/issues/562

There are a few base tests that tests these values and we are not inherited them:
- [ ] `BaseSimpleSnapshot` and `BaseSnapshotCheck`: includes hard_deletes/revives
- [ ] `BaseSnapshotNewRecordDbtValidToCurrent`: Exercises `hard_deletes: new_record` + `dbt_valid_to_current` + check strategy
- [ ] `BaseSnapshotNewRecordTimestampMode` and `BaseSnapshotNewRecordCheckMode`: Check new records with both strategies

**Existing bugs:**
- [ ] Hard-delete invalidations are not correctly working. Seems like we need to add `settings join_use_nulls = 1` to make them correctly work. https://github.com/ClickHouse/dbt-clickhouse/issues/291. This is activated with `invalidate_hard_deletes=True`
- [ ] Temporal tables not removed before a new snapshot starts https://github.com/ClickHouse/dbt-clickhouse/issues/600
- [ ] `database_engine` set as None causing Snapshot to fail https://github.com/ClickHouse/dbt-clickhouse/issues/475

Contributor guide

Open the contributing guide

Research direction

Start by auditing the listed snapshot macros, including clickhouse__snapshot_merge_sql, clickhouse__snapshot_hash_arguments, clickhouse__post_snapshot, and clickhouse__snapshot_staging_table. Review the referenced dbt-core base tests, especially BaseSimpleSnapshot and BaseSnapshotCheck and the BaseSnapshotNewRecord* cases. Done requires resolving the selected missing features and bugs and increasing coverage for the affected snapshot behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, sql
Domain
databases
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.