-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix: Source quoting ignores global configuration #10905
Fix: Source quoting ignores global configuration #10905
Conversation
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10905 +/- ##
==========================================
- Coverage 89.15% 89.14% -0.02%
==========================================
Files 183 183
Lines 23443 23466 +23
==========================================
+ Hits 20901 20919 +18
- Misses 2542 2547 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
core/dbt/context/providers.py
Outdated
return self.Relation.create_from( | ||
self.config, | ||
SourceQuotingConfig(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative approach would be to extend BaseRelation.create_from
to accept an Optional[HasQuoting]
value instead of just HasQuoting
. That said, the method in dbt-adapters is:
- abstracted to the point where it does not concern itself with the node type (e.g. model vs seed vs source, etc), and this kind of business logic feels better-suited in dbt-core
- overridable across adapters, so could lead to inconsistent handling + need updates across multiple adapters for consistent behaviour
So I went with this approach. Open to feedback :) I opted to define SourceQuotingBaseConfig
inline here since it quite specialized and likely won't need to be used elsewhere in the codebase.
|
||
generated_sql = read_file("target", "compiled", "test", "models", "model.sql") | ||
assert generated_sql == 'select * from "source_database"."source_schema"."customers"' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirmed the above test does fail on main as expected:
tests/functional/relation_quoting/test_relation_quoting.py F. [100%]
========================================================================= FAILURES ==========================================================================
_________________________________________ TestSourceQuotingGlobalConfigs.test_sources_ignore_global_quoting_configs _________________________________________
self = <test_relation_quoting.TestSourceQuotingGlobalConfigs object at 0x114bdba30>
project = <dbt.tests.fixtures.project.TestProjInfo object at 0x115b1e9b0>
def test_sources_ignore_global_quoting_configs(self, project):
run_dbt(["compile"])
generated_sql = read_file("target", "compiled", "test", "models", "model.sql")
> assert generated_sql == 'select * from "source_database"."source_schema"."customers"'
E assert 'select * fro...ema.customers' == 'select * fro..."."customers"'
E - select * from "source_database"."source_schema"."customers"
E ? - - - - - -
E + select * from source_database.source_schema.customers
/Users/michelleark/src/dbt-core/tests/functional/relation_quoting/test_relation_quoting.py:38: AssertionError
------------------------------------------------------------------- Captured stdout setup -------------------------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have also confirmed these tests pass with no code changes on 1.7.latest: #10906
Resolves #10892 (regression)
Problem
In dbt-core 1.7 and earlier, quoting behavior for sources was not controlled by the quoting configuration in dbt_project.yml, but newer versions of dbt also apply those settings to sources.
Solution
For sources, pass along an 'empty' quoting config as the base config, ignoring anything that's in self.config.quoting (ie dbt_project.yml). This was effectively the case in versions <1.7, which used create_from_source which did not incorporate the config quoting like create_from_node did.
We are not putting this behind a behaviour flag because:
False
by default, and adding quotes has been unsafe to do by default for sources when they otherwise wouldn't have been.Checklist