Merge pull request #2850 from elexisvenator/patch-1

Postgres: Prevent temp relation identifiers from being too long

CHANGELOG.md

@@ -8,12 +8,14 @@
 - Respect --project-dir in dbt clean command ([#2840](https://github.com/fishtown-analytics/dbt/issues/2840), [#2841](https://github.com/fishtown-analytics/dbt/pull/2841))
 - Fix Redshift adapter `get_columns_in_relation` macro to push schema filter down to the `svv_external_columns` view ([#2855](https://github.com/fishtown-analytics/dbt/issues/2854))
 - Add `unixodbc-dev` package to testing docker image ([#2859](https://github.com/fishtown-analytics/dbt/pull/2859))
+- Increased the supported relation name length in postgres from 29 to 51 ([#2850](https://github.com/fishtown-analytics/dbt/pull/2850))
 
 Contributors:
 - [@feluelle](https://github.com/feluelle) ([#2841](https://github.com/fishtown-analytics/dbt/pull/2841))
 - [ran-eh](https://github.com/ran-eh) [#2596](https://github.com/fishtown-analytics/dbt/pull/2596)
 - [@hochoy](https://github.com/hochoy) [#2851](https://github.com/fishtown-analytics/dbt/pull/2851)
 - [@brangisom](https://github.com/brangisom) [#2855](https://github.com/fishtown-analytics/dbt/pull/2855)
+- [@elexisvenator](https://github.com/elexisvenator) ([#2850](https://github.com/fishtown-analytics/dbt/pull/2850))
 
 ## dbt 0.19.0b1 (October 21, 2020)
 

plugins/postgres/dbt/include/postgres/macros/adapters.sql

@@ -117,8 +117,21 @@
  {{ current_timestamp() }}::timestamp without time zone
 {%- endmacro %}
 
+{#
+ Postgres tables have a maximum length off 63 characters, anything longer is silently truncated.
+ Temp relations add a lot of extra characters to the end of table namers to ensure uniqueness.
+ To prevent this going over the character limit, the base_relation name is truncated to ensure
+ that name + suffix + uniquestring is < 63 characters. 
+#}
 {% macro postgres__make_temp_relation(base_relation, suffix) %}
- {% set tmp_identifier = base_relation.identifier ~ suffix ~ py_current_timestring() %}
+ {% set dt = modules.datetime.datetime.now() %}
+ {% set dtstring = dt.strftime("%H%M%S%f") %}
+ {% set suffix_length = suffix|length + dtstring|length %}
+ {% set relation_max_name_length = 63 %}
+ {% if suffix_length > relation_max_name_length %}
+ {% do exceptions.raise_compiler_error('Temp relation suffix is too long (' ~ suffix|length ~ ' characters). Maximum length is ' ~ (relation_max_name_length - dtstring|length) ~ ' characters.') %}
+ {% endif %}
+ {% set tmp_identifier = base_relation.identifier[:relation_max_name_length - suffix_length] ~ suffix ~ dtstring %}
  {% do return(base_relation.incorporate(
  path={
  "identifier": tmp_identifier,

test/integration/063_relation_name_tests/models/my_name_is_51_characters_incremental_abcdefghijklmn.sql

@@ -0,0 +1,9 @@
+
+select * from {{ this.schema }}.seed
+
+{{
+ config({
+ "unique_key": "col_A",
+ "materialized": "incremental"
+ })
+}}

test/integration/063_relation_name_tests/models/my_name_is_52_characters_abcdefghijklmnopqrstuvwxyz0.sql

@@ -0,0 +1,8 @@
+
+select * from {{ this.schema }}.seed
+
+{{
+ config({
+ "materialized": "table"
+ })
+}}

test/integration/063_relation_name_tests/models/my_name_is_63_characters_abcdefghijklmnopqrstuvwxyz012345678901.sql

@@ -0,0 +1,8 @@
+
+select * from {{ this.schema }}.seed
+
+{{
+ config({
+ "materialized": "table"
+ })
+}}

test/integration/063_relation_name_tests/models/my_name_is_64_characters_abcdefghijklmnopqrstuvwxyz0123456789012.sql

@@ -0,0 +1,8 @@
+
+select * from {{ this.schema }}.seed
+
+{{
+ config({
+ "materialized": "table"
+ })
+}}

test/integration/063_relation_name_tests/test_relation_name.py

@@ -1,11 +1,12 @@
 from test.integration.base import DBTIntegrationTest, use_profile
+from pytest import mark
 import dbt.exceptions
 
-class TestAdapterDDL(DBTIntegrationTest):
 
+class TestAdapterDDL(DBTIntegrationTest):
  def setUp(self):
  DBTIntegrationTest.setUp(self)
- self.run_dbt(['seed'])
+ self.run_dbt(["seed"])
 
  @property
  def schema(self):
@@ -18,17 +19,61 @@ def models(self):
  @property
  def project_config(self):
  return {
- 'config-version': 2,
- 'seeds': {
- 'quote_columns': False,
+ "config-version": 2,
+ "seeds": {
+ "quote_columns": False,
  },
  }
 
- @use_profile('postgres')
- def test_postgres_long_name_fails(self):
- self.run_dbt(['run'],expect_pass=False)
+ # 63 characters is the character limit for a table name in a postgres database
+ # (assuming compiled without changes from source)
+ @use_profile("postgres")
+ def test_postgres_name_longer_than_63_fails(self):
+ self.run_dbt(
+ [
+ "run",
+ "-m",
+ "my_name_is_64_characters_abcdefghijklmnopqrstuvwxyz0123456789012",
+ ],
+ expect_pass=False,
+ )
 
- @use_profile('redshift')
- def test_redshift_long_name_succeeds(self):
- self.run_dbt(['run'],expect_pass=True)
+ @mark.skip(
+ reason="Backup table generation currently adds 12 characters to the relation name, meaning the current name limit is 51."
+ )
+ @use_profile("postgres")
+ def test_postgres_name_shorter_or_equal_to_63_passes(self):
+ self.run_dbt(
+ [
+ "run",
+ "-m",
+ "my_name_is_52_characters_abcdefghijklmnopqrstuvwxyz0"
+ "my_name_is_63_characters_abcdefghijklmnopqrstuvwxyz012345678901",
+ ],
+ expect_pass=True,
+ )
 
+ @use_profile("postgres")
+ def test_postgres_long_name_passes_when_temp_tables_are_generated(self):
+ self.run_dbt(
+ [
+ "run",
+ "-m",
+ "my_name_is_51_characters_incremental_abcdefghijklmn",
+ ],
+ expect_pass=True,
+ )
+
+ # Run again to trigger incremental materialization
+ self.run_dbt(
+ [
+ "run",
+ "-m",
+ "my_name_is_51_characters_incremental_abcdefghijklmn",
+ ],
+ expect_pass=True,
+ )
+
+ @use_profile("redshift")
+ def test_redshift_long_name_succeeds(self):
+ self.run_dbt(["run"], expect_pass=True)