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-2321] [Bug] error using inline compile with newline characters in jinja strings #7207

Closed
2 tasks done
Tracked by #7162
dave-connors-3 opened this issue Mar 22, 2023 · 9 comments · Fixed by #7208
Closed
2 tasks done
Tracked by #7162
Assignees
Labels
bug Something isn't working

Comments

@dave-connors-3
Copy link
Contributor

dave-connors-3 commented Mar 22, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When passing raw SQL to the new dbt compile --inline command, the compiler does not properly handle newline characters within jinja blocks:

Without newline characters:

❯ dbt compile --inline "select {{ dbt_utils.group_by(5) }}"
14:35:24  Running with dbt=1.5.0-b4
14:35:24  Found 3 models, 5 tests, 1 snapshot, 1 analysis, 420 macros, 0 operations, 1 seed file, 1 source, 1 exposure, 0 metrics, 0 groups
14:35:24  
14:35:24  Concurrency: 2 threads (target='postgres')
14:35:24  
14:35:24  Compiled node 'inline_query' is:
select group by 1,2,3,4,5
14:35:24  Done.

With newline characters:

❯ dbt compile --inline "select {{\n dbt_utils.group_by(5) \n}}"
14:35:34  Running with dbt=1.5.0-b4
Traceback (most recent call last):
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/clients/jinja.py", line 514, in catch_jinja
    yield
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/clients/jinja.py", line 541, in get_template
    return env.from_string(template_source, globals=ctx)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/jinja2/environment.py", line 1105, in from_string
    return cls.from_code(self, self.compile(source), gs, None)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/jinja2/environment.py", line 768, in compile
    self.handle_exception(source=source_hint)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/jinja2/environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<unknown>", line 1, in template
