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

Add quote parameter to accepted_values test #1873

Closed
clausherther opened this issue Oct 28, 2019 · 2 comments · Fixed by #1876
Closed

Add quote parameter to accepted_values test #1873

clausherther opened this issue Oct 28, 2019 · 2 comments · Fixed by #1876
Labels
enhancement New feature or request

Comments

@clausherther
Copy link
Contributor

clausherther commented Oct 28, 2019

Describe the feature

Currently only character values can be tested in the built-in accepted_values test.

Relevant code from accepted_values.sql

select
        value_field

    from all_values
    where value_field not in (
        {% for value in values -%}

            '{{ value }}' {% if not loop.last -%} , {%- endif %}

        {%- endfor %}
    )

To support testing both accepted numeric and character values, we should make quoting the values optional.

Additional context

https://getdbt.slack.com/archives/C2JRRQDTL/p1572299083481900
Per a Slack conversations with Claire and Drew, this should not break existing uses of this test.
Drew suggested added a quote parameter, the default of which should be true, which is the current behavior.

Proposed change

{% set quote_values = kwargs.get('quote', True) %}
    select
        value_field

    from all_values
    where value_field not in (
        {% for value in values -%}
            {% if quote_values == True -%}
            '{{ value }}'
            {%- else -%}
            {{ value }}
            {%- endif -%}
            {%- if not loop.last -%},{%- endif %}
        {%- endfor %}
    )

Who will this benefit?

All users that use the accepted_values test.

@clausherther clausherther added enhancement New feature or request triage labels Oct 28, 2019
@clrcrl
Copy link
Contributor

clrcrl commented Oct 28, 2019

You can change {% if quote_values == True -%} to be {% if quote_values -%}

@clausherther
Copy link
Contributor Author

Wasn't sure how that worked in Jinja, thx!

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

Successfully merging a pull request may close this issue.

3 participants