ClickHouse / ClickHouse/dbt-clickhouse
Add missing Snapshot features, fix current issues and increase test coverage
- 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
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