-
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 the filtering for external tables in the Redshift get_columns_in_relation macro #2855
Fix the filtering for external tables in the Redshift get_columns_in_relation macro #2855
Conversation
…e svv_external_columns CTE.
…svv_external_columns` CTE
…dbt into brangisom/spectrum-filter-fix
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, don't hesitate to ping @drewbanin. CLA has not been signed by users: @brangisom |
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
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.
This is the easiest approve of my life. Bravo on the research, and identification of a quick fix. Just one teensy comment about the changelog.
CHANGELOG.md
Outdated
@@ -34,6 +35,7 @@ Contributors: | |||
- Added strategy-specific validation to improve the relevancy of compilation errors for the `timestamp` and `check` snapshot strategies. (([#2787](https://github.com/fishtown-analytics/dbt/issues/2787), [#2791](https://github.com/fishtown-analytics/dbt/pull/2791)) | |||
- Changed rpc test timeouts to avoid locally run test failures ([#2803](https://github.com/fishtown-analytics/dbt/issues/2803),[#2804](https://github.com/fishtown-analytics/dbt/pull/2804)) | |||
- Added a debug_query on the base adapter that will allow plugin authors to create custom debug queries ([#2751](https://github.com/fishtown-analytics/dbt/issues/2751),[#2871](https://github.com/fishtown-analytics/dbt/pull/2817)) | |||
- Fix Redshift adapter `get_columns_in_relation` macro to push schema filter down to the `svv_external_columns` view ([#2855](https://github.com/fishtown-analytics/dbt/issues/2854)) |
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.
Tiny tiny: could you move this above, under dbt 0.19.0? You can make a new "under the hood" section there. I just don't want it here because it wasn't included in the b1 release.
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.
Got it moved!
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.
Thank you for the contribution @brangisom!
resolves #2854
Description
This change will ensure that the
table_schema = '{{ relation.schema}}'
filter gets pushed down directly against thesvv_external_columns
table to ensure that this macro does not cause performance issues by calling far more API calls than necessary.Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.dbt run
output0.19.0-b1
Explain output