Check length of escaped strings in the adapter test

[dbeatty10]

resolves #6566

See the following PRs for context and validation that it doesn't break any dbt Labs adapters:

• Check length of escaped strings in the adapter test #6567
• Add escape single quotes macro dbt-spark#586
• Validate dbt-core #6567 dbt-redshift#260
• Validate dbt-core #6567 dbt-bigquery#462
• Validate dbt-core #6567 dbt-snowflake#386

Description

Adapter maintainers can choose to extend one of the following pytests (which are expected to be mutually exclusive):

• BaseEscapeSingleQuotesQuote
• BaseEscapeSingleQuotesBackslash

We've observed at least one case where the adapter made the wrong choice but the test passed. This PR aims to trigger a CI failure if the wrong choice is made accidentally, and I validated that is does so here.

Checklist

• I have read the contributing guide and understand what's expected of me
• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
• I have run changie new to create a changelog entry

[aranke]

LGTM, I like defining the tests and expected values in SQL.

[VersusFacit]

First, we really appreciate you made those validation PRs first.

Really intriguing! So, the adapter maintainer picked a macro quoting policy that did not coordinate to the realities of their database system? Curious!

I like how the tests are defined and the numeric confirmation is key.

My only concern is this absolutely insane case that does happen and might be important to clarify:

'{{ escape_single_quotes('they're') }}'

Double quotes really aren't that sacred and on first glance, it'd be hard to know this what (at least on Snowflake) the quoting policy function does.

^So like, this is a thing...that works

I figure we should add a case to account for this. What's your thought?

[VersusFacit]

Oops didn't get my message in before you went on the merge

[dbeatty10]

the adapter maintainer picked a macro quoting policy that did not coordinate to the realities of their database system?

Yep! In this case, it was us as the maintainers of dbt-spark 😅. Since it was so easy to accidentally overlook, it provided motivation to open #6566 to make it hard to get wrong.

Based on our DM's, it seems like part of your concern is that we're covering test cases like:
'{{ escape_single_quotes("they're") }}'
but not:
'{{ escape_single_quotes('they're') }}'

I tried the latter in my local environment, and it gave the following error:

>           raise CompilationException(str(e), node) from e
E           dbt.exceptions.CompilationException: Compilation Error in model test_escape_single_quotes (models/test_escape_single_quotes.sql)
E             expected token ',', got 're'
E               line 2
E                 '{{ escape_single_quotes('they're') }}' as actual,

projects/dbt-core/core/dbt/clients/jinja.py:516: CompilationException

I think the issue is that "they're" can be parsed as valid string literal within Jinja whereas 'they're' can not.

Based on my current understanding, I'm comfortable with the changes made by this PR. But I'm open to continued discussion if we're missing something important.

[VersusFacit]

Got it. So, on some db's (like Spark) it'll be a syntax error and on others (like Snowflake), both options will work -- what a nightmare hah.

'{{ escape_single_quotes("they're") }}'

1. Outermost: SQL level
2. Innermost: will be escaped
3. In-between: what's inside these is passed to the escape function, but not these themselves

My concern was around that last of the three. I hope we have literature or guidance somewhere that makes this clear, because otherwise, that's some deep arcane type magic that would have bothered (among others) early career data engineer me 😄

Agreed this Issue is resolved. Reclosing the issue. Thanks Doug!