Update adapters.sql #15

algol68 · 2022-06-03T22:50:36Z

changes are to address issues when using custom schemas

1 - create schema call verifies database. As of 1.0.7 Oracle adapter allows for database parm not to be set since Oracle doesn't allow statement prefix to include database name. Trickle down effect of that change is 'create schema check' not working hence why lines are removed. There is even dummy-noop already in that method since you can't create schemas dbt-way in oracle (schemas are users).

2 - once #1 is addressed and custom schema is used, rename command will not find temp model table since schema is not passed as part of rename statement. Furthermore, rename statement will not work because Oracle doesn't support renaming tables that way. Alter Table to the rescue.

changed from
rename <table_original_name> to <table_new_name>

to

alter table .<original_name> rename to <table_new_name>

tested with
Python 3.9.1
dbt-core 1.0.7
dbt-oracle 1.0.2

changes are to address issues when using custom schemas 1 - create schema call verifies database. As of 1.0.7 Oracle adapter allows for database parm not to be set since Oracle doesn't allow statement prefix to include database name. Trickle down effect of that change is 'create schema check' not working hence why lines are removed. There is even dummy-noop already in that method since you can't create schemas dbt-way in oracle (schemas are users). 2 - once oracle#1 is addressed and custom schema is used, rename command will not find temp model table since schema is not passed as part of rename statement. Furthermore, rename statement will not work because Oracle doesn't support renaming tables that way. Alter Table to the rescue. changed from rename <table_original_name> to <table_new_name> to alter table <schema>.<original_name> rename to <table_new_name> tested with Python 3.9.1 dbt-core 1.0.7 dbt-oracle 1.0.2

oracle-contributor-agreement · 2022-06-03T22:50:40Z

Oracle requires that contributors to all of its open-source projects sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

PR author: @algol68
vlad@terzic.net (@algol68)

In order to sign the OCA, you need to create an Oracle account and sign the OCA in the Oracle's Contributor Agreement Application by following the steps on the homepage.

When singing the OCA, please provide your GitHub username. By doing so, this PR will be automatically updated once the signed OCA was approved by Oracle.

algol68 · 2022-06-03T23:00:01Z

I've filled out OCA document so I guess I'll check / come back in a day or two to complete any additional steps

aosingh · 2022-06-03T23:05:55Z

dbt/include/oracle/macros/adapters.sql

@@ -26,9 +26,6 @@
 {% endmacro %}

 {% macro oracle__create_schema(database_name, schema_name) -%}
-  {% if relation.database -%}


I already am working on issue which fixes this part.

aosingh · 2022-06-03T23:09:12Z

I am currently working on this issue - #14 which raises these questions.

Thank you for highlighting the problem with RENAME

1. Following macros changed or fixed in adapter.sql to use custom schema name - oracle__create_schema, oracle__create_view_as, oracle__alter_relation_comment, oracle__alter_column_type, oracle__drop_relation, oracle__truncate_relation, oracle__rename_relation, oracle__make_temp_relation 2. Changed view materialization strategy which unncessarily created an intermediate <model>__dbt_tmp view first and renamed it to the target view. Now we directly CREATE or REPLACE view <target_view_name> 3. Added generate_schema_name.sql as per dbt documentation in test project to determine the name of the schema that a model should be built in. 4. Changed a couple of models in the test project to use custom schema 5. Fixes Bug #14 6. Fixes Bug #2 7. Addresses issues raised in PR #15

aosingh · 2022-06-08T00:39:28Z

@algol68

Thank you for raising a PR.

Currently, the macro oracle__rename_relation is used at multiple places. With the proposed ALTER TABLE <schema>.<table_name> RENAME to <>, it fails if the relation is a view.

In incremental materialization while renaming existing relation, if the existing relation is a view
In view materialization again if there is an existing view

To support custom schemas, we have handled this in view and incremental materialization to incorporate ALTER TABLE <schema>.<table_name> RENAME to <>.

You can review all the changes here release/v1.0.3...fix/manage_objects_in_different_schema

Is there a way you could modify the PR's base branch to merge with release/v1.0.3 ?

algol68 · 2022-06-08T18:52:59Z

@algol68

Thank you for raising a PR.

Currently, the macro oracle__rename_relation is used at multiple places. With the proposed ALTER TABLE <schema>.<table_name> RENAME to <>, it fails if the relation is a view.
* In incremental materialization while renaming existing relation, if the existing relation is a view

* In view materialization again if there is an existing view
To support custom schemas, we have handled this in view and incremental materialization to incorporate ALTER TABLE <schema>.<table_name> RENAME to <>.

You can review all the changes here release/v1.0.3...fix/manage_objects_in_different_schema

Is there a way you could modify the PR's base branch to merge with release/v1.0.3 ?

nice catch! I will address 'alter ...' statement to switch between table/view based on selected materialization strategy.
I'll re-point pull request to suggested release as well

aosingh · 2022-06-08T20:56:41Z

@algol68
Thank you for raising a PR.
Currently, the macro oracle__rename_relation is used at multiple places. With the proposed ALTER TABLE <schema>.<table_name> RENAME to <>, it fails if the relation is a view.
* In incremental materialization while renaming existing relation, if the existing relation is a view

* In view materialization again if there is an existing view
To support custom schemas, we have handled this in view and incremental materialization to incorporate ALTER TABLE <schema>.<table_name> RENAME to <>.
You can review all the changes here release/v1.0.3...fix/manage_objects_in_different_schema
Is there a way you could modify the PR's base branch to merge with release/v1.0.3 ?
nice catch! I will address 'alter ...' statement to switch between table/view based on selected materialization strategy. I'll re-point pull request to suggested release as well

This is already handled as seen here.

release/v1.0.3...fix/manage_objects_in_different_schema#diff-bc8a7d5ce45af39780e4337b20ffc8fe5da22b1ec4fd4980fe64f7cc7b8e4cbfL281

If you could just change the base branch for this PR, I will merge the changes and resolves conflicts accordingly.

algol68

changed to pull request against 1.0.3 vs main per committer request

oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Jun 3, 2022

aosingh self-requested a review June 3, 2022 23:02

aosingh reviewed Jun 3, 2022

View reviewed changes

oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Jun 7, 2022

aosingh requested a review from ramapemmaraju June 7, 2022 16:39

ramapemmaraju approved these changes Jun 7, 2022

View reviewed changes

aosingh mentioned this pull request Jun 8, 2022

Fix/manage objects in different schema #16

Merged

algol68 changed the base branch from main to release/v1.0.3 June 8, 2022 22:58

algol68 commented Jun 8, 2022

View reviewed changes

aosingh merged commit 4795c3f into oracle:release/v1.0.3 Jun 9, 2022

aosingh mentioned this pull request Jun 10, 2022

Release/v1.0.3 #23

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update adapters.sql #15

Update adapters.sql #15

Uh oh!

algol68 commented Jun 3, 2022

Uh oh!

oracle-contributor-agreement bot commented Jun 3, 2022

Uh oh!

algol68 commented Jun 3, 2022

Uh oh!

aosingh Jun 3, 2022

Uh oh!

aosingh commented Jun 3, 2022

Uh oh!

aosingh commented Jun 8, 2022 •

edited

Loading

Uh oh!

algol68 commented Jun 8, 2022

Uh oh!

aosingh commented Jun 8, 2022

Uh oh!

algol68 left a comment

Uh oh!

Uh oh!

Update adapters.sql #15

Update adapters.sql #15

Uh oh!

Conversation

algol68 commented Jun 3, 2022

Uh oh!

oracle-contributor-agreement bot commented Jun 3, 2022

Uh oh!

algol68 commented Jun 3, 2022

Uh oh!

aosingh Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

aosingh commented Jun 3, 2022

Uh oh!

aosingh commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

algol68 commented Jun 8, 2022

Uh oh!

aosingh commented Jun 8, 2022

Uh oh!

algol68 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aosingh commented Jun 8, 2022 •

edited

Loading