jinja2.exceptions.TemplateSyntaxError: unexpected char '\\' at 9
  line 1
    select {{\n dbt_utils.group_by(5) \n}}

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/bin/dbt", line 8, in <module>
    sys.exit(cli())
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/requires.py", line 56, in wrapper
    return func(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/requires.py", line 92, in wrapper
    return func(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/requires.py", line 114, in wrapper
    return func(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/requires.py", line 140, in wrapper
    return func(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/requires.py", line 175, in wrapper
    return func(*args, **kwargs)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/cli/main.py", line 271, in compile
    results = task.run()
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/task/runnable.py", line 420, in run
    self._runtime_initialize()
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/task/compile.py", line 118, in _runtime_initialize
    sql_node = block_parser.parse_remote(self.args.inline, "inline_query")
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/parser/sql.py", line 48, in parse_remote
    return self.parse_node(contents)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/parser/base.py", line 395, in parse_node
    self.render_update(node, config)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/parser/base.py", line 371, in render_update
    context = self.render_with_context(node, config)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/parser/base.py", line 240, in render_with_context
    get_rendered(parsed_node.raw_code, context, parsed_node, capture_macros=True)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/clients/jinja.py", line 584, in get_rendered
    template = get_template(
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/clients/jinja.py", line 541, in get_template
    return env.from_string(template_source, globals=ctx)
  File "/Users/daveconnors/.pyenv/versions/3.9.16/lib/python3.9/contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/Users/daveconnors/dev/sandbox/tmp-sqlfluff-demo/env/lib/python3.9/site-packages/dbt/clients/jinja.py", line 517, in catch_jinja
    raise CompilationError(str(e), node) from e
dbt.exceptions.CompilationError: Compilation Error in sql operation inline_query (from remote system.sql)
  unexpected char '\\' at 9
    line 1
      select {{\n dbt_utils.group_by(5) \n}}

Same SQL written in a model file with the newlines:

-- in models/new_line_macro.sql

{{
    dbt_utils.group_by(5)
}}
❯ dbt compile -s new_line_macro    
14:41:56  Running with dbt=1.5.0-b4
14:41:56  Found 4 models, 5 tests, 1 snapshot, 1 analysis, 420 macros, 0 operations, 1 seed file, 1 source, 1 exposure, 0 metrics, 0 groups
14:41:56  
14:41:56  Concurrency: 2 threads (target='postgres')
14:41:56  
14:41:56  Compiled node 'new_line_macro' is:
group by 1,2,3,4,5
14:41:56  Done.

Expected Behavior

New lines should not affect the jinja rendering!

Steps To Reproduce

  1. pass in Jinja-SQL without newlines to dbt compile --inline , see rendered sql
  2. pass in same Jinja-SQL with newlines inside Jinja block {{ }} , see error
  3. paste same Jinja-SQL with newlines into a model file, compile dbt compile -s model_name see same result as step 1

Relevant log output

No response

Environment

- OS: Mac
- Python: Python 3.9.16


❯ dbt --version
Core:
  - installed: 1.5.0-b4
  - latest:    1.4.5    - Ahead of latest version!

Plugins:
  - postgres: 1.5.0b4 - Ahead of latest version!

Which database adapter are you using with dbt?

postgres

Additional Context

No response

@dave-connors-3 dave-connors-3 added bug Something isn't working triage labels Mar 22, 2023
@github-actions github-actions bot changed the title [Bug] error using inline compile with newline characters in jinja strings [CT-2321] [Bug] error using inline compile with newline characters in jinja strings Mar 22, 2023
@aranke aranke self-assigned this Mar 22, 2023
@dbeatty10 dbeatty10 removed the triage label Mar 22, 2023
@dbeatty10
Copy link
Contributor

Thanks for trying this out and reporting this bug @dave-connors-3 🙌

@jtcohen6
Copy link
Contributor

Hmm, I'm not sure if this is actually a bug we want/need to resolve! This fails independently of using dbt compile --inline. The same behavior exists in older versions, too:

-- models/any_model.sql

select {{\n dbt_utils.group_by(5) \n}}
$ dbt compile
15:47:06  Running with dbt=1.4.3
15:47:06  Unable to do partial parsing because of a version mismatch
15:47:07  Encountered an error:
Compilation Error in model my_model (models/my_model.sql)
  unexpected char '\\' at 9
    line 1
      select {{\n dbt_utils.group_by(5) \n}}

@dbeatty10
Copy link
Contributor

@jtcohen6 do you happen to know why it has an unexpected char of \\ instead of just \? It's okay if you don't know, of course.

I'm wondering if knowing that answer might unlock the details of why this one weird trick works:

private_key: "{{ env_var('BIGQUERY_KEYFILE_JSON_PRIVATE_KEY').split('\\\\n') | join('\n') }}"

Could also be unrelated.

@jtcohen6
Copy link
Contributor

Offhand, I'm not sure!

@dave-connors-3
Copy link
Contributor Author

@jtcohen6 -- is there something different about actually having returns (i.e. in the test file i included above vs. the hardcoded newlines you added to your test case? My understanding is that the raw_code that ends up in the manifest would contain the \n characters, but is still able to parse it to the correct compiled_code

@jtcohen6
Copy link
Contributor

jtcohen6 commented Mar 22, 2023

Ahh - I wonder if this has to do with encoding!

When we read files, we expect to decode them from UTF-8:

to_return = handle.read().decode("utf-8")

I don't see us doing that for --inline at all. By the time we get here, it appears that the string contents have already been escaped:

ipdb> self.args.inline
'select {{\\n dbt_utils.group_by(5) \\n}}'
ipdb> self.args.inline.encode("utf-8")
b'select {{\\n dbt_utils.group_by(5) \\n}}'
ipdb> self.args.inline.encode("utf-8").decode("utf-8")
'select {{\\n dbt_utils.group_by(5) \\n}}'

Is this why we required that SQL inputs for dbt-RPC be base64-encoded? Probably :)

I wonder if there's a way we can handle this within dbt-core, while still allowing users the convenience of passing in a human-friendly string! Perhaps with a custom click.ParamType? Should we accept either a human-friendly string, or safer bytes (UTF-8 encoded)?

@jtcohen6
Copy link
Contributor

So - I think we need to do this before the v1.5 RC!

Two options:

  • Change --inline from only "pretty" string input, to try decoding the string from base64 (using logic like this). If we can't decode it as base64, then just pass the string.
  • Add another argument, --inline-base64, and preserve the existing behavior for --inline — so that users still have the convenience of passing a "pretty" string directly into the command

Especially now that we're supporting a programmatic interface to dbt-core, users could expect to do the following in scripts that wrap around dbt:

import base64
from dbt.cli.main import dbtRunner

my_query_with_newlines = """
{{
    dbt_utils.group_by(5)
}}
"""

my_query_b64 = base64.b64encode(my_query_with_newlines.encode('utf-8'))

dbt = dbtRunner()
results, success = dbt.invoke(['compile', '--inline', my_query_b64])

@aranke
Copy link
Member

aranke commented Mar 24, 2023

@jtcohen6 I managed to fix this and add a test to my PR, so this should be resolved soon 😄

@jtcohen6
Copy link
Contributor

@aranke nice!! that's even better :)

@aranke aranke mentioned this issue Mar 24, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants