Skip to content

Conversation

@mosqueteiro
Copy link

This adds jinja rules into the AST for the purpose of being able to hook
in injections from a jinja parser. Without these hooks the parser errors
and injections are not possible.

Jinja structures:

  • Jinja Comments: {# ... #}
  • Jinja Statements: {% ... %}
  • Jinja Expressions: {{ ... }}

Addresses issue: #335

This adds jinja rules into the AST for the purpose of being able to hook
in injections from a jinja parser. Without these hooks the parser errors
and injections are not possible.
@matthias-Q
Copy link
Collaborator

We had some reservations about this topic in the past, as jinja is not part of any SQL dialect. Even though, this is a very permissive parser.

However, I would be in favor of adding this. @DerekStride what do you think?

@DerekStride
Copy link
Owner

I think it would be better to tackle this in reverse. You'd want to parse the program with a jinja parser first and then use this parser to parse the SQL blocks. Have you looked at that approach instead?

I don't know much about jinja but it doesn't seem like jinja is specific to SQL so it doesn't seem like the right solution.

@matthias-Q
Copy link
Collaborator

@DerekStride I think that request comes from users of dbt, where you write SQL code with embedded jinja block.
So it is not SQL embedded in jinja, but jinja embedded in SQL.

@DerekStride
Copy link
Owner

So it is not SQL embedded in jinja, but jinja embedded in SQL

Jinja looks like a generic template language similar to liquid, or tree-sitter-embedded-template.

nvim-treesitter defines injections on the embedded-template language. Same for the liquid injections. Unfortunately, this pattern isn't the best because you often can only partially parse the document in either parser.

I understand why this would be helpful, that said, I still don't think this belongs here.

@matthias-Q
Copy link
Collaborator

Yes, I agree. I would also be against merging it. Mainly because it is not part of the SQL language and that's what injections are for.

Nevertheless, I would like to see a working example of how to use injections in that particular case.

@mosqueteiro
Copy link
Author

So it is not SQL embedded in jinja, but jinja embedded in SQL

Jinja looks like a generic template language similar to liquid, or tree-sitter-embedded-template.

Yeah, it seems like it is similar to these. It uses python for rendering. The tree-sitter-embedded-template is for templates that use < angle brackets > but there are a couple jinja-specific tree-sitter repos out there.

nvim-treesitter defines injections on the embedded-template language. Same for the liquid injections. Unfortunately, this pattern isn't the best because you often can only partially parse the document in either parser.

I understand why this would be helpful, that said, I still don't think this belongs here.

It isn't the best with those other languages but it still works. The template blocks are either embedded in strings or, like for HTML, the language is already basically a templated text file to begin with and this other format doesn't bother the parser.

With SQL, or at least this parser, this tactic is impossible. SQL has a very rigid structure and this parser follows that well. There is just no way to parse this without having a node the SQL parser recognizes and allows.

SELECT col1 FROM {{ my_variable_object }}

If you parse with SQL first it just ERRORs immediately and there is nothing that a tree-sitter query can even do to locate for an injection. Also, anything relying on tree-sitter is now broken (highlights, indentation, etc.). If you parse with jinja first then inject SQL everywhere around it the SQL parser will freak out and just report ERROR because the structure is broken.

I think that request comes from users of dbt, where you write SQL code with embedded jinja block.

I am using this with Airflow (data pipeline orchestrator) but the most wide-spread use of this style of SQL is with dbt, to be sure. I need templated SQL because I am deploying in different environments and need the same query to work in dev, test, and prod which are deploy on different resources. The dbt use-case leans much more heavily on this templating style where it needs to work across all the database and data warehouse SQL dialects. The usage of jinja in SQL is only going to continue to grow, pushed forward from the Data Analytics and Machine Learning side.


Btw, the CREATE OR REPLACE TABLE rule was missing so I added that in here too along with a test for it. If you don't want to merge my PR feel free to cherry-pick that commit out (mosqueteiro@4f21a3f). I'll maintain my fork, as I need this feature supported.

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

Successfully merging this pull request may close these issues.

3 participants