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

commented out macros can still render into target sql in strange ways #458

Closed
cmcarthur opened this issue Jun 16, 2017 · 4 comments
Closed
Labels

Comments

@cmcarthur
Copy link
Member

cmcarthur commented Jun 16, 2017

Given a multiline macro:

{% macro multiline(field) -%}
  {{field}}
  as {{field}}
{%- endmacro %}

and model sql:

select
  *
  -- {{multiline('id')}}

the target sql is:

select
  *
  -- id
  as id

I'm not sure there's anything we can do about this besides recommend using multiline comments, it's just unexpected the first time it happens

@jthandy
Copy link
Member

jthandy commented Jun 28, 2017

I've run into this before. At first it was perplexing / frustrating to me that SQL comments don't also comment out jinja code; however, I came to see that as unreasonable. Of course the jinja compiler doesn't respect SQL comments...

Do you think there is a "solve" for this?

@drewbanin
Copy link
Contributor

@jthandy I think we have two options here:

  1. Encourage the use of jinja commments:
select
  *
  {# {{multiline('id')}} #}
...
  1. We can configure the jinja compiler to use -- as a line comment identifier. Check this out

@jthandy
Copy link
Member

jthandy commented Jun 28, 2017

Whoah! That is super-cool. I think adding -- as a comment identifier would be fucking sweet.

I also really like the line statement identifier defined in those docs—we should consider incorporating that as well. I find the {% %} syntax very ugly / un-SQL-y. The {{ }} syntax feels totally reasonable to me because it's always in-line and fairly limited in scope, but sometimes you need a ton of {% %} to get something done.

@drewbanin drewbanin added good_first_issue Straightforward + self-contained changes, good for new contributors! and removed good_first_issue Straightforward + self-contained changes, good for new contributors! labels Sep 29, 2017
@jthandy
Copy link
Member

jthandy commented Oct 12, 2017

I'm closing this. Really, the answer here is to use correct jinja commenting syntax, which I just didn't know about at the time I originally logged this (from connor's computer). The comment about line identifiers is captured in another issue.

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

No branches or pull requests

3 participants