add and incorporate macro for turning jsons (and only jsons) into strings #129
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What change does this PR introduce?
get_json_columns_in_relation
to return a list of columns that are of type JSON (on BigQuery only, as we haven't rolled out JSON support in other Fivetran destinations).get_json_columns_in_relation
infill_staging_columns
, so that each JSON field gets converted to a json-formatted STRING. This is necessary because only some connectors have JSONs currently and downstream json functions do not work seamlessly with the actual JSON-type columns.fill_staging_columns
was chosen as this is called in every (most?) source package model. therefore, we don't have to mass update a bunch of packages. however, we want this to be right and not break anything!If this PR introduces a new macro, how did you test the new macro?
get_json_columns_in_relation
I first tested
get_json_columns_in_relation
by directly calling it in a local test model.the model i ran it against was just a select * from Joe's seed file
tiktok_ads_source_integration_tests_2.tiktok_adgroup_history_data
, which contains a JSON field calledAGE_GROUPS
.When running, the test model, i was able to get the macro to return and output
['AGE_GROUPS']
fill_staging_columns
I tested the incorporation of
get_json_columns_in_relation
intofill_staging_columns
by running the tiktok ads source package (specifically from theintegration_tests
folder) locally.With
tiktok_adgroup_history_data
as the source, the model where trouble would pop up wasstg_tiktok_ads__ad_group_history
, specifically on the line where we try to coalesceage_groups
and an actual-string field,age
.With my additions, the fields CTE in
stg_tiktok_ads__ad_group_history
compiles to the followingAnd so the coalesce of
age_groups
andage
in the final CTE does not cause an error and the model runs successfully.and the data looks OK in bigquery
If this PR introduces a modification to an existing macro, which packages is the macro currently present in and what steps were taken to test compatibility across packages?
Honestly we should test this with other packages as well -- ones with JSONs and not -- given the far-reaching nature of this macro change. perhaps we could have some customers try this out
Did you update the README to reflect the macro addition/modifications?