Skip to content

Commit

Permalink
Merge pull request #998 from fishtown-analytics/fix/snowflake-quoted-…
Browse files Browse the repository at this point in the history
…identifiers-case-docs

Fix QUOTED_IDENTIFIERS_IGNORE_CASE errors (#982)
  • Loading branch information
beckjake authored Sep 13, 2018
2 parents d66f3a8 + 40f009f commit f473eae
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 17 deletions.
17 changes: 14 additions & 3 deletions dbt/adapters/default/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,15 @@ def _filter_schemas(manifest):
})

def test(row):
# the schema may be present but None
table_schema = row.get('table_schema')
if 'table_schema' not in row.keys():
# this means the get catalog operation is somehow not well formed!
raise dbt.exceptions.InternalException(
'Got a row without "table_schema" column, columns: {}'
.format(row.keys())
)
# the schema may be present but None, which is not an error and should
# be filtered out
table_schema = row['table_schema']
if table_schema is None:
return False
return table_schema.lower() in schemas
Expand Down Expand Up @@ -840,6 +847,10 @@ def run_operation(cls, profile, project_cfg, manifest, operation_name):
###
# Abstract methods involving the manifest
###
@classmethod
def _filter_table(cls, table, manifest):
return table.where(_filter_schemas(manifest))

@classmethod
def get_catalog(cls, profile, project_cfg, manifest):
try:
Expand All @@ -848,5 +859,5 @@ def get_catalog(cls, profile, project_cfg, manifest):
finally:
cls.release_connection(profile, GET_CATALOG_OPERATION_NAME)

results = table.where(_filter_schemas(manifest))
results = cls._filter_table(table, manifest)
return results
9 changes: 9 additions & 0 deletions dbt/adapters/snowflake/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,15 @@ def add_query(cls, profile, sql, model_name=None, auto_begin=True,

return connection, cursor

@classmethod
def _filter_table(cls, table, manifest):
# On snowflake, users can set QUOTED_IDENTIFIERS_IGNORE_CASE, so force
# the column names to their lowercased forms.
lowered = table.rename(
column_names=[c.lower() for c in table.column_names]
)
return super(SnowflakeAdapter, cls)._filter_table(lowered, manifest)

@classmethod
def _make_match_kwargs(cls, project_cfg, schema, identifier):
if identifier is not None and \
Expand Down
51 changes: 37 additions & 14 deletions test/integration/029_docs_generate_tests/test_docs_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
from numbers import Integral
import os
from datetime import datetime, timedelta
from mock import ANY
from mock import ANY, patch

from test.integration.base import DBTIntegrationTest, use_profile
from dbt.compat import basestring
from dbt.adapters.snowflake import impl as snowflake_impl

DATEFMT = '%Y-%m-%dT%H:%M:%S.%fZ'

Expand Down Expand Up @@ -177,7 +178,7 @@ def _snowflake_stats(self):
'row_count': {
'id': 'row_count',
'label': 'Row Count',
'value': True,
'value': 1.0,
'description': 'An approximate count of rows in this table',
'include': True,
},
Expand Down Expand Up @@ -1501,23 +1502,26 @@ def verify_manifest(self, expected_manifest):
)
self.assertEqual(manifest_without_extras, expected_manifest)

def expected_run_results(self):
def _quote(self, value):
quote_char = '`' if self.adapter_type == 'bigquery' else '"'
return '{0}{1}{0}'.format(quote_char, value)

def expected_run_results(self, quote_schema=True, quote_model=False):
"""
The expected results of this run.
"""
schema = self.unique_schema()
compiled_sql = '\n\nselect * from "{}".seed'.format(schema)
status = 'CREATE VIEW'

if self.adapter_type == 'snowflake':
status = 'SUCCESS 1'
compiled_sql = compiled_sql.replace('"', '')
compiled_schema = self._quote(schema) if quote_schema else schema
compiled_seed = self._quote('seed') if quote_model else 'seed'

if self.adapter_type == 'bigquery':
status = 'OK'
compiled_sql = '\n\nselect * from `{}`.`{}`.seed'.format(
self._profile['project'], schema
compiled_sql = '\n\nselect * from `{}`.{}.{}'.format(
self._profile['project'], compiled_schema, compiled_seed
)
status = None
else:
compiled_sql = '\n\nselect * from {}.{}'.format(compiled_schema,
compiled_seed)

return [
{
Expand Down Expand Up @@ -1573,7 +1577,7 @@ def expected_run_results(self):
'wrapped_sql': 'None'
},
'skip': False,
'status': status,
'status': None,
},
{
'error': None,
Expand Down Expand Up @@ -2025,7 +2029,26 @@ def test__snowflake__run_and_generate(self):

self.verify_catalog(self.expected_snowflake_catalog())
self.verify_manifest(self.expected_seeded_manifest())
self.verify_run_results(self.expected_run_results())
self.verify_run_results(self.expected_run_results(quote_schema=False, quote_model=False))

@use_profile('snowflake')
def test__snowflake__run_and_generate_ignore_quoting_parameter(self):
old_connect = snowflake_impl.snowflake.connector.connect
def connect(*args, **kwargs):
kwargs['session_parameters'] = {
'QUOTED_IDENTIFIERS_IGNORE_CASE':True
}
return old_connect(*args, **kwargs)
with patch.object(snowflake_impl.snowflake.connector, 'connect', connect):
self.run_and_generate({
'quoting': {
'identifier': True,
}
})

self.verify_catalog(self.expected_snowflake_catalog())
self.verify_manifest(self.expected_seeded_manifest())
self.verify_run_results(self.expected_run_results(quote_schema=False, quote_model=True))

@use_profile('bigquery')
def test__bigquery__run_and_generate(self):
Expand Down

0 comments on commit f473eae

Please sign in to comment.