Skip to content

bugfix/cast_strings_as_floats #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fivetran-savage merged 9 commits into from
Jun 9, 2026
Merged

bugfix/cast_strings_as_floats #33

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0a87567
bugfix/cast_strings_as_floats
fivetran-savage May 29, 2026
d1fa7d3
update_vars
fivetran-savage May 29, 2026
23a1548
add_casing_to_macro
fivetran-savage Jun 1, 2026
d90ca94
revert_vars
fivetran-savage Jun 1, 2026
427e7ad
update_yml
fivetran-savage Jun 1, 2026
582578f
update_macro
fivetran-savage Jun 8, 2026
19f9470
Merge branch 'bugfix/cast_strings_as_floats' of https://github.com/fi…
fivetran-savage Jun 8, 2026
f254ec0
Update CHANGELOG.md
fivetran-savage Jun 8, 2026
e12e7b8
Generate dbt docs via GitHub Actions
github-actions[bot] Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# dbt_workday v0.9.2

[PR #33](https://github.com/fivetran/dbt_workday/pull/33) includes the following update.

## Bug Fix
- Fixes an issue in `stg_workday__worker_history` where `annual_currency_summary_primary_compensation_basis`, `annual_currency_summary_total_base_pay`, and `annual_currency_summary_total_salary_and_allowances` could surface as string datatypes. These columns are now cast to float when the source type is a string.

@fivetran-avinash fivetran-avinash Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields are also present in the base stg_workday__worker model. Should we apply the macro there as well?

@fivetran-savage fivetran-savage Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per this comment in the Jira ticket, the decision was to only apply to the worker_history table.


# dbt_workday v0.9.1

[PR #31](https://github.com/fivetran/dbt_workday/pull/31) includes the following update.
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
config-version: 2
name: 'workday'
version: '0.9.1'
version: '0.9.2'
require-dbt-version: [">=1.3.0", "<3.0.0"]

models:
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/dbt_project.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 'workday_integration_tests'
version: '0.9.1'
version: '0.9.2'
config-version: 2

profile: 'integration_tests'
Expand Down Expand Up @@ -169,4 +169,4 @@ seeds:

flags:
send_anonymous_usage_stats: False
use_colors: True
use_colors: True
21 changes: 18 additions & 3 deletions models/workday_history/staging/stg_workday__worker_history.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,24 @@ final as (
active_status_date,
annual_currency_summary_currency,
annual_currency_summary_frequency,
annual_currency_summary_primary_compensation_basis,
annual_currency_summary_total_base_pay,
annual_currency_summary_total_salary_and_allowances,
{% set string_dtypes = ['char', 'text', 'string'] %}

@fivetran-avinash fivetran-avinash Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think 'text' is a string data type in Databricks. It's harmless if it's there, but might be worth double-checking if it's safe to remove.

@fivetran-savage fivetran-savage Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

{% for col in adapter.get_columns_in_relation(ref('stg_workday__worker_base')) %}
{% if col.name.lower() in ['annual_currency_summary_primary_compensation_basis', 'annual_currency_summary_total_base_pay', 'annual_currency_summary_total_salary_and_allowances'] %}
{% if target.type == 'databricks' %}

@fivetran-avinash fivetran-avinash Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a silent failure when running against databricks here. Variables set inside a {% for %} loop don't propagate out of it in Jinja2. So {% set is_str = true %} inside the {% for stype in string_dtypes %} loop has no effect, and is_str always stays false after the loop ends. This means string-typed compensation columns on Databricks are never cast to float and silently land as strings in the output table.

I tested with this configuration in integration_tests/dbt_project.yml

        annual_currency_summary_primary_compensation_basis: "{{ 'varchar(100)' if target.type in ('redshift', 'postgres') else 'string' }}"
        annual_currency_summary_total_base_pay: "{{ 'varchar(100)' if target.type in ('redshift', 'postgres') else 'string' }}"
        annual_currency_summary_total_salary_and_allowances: "{{ 'varchar(100)' if target.type in ('redshift', 'postgres') else 'string' }}"

And when running

dbt compile -s stg_workday__worker_history -t databricks --vars '{"employee_history_enabled": true}'

You end up seeing that these fields are not cast to floats and remain as strings.

Image

So the Databricks case isn't quite solved as of yet. I wonder if we can use the Jinja namespace capability, to do the data type check, and then change the value to true that way. Might be worth testing.

@fivetran-avinash fivetran-avinash Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the latest changes fixed this!

@fivetran-savage fivetran-savage Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespace was the key.

{% set is_str = false %}
{% for stype in string_dtypes %}
{% if stype in col.dtype.lower() %}{% set is_str = true %}{% endif %}
{% endfor %}
{% else %}
{% set is_str = col.is_string() %}
{% endif %}
{% if is_str %}
cast({{ col.name }} as {{ dbt.type_float() }}) as {{ col.name }},
{% else %}
{{ col.name }},
{% endif %}
{% endif %}
{% endfor %}
annual_summary_currency,
annual_summary_frequency,
annual_summary_primary_compensation_basis,
Expand Down