Issue-5: Add support for creating change streams via config option for primary key #10
Conversation
| @@ -14,7 +14,8 @@ | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| {% macro decodable__create_table_as(temporary, relation, sql, primary_key=none) -%} | |||
| {% macro decodable__create_table_as(temporary, relation, sql) -%} | |||
There was a problem hiding this comment.
Removed the primary_key parameter as it was unused, and I think there is no way to set it automatically
| @@ -29,7 +29,8 @@ | |||
| {% set should_create = {'value': false} %} | |||
| {% if existing_relation is not none %} | |||
| {% set watermark = config.get('watermark') %} | |||
| {% if adapter.has_changed(sql, target_relation, watermark) or should_full_refresh() %} | |||
| {% set primary_key = config.get('primary_key') %} | |||
| {% if adapter.has_changed(sql, target_relation, watermark, primary_key) or should_full_refresh() %} | |||
There was a problem hiding this comment.
I think it is the right thing to do to consider the primary key field in this comparison (we also consider watermark, for example). However, I am a bit uncertain about the behavior in general: If changes to the target stream are detected, it is simply deleted and recreated. Is that intended behavior @gunnarmorling @rmetzger ?
There was a problem hiding this comment.
Recreation of changed streams is the desired behavior. I will document this in the README if it isn't there already.
nicoweidner
force-pushed
the
changestream_support
branch
from
ec06bfa
to
5ec51be
Compare
|
Thanks a lot for this PR. This is a very good improvement! I validated the change locally, and it works as expected. |
Resolves #5
Resolves #6
Forgot about this: Maybe the configured primary key also needs to be considered in the change detection logic (
dbt-decodable/dbt/adapters/decodable/impl.py
Line 380 in c6d344f