Quoting changes from 1.4.0 to 1.4.1 break MSCK REPAIR TABLE pre-hooks in models

[sacundim]

I have a project that makes use of pre_hook in a handful of models to run MSCK REPAIR TABLE on source tables. Example such model:

• https://github.com/sacundim/covid-19-puerto-rico/blob/e864bd14ca6a016e92086849c7321f24e0041c27/aws/athena-dbt/models/covid19datos_v2/staging/casos.sql#L8

Just the other day I rebuilt my Docker image for this DBT project for the first time after several weeks, but the project started failing with errors like this (fuller stack trace at the bottom of the ticket):

An error occurred (InvalidRequestException) when calling the StartQueryExecution operation: line 1:1: mismatched input 'MSCK'. Expecting: 'ALTER', 'ANALYZE', 'CALL', 'COMMENT', 'COMMIT', 'CREATE', 'DEALLOCATE', 'DELETE', 'DENY', 'DESC', 'DESCRIBE', 'DROP', 'EXECUTE', 'EXPLAIN', 'GRANT', 'INSERT', 'MERGE', 'PREPARE', 'REFRESH', 'RESET', 'REVOKE', 'ROLLBACK', 'SET', 'SHOW', 'START', 'TRUNCATE', 'UNLOAD', 'UPDATE', 'USE', 'USING', <query>

I have managed to determine that pinning dbt-athena-community at version 1.4.0 doesn't experience the problem, it happens when I use v1.4.1 instead:

• sacundim/covid-19-puerto-rico@e864bd1

Looking at dbt.log, I see that with adapter v1.4.1, the schema and table name are quoted with double quotes:

18:57:41.706289 [debug] [Thread-4 (]: On model.covid_19_puerto_rico_etl.casos: 
        MSCK REPAIR TABLE "covid19datos_v2_sources"."casos_parquet_v1"

And running that statement in the AWS Athena console gets the same error "mismatched input 'MSCK'" that I show above. Athena uses the Hive-style quoting in MSCK REPAIR TABLE statements, like this one that works:

MSCK REPAIR TABLE `covid19datos_v2_sources`.`casos_parquet_v1`

---

Full stack trace:

Failed to execute query.
--
Traceback (most recent call last):
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/pyathena/common.py", line 520, in _execute
query_id = retry_api_call(
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/pyathena/util.py", line 68, in retry_api_call
return retry(func, *args, **kwargs)
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/tenacity/__init__.py", line 379, in __call__
do = self.iter(retry_state=retry_state)
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/tenacity/__init__.py", line 314, in iter
return fut.result()
File "/usr/local/lib/python3.9/concurrent/futures/_base.py", line 439, in result
return self.__get_result()
File "/usr/local/lib/python3.9/concurrent/futures/_base.py", line 391, in __get_result
raise self._exception
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/tenacity/__init__.py", line 382, in __call__
result = fn(*args, **kwargs)
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/botocore/client.py", line 530, in _api_call
return self._make_api_call(operation_name, kwargs)
File "/root/.local/pipx/venvs/dbt-core/lib/python3.9/site-packages/botocore/client.py", line 960, in _make_api_call
raise error_class(parsed_response, operation_name)
botocore.errorfactory.InvalidRequestException: An error occurred (InvalidRequestException) when calling the StartQueryExecution operation: line 1:1: mismatched input 'MSCK'. Expecting: 'ALTER', 'ANALYZE', 'CALL', 'COMMENT', 'COMMIT', 'CREATE', 'DEALLOCATE', 'DELETE', 'DENY', 'DESC', 'DESCRIBE', 'DROP', 'EXECUTE', 'EXPLAIN', 'GRANT', 'INSERT', 'MERGE', 'PREPARE', 'REFRESH', 'RESET', 'REVOKE', 'ROLLBACK', 'SET', 'SHOW', 'START', 'TRUNCATE', 'UNLOAD', 'UPDATE', 'USE', 'USING', <query>

[sacundim]

Workaround: explicitly use render_hive() in the pre-hooks:

• sacundim/covid-19-puerto-rico@e17a6c3

[sacundim]

An idea that doesn't solve the fundamental problem but may mitigate most of the instances people will encounter (and get mightily confused by): instead of automatically double quoting all identifiers, maybe check them first against a regexp that accepts only valid identifiers and quote them only if they don't match?

[nicor88]

Is it really an issue? as you find out using render_hive() is the key and needs only to be documented IMHO, the same is valid for other commands like vacuum and optimize for iceberg. I would avoid to introduce any regex that replace quoting just for specific scenarions.

[sacundim]

@nicor88 The problem with "just document it" is that it's a surprising behavior with an obscure cause, which is likely to cause significant grief to unsuspecting users and, as a result, lead to repeated inquiries and bug reports to library maintainers from people who maybe even read the documentation but didn't remember that one sentence.

It's basically XKCD's toaster that stabs users in the face.

[nicor88]

@sacundim Well, we could make it clear for users that there are breaking changes in releases notes and adding know issues in the README. Don't people read docs when working with a tool/framework? If they don't they should ;) there are some tools that needs instructions and tools that don't :P - Therefore again: adding a note in Known issues will make the job IMHO.

Also, feel free to propose your solution if you want, contributions are more than welcome:)