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

Quote config #742

Merged
merged 20 commits into from
Apr 26, 2018
Merged
Show file tree
Hide file tree
Changes from 8 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
4 changes: 4 additions & 0 deletions dbt/adapters/bigquery/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ def list_relations(cls, profile, schema, model_name=None):
project=credentials.get('project'),
schema=schema,
identifier=table.table_id,
quote_policy={
'schema': True,
'identifier': True
},
type=relation_types.get(table.table_type))
for table in all_tables]

Expand Down
7 changes: 5 additions & 2 deletions dbt/adapters/default/relation.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def include(self, database=None, schema=None, identifier=None):

return self.incorporate(include_policy=policy)

def render(self):
def render(self, use_table_name=True):
parts = []

for k in self.PATH_ELEMENTS:
Expand All @@ -128,7 +128,10 @@ def render(self):
if path_part is None:
continue
elif k == 'identifier':
path_part = self.table
if use_table_name:
path_part = self.table
else:
path_part = self.identifier

parts.append(
self.quote_if(
Expand Down
8 changes: 6 additions & 2 deletions dbt/adapters/postgres/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ def alter_column_type(cls, profile, schema, table, column_name,
def list_relations(cls, profile, schema, model_name=None):
sql = """
select tablename as name, schemaname as schema, 'table' as type from pg_tables
where schemaname = '{schema}'
where schemaname ilike '{schema}'
union all
select viewname as name, schemaname as schema, 'view' as type from pg_views
where schemaname = '{schema}'
where schemaname ilike '{schema}'
""".format(schema=schema).strip() # noqa

connection, cursor = cls.add_query(profile, sql, model_name,
Expand All @@ -133,6 +133,10 @@ def list_relations(cls, profile, schema, model_name=None):
database=profile.get('dbname'),
schema=_schema,
identifier=name,
quote_policy={
'schema': True,
'identifier': True
},
type=type)
for (name, _schema, type) in results]

Expand Down
4 changes: 4 additions & 0 deletions dbt/adapters/snowflake/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ def list_relations(cls, profile, schema, model_name=None):
database=profile.get('database'),
schema=_schema,
identifier=name,
quote_policy={
'schema': True,
'identifier': True
},
type=relation_type_lookup.get(type))
for (name, _schema, type) in results]

Expand Down
17 changes: 5 additions & 12 deletions dbt/adapters/snowflake/relation.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class SnowflakeRelation(DefaultRelation):
'quote_policy': {
'database': False,
'schema': False,
'identifier': False,
'identifier': True,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we quote both schema and identifier here? Feels weird to do one and not the other

},
'include_policy': {
'database': False,
Expand Down Expand Up @@ -61,17 +61,10 @@ def matches(self, database=None, schema=None, identifier=None):
pass

for k, v in search.items():
# snowflake upcases unquoted identiifers. so, when
# comparing unquoted identifiers, use case insensitive
# matching. when comparing quoted identifiers, use case
# sensitive matching.
if self.should_quote(k):
if self.get_path_part(k) != v:
return False

else:
if self.get_path_part(k) != v.upper():
return False
# snowflake upcases unquoted identiifers. so, use
# case insensitive matching.
if self.get_path_part(k).upper() != v.upper():
return False

return True

Expand Down
22 changes: 18 additions & 4 deletions dbt/context/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,25 @@ def _return(value):
raise dbt.exceptions.MacroReturn(value)


def get_this_relation(db_wrapper, profile, model):
def get_this_relation(db_wrapper, project, profile, model):
table_name = dbt.utils.model_immediate_name(
model, dbt.flags.NON_DESTRUCTIVE)

return db_wrapper.adapter.Relation.create_from_node(
profile, model, table_name=table_name)
profile, model, table_name=table_name,
quote_policy=project.get('quoting', {}))


def create_relation(relation_type, quoting_config):

class RelationWithContext(relation_type):
@classmethod
def create(cls, *args, **kwargs):
return relation_type.create(*args,
quote_policy=quoting_config,
**kwargs)

return RelationWithContext


def generate(model, project, flat_graph, provider=None):
Expand Down Expand Up @@ -310,7 +323,8 @@ def generate(model, project, flat_graph, provider=None):
context = dbt.utils.merge(context, {
"adapter": db_wrapper,
"api": {
"Relation": adapter.Relation,
"Relation": create_relation(adapter.Relation,
project.get('quoting')),
"Column": adapter.Column,
},
"column": adapter.Column,
Expand All @@ -336,7 +350,7 @@ def generate(model, project, flat_graph, provider=None):
"fromjson": fromjson,
"tojson": tojson,
"target": target,
"this": get_this_relation(db_wrapper, profile, model),
"this": get_this_relation(db_wrapper, project, profile, model),
"try_or_compiler_error": try_or_compiler_error(model)
})

Expand Down
4 changes: 3 additions & 1 deletion dbt/context/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ def ref(*args):
dbt.exceptions.ref_invalid_args(model, args)

adapter = get_adapter(profile)
return adapter.Relation.create_from_node(profile, model)
return adapter.Relation.create_from_node(
profile, model,
quote_policy=project.get('quoting', {}))

return ref

Expand Down
4 changes: 3 additions & 1 deletion dbt/context/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ def do_ref(*args):
identifier=add_ephemeral_model_prefix(
target_model_name)).quote(identifier=False)
else:
return adapter.Relation.create_from_node(profile, target_model)
return adapter.Relation.create_from_node(
profile, target_model,
quote_policy=project.get('quoting', {}))

return do_ref

Expand Down
1 change: 1 addition & 0 deletions dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
Required('post-hook'): [hook_contract],
Required('pre-hook'): [hook_contract],
Required('vars'): dict,
Required('quoting'): dict,
Required('column_types'): dict,
}, extra=ALLOW_EXTRA)

Expand Down
44 changes: 25 additions & 19 deletions dbt/include/global_project/macros/materializations/seed/seed.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
{{ adapter_macro('create_csv_table', model) }}
{%- endmacro %}

{% macro reset_csv_table(model, full_refresh, existing) -%}
{{ adapter_macro('reset_csv_table', model, full_refresh, existing) }}
{% macro reset_csv_table(model, full_refresh, old_relation) -%}
{{ adapter_macro('reset_csv_table', model, full_refresh, old_relation) }}
{%- endmacro %}

{% macro load_csv_rows(model) -%}
Expand All @@ -16,12 +16,12 @@
{%- set column_override = model['config'].get('column_types', {}) -%}

{% set sql %}
create table {{ adapter.quote(model['schema']) }}.{{ adapter.quote(model['name']) }} (
{% for col_name in agate_table.column_names %}
{% set inferred_type = adapter.convert_type(agate_table, loop.index0) %}
{% set type = column_override.get(col_name, inferred_type) %}
{{ col_name | string }} {{ type }} {% if not loop.last %}, {% endif %}
{% endfor %}
create table {{ this.render(False) }} (
{%- for col_name in agate_table.column_names -%}
{%- set inferred_type = adapter.convert_type(agate_table, loop.index0) -%}
{%- set type = column_override.get(col_name, inferred_type) -%}
{{ col_name | string }} {{ type }} {%- if not loop.last -%}, {%- endif -%}
{%- endfor -%}
)
{% endset %}

Expand All @@ -33,14 +33,14 @@
{% endmacro %}


{% macro default__reset_csv_table(model, full_refresh, existing) %}
{% macro default__reset_csv_table(model, full_refresh, old_relation) %}
{% set sql = "" %}
{% if full_refresh %}
{{ drop_if_exists(existing, model['schema'], model['name']) }}
{{ adapter.drop_relation(old_relation) }}
{% set sql = create_csv_table(model) %}
{% else %}
{{ adapter.truncate(model['schema'], model['name']) }}
{% set sql = "truncate table " ~ adapter.quote(model['schema']) ~ "." ~ adapter.quote(model['name']) %}
{{ adapter.truncate_relation(old_relation) }}
{% set sql = "truncate table " ~ old_relation %}
{% endif %}

{{ return(sql) }}
Expand All @@ -62,7 +62,7 @@
{% endfor %}

{% set sql %}
insert into {{ adapter.quote(model['schema']) }}.{{ adapter.quote(model['name']) }} ({{ cols_sql }}) values
insert into {{ this.render(False) }} ({{ cols_sql }}) values
{% for row in chunk -%}
({%- for column in agate_table.column_names -%}
%s
Expand All @@ -88,8 +88,14 @@

{%- set identifier = model['name'] -%}
{%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%}
{%- set existing = adapter.query_for_existing(schema) -%}
{%- set existing_type = existing.get(identifier) -%}
{%- set existing_relations = adapter.list_relations(schema=schema) -%}

{%- set old_relation = adapter.get_relation(relations_list=existing_relations,
schema=schema, identifier=identifier) -%}

{%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%}
{%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%}

{%- set csv_table = model["agate_table"] -%}

{{ run_hooks(pre_hooks, inside_transaction=False) }}
Expand All @@ -99,12 +105,12 @@

-- build model
{% set create_table_sql = "" %}
{% if existing_type and existing_type != 'table' %}
{% if exists_as_view %}
{{ dbt.exceptions.raise_compiler_error("Cannot seed to '{}', it is a view".format(identifier)) }}
{% elif existing_type is none%}
{% set create_table_sql = create_csv_table(model) %}
{% elif exists_as_table %}
{% set create_table_sql = reset_csv_table(model, full_refresh_mode, old_relation) %}
{% else %}
{% set create_table_sql = reset_csv_table(model, full_refresh_mode, existing) %}
{% set create_table_sql = create_csv_table(model) %}
{% endif %}

{% set status = 'CREATE' if full_refresh_mode else 'INSERT' %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
{%- set existing_relations = adapter.list_relations(schema=schema) -%}
{%- set old_relation = adapter.get_relation(relations_list=existing_relations,
schema=schema, identifier=identifier) -%}
{%- set target_relation = api.Relation.create(identifier=identifier, schema=schema, type='table') -%}
{%- set target_relation = api.Relation.create(identifier=identifier,
schema=schema, type='table') -%}
{%- set intermediate_relation = api.Relation.create(identifier=tmp_identifier,
schema=schema, type='table') -%}
schema=schema, type='table') -%}
{%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%}
{%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%}
{%- set create_as_temporary = (exists_as_table and non_destructive_mode) -%}
Expand Down
1 change: 1 addition & 0 deletions dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ def invoke_dbt(parsed):
profile_to_load=parsed.profile,
args=parsed
)
proj.log_warnings()
Copy link
Contributor

