Fix: Spark-compatible tmp_table_suffix for incremental materializations #759
Conversation
Generate Spark-compatible tmp_table_suffix for incremental materializations with iceberg
Generate a unique tmp table suffix if required for incremental materializations. Generate Spark-compatible tmp_table_suffix for incremental materializations.
chenp-eleos
changed the title
| {% if unique_tmp_table_suffix == True and strategy == 'insert_overwrite' and table_type == 'hive' %} | ||
| {% set tmp_table_suffix = adapter.generate_unique_temporary_table_suffix() %} | ||
| -- Generate a unique tmp table suffix if required | ||
| {% if unique_tmp_table_suffix == True %} |
There was a problem hiding this comment.
why did you remove the check on strategy == 'insert_overwrite' and table_type == 'hive'?
for sure we can simplify the statement checks and do something like this:
{% if unique_tmp_table_suffix == True and ((strategy == 'insert_overwrite' and table_type == 'hive') or table_type == 'iceberg') %}
But I will avoid to don't check the strategy type.
There was a problem hiding this comment.
I've some doubts about my comment, your idea might not be bad overall, but to guarantee that works as expected, consider to add some Integrations tests for different scenarios.
| -- Generate a unique tmp table suffix if required | ||
| {% if unique_tmp_table_suffix == True %} | ||
| {% set raw_suffix = adapter.generate_unique_temporary_table_suffix() %} | ||
| {% set tmp_table_suffix = raw_suffix.replace('-', '_') %} |
There was a problem hiding this comment.
please move this inside the generate_unique_temporary_table_suffix.
replacing - with _ make sense also for an athena/trino SQL prospective.
Also, remember to adapt the functional tests, otherwise they will fail.
There was a problem hiding this comment.
#754 was merged, therefore please remove this, as not needed anymore
|
#754 was merged, please readapt this PR based on my comments. |
Description
This PR addresses the issue related to unique_tmp_table_suffix when working with Spark. While the proposed changes in issue #668 worked for part of the scenarios, they did not account for Spark’s restrictions on table and view names. Specifically:
• Spark does not allow hyphens (-) in table/view names, causing failures when creating temporary tables.
• This PR modifies the logic to replace hyphens in the generated suffix with underscores (_), ensuring Spark compatibility.
Additionally:
• Ensures unique_tmp_table_suffix is respected and functional when set to True for all table types and strategies.
• Supports concurrent runs of the same model by avoiding naming conflicts.
Models used to test - Optional
Checklist