Override list_relations_without_caching #428
Conversation
…ist_relation_without_caching
…ft into fix_get_relation
…datashared objects of a connected database
jiezhen-chen
changed the title
jiezhen-chen
changed the title
Is there a way to dynamically infer whether the user needs multi-db querying and/or datashare support? It would be awesome if we could have both of these without the user needing to add extra configuration themselves:
What would be the performance and/or cost implications to always allowing cross-db querying? e.g., How much extra latency would it add? And how much monetary cost? |
…jiezhen-chen/dbt-redshift into override_list_relations_without_caching
jiezhen-chen
changed the title
jiezhen-chen
changed the title
Since dbt is only creating tables in the target database the only time we would need this set would be if the user wanted to refer to a source that was outside the database. We could have this set based on the source config In any case I'd suggest we split this out into it's own PR as I don't think it's necessary to the rest of the PR right? |
|
Great that this has been merged into main! @mikealfare @jiezhen-chen which dbt-redshift release will this be available in? We are operating on dbt 1.3. Do you know if I will need to upgrade that to 1.5 before taking advantage of this upgrade? |
|
@rarup1 currently, this is slated to require dbt 1.6, which will have its final release in about two weeks. |
|
@jiezhen-chen the changelog entry indicated that this issue resolves #17, so I added But maybe I shouldn't have done either! Now that I'm looking at this implementation more closely, I'm not seeing where external tables are covered. I wonder if support for external tables did exist in this PR when get_tables was included, but that support was dropped when switching over to If this PR does not resolve dbt-labs/dbt-core#17, then we'll want to do a couple follow-up items:
Bonus follow-up items: |
|
@dbeatty10 You're correct. Because we've pivoted our solution to NOT use get_tables, this change won't solve issue 17 and 217. I'm sorry I didn't update the PR with our latest decision prior to the merge. I'll open a new PR to update the changelog to reflect this. Thanks! |
|
No prob @jiezhen-chen. Thank you for updating that changelog entry to reflect the relevant issue in dbt-labs/dbt-core#535, and thanks to @dataders for re-opening dbt-labs/dbt-core#17 and dbt-labs/dbt-core#217 👍 |
* add support for datashared objects and external tables for redshift list_relation_without_caching * changie * refactor inner joins to cte * shorten list_relations_without_caching query * add database filter to get_columns_in_relation to only query inbound datashared objects of a connected database * leverage redshift_conn method for list_schemas * add list_schemas method * migrate from macros to redshift driver * add database_metadata_current_db_only to unit tests * override get_columns_in_relation * cast relation types to lower case * add functional test to ensure list_relations_without_chacing works * revert changes in macros adapter.sql * revert changes in macros adapter.sql * revert changes in macros adapter.sql * update test_adapter_methods with project test * remove snowflake comment in test_adapter_methods Co-authored-by: Doug Beatty <[email protected]> * unpack driver metadata results with dictionary * refactor parse_relation_results to its own method * replace dict[] with Dict[] Co-authored-by: colin-rogers-dbt <[email protected]> * correct unit tests and functional tests * remove irrelevant comment * set current_db_only to default true * remove unused import * remove overriding list_relations_without_caching and rewrite the sql query * update unit tests * remove overriding list_relations_without_caching and rewrite the sql query * remove unused tests * add schema filter --------- Co-authored-by: Anders <[email protected]> Co-authored-by: Doug Beatty <[email protected]> Co-authored-by: colin-rogers-dbt <[email protected]>
resolves #179
Description
Currently, redshift__list_relations_without_caching leverages postgres__list_relation_without_caching. Since redshift_connector has its own metadata call(get_tables) that does the same thing as postgres__list_relation_without_caching, we are migrating to use the native redshift driver.Changes were made to override the method list_relations_without_caching directly, instead of changing the underlying macro sql query. Unit tests were also created under test_redshift_adapter, functional tests created in test_adapter_methods.Because redshift_connector get_tables api call does not support MV, we are switching back to writing hard coded queries to fetch this metadata. Instead of
pg_viewsandpg_tablesthe new queries useinformation_schema.viewsandinformation_schema.tables.Checklist
changie newto create a changelog entry