add fivetran_union_relations macro#139
add fivetran_union_relations macro#139fivetran-jamie wants to merge 50 commits intoreleases/v0.4.latestfrom
Conversation
Bug/redshift constant exp
…ual table identifiers <> source table names
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
@fivetran-jamie I just have a few comments and minor changes. Would you be able to address these. Once they are addressed I can take a final look and ensure buildkite is passing before releasing.
Also I asked eng to remove the quickstart.yml test from this repo.
| database=source(default_schema, table_identifier).database, | ||
| schema=source(default_schema, table_identifier).schema, | ||
| identifier=var(identifier_var, table_identifier) | ||
| identifier=connector_table_name_override if (connector_table_name_override and not var('use_table_name_identifer_override', false)) else var(identifier_var, connector_table_name_override if connector_table_name_override else table_identifier) |
There was a problem hiding this comment.
I believe I am understanding this; however, for posterity sake would you be able to provide an example for each of these conditionals and what the expected behavior would be?
There was a problem hiding this comment.
Sure, let's take the account_type table in Netsuite2. The source table name is account_type, while the actual connector table name is accounttype.
In the _tmp model, we provide accounttype as the connector_table_name_override instead of the typical table_identifier, which would be account_type in this case. we have not configured the use_table_name_identifier_override variable, so that remains false and the macro uses the connector_table_name_override. For tables that don't have special/wonky connector names, the macro uses the default table_identifier (example: accounts)
Back to account_types -- in our integration tests, the seed file is named netsuite2_account_type_data. We use the netsuite2_account_type_identifier variable to point to this name here. However, because the respective _tmp model has a connector_table_name_override, i was seeing the no ACCOUNT_TYPE table found in your schema. Fivetran will create an empty staging model yadda yadda log message for this and every table for which we used connector_table_name_override.
To ensure that we could use identifier variables even for these weird tables, we now set the global use_table_name_identifier_override variable to True in the integration_tests/dbt_project.yml file. This means that we'll use the connector_table_name_override if it's provided, but only if we're not overriding the table identifier completely with our own custom identifier (in this case netsuite2_account_type_data).
But let's say that our seed file for accounting_book_subsidiaries aligns with the actual connector table name of accountingbooksubsidiaries (which we have configured in the _tmp model as well), so it's accountingbooksubsidiaries.csv.
The last part of the highlighted line, var(identifier_var, connector_table_name_override if connector_table_name_override else table_identifier) makes it so that we can use identifier variables for some tables but we don't need to use them for all of the tables (despite our globally-defined variable). The package will pick up accountingbooksubsidiaries for the respective staging model and still pick up netsuite2_account_type_data for the account_type staging model. This would work as well if we had a seed file called accounts.csv for the accounts table, which doesn't use the connector_table_name_override.
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
Feature/add lookback macro
What change does this PR introduce?
Adds
fivetran_union_relationsmacro. This was necessary for rolling union_data out to Zendesk, which has some source tables that have reserved-keywords for names (ietimezone). In the past, we've solved this error by aliasing the problematic source table, but there's no way to getdbt_utils.union_relationsto do so (besides opening a PR on dbt utils or making our own version of the macro).We already do have our own version of
union_relationsin this package, but as we learned recently, using the same exact name as a macro in debt_utils can cause failures for customers with certain dispatch configsThis PR also adjusts union_data (in order to work with Netsuite, which is a weird package due to our support of both of its endpoints).
connector_table_name_overridefield. this is to be used when the definedsourcetable name is different from the actual table name as it appears in the connector/warehouse. This is only a thing for Netsuite2use_table_name_identifer_override. this was necessary to add for our integration tests (and customers) to still be able to use identifier variables for tables in whichconnector_table_name_overrideis implemented.If this PR introduces a new macro, how did you test the new macro?
Ran it with zendesk, hubspot, and netsuite in their
feature/add-union-databranches. moreover, i tested this fivetran_utils branch on Shopify_source, which has the union ability already.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?
union_relationsmacro to deprecate it in the futureRan it with zendesk, hubspot, and netsuite in their
feature/add-union-databranches. moreover, i tested this fivetran_utils branch on Shopify_source, which has the union ability already.Did you update the README to reflect the macro addition/modifications?