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

Broken jinja2 templating in snowflake sql #168

Open
Curtis-Jiang-2020 opened this issue Aug 10, 2020 · 3 comments
Open

Broken jinja2 templating in snowflake sql #168

Curtis-Jiang-2020 opened this issue Aug 10, 2020 · 3 comments

Comments

@Curtis-Jiang-2020
Copy link

Currently, Jinja2 templating is done here:

lore/lore/io/connection.py

Lines 413 to 434 in f4789b1

def __prepare(self, sql=None, extract=None, filename=None, **kwargs):
if extract is None:
extract = filename
if sql is None and extract is not None:
sql_filename = Connection.path(extract, '.sql')
template_filename = Connection.path(extract, '.sql.j2')
if os.path.exists(sql_filename):
logger.debug('READ SQL FILE: ' + sql_filename)
if os.path.exists(template_filename):
logger.warning('ignoring template with the same base file name')
with open(sql_filename) as file:
sql = file.read()
elif os.path.exists(template_filename):
logger.debug('READ SQL TEMPLATE: ' + template_filename)
sql = jinja2_env.get_template(extract + '.sql.j2').render(**kwargs)
else:
raise IOError('There is no template or sql file for %s' % extract)
# support mustache style bindings
sql = re.sub(r'\{(\w+?)\}', r'%(\1)s', sql)
return sql

And this is how they are called:

lore/lore/io/connection.py

Lines 168 to 169 in f4789b1

def execute(self, sql=None, extract=None, filename=None, **kwargs):
self.__execute(self.__prepare(sql=sql, extract=extract, filename=filename, **kwargs), kwargs)

The problem here is all the args are passed twice, one to jinja2, the other to snowflake connector.

It will break down right here, because nothing could be formated this way.

For example if we want to run

lore.io.analysis.execute(filename='somefile', job_type=self.job_type)

we will see processed_params is not empty and breaks the program here:

https://github.com/snowflakedb/snowflake-connector-python/blob/83dfe771c5404657f2008fec15bf0386185b6d70/cursor.py#L491

@Curtis-Jiang-2020
Copy link
Author

def __execute(self, sql, bindings):

The bindings (which is actually used in jinja2 template) is passed again in this function, in to snowflake connector's execute.

One suggestion is to give jinja2 template args a dict argument that works like kwargs, so these values will not interfere snowflake connector's execute method

@ganesh-krishnan
Copy link
Contributor

@Curtis-Jiang-2020 makes sense. feel free to open a PR with this improvement.

@metatron1973

This comment was marked as spam.

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

No branches or pull requests

3 participants