oracle-enhanced icon indicating copy to clipboard operation
oracle-enhanced copied to clipboard

add_synonym with database link

Open davinlagerroos opened this issue 4 years ago • 5 comments

First, thank you for all your work on this gem. Our institution has a large investment in oracle and this gem allows us to work in our preferred framework while taking advantage of the oracle resources and expertise of the institution.

While testing our upgrade to rails 6, we started getting errors like "DESC OTHER_USER.TABLE" failed; does it exist? After some time we traced the issue to models that were using a synonym that had a database link. With the removal of database link support in https://github.com/rsim/oracle-enhanced/pull/1668 the synonyms were still getting built, but without the link, so the synonym was pointing to a non-existent object.

We've worked around the issue, but I wonder if there is a way to save other developers the debugging time.

Steps to reproduce

add_synonym "new_synonym" "other_user.table@dblink", force: true

Expected behavior

Not sure. Perhaps it should raise a error.

It looks like add_synonym uses quote_table_name, which is silently ignoring the link. Could quote_table_name instead raise like Connection#describe? That could also flag other legacy uses of db links as well.

Actual behavior

select * from user_synonyms

SYNONYM_NAME     TABLE_OWNER     TABLE_NAME       DB_LINK ORIGIN_CON_ID
---------------- --------------- ---------------- ------- -------------
NEW_SYNONYM      OTHER_USER      TABLE                    0

System configuration

Rails version: 6.0.3.2

Oracle enhanced adapter version: 6.0.4

Ruby version: ruby 2.6.5

Oracle Database version: Oracle Database 12c Enterprise Edition Release 12.2.0.1.0 - 64bit Production

davinlagerroos avatar Sep 17 '20 16:09 davinlagerroos

I use "dblink" for many databases and I'm trying to find a workaround. Perhaps you could share your way to workaround this problem?

net1957 avatar Oct 05 '20 06:10 net1957

We build a view that returned the information from the database link and used that view as our active record source. The downside is we have to maintain the link and the view outside of active record migrations, but it does work.

ActiveRecord's multiple database support looks promising as well, but we did not have time to bring that into our project on our last sprint.

davinlagerroos avatar Oct 05 '20 14:10 davinlagerroos

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 25 '20 13:12 stale[bot]

Thanks for opening an issue. I understand dropping database link support causes unexpected issues in your environment. It would be appreciated to implement workarounds to create views on synonyms pointing to database links.

yahonda avatar Dec 30 '20 06:12 yahonda

Thanks for your comment on this issue.

Once we figured out what was going on, we did the work around you suggested. We stopped creating the synonyms and switched to views that returned data from the dblink tables. That has worked well for us, so thanks again for documenting those workarounds here and in #1668.

The main challenge was figuring out what was happening. add_synonym succeeded but was silently dropping the db link from the synonym, so the new synonym it was pointing to a non-existent table. Instead of getting feedback when we tried to create the synonym (where our code was in error), we got an Oracle error when we tried to do something with a model that used the synonym.

My hope with this issue was to see if there was a way to save some debugging time for other developers. One thought was that if add_synonym raised when the target table_name had a dblink in it, that would put the error closer to the code that needs to be updated. Adding something to the "Upgrade" section of the README that highlights the changes around database links could also work.

Thanks again for all your work on this project.

davinlagerroos avatar Dec 30 '20 22:12 davinlagerroos