-
Notifications
You must be signed in to change notification settings - Fork 78
Jinja hooks #336
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
base: main
Are you sure you want to change the base?
Jinja hooks #336
Conversation
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.
|
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? |
|
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. |
|
@DerekStride I think that request comes from users of |
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. |
|
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. |
Yeah, it seems like it is similar to these. It uses python for rendering. The tree-sitter-embedded-template is for templates that use
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 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 Btw, the |
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:
{# ... #}{% ... %}{{ ... }}Addresses issue: #335