Use dbt.type_string when no character limit provided in snowplow_utils.type_string()

[rlh1994]

Summary

Currently we have a macro called type_string that overwrites the default dbt one where used. Until recently this was basically never called as we usually call type_string raw (which is why we didn't notice the error that needed fixing, we now notice this due to the dispatch).

The main difference is we support adding a specified length (where the warehouses support it) which is used in web for the model name in the manifest. We also provide a type_max_string macro but as far as I can tell it's not used in any of our packages.

Options

My suggestion would be to change the macro so that we explicitly define the redshift size (as all other warehouses provide max size by default), and in all other cases we defer the macro to the dbt type_string.

The other option would be to rename the macro entirely and explicitly call it across all our packages to avoid clashing with the newly required dispatch setting.

Final option is to rely on dbt to accept and make this change (dbt-labs/dbt-core#7103) but it would mean we would require an even higher version of dbt, and I am not hopeful this will be quickly added.

Us:

dbt-snowplow-utils/macros/utils/cross_db/datatypes.sql

Lines 1 to 42 in f5b4ce6

	{# string ------------------------------------------------- #}
	{%- macro type_string(max_characters) -%}
	{% if max_characters %}
	{{ return(adapter.dispatch('type_string', 'snowplow_utils')(max_characters)) }}
	{% else %}
	{{ return(adapter.dispatch('type_max_string', 'snowplow_utils')()) }}
	{% endif %}
	{%- endmacro -%}
	{% macro default__type_string(max_characters) %}
	varchar( {{max_characters }} )
	{% endmacro %}
	{% macro bigquery__type_string(max_characters) %}
	string
	{% endmacro %}
	{% macro spark__type_string(max_characters) %}
	string
	{% endmacro %}
	{%- macro type_max_string() -%}
	{{ return(adapter.dispatch('type_max_string', 'snowplow_utils')()) }}
	{%- endmacro -%}
	{% macro default__type_max_string() %}
	string
	{% endmacro %}
	{% macro snowflake__type_max_string() %}
	varchar
	{% endmacro %}
	{% macro redshift__type_max_string() %}
	varchar(max)
	{% endmacro %}
	{% macro postgres__type_max_string() %}
	text
	{% endmacro %}

dbt:
https://github.com/dbt-labs/dbt-core/blob/8019498f0912e9ea921a5579c454e3408764960c/core/dbt/include/global_project/macros/utils/data_types.sql#L1-L15

[agnessnowplow]

I agree with the initially proposed change, meaning that we change the snowplow_utils.type_string() macro so that the input parameter would only take effect for redshift and direct the rest of the warehouses to a new default definition which would be the dbt's main type_string().

Naming it differently would in my eyes complicate things further, adding one more macro to keep in mind that has close links to a dbt core defined one. With this change we gain time to wait until we can comfortably remove this macro altogether and rely on dbt core, as and when it is optional dependence-wise.

[rlh1994]

After a discussion on an issue over on dbt-core, it seems we can use the built in api.Column functions to generate the lengths instead, so we should be able to deprecate and remove this entirely from our package.

[rlh1994]

It turns out this is only supported for the default warehouses, so there is still a need to keep a type specifically for redshift greater than 256 chars, but it will be simplified and not clash in name.