-
Notifications
You must be signed in to change notification settings - Fork 18
Fix list relations macro #14
base: develop
Are you sure you want to change the base?
Fix list relations macro #14
Conversation
@LewisDavies very sorry for taking this long to reply, thank you for the pull request! I'll have a look this weekend. I'm not sure if making everything upper case might cause some issue when using quoting for the relations names. Are you aware of the quoting configs @LewisDavies ? |
Hey @vitoravancini, no worries. I'm aware the quoting options in the dbt profile but I'm not sure how to incorporate them here. Happy to help test this if you've got any suggestions. |
I've started bringing the newer dbt integrationt tests for adapter last night(https://github.com/fishtown-analytics/dbt-adapter-tests) My suggestion for testing quoting options with incremental mode for now is using the dbt_test_project included in this repo. Maybe create a model with a mix of uppercase and lowercase column names, and try to make some assertion using dbt test on this columns. Does that makes sense? |
@vitoravancini does this mean you are also working on making the dbt-oracle compatible with newer versions of dbt? Would like to use the dbt run state filters in v18+, but dbt-oracle is currently supporting up to dbt 17.2. Either way, thanks much for your efforts on this project. |
@jeffw82 yes we intend to make this oracle adapter compatible with 18+ dbt versions |
I just tried using the test project to test my changes but haven't been able to do so; the project seems tailored to a specific environment. It would take a non-trivial amount of work to getting it running with my setup and meaningfully test an incremental table. I'm leaving my current role next week so I can't dedicate the time to it, and I won't have access to an Oracle DB afterwards. Anything I can do to help before then? |
@LewisDavies can you tell more about your setup? And when you say the test project are you talking about the test project in this repo or the dbt integration test? I was able to use the dbt integration tests(https://github.com/fishtown-analytics/dbt-adapter-tests) and runs most tests sucessfully. I'll merge it tonight. |
First of all, thanks for building this adapter - appreciate the hard work! I've made some changes that should make incremental loading work properly. The main problem was that
adapter.get_relation
wasn't returning any results, which I traced back to theoracle__list_relations_without_caching
macro.The database and schema arguments were being changed to uppercase, but the output of
sys.all_tables
andsys.all_views
was not. I've updated all comparisons inadapters.sql
to use uppercase and the macros now provide usable output.I haven't been able to run all tests yet because I'm having environment issues, but wanted to open a PR for feedback anyway.