refactor: delegate generate_series/get_powers_of_two to dbt-core equivalents#1088
Open
sdebruyn wants to merge 1 commit into
Open
refactor: delegate generate_series/get_powers_of_two to dbt-core equivalents#1088sdebruyn wants to merge 1 commit into
sdebruyn wants to merge 1 commit into
Conversation
The default__ implementations of dbt_utils.generate_series and dbt_utils.get_powers_of_two are byte-for-byte identical to the equivalents that now live in dbt-adapters (formerly dbt-core's global_project). Maintaining two copies forces adapter maintainers to register the same override on two dispatch namespaces and means behavior can silently diverge. Replace both default__ bodies with a return to the dbt namespace equivalent. Adapter overrides on dbt_utils.generate_series keep working through the existing dispatch path; users without any override get identical SQL as before. Bump require-dbt-version minimum to 1.7.0, the release in which dbt.generate_series and dbt.get_powers_of_two were added to core (dbt-labs/dbt-core PR 8616, commit aa86fdfe7).
Contributor
Author
|
Human reviewed. This would simplify the dbt-fabric adapter. Also, why have the macro in 2 places? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
dbt_utils.generate_seriesanddbt_utils.get_powers_of_twoare byte-for-byte identical to thedbt.generate_series/dbt.get_powers_of_twomacros that now live in dbt-adapters' global_project. The only difference between the two pairs is the dispatch namespace argument.Maintaining two copies has concrete downsides:
dbt_utilsnamespace and thedbtnamespace, to give users consistent behavior across entry points.dbt_utils.generate_seriesdirectly or reach it indirectly throughdbt.date_spine(which callsdbt.generate_series).Concrete impact (one example)
dbt-fabric-samdebruynships afabric__generate_seriesoverride on thedbt_utilsnamespace that uses Microsoft Fabric's nativeGENERATE_SERIEStable-valued function. The TVF is dramatically faster than the cross-join power-of-two CTE for non-trivial bounds. But users who reach this code path viadbt.date_spine(which callsdbt.generate_series, notdbt_utils.generate_series) get the slow CTE version, because the override sits on the other namespace. The same trap exists for every adapter that wants to optimizegenerate_series.History
The duplication was introduced intentionally in dbt-labs/dbt-core#8616 (commit
aa86fdfe7, "Add date spine macros to core", released in dbt-core 1.7.0). The PR's commit message gives the motivation:In late 2023 the
global_projectmigrated fromdbt-coretodbt-adapters(commit615ad1fe2). The macro now lives at https://github.com/dbt-labs/dbt-adapters/blob/main/dbt-adapters/src/dbt/include/global_project/macros/utils/generate_series.sql and is functionally interchangeable with the dbt-utils version.Fix
Replace the bodies of
default__generate_seriesanddefault__get_powers_of_twowith a single line that returns thedbt.*equivalent:For users without an adapter override: emitted SQL is identical (delegate target is byte-for-byte the same body).
For users with an adapter override on
dbt_utils.generate_series: their override still wins via the existing dispatch path, no behavior change.For users with an adapter override on
dbt.generate_seriesonly (e.g. anything inherited viadbt_external_tables-style global namespace overrides): they now also get their fast path when called throughdbt_utils.generate_series. This is the desired new behavior.Compatibility
dbt.generate_seriesanddbt.get_powers_of_twowere added in dbt-core 1.7.0 (September 2023). This PR bumpsrequire-dbt-versionfrom>=1.3.0to>=1.7.0so the delegate target is guaranteed to exist. dbt-core 1.6 and earlier are no longer supported upstream, so the practical impact for active users should be minimal — but flagging this explicitly because the version floor change is the only externally visible behavior in this PR.If raising the floor is undesirable, an alternative is wrapping the delegate in an
{% if context.get('dbt') and ... %}guard, but that adds complexity for a code path that targets versions the project no longer supports.Testing
I have not run the integration test suite locally for this change. The intent is for CI to validate that
dbt_utils.generate_seriescontinues to emit equivalent SQL on all supported adapters — the existing tests should catch any regression because the delegate target is the same code.Open question for maintainers
Given that
dbt.generate_seriesnow lives in dbt-adapters and the semantic layer no longer needs to take a dependency on dbt-utils, shoulddbt_utils.generate_seriesanddbt_utils.get_powers_of_twobe deprecated outright in favor of thedbt.*equivalents? This PR is a no-op-behavior bridge that makes deprecation safe — a future major release could remove thedbt_utilsvariants entirely without affecting users who have migrated their calls. Not assuming this is the goal, just flagging it because the duplication question naturally surfaces it.References
dbt.generate_seriesin dbt-adaptersfabric__generate_seriesoverride using the T-SQL TVF