Add column quotes in macro to avoid build failure

[seanglynn-thrive]

Resolves: dbt-labs/dbt-adapters#157

Problem

The macro: default__alter_relation_add_remove_columns generated by incremental config: on_schema_change = 'append_new_columns' is causing our build to fail when it encounters columns that are named as reserved keywords BigQuery SQL syntax.

Solution

Wrap column reference with quotes to avoid a SQL exception being thrown.

Checklist

• I have read the contributing guide and understand what's expected of me
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests
• This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc)

[cla-bot]

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @seanglynn-thrive

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.11%. Comparing base (d597b80) to head (a1d17ab).
Report is 246 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    dbt-labs/dbt-core#8529      +/-   ##
==========================================
- Coverage   86.60%   85.11%   -1.50%     
==========================================
  Files         175      175              
  Lines       25638    25638              
==========================================
- Hits        22205    21822     -383     
- Misses       3433     3816     +383     

Flag	Coverage Δ
integration	81.09% <ø> (-2.37%)	⬇️
unit	65.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dbeatty10]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[dbeatty10]

Thanks for opening this PR @seanglynn-thrive !

I just created and added changelog entry file for this PR: .changes/unreleased/Fixes-20230918-145127.yaml

Could you do the following?

1. Open an associated issue here (so there is a more detailed description of the problem, including a simple code example, that we can link to it in the changelog).
2. Update .changes/unreleased/Fixes-20230918-145127.yaml with the resulting issue number.
3. Review each of the items in the PR checklist and check off those that have been completed already.
4. Make an attempt at checking off the rest.

In terms of adding new test cases, the problem and solution are similar to #919, and you can use that PR for inspiration for how to implement.

Specifically, we'll want to make sure the tests verify that both unquoted and quoted identifiers still work when appending new columns (in addition to a quoted identifier with a reserved keyword).

[seanglynn-thrive]

Thanks for opening this PR @seanglynn-thrive !

I just created and added changelog entry file for this PR: .changes/unreleased/Fixes-20230918-145127.yaml

Could you do the following?

1. Open an associated issue here (so there is a more detailed description of the problem, including a simple code example, that we can link to it in the changelog).
2. Update .changes/unreleased/Fixes-20230918-145127.yaml with the resulting issue number.
3. Review each of the items in the PR checklist and check off those that have been completed already.
4. Make an attempt at checking off the rest.

In terms of adding new test cases, the problem and solution are similar to #919, and you can use that PR for inspiration for how to implement.

Specifically, we'll want to make sure the tests verify that both unquoted and quoted identifiers still work when appending new columns (in addition to a quoted identifier with a reserved keyword).

@dbeatty10
I've opened an issue here: dbt-labs/dbt-adapters#157 and updated the changelog in the latest commit. However, I'm not really sure where I can add unit tests for this change. The PR you have referenced seems to refer to pytest files that no longer exists on the latest branch :(
Would you mind directing me to the best location to add these unit tests and I will be more than happy to add them?

[dbeatty10]

Nice @seanglynn-thrive 😎

I left a review comment within the changelog entry -- could you take a look?

Here's the example test from dbt-labs/dbt-bigquery#919.

Since the example above is from the dbt-bigquery repo, but your PR is within the dbt-core repo, you can use a search similar to the following to find a relevant test to modify (or copy-paste from):

• GitHub code search: repo:dbt-labs/dbt-core append_new_columns

Here's some lines of code you could take a peek at:

1. fixture model definition
2. import fixture
3. add the fixture to the dbt project for a pytest case
4. run the dbt project within a test case that pytest will pick up

Adding / modifying tests can be tricky sometimes, so just let us know if you run into any issues and need a hand.

Accidental approval -- need to wait for test cases to approve

[github-actions]

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

[dbeatty10]

Thanks for taking the time to open this PR @seanglynn-thrive! Since opening, we've decoupled dbt Adapters from dbt Core, and this code now lives in a separate repo: dbt-adapters.

A consequence of the decoupling is that PR can't be merged anymore as is, so we're closing it. For more context see #9171.

The linked issue has already been transferred. Please don't hesitate to re-open your proposed code changes within this PR in the dbt-adapters repo.