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

[CT-503] Add method to target keys #335

Closed
jtcohen6 opened this issue Apr 18, 2022 · 1 comment
Closed

[CT-503] Add method to target keys #335

jtcohen6 opened this issue Apr 18, 2022 · 1 comment
Labels

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 18, 2022

In #250, we added logic to explicitly render the SQL binding character as ? instead of %s if the method is odbc:

{% macro spark__get_binding_char() %}
{{ return('?' if target.method == 'odbc' else '%s') }}
{% endmacro %}

Turns out that didn't work as we hoped, because {{ target.method }} doesn't actually return anything! Why? The method attribute isn't included among its connection keys, exposed to the Jinja context:

def _connection_keys(self):
return ('host', 'port', 'cluster',
'endpoint', 'schema', 'organization')

This makes bugs like #334 much trickier to debug than they should be. dbt's debug-level logs (logs/dbt.log) show %s for all SQL binding parameters, and we just have to trust that sqlparams is magically catching and converting those characters right as the query is being sent off via pyodbc:

# pyodbc does not handle a None type binding!
if bindings is None:
self._cursor.execute(sql)
else:
# pyodbc only supports `qmark` sql params!
query = sqlparams.SQLParams('format', 'qmark')
sql, bindings = query.format(sql, bindings)
self._cursor.execute(sql, *bindings)

@github-actions github-actions bot changed the title Add method to target keys [CT-503] Add method to target keys Apr 18, 2022
@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant