dbt-sqlserver icon indicating copy to clipboard operation
dbt-sqlserver copied to clipboard

Max Recursion config property

Open cody-scott opened this issue 8 months ago • 2 comments

We have come across some tables that require a recursion limit of greater then 100, the SQL Server default. Proposing to add a config property to insert a variable max recursion amount.

The current adapter implements the options here https://github.com/dbt-msft/dbt-sqlserver/blob/master/dbt/include/sqlserver/macros/adapter/metadata.sql

Since the option maxrecursion cannot be apart of the create view statement, this should only apply to the create table statements. Based on how this is set now it would either need to be changed for all calls to check if its a table materialization or we have an alternative table version that adds maxrecursion.

My suggestion would be to have it always include the maxrecursion in the option, but have the default be 100 to mimic the sql server default.

apply_label appears in a few spots, so perhaps the simpler option is have apply_label_for_table as a starting point. We wouldn't have to change the apply_label signature to require a materialization type argument in that case.

https://github.com/search?q=repo%3Adbt-msft%2Fdbt-sqlserver%20apply_label&type=code

Option

{% macro apply_label() %}
    {{ log (config.get('query_tag','dbt-sqlserver'))}}
    {%- set query_label = config.get('query_tag','dbt-sqlserver') -%}
    OPTION (LABEL = '{{query_label}}');
{% endmacro %}

{% macro apply_label_for_table() %}
    {{ log (config.get('query_tag','dbt-sqlserver'))}}
    {%- set query_label = config.get('query_tag','dbt-sqlserver') -%}
    {%- set max_recursion = config.get('max_recursion',100) -%}
    OPTION (
        LABEL = '{{query_label}}', 
        MAXRECURSION = {{ max_recursion }}
    );
{% endmacro %}

then change the call here: https://github.com/dbt-msft/dbt-sqlserver/blob/bcf4ac910528c9bc681df5fd487818a19094838a/dbt/include/sqlserver/macros/relations/table/create.sql#L2

{% macro sqlserver__create_table_as(temporary, relation, sql) -%}
    {%- set query_label = apply_label_for_table() -%}
    ...

cody-scott avatar May 16 '25 14:05 cody-scott

apply_label also doesn't really capture the scope properly, but not sure if there is cascading issues from the fabric adapter if that was changed to apply_option

cody-scott avatar May 16 '25 14:05 cody-scott

Because there are a lot more useful options someone may want to set in the option hints this might be a good opportunity to add an additional property that we parse to the materializations that can be a dictionary of the option values you want to set.

That would mean we want to make the options macro materialization aware so we do not output options into views but would provide very useful functionality.

If we were to setup an apply_option macro we could just route apply_label calls through to it as its applying a specific option.

Benjamin-Knight avatar May 24 '25 21:05 Benjamin-Knight