Choose a reason for hiding this comment

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

we should do this check on line 235. If --target is used on the CLI, then this will refer to the wrong target. Probably not an issue for most, but i just got snagged locally

proj.validate()
except project.DbtProjectError as e:
logger.info("Encountered an error while reading the project:")
Expand Down
5 changes: 3 additions & 2 deletions dbt/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class SourceConfig(object):
ConfigKeys = DBTConfigKeys

AppendListFields = ['pre-hook', 'post-hook']
ExtendDictFields = ['vars', 'column_types']
ExtendDictFields = ['vars', 'column_types', 'quoting']
ClobberFields = [
'schema',
'enabled',
Expand Down Expand Up @@ -77,7 +77,8 @@ def config(self):
active_config = self.load_config_from_active_project()

if self.active_project['name'] == self.own_project['name']:
cfg = self._merge(defaults, active_config, self.in_model_config)
cfg = self._merge(defaults, active_config,
self.in_model_config)
else:
own_config = self.load_config_from_own_project()

Expand Down
18 changes: 18 additions & 0 deletions dbt/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ def __init__(self, cfg, profiles, profiles_dir, profile_to_load=None,
if self.cfg.get('models') is None:
self.cfg['models'] = {}

if self.cfg.get('quoting') is None:
self.cfg['quoting'] = {}

if self.cfg['models'].get('vars') is None:
self.cfg['models']['vars'] = {}

Expand Down Expand Up @@ -223,6 +226,21 @@ def validate(self):
"Expected project configuration '{}' was not supplied"
.format('.'.join(e.path)), self)

def log_warnings(self):
target_cfg = self.run_environment()
db_type = target_cfg.get('type')

if db_type == 'snowflake' and self.cfg \
.get('quoting', {}) \
.get('identifier') is None:
logger.warn(
'You are using Snowflake, but you did not specify a '
'quoting strategy for your identifiers. Quoting '
'behavior for Snowflake will change in a future release, '
'so it is recommended that you define this explicitly. '
'\n\n'
'For more information, see ADD LINK')
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO


def hashed_name(self):
if self.cfg.get("name", None) is None:
return None
Expand Down
1 change: 1 addition & 0 deletions dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
'vars',
'column_types',
'bind',
'quoting',
]


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
)
}}

select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
2 changes: 1 addition & 1 deletion test/integration/001_simple_copy_test/models/disabled.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
)
}}

select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
)
}}

select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
)
}}

select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
}}

-- this is a unicode character: å
select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
)
}}

select * from {{ target.schema }}.seed
select * from {{ ref('seed') }}
Loading