Skip to content
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.

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

create_nonclustered_index macro does not check for index name length #317

Closed
Freia3 opened this issue Dec 13, 2022 · 3 comments · Fixed by #386
Closed

create_nonclustered_index macro does not check for index name length #317

Freia3 opened this issue Dec 13, 2022 · 3 comments · Fixed by #386
Labels
Milestone

Comments

@Freia3
Copy link

Freia3 commented Dec 13, 2022

I get the following error:
Failed to execute query. Error: The identifier that starts with 'test__index_on_person_id_includes_l1234567_source_value_gender_concept_id_l_source_collection_elt_loaded_at_race_conce' is too long. Maximum length is 128.

When executing:

{{
config({
"materialized": 'table',
"as_columnstore": False,
"post-hook": [
"{{ create_nonclustered_index(columns = ['person_id'], includes = ['l1234567_id_source_value', 'gender_concept_id','l1234567_source_collection','elt_loaded_at','race_concept_id','hospital_id_source_value' ]) }}",
]
})
}}

The create_nonclustered_index macro does not check the length of the index name:
{% set idx_name = (this.table + '__index_on_' + columns|join('_')|replace(" ", "_") + '_includes_' + includes|join('_')|replace(" ", "_")) %}

So when there are too many included columns, the index name is too long.

@Freia3 Freia3 changed the title create_nonclustered_index macro does not checkout for index name length create_nonclustered_index macro does not check for index name length Jan 19, 2023
@cbini
Copy link

cbini commented Mar 30, 2023

Just ran into this myself. This could probably be avoided using local_md5 function to hash the includes list. Probably, a good idea to do it on the keys list as well.

{% set idx_name = this.table + '__index_on_' + columns|join('_')|replace(" ", "_") + '_includes_' + includes|join('_')|replace(" ", "_") %}

@cbini
Copy link

cbini commented Mar 30, 2023

Found a workaround, you can drop these into your macros folder. Requires dbt-core 1.4+

{# https://github.com/dbt-msft/dbt-sqlserver/blob/73537af0cbb1162f585b3bebf06f1a043d1a28d0/dbt/include/sqlserver/macros/adapters/indexes.sql #}
{% macro create_clustered_index(columns, unique=False) -%}

{{ log("Creating clustered index...") }}

{% set idx_name = "clustered_" + local_md5(columns | join("_")) %}

if not exists (
    select *
    from sys.indexes
    where name = '{{ idx_name }}' and object_id = object_id('{{ this }}')
)
begin

create
{% if unique -%}
unique
{% endif %}
clustered index
    {{ idx_name }}
      on {{ this }} ({{ '[' + columns|join("], [") + ']' }})
end

{%- endmacro %}

{% macro create_nonclustered_index(columns, includes=False) %}

{{ log("Creating nonclustered index...") }}

{% if includes -%}
{% set idx_name = (
    "nonclustered_"
    + local_md5(columns | join("_"))
    + "_incl_"
    + local_md5(includes | join("_"))
) %}
{% else -%} {% set idx_name = "nonclustered_" + local_md5(columns | join("_")) %}
{% endif %}

if not exists (
    select *
    from sys.indexes
    where name = '{{ idx_name }}' and object_id = object_id('{{ this }}')
)
begin
create nonclustered index
    {{ idx_name }}
      on {{ this }} ({{ '[' + columns|join("], [") + ']' }})
      {% if includes -%}
        include ({{ '[' + includes|join("], [") + ']' }})
      {% endif %}
end

{% endmacro %}

@sdebruyn
Copy link
Member

Good idea for the md5!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants