Repository navigation
chore: updates for openflow migration - #115
Conversation
|
|
||
| _SNOWFLAKE_DELETED: | ||
| name: _FIVETRAN_DELETED | ||
| expression: COALESCE(_SNOWFLAKE_DELETED, FALSE) |
There was a problem hiding this comment.
(MINOR)
The COALESCE in this example never changes a value in the generated views.
Schema Builder applies the SOFT_DELETE filter to the raw column. With _SNOWFLAKE_DELETED: =FALSE, the WHERE clause keeps only rows where the raw value is FALSE. SQL runs WHERE before SELECT, so no NULL rows reach the COALESCE.
Snowflake's Openflow connector docs recommend the same filter for active rows: SELECT * FROM my_table WHERE _SNOWFLAKE_DELETED = FALSE; (MySQL, PostgreSQL). That implies live rows are FALSE, not NULL.
Suggestion: remove the COALESCE from this example and from the get_column_renames() docstring in dbt_schema_builder/builder.py. Then the column_renames.yml in warehouse-transforms won't copy it.
Have you seen NULLs in _SNOWFLAKE_DELETED? If so, the example should pair the rename with SOFT_DELETE: _SNOWFLAKE_DELETED: IS DISTINCT FROM TRUE, because =FALSE drops those rows.
seungjunone
left a comment
There was a problem hiding this comment.
LGTM!
I tested this offline against the real applications config, and the code works as intended. The 53 unit tests pass. With no column_renames.yml, the output is unchanged from main. The renames are correct in both the safe and PII views. Columns dropped at the source (__SNOWFLAKE_DELETED) no longer leak into the safe views unredacted, as they do on main.
Most of what I found is about configuration. A wrong schema_config.yml produces wrong views without failing the build:
- SOFT_DELETE must use _SNOWFLAKE_DELETED in Openflow blocks. If it keeps _FIVETRAN_DELETED, the view gets no WHERE clause and soft-deleted rows show up.
- For a partial cutover, each table must be in exactly one raw schema. Keep the Fivetran EXCLUDE list and the Openflow INCLUDE list in sync. Otherwise the raw schema listed last silently wins.
- The Openflow raw schema needs a different schema name from the Fivetran one, even in a different database. Otherwise every table in the app points at the last database.
Two other points to watch during the migration:
- The partner exports depend on column order. ent_report_pearson_block_completion and ent_report_herovired_block_completion select * and are exported as CSV files. When LMS moves, their column order changes and _SNOWFLAKE_INSERTED_AT appears. They should get explicit column lists first, agreed with the enterprise team.
- _SNOWFLAKE_UPDATED_AT is TIMESTAMP_NTZ. The docs example casts it with ::TIMESTAMP_TZ. That cast may use the session time zone, so please confirm the dbt runs use UTC.
As a follow-up, Schema Builder could turn the three config mistakes into build errors. The full test report, with the generated SQL and the scripts, is here: ⧉ https://claude.ai/artifact/CHX6L3h4LNQagUao31E65B
Description: Describe in a couple of sentences what this PR adds
JIRA: Link to JIRA ticket
Dependencies: dependencies on other outstanding PRs, issues, etc.
Merge deadline: List merge deadline (if any)
Installation instructions: List any non-trivial installation
instructions.
Testing instructions:
Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.