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

EOL SQL comments remove entire line from execution #2731

Closed
1 of 5 tasks
mroy-seedbox opened this issue Aug 28, 2020 · 1 comment · Fixed by #2974
Closed
1 of 5 tasks

EOL SQL comments remove entire line from execution #2731

mroy-seedbox opened this issue Aug 28, 2020 · 1 comment · Fixed by #2974
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors! snowflake
Milestone

Comments

@mroy-seedbox
Copy link

mroy-seedbox commented Aug 28, 2020

Describe the bug

When using SQL line comment (-- comment) at the end of lines and running multiple SQL queries (separated by semicolons) within one statement block, if any one-line query has a line comment after it (on the same line), that command won't be executed (i.e. not submitted to the database at all) - except if it's the first query.

  • This affects all statement blocks, whether they come from a model, macro, materialization, etc.
  • Block comments (/* comment */) do not have this effect (which provides a workaround)

This will work (all three queries will be executed):

{%- call statement('test') -%}
  select 1;
  select 2;
  select 3;
{%- endcall %}

This will work (all three queries will be executed):

{%- call statement('test') -%}
  select 1; /* comment */
  select 2; /* comment */
  select 3; /* comment */
{%- endcall %}

This will not work (only the first query will be executed):

{%- call statement('test') -%}
  select 1; -- comment
  select 2; -- comment
  select 3; -- comment
{%- endcall %}

This will work (all three queries will be executed):

{%- call statement('test') -%}
  select
    1; -- comment
  select
    2; -- comment
  select
    3; -- comment
{%- endcall %}

Steps To Reproduce

Just put the examples above in a model or macro, and take a look at the queries executed on your database (query log, etc.)

Expected behavior

All three queries should be executed in all examples.

System information

Which database are you using dbt with?

  • postgres
  • redshift
  • bigquery
  • snowflake
  • other (specify: ____________)

The output of dbt --version:

installed version: 0.18.0-rc1
   latest version: 0.17.2

Your version of dbt is ahead of the latest release!

Plugins:
  - snowflake: 0.18.0rc1

The operating system you're using:

macOS Catalina
Version 10.15.6 (19G2021)

The output of python --version:

Python 3.8.3
@mroy-seedbox mroy-seedbox added bug Something isn't working triage labels Aug 28, 2020
@jtcohen6
Copy link
Contributor

Thanks for the detailed walkthrough, @mroy-seedbox.

I'm guessing that the offending code is our regex hack here:
https://github.com/fishtown-analytics/dbt/blob/2cc2d971c69ecf4143deb16f961f2c9ed8f22199/plugins/snowflake/dbt/adapters/snowflake/connections.py#L298-L304

This reflects the requirement of snowflake-connector-python to split a stream of semicolon-delimited statements into separate, individual queries. We rely on their split_statements function to do this, too.

As you note, there are ways to work around this, so it's not a high-priority fix from our end. If you're interested in contributing the fix, I'd be happy to help!

@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! snowflake and removed triage labels Aug 31, 2020
@jtcohen6 jtcohen6 added this to the Margaret Mead milestone Jan 19, 2021
jtcohen6 added a commit that referenced this issue Jan 21, 2021
Fix bug #2731 on stripping query comments for snowflake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors! snowflake
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants