[CT-1946] [Bug] dbt snapshot fails if the last line is a comment

[RobbertDM]

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 making a snapshot and using the check strategy with specified columns (not all), if the last line of your snapshot is a comment, it yields
14:50:11 TrinoUserError(type=USER_ERROR, name=SYNTAX_ERROR, message="line 12:16: mismatched input '<EOF>'. Expecting: ')'", query_id=20230130_145011_02592_b7axv)
because the produced query is:

select * from (
            select col1, col2, col3 from (
select * from iceberg.dbt_robbert.my_table
-- some comment ) subq
        ) as __dbt_sbq
        where false
        limit 0

Check out the -- some comment ) subq line.

Expected Behavior

When making a snapshot and using the check strategy with specified columns (not all), if the last line of your snapshot is a comment, it yields
a working snapshot
because the produced query is:

select * from (
            select col1, col2, col3 from (
select * from iceberg.dbt_robbert.my_table
-- some comment 
) subq
        ) as __dbt_sbq
        where false
        limit 0

Check out the ) subq line.

Steps To Reproduce

1. Configure a snapshot to use check strategy
2. Configure the snapshot to check_cols=['col1','col2'], not all
3. Make sure the last line of the snapshot is a -- comment
4. Run the snapshot

Relevant log output

No response

Environment

- OS: Ubuntu 22.04
- Python: 3.10.6
- dbt: 1.3.2

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

adapter used: dbt-trino

[dbeatty10]

Thank you for reporting this (and opening a PR!) @RobbertDM

This is similar to this one for incremental models:

• [CT-1725] [CT-1722] [CT-1721] [Bug] 1.4.0-b1 Bigquery Incremental Query breaks when ending in comment #6485

How we should test for this (and similar issues):

• We should add a comment to the end of models in integration tests for snapshots + each of our materialization types + singular and generic tests to catch any regressions in the future

[VersusFacit]

Extremely naive question -- why would these models end in comments? What's a use case? I just find it strange that adding whitespace is apparently a fix here. Not used to seeing that

[jtcohen6]

It's a fair question — but also something we should support. Comments, wherever they are, should work.

We should add a comment to the end of models in integration tests for snapshots + each of our materialization types + singular and generic tests to catch any regressions in the future

During BLG:

• We should add a test like this. A model with a comment at the end, every materialization (including singular test + snapshot), every incremental_strategy, every adapter, every day of the week.
• Neat chat with @colin-rogers-dbt about how we might support property-based or "matrix"-style testing. This feels like a good example, and maybe we'd want to spike an experiment with pytest_matrix

[RobbertDM]

Extremely naive question -- why would these models end in comments? What's a use case? I just find it strange that adding whitespace is apparently a fix here. Not used to seeing that

• For me, I had a post_hook that I didn't want to run, but also wanted to keep somewhere. I just moved it to the end of the snapshot and commented it out. That broke things in an unexpected way, and it took me a while to figure out where the problem was. If this is fixed, we prevent people from going through the same struggle in the future.
• Whitespace indeed is seldom a fix, but in this case, -- is valid for only one line, so adding whitespace transforms ) subq from a comment to actual code.

[VersusFacit]

Makes total sense. I knew I was just lacking imagination and I appreciate you filling me in!

edit: just circle back to say well of course comment should work everywhere. I regret ever being questioning here