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

Postgres: Prevent temp relation identifiers from being too long #2850

Merged
merged 3 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
15 changes: 14 additions & 1 deletion plugins/postgres/dbt/include/postgres/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitate about replacing one silent truncation (by postgres) with another (by dbt), but I suppose this is only for temp relation naming, for use mid-materialization, and it won't have any effect on how users actually reference the final model

{% do return(base_relation.incorporate(
path={
"identifier": tmp_identifier,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

select * from {{ this.schema }}.seed

{{
config({
"unique_key": "col_A",
"materialized": "incremental"
})
}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

select * from {{ this.schema }}.seed

{{
config({
"materialized": "table"
})
}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

select * from {{ this.schema }}.seed

{{
config({
"materialized": "table"
})
}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

select * from {{ this.schema }}.seed

{{
config({
"materialized": "table"
})
}}
67 changes: 56 additions & 11 deletions test/integration/063_relation_name_tests/test_relation_name.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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)