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

Fix QUOTED_IDENTIFIERS_IGNORE_CASE errors (#982) #998

Merged
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
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):
Copy link
Member

Choose a reason for hiding this comment

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

clever!

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