dbt-labs / dbt-labs/dbt

[spike+] option to generate dbt_scd_id as an integer column instead of a string for performance improvements

Open
#10,300 1 comment 0 reactions 0 assignees View on GitHub
engine:v1 performance snapshots type:feature
Dominant language
Rust
Stars
13.8k
Forks
2.6k
Avg merge
21h 31m
Merged PRs (30d)
56

Description

### Is this your first time submitting a feature request?

- [X] I have read the [expectations for open source contributors](https://docs.getdbt.com/docs/contributing/oss-expectations)
- [X] I have searched the existing issues, and I could not find an existing issue for this feature
- [X] I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

### Describe the feature

When dbt generates your snapshot, one of the meta-fields it creates is `dbt_scd_id` - a unique key generated for each snapshotted record, used internally by dbt.

Currently, `dbt_scd_id` is a string because of the hashing function used. `dbt_scd_id` is a combo of `unique_key` + `updated_at` for `timestamp` strategy (essentially creates a surrogate key).

Some folks want `dbt_scd_id` to instead be an integer for better performance. Specifically, it’s the performance of the MERGE in the snapshot process itself that’s the question.

See [here](https://github.com/dbt-labs/dbt-adapters/blob/cc526ba10b06d3fc966427f16f054beef4387b09/dbt/include/global_project/macros/materializations/snapshots/snapshot_merge.sql#L14).

```
merge into {{ target.render() }} as DBT_INTERNAL_DEST
using {{ source }} as DBT_INTERNAL_SOURCE
on DBT_INTERNAL_SOURCE.{{ columns.dbt_scd_id }} = DBT_INTERNAL_DEST.{{ columns.dbt_scd_id }}
```

If we swapped to an integer, we’d have to use a hashing function that outputted an integer instead of a string.
- integers collide a lot more easily than a string
- if we get a collision, we get unintended behavior (will fail silently!)

Because of the risk of collision, we should consider how to proceed (behavior change flag, new config, etc.).

### Game plan

Let's:
- spiking if there’s a perf improvement on this `MERGE` when `dbt_scd_id` is an integer (instead of a string)
- my understanding is that this isn’t as painful on snowflake as it is for other data warehouses (Databricks, Redshift, Bigquery)
- if there is a significant performance improvement, decide if we should:
- make this the default for certain warehouses moving forward (behind a behavior change flag)
- make this a new configuration to allow you to control the hashing function used when generating `dbt_scd_id`
- do the math on the risk of collision (int hash vs string)

### Describe alternatives you've considered

Creating a custom materialization to override the outputs from `generate_surrogate_key`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.